Skip to content

Conversation

@Ihor-Bilous
Copy link
Collaborator

@Ihor-Bilous Ihor-Bilous commented Dec 1, 2025

Motivation

In this MR I added SendingDomainsApi, related models, tests, examples.

Changes

  • README.md updated with new link to examples

  • Change 1

  • Change 2

How to test

  • see examples/sending_domains/sending_domains.py

Summary by CodeRabbit

  • New Features

    • Added a Sending Domains API: list domains, view details, create, delete, and send setup instructions.
  • Documentation

    • Added Sending Domains API reference and usage example.
  • Tests

    • Added unit tests covering API operations, models, success paths, and error handling.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 1, 2025

Walkthrough

Adds a Sending Domains feature: new pydantic models, an API resource class with CRUD and "send setup instructions" endpoints, client integration exposing the feature, example usage script, and unit tests covering success and error paths.

Changes

Cohort / File(s) Summary
Documentation & Example
README.md, examples/sending_domains/sending_domains.py
README: added Sending Domains API entry and link to example. New example script demonstrating list/get/create/delete and send setup instructions flows using placeholder credentials.
Models
mailtrap/models/sending_domains.py
New pydantic model types: SendingDomainPermissions, DnsRecord, SendingDomain, CreateSendingDomainParams, SendSetupInstructionsParams, SendSetupInstructionsResponse.
API Resource
mailtrap/api/resources/sending_domains.py
New SendingDomainsApi class implementing get_list, get_by_id, create, delete, send_setup_instructions, and helper _api_path.
API Surface / Base
mailtrap/api/sending_domains.py
New SendingDomainsBaseApi exposing sending_domains property that returns a configured SendingDomainsApi.
Client Integration
mailtrap/client.py, mailtrap/__init__.py
MailtrapClient gains sending_domains_api property; public imports CreateSendingDomainParams and SendSetupInstructionsParams added to package init.
Tests
tests/unit/api/sending_domains/test_sending_domains.py, tests/unit/models/test_sending_domains.py
New unit tests covering success and error cases for API methods and verifying model serialization for request params.

Sequence Diagram(s)

sequenceDiagram
    participant User as Example Script / Caller
    participant Client as MailtrapClient
    participant BaseAPI as SendingDomainsBaseApi
    participant ResAPI as SendingDomainsApi
    participant HTTP as HttpClient
    participant Models as Response Models

    User->>Client: access sending_domains_api
    Client->>Client: validate account_id
    Client->>BaseAPI: construct with client & account_id
    BaseAPI->>ResAPI: instantiate SendingDomainsApi
    ResAPI-->>User: configured API returned

    Note over User,ResAPI: Example flow — create then fetch
    User->>ResAPI: create(CreateSendingDomainParams)
    ResAPI->>ResAPI: build POST /api/accounts/{id}/sending_domains
    ResAPI->>HTTP: POST request (body params)
    HTTP-->>Models: return JSON
    Models->>ResAPI: map to SendingDomain
    ResAPI-->>User: return SendingDomain

    User->>ResAPI: send_setup_instructions(id, SendSetupInstructionsParams)
    ResAPI->>HTTP: POST /api/accounts/{id}/sending_domains/{id}/send_setup_instructions
    HTTP-->>Models: return JSON {message}
    Models->>ResAPI: map to SendSetupInstructionsResponse
    ResAPI-->>User: return SendSetupInstructionsResponse
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

  • Review model field defaults and Optional usage (especially dns_records = Field(default_factory=list)).
  • Verify _api_path() builds correct endpoints with and without domain ID.
  • Check serialization behavior of CreateSendingDomainParams and SendSetupInstructionsParams (inheritance from RequestParams).
  • Inspect unit tests to ensure error-case HTTP status handling matches client error mapping.
  • Confirm MailtrapClient.sending_domains_api account_id validation and returned instance wiring.

Suggested reviewers

  • IgorDobryn
  • VladimirTaytor
  • andrii-porokhnavets

Poem

