-
Notifications
You must be signed in to change notification settings - Fork 11
[3.6.0] Contacts Batch API Import #47
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
WalkthroughThis update introduces bulk contact import functionality to the Contact API, including new DTOs, API methods, documentation, usage examples, and comprehensive test coverage for importing contacts and retrieving import status. The changelog and README are updated to reflect the new feature. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ContactAPI
participant MailtrapAPI
Client->>ContactAPI: importContacts(array of ImportContact)
ContactAPI->>MailtrapAPI: POST /contacts/imports (bulk contacts data)
MailtrapAPI-->>ContactAPI: Response (importId, status)
ContactAPI-->>Client: Response (importId, status)
Client->>ContactAPI: getContactImport(importId)
ContactAPI->>MailtrapAPI: GET /contacts/imports/{importId}
MailtrapAPI-->>ContactAPI: Response (import status/results)
ContactAPI-->>Client: Response (import status/results)
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 1
🧹 Nitpick comments (3)
src/DTO/Request/Contact/ImportContact.php (3)
20-27: Consider removing the redundant static factory method.The
initmethod duplicates the functionality of the constructor without adding any value. This violates the DRY principle and creates unnecessary code duplication.- public static function init( - string $email, - array $fields = [], - array $listIdsIncluded = [], - array $listIdsExcluded = [] - ): self { - return new self($email, $fields, $listIdsIncluded, $listIdsExcluded); - }
49-60: Optimize the array filtering logic.The
array_filtercallback checking for!== nullis unnecessary since none of the properties can be null based on the constructor (the string is required and arrays have default empty values).public function toArray(): array { - return array_filter( - [ - 'email' => $this->getEmail(), - 'fields' => $this->getFields(), - 'list_ids_included' => $this->getListIdsIncluded(), - 'list_ids_excluded' => $this->getListIdsExcluded(), - ], - fn($value) => $value !== null - ); + return [ + 'email' => $this->getEmail(), + 'fields' => $this->getFields(), + 'list_ids_included' => $this->getListIdsIncluded(), + 'list_ids_excluded' => $this->getListIdsExcluded(), + ]; }
12-16: Consider adding email validation.The constructor accepts any string as email without validation. Consider adding basic email format validation to prevent invalid data from being processed.
public function __construct( private string $email, private array $fields = [], private array $listIdsIncluded = [], private array $listIdsExcluded = [] ) { + if (!filter_var($email, FILTER_VALIDATE_EMAIL)) { + throw new \InvalidArgumentException('Invalid email format'); + } }
📜 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/ImportContact.php(1 hunks)tests/Api/General/ContactTest.php(2 hunks)
🔇 Additional comments (14)
src/DTO/Request/Contact/ImportContact.php (1)
12-18: LGTM! Constructor parameters are well-structured.The constructor properly initializes all required properties with sensible defaults for optional array parameters.
CHANGELOG.md (1)
1-3: LGTM! Changelog entry is properly formatted.The new version entry correctly documents the Contact Imports API functionality addition and follows the established changelog format.
README.md (1)
36-36: LGTM! Documentation accurately reflects the new import feature.The addition of "Import" to the contact management features list correctly documents the new bulk import functionality.
src/Api/General/Contact.php (2)
10-10: LGTM! Import statement is correctly added.The ImportContact DTO is properly imported to support the new bulk import functionality.
266-277: LGTM! Import status retrieval method is well-implemented.The
getContactImportmethod follows the established patterns and correctly handles the GET request for import status retrieval.examples/general/contacts.php (3)
5-5: LGTM! Import statement is correctly added.The ImportContact DTO is properly imported to support the example usage.
277-306: Excellent example demonstrating bulk contact import.The example is comprehensive, well-documented, and shows practical usage of the ImportContact DTO with various field types and list management options. The comment explaining the asynchronous nature and import limits is particularly helpful.
309-322: LGTM! Import status retrieval example is clear and complete.The example properly demonstrates how to check import status and includes appropriate error handling.
tests/Api/General/ContactTest.php (6)
9-9: LGTM: Proper import additionThe import for
ImportContactDTO is correctly added to support the new test methods.
480-518: Comprehensive test for bulk contact importThe test properly validates the
importContacts()functionality with:
- Correct HTTP POST method and endpoint
- Proper payload structure with array mapping
- Appropriate response assertions
The test data includes realistic scenarios with multiple contacts having different field combinations and list assignments.
520-540: Good coverage for in-progress import statusThis test appropriately validates the
getContactImport()method for imports that are still being processed, checking the correct endpoint and response structure.
542-568: Thorough validation of completed import statusThe test comprehensively checks all fields returned when an import is finished, including the contact count metrics (
created_contacts_count,updated_contacts_count,contacts_over_limit_count).
570-585: Proper error handling for non-existent importsThe test correctly validates the 404 error scenario with appropriate exception expectations and message verification.
587-627: Comprehensive validation error testingThis test thoroughly validates the complex nested error structure returned during validation failures. The expected error message on line 624 properly flattens the nested error structure for verification.
The test covers a realistic scenario with invalid email format and ensures proper exception handling.
Motivation
Support new functionality (Contacts Batch Import)
https://help.mailtrap.io/article/147-contacts
Changes
How to test
composer testOR run PHP code from the example
Summary by CodeRabbit
New Features
Documentation
Tests