-
Notifications
You must be signed in to change notification settings - Fork 10
Add Contact Export functionality with filters and related tests #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds Contact Export functionality: new DTO for export filters, new API methods to create and fetch contact exports, example usage, tests, and documentation updates including changelog and README list item. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as Caller
participant SDK as Contact API
participant HTTP as HTTP Client
participant Svc as Mail Service
rect rgba(230,245,255,0.6)
note over User,SDK: Create Contact Export (async task)
User->>SDK: createContactExport(filters: ContactExportFilter[])
SDK->>SDK: validate filters are ContactExportFilter
SDK->>HTTP: POST /contacts/exports {filters:[...]}
HTTP->>Svc: Request
Svc-->>HTTP: 202 Accepted {export_id}
HTTP-->>SDK: Response
SDK-->>User: Response (export_id)
end
rect rgba(240,255,230,0.6)
note over User,SDK: Poll Export Status / Get Download URL
loop until completed
User->>SDK: getContactExport(export_id)
SDK->>HTTP: GET /contacts/exports/{id}
HTTP->>Svc: Request
Svc-->>HTTP: 200 {status, url?}
HTTP-->>SDK: Response
SDK-->>User: Response
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/general/contacts.php (1)
353-388: Consider documenting valid filter parameters.The example demonstrates the export workflow well, including the async polling pattern. However, users may benefit from inline comments or documentation explaining:
- Valid filter names (e.g.,
list_id,subscription_status)- Valid operators (e.g.,
equal,not_equal, etc.)- Expected value types for each filter
This would help developers understand what filters are available without consulting external API documentation.
Example enhancement:
/** * Create a new Contact Export (asynchronous task) * * POST https://mailtrap.io/api/accounts/{account_id}/contacts/exports * * Common filter names: list_id, subscription_status, created_at, etc. * Common operators: equal, not_equal, greater_than, less_than, etc. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CHANGELOG.md(1 hunks)README.md(1 hunks)examples/general/contacts.php(2 hunks)src/Api/General/Contact.php(2 hunks)src/DTO/Request/Contact/ContactExportFilter.php(1 hunks)tests/Api/General/ContactTest.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
examples/general/contacts.php (3)
src/Api/General/Contact.php (3)
Contact(20-408)createContactExport(320-339)getContactExport(349-354)src/DTO/Request/Contact/ContactExportFilter.php (3)
ContactExportFilter(12-49)init(21-24)toArray(41-48)src/Helper/ResponseHelper.php (1)
ResponseHelper(14-37)
src/Api/General/Contact.php (2)
src/DTO/Request/Contact/ContactExportFilter.php (2)
ContactExportFilter(12-49)toArray(41-48)src/Api/AbstractApi.php (3)
handleResponse(84-111)httpPost(43-50)httpGet(34-41)
src/DTO/Request/Contact/ContactExportFilter.php (1)
src/Api/General/Contact.php (1)
Contact(20-408)
tests/Api/General/ContactTest.php (5)
src/Api/General/Contact.php (3)
Contact(20-408)createContactExport(320-339)getContactExport(349-354)src/DTO/Request/Contact/ContactExportFilter.php (2)
ContactExportFilter(12-49)toArray(41-48)src/Api/AbstractApi.php (1)
AbstractApi(18-150)src/Helper/ResponseHelper.php (1)
ResponseHelper(14-37)src/Exception/HttpClientException.php (1)
HttpClientException(14-84)
🔇 Additional comments (9)
README.md (1)
37-37: LGTM!The documentation update accurately reflects the new Contact Export capability alongside the existing Import feature.
CHANGELOG.md (1)
1-3: LGTM!The changelog entry follows the established format and clearly documents the new feature.
examples/general/contacts.php (1)
10-10: LGTM!The import is correctly placed alongside other Contact-related DTOs.
src/Api/General/Contact.php (3)
11-11: LGTM!The import is correctly placed with other Contact-related DTO imports.
312-339: LGTM!The implementation is solid and follows the established pattern from
importContacts. The filter validation ensures type safety, and the array transformation viatoArray()is consistent with other DTO usage in the codebase.
341-354: LGTM!The implementation correctly retrieves export status by ID using the established patterns and error handling.
src/DTO/Request/Contact/ContactExportFilter.php (1)
1-50: LGTM!The DTO implementation is clean and follows PHP best practices:
- Final class prevents unintended inheritance
- Constructor property promotion (PHP 8.0+)
- Static factory method for convenient instantiation
- Simple getters and serialization method
The
mixedtype forvalueprovides flexibility for different filter types (arrays, strings, etc.), which aligns with the example usage showing both array and string values.tests/Api/General/ContactTest.php (2)
9-9: LGTM!The import is correctly placed with other DTO imports.
929-1061: LGTM!The test suite provides excellent coverage of the contact export functionality:
- Success scenarios for both create and get operations
- Comprehensive error handling (401, 403, 404, 422, 429)
- Proper mocking and assertions
- Realistic response structures
- Follows established testing patterns in the codebase
The tests demonstrate both filter construction approaches (
ContactExportFilter::init()andnew ContactExportFilter()) and validate the complete workflow from creation to retrieval.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
examples/general/contacts.php (1)
353-388: Consider adding a comment linking the two examples.The create and get export examples are well-structured. However, it might be helpful to add a brief comment in the get export example (around line 381) explaining how to obtain the
exportIdfrom the create response, since these two operations are typically used together in a polling pattern.Example enhancement:
/** * Get Contact Export status / download URL * (Poll this endpoint until status becomes `finished` and `url` is not null) * * GET https://mailtrap.io/api/accounts/{account_id}/contacts/exports/{export_id} */ try { $exportId = 1; // Replace 1 with the actual export ID obtained from createContactExport response (see previous example) $response = $contacts->getContactExport($exportId);tests/Api/General/ContactTest.php (1)
929-1061: Add test for invalid filter type validation.The test suite comprehensively covers the export functionality with various HTTP response scenarios (unauthorized, forbidden, validation errors, rate limits, not found). However, there's a missing test case for the
InvalidArgumentExceptionthrown when a non-ContactExportFilterinstance is passed tocreateContactExport().Compare with the
importContactstest suite, which includestestImportContactsThrowsExceptionForInvalidInput(line 632) to verify the validation logic. Adding a similar test for exports would ensure consistent test coverage for the validation logic increateContactExport(lines 328-330 in Contact.php).Add this test case after line 1030:
public function testCreateContactExportThrowsExceptionForInvalidInput(): void { $filters = [ new ContactExportFilter('list_id', 'equal', [1]), // Invalid input - should be ContactExportFilter new ImportContact( email: '[email protected]', fields: [], listIdsIncluded: [], listIdsExcluded: [] ), ]; $this->contact->expects($this->never())->method('httpPost'); $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('Each filter must be an instance of ContactExportFilter.'); $this->contact->createContactExport($filters); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CHANGELOG.md(1 hunks)README.md(1 hunks)examples/general/contacts.php(2 hunks)src/Api/General/Contact.php(2 hunks)src/DTO/Request/Contact/ContactExportFilter.php(1 hunks)tests/Api/General/ContactTest.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/Api/General/Contact.php (3)
src/DTO/Request/Contact/ContactExportFilter.php (2)
ContactExportFilter(12-49)toArray(41-48)src/Api/AbstractApi.php (3)
handleResponse(84-111)httpPost(43-50)httpGet(34-41)src/Exception/InvalidArgumentException.php (1)
InvalidArgumentException(10-12)
examples/general/contacts.php (3)
src/Api/General/Contact.php (3)
Contact(20-408)createContactExport(320-339)getContactExport(349-354)src/DTO/Request/Contact/ContactExportFilter.php (3)
ContactExportFilter(12-49)init(21-24)toArray(41-48)src/Helper/ResponseHelper.php (1)
ResponseHelper(14-37)
tests/Api/General/ContactTest.php (5)
src/Api/General/Contact.php (3)
Contact(20-408)createContactExport(320-339)getContactExport(349-354)src/DTO/Request/Contact/ContactExportFilter.php (2)
ContactExportFilter(12-49)toArray(41-48)src/Api/AbstractApi.php (1)
AbstractApi(18-150)src/Helper/ResponseHelper.php (1)
ResponseHelper(14-37)src/Exception/HttpClientException.php (1)
HttpClientException(14-84)
src/DTO/Request/Contact/ContactExportFilter.php (1)
src/Api/General/Contact.php (1)
Contact(20-408)
🔇 Additional comments (5)
src/Api/General/Contact.php (3)
11-11: LGTM!The import for
ContactExportFilteris correctly placed and necessary for the new export methods.
312-339: LGTM!The
createContactExportmethod implementation is solid:
- Validates each filter is an instance of
ContactExportFilter(lines 328-330)- Throws
InvalidArgumentExceptionfor invalid filter types (consistent withimportContactspattern)- Properly transforms filters to array format using
array_mapandtoArray()(lines 326-335)- Empty filters array is allowed by default parameter, which is reasonable for exporting all contacts
341-354: LGTM!The
getContactExportmethod is straightforward and correct:
- Simple GET request to the appropriate endpoint
- Uses
handleResponsefor consistent error handling- Proper path construction with exportId
src/DTO/Request/Contact/ContactExportFilter.php (1)
1-49: LGTM!The
ContactExportFilterclass is well-implemented as an immutable value object:
- Final class prevents unintended extension
- Private properties with public getters ensure immutability
- Static factory method
init()provides a convenient construction patterntoArray()method properly serializes the filter for API requestsmixedtype for$valueallows flexibility for different filter types (arrays, strings, etc.)- Consistent with other DTO patterns in the codebase (e.g.,
ImportContact,CreateContact)tests/Api/General/ContactTest.php (1)
9-9: LGTM!The import for
ContactExportFilteris correctly placed and necessary for the new export tests.
leonid-shevtsov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good although I'm not an expert on the contacts features.
Motivation
Support new export contact functionality
Changes
How to test
composer testSummary by CodeRabbit
New Features
Documentation
Tests