🐰 I hopped through code and planted a domain,
Endpoints sprouted up after the rain.
Models and tests snug in a row,
Clients and examples ready to go —
A carrot of features, delivered with gain. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.13% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ❓ Inconclusive The description lacks detail and contains incomplete placeholder sections ('Change 1', 'Change 2'). While it mentions the main additions, it doesn't comprehensively describe the implementation or provide proper testing guidance. Replace placeholder sections with specific changes made (e.g., new API resource class, model definitions, HTTP endpoints). Expand 'How to test' with clear testing steps beyond just referencing the example file.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and clearly summarizes the main change: adding SendingDomainsApi with related models, tests, and examples.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ISSUE-54

📜 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 96b9ce7 and 97d92e4.

📒 Files selected for processing (9)
  • README.md (1 hunks)
  • examples/sending_domains/sending_domains.py (1 hunks)
  • mailtrap/__init__.py (1 hunks)
  • mailtrap/api/resources/sending_domains.py (1 hunks)
  • mailtrap/api/sending_domains.py (1 hunks)
  • mailtrap/client.py (2 hunks)
  • mailtrap/models/sending_domains.py (1 hunks)
  • tests/unit/api/sending_domains/test_sending_domains.py (1 hunks)
  • tests/unit/models/test_sending_domains.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
  • README.md
  • tests/unit/api/sending_domains/test_sending_domains.py
  • mailtrap/api/sending_domains.py
  • mailtrap/models/sending_domains.py
  • mailtrap/init.py
  • tests/unit/models/test_sending_domains.py
🧰 Additional context used
🧬 Code graph analysis (2)
mailtrap/client.py (3)
mailtrap/api/sending_domains.py (2)
  • sending_domains (11-12)
  • SendingDomainsBaseApi (5-12)
tests/unit/api/sending_domains/test_sending_domains.py (1)
  • sending_domains_api (25-26)
mailtrap/http.py (1)
  • HttpClient (14-106)
examples/sending_domains/sending_domains.py (5)
mailtrap/api/sending_domains.py (1)
  • sending_domains (11-12)
mailtrap/models/sending_domains.py (4)
  • SendingDomain (27-42)
  • SendSetupInstructionsResponse (56-57)
  • CreateSendingDomainParams (46-47)
  • SendSetupInstructionsParams (51-52)
mailtrap/client.py (1)
  • sending_domains_api (98-103)
tests/unit/api/sending_domains/test_sending_domains.py (1)
  • sending_domains_api (25-26)
mailtrap/api/resources/sending_domains.py (5)
  • get_list (16-22)
  • get_by_id (24-29)
  • create (31-40)
  • delete (42-47)
  • send_setup_instructions (49-63)
🪛 Ruff (0.14.6)
examples/sending_domains/sending_domains.py

6-6: Possible hardcoded password assigned to: "API_TOKEN"

(S105)

⏰ 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). (5)
  • GitHub Check: Test python3.10 on windows-latest
  • GitHub Check: Test python3.12 on windows-latest
  • GitHub Check: Test python3.13 on windows-latest
  • GitHub Check: Test python3.9 on windows-latest
  • GitHub Check: Test python3.11 on windows-latest
🔇 Additional comments (15)
mailtrap/client.py (2)

11-11: LGTM!

The import follows the established pattern and alphabetical ordering.


97-103: LGTM!

The implementation follows the exact pattern of other API properties in the client, with proper account_id validation and consistent configuration.

mailtrap/api/resources/sending_domains.py (7)

1-14: LGTM!

Imports are well-organized and the class initialization follows the established pattern from other API resources.


16-22: LGTM!

The method correctly extracts the domain list from the response and safely handles the data structure.


24-29: LGTM!

The method is correctly implemented and the docstring grammar is accurate.


31-40: LGTM!

The method correctly wraps the domain parameters in the expected API structure and provides helpful guidance in the docstring about checking domain status.


42-47: LGTM!

The delete method correctly calls the endpoint and returns a consistent DeletedObject, which is a reasonable pattern for delete operations.


49-63: LGTM!

The method correctly constructs the endpoint path and creates an appropriate response object. Hardcoding the success message is acceptable if the API returns minimal response data.


65-69: LGTM!

The helper method cleanly constructs API paths for both collection and individual resource endpoints.

examples/sending_domains/sending_domains.py (6)

1-10: LGTM!

The imports and client setup are correct. Line 10 demonstrates the proper way to access the SendingDomainsApi through the client's property chain.

Note: The static analysis warning about a hardcoded password on line 6 is a false positive—these are clearly example placeholders.


13-14: LGTM!

Clean demonstration of listing sending domains.


17-18: LGTM!

Clear demonstration of fetching a domain by ID.


21-23: LGTM!

Well-structured example showing how to create a sending domain with parameters.


26-32: LGTM!

Both functions clearly demonstrate their respective operations—delete by ID and sending setup instructions with email parameter.


35-49: LGTM!

The main block provides a complete walkthrough of all available operations in a logical sequence, serving as an excellent reference for users.


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: 1

🧹 Nitpick comments (2)
mailtrap/models/sending_domains.py (2)

16-23: DNS record model is straightforward; consider stricter types only if API is stable

The DnsRecord dataclass cleanly mirrors a typical DNS record payload. If the API’s status or type fields have a known finite set of values, you might later consider Literal[...]/Enum types for stronger validation, but it’s not required for this PR.


26-42: SendingDomain aggregation and defaults look correct; one minor optional improvement

  • Composition of booleans, permissions, and dns_records is consistent and matches how you’d typically model the resource.
  • Using Field(default_factory=list) for dns_records avoids shared mutable defaults, which is correct in combination with pydantic.dataclasses.dataclass.
  • Optional string fields for alert_recipient_email and dns_verified_at work fine with exclude_none=True in RequestParams.api_data for request-type usage.

If the backend returns dns_verified_at as an ISO8601 timestamp and you want automatic parsing, you could switch that field to Optional[datetime] later; otherwise leaving it as str is perfectly acceptable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 64f1941 and 96b9ce7.

📒 Files selected for processing (9)
  • README.md (1 hunks)
  • examples/sending_domains/sending_domains.py (1 hunks)
  • mailtrap/__init__.py (1 hunks)
  • mailtrap/api/resources/sending_domains.py (1 hunks)
  • mailtrap/api/sending_domains.py (1 hunks)
  • mailtrap/client.py (2 hunks)
  • mailtrap/models/sending_domains.py (1 hunks)
  • tests/unit/api/sending_domains/test_sending_domains.py (1 hunks)
  • tests/unit/models/test_sending_domains.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
examples/sending_domains/sending_domains.py (4)
mailtrap/models/common.py (1)
  • DeletedObject (23-24)
mailtrap/models/sending_domains.py (4)
  • SendingDomain (27-42)
  • SendSetupInstructionsResponse (56-57)
  • CreateSendingDomainParams (46-47)
  • SendSetupInstructionsParams (51-52)
mailtrap/client.py (1)
  • sending_domains_api (98-103)
mailtrap/api/resources/sending_domains.py (5)
  • get_list (16-22)
  • get_by_id (24-29)
  • create (31-40)
  • delete (42-47)
  • send_setup_instructions (49-63)
tests/unit/models/test_sending_domains.py (1)
mailtrap/models/sending_domains.py (2)
  • CreateSendingDomainParams (46-47)
  • SendSetupInstructionsParams (51-52)
mailtrap/models/sending_domains.py (2)
mailtrap/models/common.py (1)
  • RequestParams (13-19)
mailtrap/api/general.py (1)
  • permissions (25-26)
mailtrap/api/sending_domains.py (2)
mailtrap/api/resources/sending_domains.py (1)
  • SendingDomainsApi (11-69)
mailtrap/http.py (1)
  • HttpClient (14-106)
tests/unit/api/sending_domains/test_sending_domains.py (5)
mailtrap/api/resources/sending_domains.py (6)
  • SendingDomainsApi (11-69)
  • get_list (16-22)
  • get_by_id (24-29)
  • create (31-40)
  • delete (42-47)
  • send_setup_instructions (49-63)
mailtrap/http.py (2)
  • HttpClient (14-106)
  • post (32-34)
mailtrap/models/common.py (1)
  • DeletedObject (23-24)
mailtrap/models/sending_domains.py (4)
  • CreateSendingDomainParams (46-47)
  • SendingDomain (27-42)
  • SendSetupInstructionsParams (51-52)
  • SendSetupInstructionsResponse (56-57)
mailtrap/client.py (1)
  • sending_domains_api (98-103)
mailtrap/__init__.py (2)
mailtrap/api/sending_domains.py (1)
  • sending_domains (11-12)
mailtrap/models/sending_domains.py (2)
  • CreateSendingDomainParams (46-47)
  • SendSetupInstructionsParams (51-52)
🪛 Ruff (0.14.6)
examples/sending_domains/sending_domains.py

6-6: Possible hardcoded password assigned to: "API_TOKEN"

(S105)

⏰ 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)
  • GitHub Check: Test python3.9 on windows-latest
🔇 Additional comments (14)
README.md (1)

244-246: LGTM!

The new Sending Domains API section follows the established documentation pattern and correctly links to the example file.

tests/unit/models/test_sending_domains.py (1)

1-14: LGTM!

The model tests are well-structured and follow the existing test patterns. They correctly verify that the api_data property returns the expected dictionary for both parameter classes.

mailtrap/client.py (1)

97-103: LGTM!

The sending_domains_api property follows the established pattern used by other API properties in the client. It correctly validates the account_id and initializes the SendingDomainsBaseApi with the appropriate configuration.

mailtrap/__init__.py (1)

33-34: LGTM!

The new model exports follow the existing pattern and are properly placed in alphabetical order with other model imports.

mailtrap/api/sending_domains.py (1)

1-12: LGTM!

The SendingDomainsBaseApi class follows the established pattern for base API wrappers in the codebase. It provides a clean abstraction layer for accessing the SendingDomainsApi with pre-configured account and client instances.

tests/unit/api/sending_domains/test_sending_domains.py (1)

1-346: LGTM!

This test suite is comprehensive and well-organized. It provides excellent coverage of all API methods with both success and error scenarios. The use of pytest fixtures and parametrization makes the tests clean and maintainable.

examples/sending_domains/sending_domains.py (1)

1-49: LGTM!

The example script clearly demonstrates all available operations for the Sending Domains API. It follows the patterns established by other example scripts in the repository.

Note: The static analysis warning about a hardcoded password on line 6 is a false positive—this is just a placeholder string in example code that users need to replace with their actual API token.

mailtrap/api/resources/sending_domains.py (5)

16-22: LGTM!

The get_list method correctly handles the API response structure by extracting the "data" field with a defensive default. The list comprehension properly instantiates SendingDomain objects from the response data.


31-40: LGTM!

The create method properly wraps the domain parameters in a {"sending_domain": ...} structure, which aligns with the API's expected request format. The response is correctly parsed into a SendingDomain object.


42-47: LGTM!

The delete method follows a clean pattern by delegating to the HTTP client and returning a DeletedObject with the domain ID. This is consistent with similar delete operations in the codebase.


49-63: LGTM!

The send_setup_instructions method correctly handles the 204 (No Content) response from the API by constructing a SendSetupInstructionsResponse with a descriptive success message. This is an appropriate pattern when the API doesn't return meaningful response data.


65-69: LGTM!

The _api_path helper method cleanly constructs API paths with optional domain ID. The implementation is straightforward and follows good practices.

mailtrap/models/sending_domains.py (2)

9-13: Permissions model looks clean and sufficient

The SendingDomainPermissions dataclass is minimal, well-typed, and aligns with the common read/update/destroy permission pattern. No changes needed here.


45-57: Params/response modeling is compatible with RequestParams.api_data

  • CreateSendingDomainParams and SendSetupInstructionsParams inheriting from RequestParams as pydantic dataclasses should integrate cleanly with the existing api_data helper (single required field each, no defaults).
  • SendSetupInstructionsResponse as a simple dataclass with message: str matches a typical API “acknowledge” payload and will be easy to validate/serialize via pydantic.

No issues spotted here.

@Ihor-Bilous Ihor-Bilous self-assigned this Dec 1, 2025
@yanchuk yanchuk linked an issue Dec 1, 2025 that may be closed by this pull request
@yanchuk yanchuk merged commit 858e834 into main Dec 2, 2025
16 checks passed
@yanchuk yanchuk deleted the ISSUE-54 branch December 2, 2025 10:27
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.

Add Domains management support

4 participants