Fix issue #24: Add batch_send method to SendingApi, add models #47
Conversation
WalkthroughIntroduces batch email sending: new batch models and response types, a SendingApi.batch_send method hitting /api/batch, a MailtrapClient.batch_send wrapper with a new response alias, example updates to use batch params, public exports adjusted, legacy base/template model files removed, and tests added/updated for batch flows and model api_data. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Caller
participant MC as MailtrapClient
participant SA as SendingApi
participant HTTP as Mailtrap REST API
Dev->>MC: batch_send(params: BatchSendEmailParams)
MC->>SA: batch_send(params)
SA->>HTTP: POST /api/batch<br/>Body: params.api_data
HTTP-->>SA: 200/4xx/5xx + JSON
SA-->>MC: BatchSendResponse
MC-->>Dev: BATCH_SEND_ENDPOINT_RESPONSE (serialized)
note over SA,HTTP: New endpoint and response shape
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches<
8000
/summary>
🧪 Generate unit tests
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.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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.
Actionable comments posted: 0
🧹 Nitpick comments (18)
mailtrap/models/mail/mail.py (1)
12-21: Reduce duplication between BaseMail and BaseBatchMail (optional)Shared fields (sender alias, attachments, headers, custom_variables, reply_to) exist in both hierarchies. Consider a small shared mixin to DRY this up.
Also applies to: 32-35
tests/unit/models/mail/test_batch_mail.py (5)
1-6: Import ClassVar for class-level test fixturesRuff RUF012: annotate mutable class attributes with ClassVar to silence lint. Add import here.
Apply this diff:
+from typing import ClassVar from mailtrap.models.mail import Address from mailtrap.models.mail import Attachment from mailtrap.models.mail im 8000 port BatchEmailRequest from mailtrap.models.mail import BatchMail from mailtrap.models.mail import BatchMailFromTemplate
8-14: Annotate class attributes with ClassVar (RUF012)Prevents false positives about mutable class attributes.
Apply this diff:
class TestBatchMail: - ADDRESS = Address(email="joe@mail.com") - ADDRESS_API_DATA = {"email": "joe@mail.com"} + ADDRESS: ClassVar[Address] = Address(email="joe@mail.com") + ADDRESS_API_DATA: ClassVar[dict[str, str]] = {"email": "joe@mail.com"} - ATTACHMENT = Attachment(content=b"base64_content", filename="file.txt") - ATTACHMENT_API_DATA = {"content": "base64_content", "filename": "file.txt"} + ATTACHMENT: ClassVar[Attachment] = Attachment( + content=b"base64_content", filename="file.txt" + ) + ATTACHMENT_API_DATA: ClassVar[dict[str, str]] = { + "content": "base64_content", + "filename": "file.txt", + }
53-59: Fix typo in test class name“Tremplate” → “Template” for consistency.
Apply this diff:
-class TestBatchMailFromTremplate: +class TestBatchMailFromTemplate:
60-68: Annotate class attributes with ClassVar (RUF012)Repeat pattern for this class.
Apply this diff:
- ADDRESS = Address(email="joe@mail.com") - ADDRESS_API_DATA = {"email": "joe@mail.com"} + ADDRESS: ClassVar[Address] = Address(email="joe@mail.com") + ADDRESS_API_DATA: ClassVar[dict[str, str]] = {"email": "joe@mail.com"} - ATTACHMENT = Attachment(content=b"base64_content", filename="file.txt") - ATTACHMENT_API_DATA = {"content": "base64_content", "filename": "file.txt"} + ATTACHMENT: ClassVar[Attachment] = Attachment( + content=b"base64_content", filename="file.txt" + ) + ATTACHMENT_API_DATA: ClassVar[dict[str, str]] = { + "content": "base64_content", + "filename": "file.txt", + }
92-103: Annotate class attributes with ClassVar (RUF012)Repeat pattern for the third test class.
Apply this diff:
- ADDRESS = Address(email="joe@mail.com") - ADDRESS_API_DATA = {"email": "joe@mail.com"} + ADDRESS: ClassVar[Address] = Address(email="joe@mail.com") + ADDRESS_API_DATA: ClassVar[dict[str, str]] = {"email": "joe@mail.com"} - ATTACHMENT = Attachment(content=b"base64_content", filename="file.txt") - ATTACHMENT_API_DATA = {"content": "base64_content", "filename": "file.txt"} + ATTACHMENT: ClassVar[Attachment] = Attachment( + content=b"base64_content", filename="file.txt" + ) + ATTACHMENT_API_DATA: ClassVar[dict[str, str]] = { + "content": "base64_content", + "filename": "file.txt", + }mailtrap/models/mail/__init__.py (1)
4-13: Exports look consistent; consider adding BatchSendResponseItemPublic surface includes batch types. If external users need to type individual response entries, re-export BatchSendResponseItem too.
Also applies to: 15-29
tests/unit/models/mail/test_mail.py (4)
1-5: Import ClassVar for class-level fixtures (RUF012)Add the import to annotate fixtures.
Apply this diff:
from mailtrap.models.mail import Address from mailtrap.models.mail import Attachment from mailtrap.models.mail import Mail from mailtrap.models.mail import MailFromTemplate +from typing import ClassVar
7-13: Annotate class attributes with ClassVar (RUF012)Silences mutable class attribute warnings.
Apply this diff:
class TestMail: - ADDRESS = Address(email="joe@mail.com") - ADDRESS_API_DATA = {"email": "joe@mail.com"} + ADDRESS: ClassVar[Address] = Address(email="joe@mail.com") + ADDRESS_API_DATA: ClassVar[dict[str, str]] = {"email": "joe@mail.com"} - ATTACHMENT = Attachment(content=b"base64_content", filename="file.txt") - ATTACHMENT_API_DATA = {"content": "base64_content", "filename": "file.txt"} + ATTACHMENT: ClassVar[Attachment] = Attachment( + content=b"base64_content", filename="file.txt" + ) + ATTACHMENT_API_DATA: ClassVar[dict[str, str]] = { + "content": "base64_content", + "filename": "file.txt", + }
58-58: Fix typo in test class name“Tremplate” → “Template”.
Apply this diff:
-class TestMailFromTremplate: +class TestMailFromTemplate:
59-66: Annotate class attributes with ClassVar (RUF012)Repeat pattern for this class.
Apply this diff:
- ADDRESS = Address(email="joe@mail.com") - ADDRESS_API_DATA = {"email": "joe@mail.com"} + ADDRESS: ClassVar[Address] = Address(email="joe@mail.com") + ADDRESS_API_DATA: ClassVar[dict[str, str]] = {"email": "joe@mail.com"} - ATTACHMENT = Attachment(content=b"base64_content", filename="file.txt") - ATTACHMENT_API_DATA = {"content": "base64_content", "filename": "file.txt"} + ATTACHMENT: ClassVar[Attachment] = Attachment( + content=b"base64_content", filename="file.txt" + ) + ATTACHMENT_API_DATA: ClassVar[dict[str, str]] = { + "content": "base64_content", + "filename": "file.txt", + }mailtrap/client.py (1)
24-27: Consider TypedDict for response types (optional)TypedDicts would document response keys more precisely than dict[str, Union[…]].
mailtrap/__init__.py (1)
19-22: Consider also exporting BatchSendResponse and BatchSendResponseItemFor parity with SEND types and to simplify imports in user code, consider re-exporting these response models at the root.
Apply this diff:
from .models.mail import BatchEmailRequest from .models.mail import BatchMail from .models.mail import BatchMailFromTemplate from .models.mail import BatchSendEmailParams +from .models.mail import BatchSendResponse +from .models.mail import BatchSendResponseItemexamples/sending.py (2)
33-46: Example is minimal; consider showing multiple requestsAdd a second BatchEmailRequest to better illustrate batching and per-request overrides (e.g., subject or cc/bcc).
47-68: batch_mail_from_template is defined but not demonstratedUse it in main to showcase the template-based path.
Apply this diff:
if __name__ == "__main__": print(send(default_client, mail)) print(batch_send(default_client, batch_mail)) + print(batch_send(default_client, batch_mail_from_template))mailtrap/api/sending.py (1)
25-36: Preflight-validate batch size and emptinessThe docstring notes a 500-message cap; add client-side checks to fail fast and offer clearer errors.
Apply this diff:
def batch_send(self, mail: BatchSendEmailParams) -> BatchSendResponse: """ Batch send email (text, html, text&html, templates). Please note that @@ - response = self._client.post(self._get_api_url("/api/batch"), json=mail.api_data) + # Fail fast on obvious contract violations before making a network call. + if not mail.requests: + raise ValueError("Batch requests cannot be empty") + if len(mail.requests) > 500: + raise ValueError("Batch supports up to 500 messages per API call") + + response = self._client.post(self._get_api_url("/api/batch"), json=mail.api_data) return BatchSendResponse(**response)mailtrap/models/mail/batch_mail.py (2)
36-50: Avoid overriding 'sender' from required to optional in a subclassBatchEmailRequest inherits BaseBatchMail (sender: Address required) and relaxes to Optional[Address] with type: ignore. This is a type safety smell and easy to miss at call sites.
Consider extracting shared fields and splitting hierarchies:
- Create a common mixin for attachments/headers/custom_variables/reply_to.
- Keep BaseBatchMail requiring sender.
- Make BatchEmailRequest inherit the common mixin (not BaseBatchMail) and declare sender as Optional[Address].
Illustrative refactor:
+@dataclass +class _BatchCommon(RequestParams): + attachments: Optional[list[Attachment]] = None + headers: Optional[dict[str, str]] = None + custom_variables: Optional[dict[str, Any]] = None + reply_to: Optional[Address] = None + -@dataclass -class BaseBatchMail(RequestParams): - sender: Address = Field(..., serialization_alias="from") - attachments: Optional[list[Attachment]] = None - headers: Optional[dict[str, str]] = None - custom_variables: Optional[dict[str, Any]] = None - reply_to: Optional[Address] = None +@dataclass +class BaseBatchMail(_BatchCommon): + sender: Address = Field(..., serialization_alias="from") @@ -@dataclass -class BatchEmailRequest(BaseBatchMail): +@dataclass +class BatchEmailRequest(_BatchCommon): to: list[Address] = Field(...) # type:ignore sender: Optional[Address] = Field( None, serialization_alias="from" ) # type: ignore[assignment]This removes the need for type: ignore and clarifies intent.
52-56: Optional client-side constraint for requests lengthYou may enforce the 1..500 constraint at model level (validator) to complement API-side checks. If you prefer not to validate in models, the API preflight check suffices.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
examples/sending.py(1 hunks)mailtrap/__init__.py(2 hunks)mailtrap/api/sending.py(1 hunks)mailtrap/client.py(2 hunks)mailtrap/models/mail/__init__.py(1 hunks)mailtrap/models/mail/base.py(0 hunks)mailtrap/models/mail/batch_mail.py(1 hunks)mailtrap/models/mail/from_template.py(0 hunks)mailtrap/models/mail/mail.py(2 hunks)tests/unit/api/test_sending.py(3 hunks)tests/unit/models/mail/test_batch_mail.py(1 hunks)tests/unit/models/mail/test_from_template.py(0 hunks)tests/unit/models/mail/test_mail.py(2 hunks)
💤 Files with no reviewable changes (3)
- tests/unit/models/mail/test_from_template.py
- mailtrap/models/mail/base.py
- mailtrap/models/mail/from_template.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-12T23:07:25.653Z
Learnt from: Ihor-Bilous
PR: railsware/mailtrap-python#31
File: mailtrap/config.py:1-1
Timestamp: 2025-08-12T23:07:25.653Z
Learning: In Mailtrap's API architecture, Testing API resources (Projects, Inboxes, etc.) use the main "mailtrap.io" host, while only email sending functionality uses "sandbox.api.mailtrap.io" as the host.
Applied to files:
tests/unit/api/test_sending.py
🧬 Code graph analysis (10)
mailtrap/models/mail/__init__.py (2)
mailtrap/models/mail/batch_mail.py (6)
BaseBatchMail(14-19)BatchEmailRequest(37-49)BatchMail(23-27)BatchMailFromTemplate(31-33)BatchSendEmailParams(53-55)BatchSendResponse(66-69)mailtrap/models/mail/mail.py (4)
BaseMail(13-21)MailFromTemplate(33-35)SendingMailResponse(39-41)
tests/unit/models/mail/test_batch_mail.py (3)
mailtrap/models/mail/address.py (1)
Address(9-11)mailtrap/models/mail/batch_mail.py (3)
BatchEmailRequest(37-49)BatchMail(23-27)BatchMailFromTemplate(31-33)tests/unit/models/mail/test_mail.py (4)
test_api_data_should_return_dict_with_required_props_only(14-26)test_api_data_should_return_dict_with_required_props_only(65-75)test_api_data_should_return_dict_with_all_props(28-55)test_api_data_should_return_dict_with_all_props(77-100)
tests/unit/models/mail/test_mail.py (4)
mailtrap/models/mail/mail.py (1)
MailFromTemplate(33-35)mailtrap/models/mail/address.py (1)
Address(9-11)mailtrap/models/mail/attachment.py (1)
Attachment(18-27)mailtrap/models/common.py (1)
api_data(15-19)
mailtrap/models/mail/batch_mail.py (2)
mailtrap/models/common.py (1)
RequestParams(13-19)mailtrap/models/mail/address.py (1)
Address(9-11)
mailtrap/client.py (3)
mailtrap/models/mail/batch_mail.py (2)
BatchSendResponse(66-69)BatchSendEmailParams(53-55)examples/sending.py (1)
batch_send(75-78)mailtrap/api/sending.py (1)
batch_send(25-36)
mailtrap/api/sending.py (5)
mailtrap/models/mail/mail.py (2)
BaseMail(13-21)SendingMailResponse(39-41)mailtrap/models/mail/batch_mail.py (2)
BatchSendEmailParams(53-55)BatchSendResponse(66-69)mailtrap/http.py (2)
HttpClient(14-106)post(32-34)mailtrap/client.py (2)
send(94-99)batch_send(101-106)mailtrap/models/common.py (1)
api_data(15-19)
mailtrap/__init__.py (1)
mailtrap/models/mail/batch_mail.py (4)
BatchEmailRequest(37-49)BatchMail(23-27)BatchMailFromTemplate(31-33)BatchSendEmailParams(53-55)
tests/unit/api/test_sending.py (6)
mailtrap/models/mail/mail.py (1)
SendingMailResponse(39-41)mailtrap/models/mail/batch_mail.py (4)
BatchEmailRequest(37-49)BatchMail(23-27)BatchSendEmailParams(53-55)BatchSendResponse(66-69)mailtrap/http.py (1)
post(32-34)mailtrap/exceptions.py (2)
AuthorizationError(18-20)APIError(10-15)mailtrap/api/sending.py (1)
batch_send(25-36)mailtrap/client.py (1)
batch_send(101-106)
mailtrap/models/mail/mail.py (3)
mailtrap/models/common.py (1)
RequestParams(13-19)mailtrap/models/mail/address.py (1)
Address(9-11)mailtrap/models/mail/attachment.py (1)
Attachment(18-27)
examples/sending.py (4)
mailtrap/models/mail/batch_mail.py (4)
BatchSendEmailParams(53-55)BatchMail(23-27)BatchEmailRequest(37-49)BatchMailFromTemplate(31-33)mailtrap/models/mail/address.py (1)
Address(9-11)mailtrap/api/sending.py (2)
send(20-23)batch_send(25-36)mailtrap/client.py (3)
send(94-99)MailtrapClient(30-163)batch_send(101-106)
🪛 Ruff (0.13.1)
tests/unit/models/mail/test_batch_mail.py
10-10: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
13-13: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
55-55: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
58-58: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
94-94: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
97-97: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
tests/unit/models/mail/test_mail.py
60-60: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
63-63: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
🔇 Additional comments (19)
mailtrap/models/mail/mail.py (2)
12-21: Solid BaseMail refactor; aliasing “from” is correctFields align with API payload expectations; by-alias + exclude_none will serialize cleanly. No blocking issues.
32-35: MailFromTemplate model looks correctRequired/optional fields match tests and expected API shape.
mailtrap/client.py (2)
101-107: batch_send wrapper is correct and consistent with send()Delegates to SendingApi, serializes via TypeAdapter; return alias matches payload shape.
20-23: BatchSendResponse uses Pydantic dataclassBatchSendResponse and BatchSendResponseItem are decorated with
@dataclassfrompydantic.dataclasses, satisfying TypeAdapter requirements.mailtrap/__init__.py (2)
1-1: Re-export of batch response alias looks goodExposes BATCH_SEND_ENDPOINT_RESPONSE at the package root for examples/types. OK.
19-22: Good: expose batch request models at the rootBatchEmailRequest, BatchMail, BatchMailFromTemplate, BatchSendEmailParams are now first-class exports. Consistent with existing Mail/MailFromTemplate.
examples/sending.py (2)
75-78: LGTM: batch_send wrapperSignature and delegation to client.batch_send are correct and aligned with public types.
83-83: LGTM: prints batch_send resultRunning example shows the new flow. Note: placeholders must be replaced with real values before running.
mailtrap/api/sending.py (4)
4-7: Imports aligned with model module consolidationMoving to mailtrap.models.mail and adding batch types looks correct.
15-19: URL builder is clear_appends inbox_id when present; keeps paths clean.
22-23: LGTM: send endpoint path switchUsing _get_api_url("/api/send") is correct and consistent with HttpClient expectations.
25-36: Add a test to cover inbox-specific batch URLWe don’t exercise "/api/batch/{inbox_id}". Recommend adding a unit test.
Apply this diff to tests/unit/api/test_sending.py (see placement near other batch tests):
@responses.activate def test_batch_send_should_handle_partial_failure_response(self) -> None: @@ assert result.responses[1].errors == ["Invalid email address"] + + @responses.activate + def test_batch_send_should_use_inbox_specific_url_when_inbox_id_is_set(self) -> None: + api = SendingApi(client=HttpClient(SENDING_HOST), inbox_id=INBOX_ID) + response_body = {"success": True, "responses": [{"success": True, "message_ids": ["id"]}]} + responses.post(f"https://{SENDING_HOST}/api/batch/{INBOX_ID}", json=response_body) + + result = api.batch_send(DUMMY_BATCH_PARAMS) + assert result.success is True + assert len(responses.calls) == 1Based on learnings
tests/unit/api/test_sending.py (5)
10-14: Imports updated to new locationsSwitching to mailtrap.models.mail and explicit batch types is correct.
35-52: Batch fixtures look soundCovers base payload, per-request overrides, and HTML/text forms.
54-54: Batch endpoint URL constant is correctMatches SendingApi._get_api_url("/api/batch") with default host.
122-170: Robust API behavior coverage for batch_send401, 400/500, and success paths are well covered, including body assertion.
171-191: Good: partial failure scenario coveredValidates per-item success/errors semantics.
mailtrap/models/mail/batch_mail.py (2)
13-20: BaseBatchMail fields/aliases are consistent with single-send modelsAlignment (sender alias "from", attachments/headers/custom_variables/reply_to) looks correct.
58-70: Response models look correctShapes match tests: overall success, per-item responses, and optional errors.
Motivation
In this PR I added batch_send method to SendingApi, related models, tests, examples
Changes
How to test
Summary by CodeRabbit
New Features
Documentation
Breaking Changes