-
Notifications
You must be signed in to change notification settings - Fork 0
Implemented delete for Sending Domains API #37
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 a deleteSendingDomain API method to the SendingDomains interface and its implementation, adds tests for the DELETE endpoint, and introduces a Java example demonstrating create, get, list, send setup instructions, and delete operations for sending domains. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Ex as Example App
participant SD as SendingDomainsImpl
participant HTTP as HTTP Client
participant MT as Mailtrap API
rect rgba(200,230,255,0.25)
note over Ex: Create domain
Dev->>Ex: run main()
Ex->>SD: createSendingDomain(accountId, domainName)
SD->>HTTP: POST /api/accounts/{id}/sending_domains
HTTP->>MT: POST
MT-->>HTTP: 201 Created
HTTP-->>SD: domain
SD-->>Ex: domain
end
rect rgba(220,255,220,0.18)
note over Ex: Retrieve / List / Send instructions
Ex->>SD: getSendingDomain(accountId, domainId)
SD->>HTTP: GET /api/.../sending_domains/{domainId}
HTTP-->>SD: 200 OK
Ex->>SD: listSendingDomains(accountId)
SD->>HTTP: GET /api/.../sending_domains
HTTP-->>SD: 200 OK
Ex->>SD: sendSetupInstructions(accountId, domainId, email)
SD->>HTTP: POST /api/.../sending_domains/{domainId}/setup_instructions
HTTP-->>SD: 204 No Content
end
rect rgba(255,230,230,0.18)
note over Ex,SD: Delete domain (new)
Ex->>SD: deleteSendingDomain(accountId, domainId)
SD->>HTTP: DELETE /api/accounts/{accountId}/sending_domains/{sendingDomainId}
HTTP->>MT: DELETE
MT-->>HTTP: 204 No Content
HTTP-->>SD: void
SD-->>Ex: void
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks (1 passed, 2 warnings)❌ Failed Checks (2 warnings)
✅ Passed Checks (1 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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 (3)
src/main/java/io/mailtrap/api/sendingdomains/SendingDomainsImpl.java (1)
62-62: Unify String.format specifiers for consistencyOther methods in this class use %s for IDs. Optional nit to keep style consistent.
Apply if you prefer consistency with the rest of the file:
- String.format(apiHost + "/api/accounts/%d/sending_domains/%d", accountId, sendingDomainId), + String.format(apiHost + "/api/accounts/%s/sending_domains/%s", accountId, sendingDomainId),examples/java/io/mailtrap/examples/sendingdomains/SendingDomains.java (2)
10-10: Avoid name clash with API interfaceRenaming the example class reduces confusion with io.mailtrap.api.sendingdomains.SendingDomains.
-public class SendingDomains { +public class SendingDomainsExample {Note: rename the file to SendingDomainsExample.java accordingly.
12-12: Don’t hardcode tokens; read from environmentSafer for copy-paste usage and avoids accidental commits of secrets.
- private static final String TOKEN = "<YOUR MAILTRAP TOKEN>"; + private static final String TOKEN = System.getenv("MAILTRAP_TOKEN"); @@ - final var config = new MailtrapConfig.Builder() - .token(TOKEN) - .build(); + if (TOKEN == null || TOKEN.isBlank()) { + throw new IllegalStateException("Set MAILTRAP_TOKEN env var."); + } + final var config = new MailtrapConfig.Builder().token(TOKEN).build();Also applies to: 18-21
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
examples/java/io/mailtrap/examples/sendingdomains/SendingDomains.java(1 hunks)src/main/java/io/mailtrap/api/sendingdomains/SendingDomains.java(1 hunks)src/main/java/io/mailtrap/api/sendingdomains/SendingDomainsImpl.java(1 hunks)src/test/java/io/mailtrap/api/sendingdomains/SendingDomainsImplTest.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
examples/java/io/mailtrap/examples/sendingdomains/SendingDomains.java (1)
src/main/java/io/mailtrap/factory/MailtrapClientFactory.java (1)
MailtrapClientFactory(31-116)
src/test/java/io/mailtrap/api/sendingdomains/SendingDomainsImplTest.java (1)
src/main/java/io/mailtrap/Constants.java (1)
Constants(6-15)
🔇 Additional comments (4)
src/main/java/io/mailtrap/api/sendingdomains/SendingDomains.java (1)
46-52: Delete API addition — LGTMInterface surface looks consistent with existing methods.
src/main/java/io/mailtrap/api/sendingdomains/SendingDomainsImpl.java (1)
58-66: DELETE implementation is correctEndpoint path, HTTP verb, and Void response match expectations and existing patterns.
src/test/java/io/mailtrap/api/sendingdomains/SendingDomainsImplTest.java (2)
26-45: Mocks cover the new DELETE endpointThe DataMock for DELETE with null request/response aligns with the Void contract.
90-93: Happy-path delete test — LGTMassertDoesNotThrow is appropriate given a Void delete.
Motivation
Implement the DELETE method for the Sending Domains API to keep the SDK up-to-date
Changes
Summary by CodeRabbit
New Features
Documentation
Tests