Implemented ContactEvents and missing ContactLists APIs #46
Conversation
📝 WalkthroughWalkthroughThe changes add contact events functionality to the Mailtrap Java SDK with new request/response models and API operations, expand contact lists with full CRUD support, and refactor the projects API request type from CreateUpdateProjectRequest to ProjectRequest for consistency. Corresponding example code and comprehensive test coverage are included. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
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.
|
|
||
| @Getter | ||
| @AllArgsConstructor | ||
| public class CreateUpdateContactListRequest extends AbstractModel { |
There was a problem hiding this comment.
| public class CreateUpdateContactListRequest extends AbstractModel { | |
| public class UpdateContactListRequest extends AbstractModel { |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/io/mailtrap/model/request/contactlists/ContactListRequest.java (1)
7-11: Consider adding input validation for the name field.The
ProjectRequestclass applies@Size(min = 2, max = 100)validation to its name field. For consistency and to prevent invalid input (empty or excessively long names), consider adding similar validation here.🔎 Proposed validation addition
+import jakarta.validation.constraints.Size; + @Getter @AllArgsConstructor public class ContactListRequest extends AbstractModel { + @Size(min = 2, max = 100) private String name; }Based on the pattern established in
ProjectRequest.java(lines 20-21).src/test/java/io/mailtrap/api/contactlists/ContactListsImplTest.java (1)
16-16: Consider using specific assertion imports instead of wildcard.While wildcard imports are common for test assertions, specific imports improve IDE navigation and make dependencies more explicit.
examples/java/io/mailtrap/examples/contactlists/ContactListsExample.java (1)
21-34: Consider adding feedback after the delete operation.The example demonstrates a complete CRUD flow, which is excellent. However, the delete operation at line 33 has no confirmation or verification. Consider adding a success message or attempting to retrieve the deleted item to demonstrate that deletion succeeded.
🔎 Suggested addition
client.contactsApi().contactLists().deleteContactList(ACCOUNT_ID, byId.getId()); +System.out.println("Contact list deleted successfully");Or verify deletion:
client.contactsApi().contactLists().deleteContactList(ACCOUNT_ID, byId.getId()); +System.out.println("Deleted. Remaining lists: " + client.contactsApi().contactLists().findAll(ACCOUNT_ID).size());
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
examples/java/io/mailtrap/examples/contactlists/ContactListsExample.javaexamples/java/io/mailtrap/examples/testing/ProjectsExample.javasrc/main/java/io/mailtrap/api/contactlists/ContactLists.javasrc/main/java/io/mailtrap/api/contactlists/ContactListsImpl.javasrc/main/java/io/mailtrap/api/projects/Projects.javasrc/main/java/io/mailtrap/api/projects/ProjectsImpl.javasrc/main/java/io/mailtrap/model/request/contactlists/ContactListRequest.javasrc/main/java/io/mailtrap/model/request/projects/ProjectRequest.javasrc/test/java/io/mailtrap/api/contactlists/ContactListsImplTest.javasrc/test/java/io/mailtrap/api/projects/ProjectsImplTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/io/mailtrap/api/contactlists/ContactListsImpl.java
- src/main/java/io/mailtrap/api/contactlists/ContactLists.java
🧰 Additional context used
🧬 Code graph analysis (3)
src/test/java/io/mailtrap/api/contactlists/ContactListsImplTest.java (1)
src/main/java/io/mailtrap/Constants.java (1)
Constants(6-15)
examples/java/io/mailtrap/examples/contactlists/ContactListsExample.java (1)
src/main/java/io/mailtrap/factory/MailtrapClientFactory.java (1)
MailtrapClientFactory(34-132)
src/main/java/io/mailtrap/model/request/contactlists/ContactListRequest.java (3)
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/model/request/projects/ProjectRequest.java (2)
Getter(9-26)Getter(16-24)
🔇 Additional comments (10)
src/test/java/io/mailtrap/api/projects/ProjectsImplTest.java (1)
6-7: LGTM! Test updates are consistent with the class rename.All test references have been properly updated to use
ProjectRequestandProjectRequest.ProjectData. The test logic remains unchanged.Also applies to: 54-54, 83-83
src/main/java/io/mailtrap/api/projects/Projects.java (1)
3-3: LGTM! Interface signatures properly updated.The method signatures for
createProjectandupdateProjecthave been correctly updated to useProjectRequest, maintaining consistency with the renamed class.Also applies to: 21-21, 48-48
examples/java/io/mailtrap/examples/testing/ProjectsExample.java (1)
5-5: LGTM! Example code updated correctly.The example demonstrates proper usage of the renamed
ProjectRequestclass with correct import and constructor syntax.Also applies to: 25-25
src/main/java/io/mailtrap/api/projects/ProjectsImpl.java (1)
8-8: LGTM! Implementation properly updated to use the renamed class.The method implementations for
createProjectandupdateProjecthave been correctly updated to acceptProjectRequestparameters. The implementation logic remains unchanged.Also applies to: 22-22, 53-53
src/main/java/io/mailtrap/model/request/projects/ProjectRequest.java (1)
11-11: LGTM! Class rename improves naming consistency.The rename from
CreateUpdateProjectRequesttoProjectRequestaligns with the PR's objective to standardize request type naming. All references to the old class name have been updated throughout the codebase. This is a breaking change for external SDK users, but the consistency improvement is worthwhile.src/main/java/io/mailtrap/model/request/contactlists/ContactListRequest.java (1)
1-11: LGTM! Clean and simple request model.The structure follows the established pattern of extending
AbstractModeland using Lombok annotations to minimize boilerplate.src/test/java/io/mailtrap/api/contactlists/ContactListsImplTest.java (3)
23-34: LGTM! Comprehensive mock setup for CRUD operations.The HTTP stubs correctly cover all CRUD operations with appropriate HTTP methods (POST, GET, PATCH, DELETE) and properly constructed endpoint paths.
52-66: LGTM! Create and read operations properly tested.Both
test_createandtest_findOneinclude appropriate assertions to verify the operations succeed and return expected data.
68-80: LGTM! Update and delete operations properly tested.The
test_updatemethod validates both the ID and the updated name value, whiletest_deleteappropriately verifies that deletion completes without throwing an exception.examples/java/io/mailtrap/examples/contactlists/ContactListsExample.java (1)
11-12: LGTM! Clear constant naming.The constants clearly indicate their purpose in the CRUD flow demonstration.
Motivation
Keep SDK up-to-date, implement missing APIs
Changes
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.