Skip to content

Conversation

@vitalii-t
Copy link
Collaborator

@vitalii-t vitalii-t commented Sep 7, 2025

Motivation

Implement the DELETE method for the Sending Domains API to keep the SDK up-to-date

Changes

  • Added DELETE method, unit test
  • Added example class

Summary by CodeRabbit

  • New Features

    • Added ability to delete a sending domain via the API client.
  • Documentation

    • Added a Java example demonstrating an end-to-end Sending Domains workflow: configuration, creation, retrieval, listing, sending setup instructions, and deletion.
  • Tests

    • Added test coverage for deleting a sending domain to ensure stability and reliability.

@coderabbitai
Copy link

coderabbitai bot commented Sep 7, 2025

Walkthrough

Adds 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

Cohort / File(s) Summary
Example: Sending Domains end-to-end
examples/java/io/mailtrap/examples/sendingdomains/SendingDomains.java
New example app showing create, get, list, send setup instructions, and delete operations for sending domains.
API Interface: SendingDomains
src/main/java/io/mailtrap/api/sendingdomains/SendingDomains.java
Added void deleteSendingDomain(long accountId, long sendingDomainId); to the interface.
API Implementation: SendingDomainsImpl
src/main/java/io/mailtrap/api/sendingdomains/SendingDomainsImpl.java
Implemented deleteSendingDomain performing HTTP DELETE to /api/accounts/{accountId}/sending_domains/{sendingDomainId}.
Tests: SendingDomainsImpl
src/test/java/io/mailtrap/api/sendingdomains/SendingDomainsImplTest.java
Added mock entry for DELETE endpoint and test test_deleteSendingDomain() asserting no exception is thrown. Minor formatting tweaks.

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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks (1 passed, 2 warnings)

❌ Failed Checks (2 warnings)
Check Name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description does not follow the repository template because it is missing the “How to test” section with required test steps and the “Images and GIFs” section. Please update the description to include a “## How to test” section with detailed steps (with checkboxes) and the “## Images and GIFs” section following the template format.
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed Checks (1 passed)
Check Name Status Explanation
Title Check ✅ Passed The title concisely and accurately summarizes the primary change of adding delete functionality to the Sending Domains API, making it clear and relevant for reviewers scanning the history.

Poem

I nibble keys and hop through code,
I spin up domains down the road.
Create, inspect, instruct — then flee,
A gentle DELETE, and free am I.
Carrots crunch; the repo hums with glee. 🥕

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.

  - Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.
  - Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 61ae2dd and eaded98.

📒 Files selected for processing (1)
  • examples/java/io/mailtrap/examples/sendingdomains/SendingDomains.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • examples/java/io/mailtrap/examples/sendingdomains/SendingDomains.java
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/delete-sending-domain

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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 consistency

Other 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 interface

Renaming 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 environment

Safer 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

📥 Commits

Reviewing files that changed from the base of the PR and between b3ae7da and 61ae2dd.

📒 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 — LGTM

Interface surface looks consistent with existing methods.

src/main/java/io/mailtrap/api/sendingdomains/SendingDomainsImpl.java (1)

58-66: DELETE implementation is correct

Endpoint 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 endpoint

The DataMock for DELETE with null request/response aligns with the Void contract.


90-93: Happy-path delete test — LGTM

assertDoesNotThrow is appropriate given a Void delete.

@vitalii-t vitalii-t merged commit 7841c8c into main Sep 8, 2025
2 checks passed
@vitalii-t vitalii-t deleted the feature/delete-sending-domain branch September 8, 2025 20:30
@vitalii-t vitalii-t mentioned this pull request Sep 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants