-
Notifications
You must be signed in to change notification settings - Fork 0
Implemented ContactEvents and missing ContactLists APIs #46
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
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds a Contact Events create endpoint and expands Contact Lists with full CRUD; wires new APIs into the client factory; introduces request/response models, tests, JSON fixtures, and Java example code. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MailtrapContactsApi
participant ContactEventsImpl
participant HTTP_API
Client->>MailtrapContactsApi: contactEvents()
MailtrapContactsApi-->>Client: ContactEvents instance
rect rgb(220, 235, 250)
Note over Client,HTTP_API: Create Contact Event Flow
Client->>ContactEventsImpl: createContactEvent(accountId, contactIdentifier, request)
activate ContactEventsImpl
ContactEventsImpl->>ContactEventsImpl: URLEncode(contactIdentifier)
ContactEventsImpl->>HTTP_API: POST /api/accounts/{accountId}/contacts/{encoded}/events (body)
HTTP_API-->>ContactEventsImpl: 200 ContactEventResponse
deactivate ContactEventsImpl
ContactEventsImpl-->>Client: ContactEventResponse
end
sequenceDiagram
participant Client
participant MailtrapContactsApi
participant ContactListsImpl
participant HTTP_API
Client->>MailtrapContactsApi: contactLists()
MailtrapContactsApi-->>Client: ContactLists instance
rect rgb(220, 235, 250)
Note over Client,HTTP_API: Contact Lists CRUD Workflow
Client->>ContactListsImpl: createContactList(accountId, request)
ContactListsImpl->>HTTP_API: POST /api/accounts/{accountId}/contacts/lists
HTTP_API-->>ContactListsImpl: ContactListResponse
Client->>ContactListsImpl: getContactList(accountId, listId)
ContactListsImpl->>HTTP_API: GET /api/accounts/{accountId}/contacts/lists/{listId}
HTTP_API-->>ContactListsImpl: ContactListResponse
Client->>ContactListsImpl: updateContactList(accountId, listId, request)
ContactListsImpl->>HTTP_API: PATCH /api/accounts/{accountId}/contacts/lists/{listId}
HTTP_API-->>ContactListsImpl: ContactListResponse
Client->>ContactListsImpl: deleteContactList(accountId, listId)
ContactListsImpl->>HTTP_API: DELETE /api/accounts/{accountId}/contacts/lists/{listId}
HTTP_API-->>ContactListsImpl: 204 No Content
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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: 1
🧹 Nitpick comments (3)
examples/java/io/mailtrap/examples/contactevents/ContactEventsExample.java (1)
14-14: Misleading constant name.The constant
CONTACT_EVENT_IDis used as thecontactIdentifierparameter (which per the Javadoc is "Contact UUID or Email"). Consider renaming toCONTACT_IDENTIFIERorCONTACT_IDto accurately reflect its purpose and avoid confusion for developers using this example.🔎 Proposed fix
- private static final String CONTACT_EVENT_ID = "b691272b-3e50-4813-997b-c7c9b317dcb2"; + private static final String CONTACT_IDENTIFIER = "b691272b-3e50-4813-997b-c7c9b317dcb2";And update the usage on line 29:
- final ContactEventResponse contactEvent = client.contactsApi().contactEvents().createContactEvent(ACCOUNT_ID, CONTACT_EVENT_ID, request); + final ContactEventResponse contactEvent = client.contactsApi().contactEvents().createContactEvent(ACCOUNT_ID, CONTACT_IDENTIFIER, request);examples/java/io/mailtrap/examples/contactlists/ContactListsExample.java (1)
30-34: Example deletes all contact lists, not just the one created.The loop deletes every contact list returned by
findAll(), which includes pre-existing lists in the account—not just the one created by this example. This could cause unintended data loss for users running the example against a real account.Consider deleting only the created list to make the example safer:
🔎 Proposed fix
final var contactLists = client.contactsApi().contactLists().findAll(ACCOUNT_ID); System.out.println(contactLists); - contactLists.forEach(contactList -> client.contactsApi().contactLists().deleteContactList(ACCOUNT_ID, contactList.getId())); + // Delete only the contact list created by this example + client.contactsApi().contactLists().deleteContactList(ACCOUNT_ID, created.getId());src/test/java/io/mailtrap/api/contactevents/ContactEventsImplTest.java (1)
38-52: LGTM! Consider expanding test coverage.The test correctly validates the happy path for creating a contact event. The assertions appropriately verify the response structure and data.
For more comprehensive coverage, consider adding tests for:
- Error scenarios (e.g., invalid accountId, null request)
- Edge cases (e.g., empty params map, special characters in contactIdentifier)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
examples/java/io/mailtrap/examples/contactevents/ContactEventsExample.javaexamples/java/io/mailtrap/examples/contactlists/ContactListsExample.javasrc/main/java/io/mailtrap/api/contactevents/ContactEvents.javasrc/main/java/io/mailtrap/api/contactevents/ContactEventsImpl.javasrc/main/java/io/mailtrap/api/contactlists/ContactLists.javasrc/main/java/io/mailtrap/api/contactlists/ContactListsImpl.javasrc/main/java/io/mailtrap/client/api/MailtrapContactsApi.javasrc/main/java/io/mailtrap/factory/MailtrapClientFactory.javasrc/main/java/io/mailtrap/model/request/contactevents/CreateContactEventRequest.javasrc/main/java/io/mailtrap/model/request/contactlists/CreateUpdateContactListRequest.javasrc/main/java/io/mailtrap/model/response/contactevents/ContactEventResponse.javasrc/test/java/io/mailtrap/api/contactevents/ContactEventsImplTest.javasrc/test/java/io/mailtrap/api/contactlists/ContactListsImplTest.javasrc/test/java/io/mailtrap/testutils/BaseTest.javasrc/test/resources/api/contact_lists/contactList.jsonsrc/test/resources/api/contact_lists/createRequest.jsonsrc/test/resources/api/contact_lists/updateRequest.jsonsrc/test/resources/api/contact_lists/updatedContactList.jsonsrc/test/resources/api/contactevents/contactEvent.jsonsrc/test/resources/api/contactevents/createRequest.json
🧰 Additional context used
🧬 Code graph analysis (6)
src/main/java/io/mailtrap/model/request/contactlists/CreateUpdateContactListRequest.java (4)
src/main/java/io/mailtrap/model/AbstractModel.java (1)
AbstractModel(10-29)src/main/java/io/mailtrap/model/request/sendingdomains/SendingDomainsSetupInstructionsRequest.java (1)
AllArgsConstructor(7-13)src/main/java/io/mailtrap/client/api/MailtrapContactsApi.java (1)
Getter(16-26)src/main/java/io/mailtrap/model/request/contactevents/CreateContactEventRequest.java (1)
Getter(9-14)
src/main/java/io/mailtrap/model/request/contactevents/CreateContactEventRequest.java (4)
src/main/java/io/mailtrap/model/AbstractModel.java (1)
AbstractModel(10-29)src/main/java/io/mailtrap/model/request/sendingdomains/SendingDomainsSetupInstructionsRequest.java (1)
AllArgsConstructor(7-13)src/main/java/io/mailtrap/client/api/MailtrapContactsApi.java (1)
Getter(16-26)src/main/java/io/mailtrap/model/request/contactlists/CreateUpdateContactListRequest.java (1)
Getter(7-11)
src/main/java/io/mailtrap/factory/MailtrapClientFactory.java (1)
src/main/java/io/mailtrap/api/contactevents/ContactEventsImpl.java (1)
ContactEventsImpl(13-29)
examples/java/io/mailtrap/examples/contactevents/ContactEventsExample.java (1)
src/main/java/io/mailtrap/factory/MailtrapClientFactory.java (1)
MailtrapClientFactory(34-132)
src/main/java/io/mailtrap/api/contactevents/ContactEventsImpl.java (2)
src/main/java/io/mailtrap/Constants.java (1)
Constants(6-15)src/main/java/io/mailtrap/api/apiresource/ApiResource.java (1)
ApiResource(10-32)
src/test/java/io/mailtrap/api/contactevents/ContactEventsImplTest.java (3)
src/main/java/io/mailtrap/Constants.java (1)
Constants(6-15)src/test/java/io/mailtrap/testutils/BaseTest.java (1)
BaseTest(6-32)src/test/java/io/mailtrap/testutils/TestHttpClient.java (1)
TestHttpClient(19-160)
🔇 Additional comments (21)
src/test/resources/api/contactevents/contactEvent.json (1)
1-10: LGTM!The test resource JSON is well-structured with appropriate field types for a contact event response. The UUID format for
contact_idand the nestedparamsobject structure are consistent with typical API response patterns.src/test/java/io/mailtrap/testutils/BaseTest.java (1)
29-31: LGTM!The new test utility fields follow the established pattern in the class. The
contactListIdvalue (1337L) aligns with the test resources, and the URL encoding pattern forcontactEventIdEncodedis consistent with other encoded fields in the class.src/test/resources/api/contact_lists/updateRequest.json (1)
1-3: LGTM!The update request test resource is well-formed and consistent with the corresponding
updatedContactList.jsonresponse, which contains the same "Old-Customers" name value.src/test/resources/api/contact_lists/createRequest.json (1)
1-3: LGTM!The create request test resource is properly structured and consistent with the corresponding
contactList.jsonresponse, which includes the same "Customers" name value.src/test/resources/api/contact_lists/updatedContactList.json (1)
1-4: LGTM!The updated contact list response is well-structured and maintains consistency across test resources. The
idvalue (1337) aligns withBaseTest.contactListId, and thenamevalue matches the update request payload.src/test/resources/api/contact_lists/contactList.json (1)
1-4: LGTM!The contact list response resource is properly structured and maintains consistency with both the create request payload ("Customers" name) and the test infrastructure (
id1337 matchesBaseTest.contactListId).src/test/resources/api/contactevents/createRequest.json (1)
1-8: LGTM!The contact event create request is well-structured with a clear event name ("UserLogin") and relevant parameters. The structure aligns with the expected contact event format shown in
contactEvent.json.src/main/java/io/mailtrap/client/api/MailtrapContactsApi.java (2)
3-3: LGTM!The import for
ContactEventsis properly placed in alphabetical order within the existing imports.
25-25: LGTM!The
contactEventsfield integration follows the established pattern in this class. With Lombok's@RequiredArgsConstructorand@Getter(fluent = true), this automatically provides constructor injection and a publiccontactEvents()accessor method, consistent with the other API surface fields.src/main/java/io/mailtrap/model/request/contactevents/CreateContactEventRequest.java (1)
1-14: LGTM!The request class follows the established patterns in the codebase (e.g.,
SendingDomainsSetupInstructionsRequest). The use ofMap<String, Object>forparamsprovides appropriate flexibility for arbitrary event metadata.src/main/java/io/mailtrap/model/request/contactlists/CreateUpdateContactListRequest.java (1)
1-11: LGTM!Simple and clean DTO following the established patterns in the codebase.
examples/java/io/mailtrap/examples/contactevents/ContactEventsExample.java (1)
16-31: LGTM!The example demonstrates the API usage clearly with appropriate configuration setup and request construction using
Map.of()for the params.src/test/java/io/mailtrap/api/contactlists/ContactListsImplTest.java (2)
21-42: LGTM!The test setup properly mocks all CRUD endpoints with appropriate HTTP methods and request/response mappings. The initialization follows the established testing patterns in the codebase.
44-81: LGTM!Good test coverage for the new CRUD operations. Tests verify the expected outcomes appropriately, including using
assertDoesNotThrowfor the delete operation which returns void.src/main/java/io/mailtrap/model/response/contactevents/ContactEventResponse.java (1)
1-21: LGTM!The response DTO correctly uses
@JsonPropertyannotations for snake_case to camelCase mappings where needed. Using Lombok@Datais appropriate for response objects that need full JavaBean functionality.examples/java/io/mailtrap/examples/contactlists/ContactListsExample.java (1)
14-29: LGTM!The CRUD workflow demonstration is clear and well-structured, showing create → update → get by ID → find all operations in a logical sequence.
src/main/java/io/mailtrap/api/contactevents/ContactEvents.java (1)
1-17: LGTM!Clean interface design with appropriate Javadoc documentation. The method signature correctly accepts flexible contact identification (UUID or Email) as documented.
src/main/java/io/mailtrap/factory/MailtrapClientFactory.java (1)
74-83: LGTM!The
ContactEventsImplis correctly wired into the factory following the established patterns. The constructor call appropriately omits theVALIDATORparameter sinceContactEventsImpldoesn't require request validation, consistent with the request model having no validation annotations.src/main/java/io/mailtrap/api/contactevents/ContactEventsImpl.java (1)
20-28: LGTM! Implementation follows correct patterns.The method properly URL-encodes the contact identifier before constructing the API path, which is essential for handling special characters (like the UUID format used in tests).
src/main/java/io/mailtrap/api/contactlists/ContactListsImpl.java (1)
28-64: LGTM! Well-structured CRUD implementation.The new methods follow consistent patterns with proper HTTP verbs:
- POST for creation
- GET for retrieval
- PATCH for updates
- DELETE for removal
All methods correctly use
ContactListResponseas the return type (except delete which appropriately returns void).src/main/java/io/mailtrap/api/contactlists/ContactLists.java (1)
27-52: LGTM! Well-documented API methods.The JavaDoc comments for get, update, and delete methods are clear and accurate, properly describing the parameters and return values.
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 (3)
src/main/java/io/mailtrap/api/contactlists/ContactLists.java (1)
18-52: Standardize JavaDoc style for consistency.The JavaDoc summaries for the new methods have inconsistent capitalization and article usage:
- Line 28: "Get a contact list by ID" (lowercase, with article)
- Line 19: "Create new Contact List" (capitalized, no article)
- Line 37: "Update existing Contact List" (capitalized, no article)
- Line 47: "Delete existing Contact List" (capitalized, no article)
Consider standardizing to lowercase with articles for consistency, e.g., "Create a new contact list", "Update an existing contact list", etc.
🔎 Proposed JavaDoc standardization
/** - * Create new Contact List + * Create a new contact list * * @param accountId unique account ID * @param request body * @return created contact list */ ContactListResponse createContactList(long accountId, CreateUpdateContactListRequest request); /** * Get a contact list by ID * * @param accountId unique account ID * @param contactListId unique contact list ID * @return found contact list */ ContactListResponse getContactList(long accountId, long contactListId); /** - * Update existing Contact List + * Update an existing contact list * * @param accountId unique account ID * @param contactListId unique contact list ID * @param request body * @return updated contact list */ ContactListResponse updateContactList(long accountId, long contactListId, CreateUpdateContactListRequest request); /** - * Delete existing Contact List + * Delete an existing contact list * * @param accountId unique account ID * @param contactListId unique contact list ID */ void deleteContactList(long accountId, long contactListId);examples/java/io/mailtrap/examples/contactevents/ContactEventsExample.java (2)
12-14: Consider adding clarifying comments for placeholder values.To improve user experience, consider adding inline comments indicating that users must replace these placeholder values with their actual credentials and identifiers before running the example.
🔎 Suggested enhancement
- private static final String TOKEN = "<YOUR MAILTRAP TOKEN>"; - private static final long ACCOUNT_ID = 1L; - private static final String CONTACT_IDENTIFIER = "b691272b-3e50-4813-997b-c7c9b317dcb2"; + // Replace with your actual Mailtrap API token + private static final String TOKEN = "<YOUR MAILTRAP TOKEN>"; + // Replace with your actual account ID + private static final long ACCOUNT_ID = 1L; + // Replace with your actual contact identifier (email or UUID) + private static final String CONTACT_IDENTIFIER = "b691272b-3e50-4813-997b-c7c9b317dcb2";
16-31: Error handling is intentionally separated into a dedicated example.The codebase follows a consistent pattern where 24 of 26 examples demonstrate the happy path without error handling. Error handling strategies are covered in
ErrorsExample.java, which demonstrates catchingInvalidRequestBodyExceptionandHttpClientException. If the team decides to include error handling in all examples, this example could be updated to match; however, the current design intentionally keeps examples focused on the primary use case.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/java/io/mailtrap/examples/contactevents/ContactEventsExample.javaexamples/java/io/mailtrap/examples/contactlists/ContactListsExample.javasrc/main/java/io/mailtrap/api/contactlists/ContactLists.java
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/java/io/mailtrap/examples/contactlists/ContactListsExample.java
🧰 Additional context used
🧬 Code graph analysis (1)
examples/java/io/mailtrap/examples/contactevents/ContactEventsExample.java (1)
src/main/java/io/mailtrap/factory/MailtrapClientFactory.java (1)
MailtrapClientFactory(34-132)
🔇 Additional comments (2)
src/main/java/io/mailtrap/api/contactlists/ContactLists.java (1)
18-25: JavaDoc correction applied successfully.The plural form issue flagged in the previous review has been addressed. The JavaDoc now correctly uses the singular form "Create new Contact List."
examples/java/io/mailtrap/examples/contactevents/ContactEventsExample.java (1)
23-27: Request construction looks correct.The use of
Map.of()with mixed types (Integer, String, Boolean) is valid for theMap<String, Object>params field. The event name "UserLogin" and metadata structure demonstrate typical usage patterns.
Motivation
Keep SDK up-to-date, implement missing APIs
Changes
Summary by CodeRabbit
New Features
Examples
Tests
✏️ Tip: You can customize this high-level summary in your review settings.