Fix issue #28: Add AttachmentsApi, related models, tests, examples #44
Conversation
WalkthroughIntroduces Attachments API support: new Attachment model, AttachmentsApi resource, and TestingApi.attachments accessor with corresponding unit tests and a new example script. Renames MessagesApi body methods to new “…_message” names and updates tests/examples. Also standardizes example placeholder credentials and one template UUID placeholder. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer Script
participant Client as MailtrapClient
participant TAPI as TestingApi
participant AAPI as AttachmentsApi
participant HTTP as HttpClient
participant SVC as Mailtrap API
Dev->>Client: initialize(token, account_id)
Dev->>TAPI: client.testing_api
Dev->>AAPI: TAPI.attachments
note right of AAPI: New accessor
Dev->>AAPI: get_list(inbox_id, message_id)
AAPI->>HTTP: GET /api/accounts/{acc}/inboxes/{inbox}/messages/{msg}/attachments
HTTP->>SVC: Request
SVC-->>HTTP: 200 OK [attachments...]
HTTP-->>AAPI: JSON list
AAPI-->>Dev: list[Attachment]
Dev->>AAPI: get(inbox_id, message_id, attachment_id)
AAPI->>HTTP: GET .../attachments/{attachment_id}
HTTP->>SVC: Request
SVC-->>HTTP: 200 OK {attachment}
HTTP-->>AAPI: JSON
AAPI-->>Dev: Attachment
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
examples/testing/inboxes.py (1)
6-7: Same placeholder consistency note appliesMatches the new "YOUR_*" convention; ensure repo‑wide consistency per earlier comment in examples/contacts/contacts.py.
examples/suppressions/suppressions.py (1)
6-7: Placeholders updated — OKNo functional changes. Consistency note and S105 handling as mentioned earlier.
examples/contacts/contact_imports.py (1)
4-5: Placeholders updated — OKChange is benign. Follow the repo‑wide consistency and S105 guidance from the first comment.
examples/contacts/contact_lists.py (1)
5-6: Placeholders updated — OKAligned with the new convention; see earlier note on verifying all examples and ignoring S105 for examples.
examples/contacts/contact_fields.py (1)
7-8: Placeholders updated — OKLooks good; same repo‑wide consistency + S105 config advice applies.
examples/email_templates/templates.py (1)
7-8: Placeholders updated — OKNo functional change; consistent with other examples. Refer to the first comment for the verification script and Ruff config.
🧹 Nitpick comments (15)
examples/contacts/contacts.py (1)
8-9: Standardize placeholders in examples; silence Ruff S105 via configChange to "YOUR_" is applied across examples — no "YOU_" placeholders remain. Silence S105 for examples in pyproject.toml instead of using inline noqa.
+[tool.ruff] +per-file-ignores = { "examples/**" = ["S105"] }examples/testing/projects.py (1)
4-5: Revert placeholders to repo-wide “YOU_” convention.Other examples use YOU_API_TOKEN / YOU_ACCOUNT_ID; keep consistency.
[ suggest_nitpick_refactor ]
-API_TOKEN = "YOUR_API_TOKEN" -ACCOUNT_ID = "YOUR_ACCOUNT_ID" +API_TOKEN = "YOU_API_TOKEN" +ACCOUNT_ID = "YOU_ACCOUNT_ID"tests/unit/api/testing/test_attachments.py (2)
54-54: Rename the test class.Likely copy-paste; should be TestAttachmentsApi to match subject.
-class TestProjectsApi: +class TestAttachmentsApi:
37-39: Use a realistic MIME type in sample data.“plain/text” → “text/plain” to mirror real responses.
- "content_type": "plain/text", + "content_type": "text/plain",examples/testing/attachments.py (3)
4-7: Align placeholders with repo convention (“YOU_”, not “YOUR_”).Keeps examples uniform across the repo.
-API_TOKEN = "YOUR_API_TOKEN" -ACCOUNT_ID = "YOUR_ACCOUNT_ID" -INBOX_ID = "YOUR_INBOX_ID" -MESSAGE_ID = "YOUR_MESSAGE_ID" +API_TOKEN = "YOU_API_TOKEN" +ACCOUNT_ID = "YOU_ACCOUNT_ID" +INBOX_ID = "YOU_INBOX_ID" +MESSAGE_ID = "YOU_MESSAGE_ID"
13-15: Loosen example type hints to accept placeholders without surprises.Placeholders are strings; allow str|int in example functions to avoid type hint mismatch.
-def list_attachments(inbox_id: int, message_id: int) -> list[Attachment]: +def list_attachments(inbox_id: int | str, message_id: int | str) -> list[Attachment]: return attachments_api.get_list(inbox_id=inbox_id, message_id=message_id) -def get_attachment(inbox_id: int, message_id: int, attachment_id: int) -> Attachment: +def get_attachment(inbox_id: int | str, message_id: int | str, attachment_id: int | str) -> Attachment: return attachments_api.get( inbox_id=inbox_id, message_id=message_id, attachment_id=attachment_id, )Also applies to: 17-22
4-4: Optional: silence Ruff S105 in examples.If Ruff checks examples, add an inline ignore or exclude the path in config.
-API_TOKEN = "YOU_API_TOKEN" +API_TOKEN = "YOU_API_TOKEN" # noqa: S105Quick check script (same as above) can confirm current config.
mailtrap/api/resources/attachments.py (2)
27-29: Grammar nit in docstring.Small readability tweak.
- """Get message single attachment by inbox_id, message_id and attachment_id.""" + """Get a single message attachment by inbox_id, message_id, and attachment_id."""
43-45: Truthiness check may misbehave for falsy IDs; prefer explicit None check.Defensive, even if IDs are always >= 1.
- if attachment_id: + if attachment_id is not None: return f"{path}/{attachment_id}"mailtrap/api/resources/messages.py (2)
110-115: Renames align with other SDKs/docs. Consider a temporary deprecation shim.To avoid breaking users immediately, alias old names to new ones with a deprecation note (or bump major version).
def get_message_as_eml(self, inbox_id: int, message_id: int) -> str: """Get email message in .eml format.""" return cast( str, self._client.get(f"{self._api_path(inbox_id, message_id)}/body.eml") ) + + # Backward-compatibility aliases (deprecated). Consider removing in next major. + # get_text_body -> get_text_message, get_raw_body -> get_raw_message, etc. + # These assignments should live at class scope (not inside the method).Add at class scope (after the last new method):
+ # Deprecated aliases for backward compatibility + get_text_body = get_text_message + get_raw_body = get_raw_message + get_html_body = get_html_message + get_eml_body = get_message_as_emlIf you prefer explicit warnings, we can add wrapper methods emitting DeprecationWarning.
Also applies to: 116-121, 129-134, 135-140
149-152: Preferis not Nonefor message_id gate.Avoids accidental omission when
message_id == 0(defensive).- if message_id: + if message_id is not None: return f"{path}/{message_id}"examples/testing/messages.py (4)
56-57: Type hint: message_id should be int to match API
The underlying MessagesApi expects an int message_id; keep the wrapper signature consistent.-def get_text_message(inbox_id: int, message_id: str) -> str: +def get_text_message(inbox_id: int, message_id: int) -> str:Apply the same int hint to other wrappers in this file for consistency.
60-61: Align type hint with API (int)-def get_raw_message(inbox_id: int, message_id: str) -> str: +def get_raw_message(inbox_id: int, message_id: int) -> str:
68-69: Align type hint with API (int)-def get_html_message(inbox_id: int, message_id: str) -> str: +def get_html_message(inbox_id: int, message_id: int) -> str:
72-73: Align type hint with API (int)-def get_message_as_eml(inbox_id: int, message_id: str) -> str: +def get_message_as_eml(inbox_id: int, message_id: int) -> str:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
examples/contacts/contact_fields.py(1 hunks)examples/contacts/contact_imports.py(1 hunks)examples/contacts/contact_lists.py(1 hunks)examples/contacts/contacts.py(1 hunks)examples/email_templates/templates.py(1 hunks)examples/sending.py(2 hunks)examples/suppressions/suppressions.py(1 hunks)examples/testing/attachments.py(1 hunks)examples/testing/inboxes.py(1 hunks)examples/testing/messages.py(2 hunks)examples/testing/projects.py(1 hunks)mailtrap/api/resources/attachments.py(1 hunks)mailtrap/api/resources/messages.py(2 hunks)mailtrap/api/testing.py(2 hunks)mailtrap/models/attachments.py(1 hunks)tests/unit/api/testing/test_attachments.py(1 hunks)tests/unit/api/testing/test_messages.py(12 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-09-04T19:31:01.169Z
Learnt from: Ihor-Bilous
PR: railsware/mailtrap-python#39
File: examples/suppressions.py:1-10
Timestamp: 2025-09-04T19:31:01.169Z
Learning: In the mailtrap-python repository, all example files consistently use placeholder strings like `API_TOKEN = "YOU_API_TOKEN"` and `ACCOUNT_ID = "YOU_ACCOUNT_ID"` instead of environment variable lookups. This pattern should be maintained for consistency across examples.
Applied to files:
examples/testing/messages.pyexamples/contacts/contacts.pyexamples/testing/inboxes.pyexamples/contacts/contact_imports.pyexamples/sending.pyexamples/email_templates/templates.pyexamples/suppressions/suppressions.pyexamples/contacts/contact_fields.pyexamples/testing/projects.pyexamples/contacts/contact_lists.py
📚 Learning: 2025-08-29T21:15:03.708Z
Learnt from: Ihor-Bilous
PR: railsware/mailtrap-python#38
File: mailtrap/api/resources/contact_imports.py:13-18
Timestamp: 2025-08-29T21:15:03.708Z
Learning: In mailtrap/models/contacts.py, ImportContactParams.api_data is defined as a property, not a method, so it can be accessed without parentheses like contact.api_data.
Applied to files:
examples/contacts/contacts.pyexamples/contacts/contact_imports.pyexamples/contacts/contact_fields.py
📚 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:
examples/testing/inboxes.pyexamples/testing/projects.py
📚 Learning: 2025-09-04T16:28:41.602Z
Learnt from: Ihor-Bilous
PR: railsware/mailtrap-python#39
File: mailtrap/api/resources/suppressions.py:16-18
Timestamp: 2025-09-04T16:28:41.602Z
Learning: Mailtrap Suppressions API: DELETE /api/accounts/{account_id}/suppressions/{id} returns HTTP 200 OK with a JSON body of the deleted suppression. Keep SDK delete() typed as -> Suppression (not Optional).
Applied to files:
examples/suppressions/suppressions.py
📚 Learning: 2025-08-22T13:51:31.437Z
Learnt from: Ihor-Bilous
PR: railsware/mailtrap-python#35
File: examples/contacts/contact_fields.py:11-11
Timestamp: 2025-08-22T13:51:31.437Z
Learning: In mailtrap/api/contacts.py, ContactsBaseApi.contact_fields is defined as a property, not a method, so it can be accessed without parentheses like client.contacts_api.contact_fields.
Applied to files:
examples/contacts/contact_fields.py
🧬 Code graph analysis (8)
mailtrap/models/attachments.py (1)
mailtrap/models/mail/attachment.py (1)
Disposition(12-14)
mailtrap/api/testing.py (2)
mailtrap/api/resources/attachments.py (1)
AttachmentsApi(7-45)tests/unit/api/testing/test_attachments.py (1)
client(27-28)
examples/testing/messages.py (1)
mailtrap/api/resources/messages.py (5)
get_text_message(110-114)get_raw_message(116-120)get_html_source(122-127)get_html_message(129-133)get_message_as_eml(135-139)
tests/unit/api/testing/test_attachments.py (4)
mailtrap/api/resources/attachments.py (3)
AttachmentsApi(7-45)get(21-29)get_list(12-19)mailtrap/exceptions.py (1)
APIError(10-15)mailtrap/http.py (1)
HttpClient(14-106)mailtrap/models/attachments.py (1)
Attachment(10-22)
mailtrap/api/resources/messages.py (2)
examples/testing/messages.py (4)
get_text_message(56-57)get_raw_message(60-61)get_html_message(68-69)get_message_as_eml(72-73)mailtrap/http.py (1)
get(26-30)
mailtrap/api/resources/attachments.py (4)
mailtrap/http.py (1)
HttpClient(14-106)mailtrap/api/testing.py (1)
attachments(31-32)mailtrap/models/attachments.py (1)
Attachment(10-22)tests/unit/api/testing/test_attachments.py (1)
client(27-28)
examples/testing/attachments.py (4)
mailtrap/api/testing.py (1)
attachments(31-32)mailtrap/models/attachments.py (1)
Attachment(10-22)mailtrap/client.py (1)
testing_api(52-58)mailtrap/api/resources/attachments.py (2)
get_list(12-19)get(21-29)
tests/unit/api/testing/test_messages.py (2)
examples/testing/messages.py (4)
get_text_message(56-57)get_html_message(68-69)get_raw_message(60-61)get_message_as_eml(72-73)mailtrap/api/resources/messages.py (4)
get_text_message(110-114)get_html_message(129-133)get_raw_message(116-120)get_message_as_eml(135-139)
🪛 Ruff (0.12.2)
examples/testing/messages.py
9-9: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/contacts/contacts.py
8-8: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/testing/inboxes.py
6-6: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/contacts/contact_imports.py
4-4: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/sending.py
3-3: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/email_templates/templates.py
7-7: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/suppressions/suppressions.py
6-6: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/contacts/contact_fields.py
7-7: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/testing/projects.py
4-4: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/testing/attachments.py
4-4: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/contacts/contact_lists.py
5-5: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
🔇 Additional comments (6)
examples/sending.py (1)
3-3: LGTM: placeholder update and template UUID typo fixGood catch on "<YOUR_TEMPLATE_UUID>" and aligning API token placeholder.
Also applies to: 22-22
mailtrap/api/testing.py (1)
30-33: Nice addition: TestingApi.attachments accessor is correctly wired.Reuses the existing client/headers/host for Testing API. LGTM.
mailtrap/models/attachments.py (1)
9-22: Attachment model looks correct and aligns with API fields.Enum mapping and timestamps are typed properly.
tests/unit/api/testing/test_messages.py (1)
574-591: Method rename coverage looks complete.Tests now call get_text_message, get_html_message, get_raw_message, and get_message_as_eml; endpoints unchanged. LGTM.
Also applies to: 596-609, 636-654, 658-671, 698-716, 720-734, 821-839, 843-859
examples/testing/projects.py (1)
4-4: Decide on Ruff S105 handling for examples.Search of pyproject.toml found no ruff / S105 / exclude / ignore entries.
- Option A — Inline ignore: examples/testing/projects.py (line 4): API_TOKEN = "YOUR_API_TOKEN" # noqa: S105
- Option B — Exclude examples in pyproject.toml: add a [tool.ruff] section with exclude = ["examples"]
Also verify other Ruff config locations (.ruff.toml, setup.cfg) or CI/pre-commit that might lint examples.
examples/testing/messages.py (1)
9-10: Keep 'YOUR_*' placeholders and add S105 ignores in examplesRepository examples use "YOUR_" placeholders (multiple matches found); do not change them to "YOU_". Add an inline ignore
# noqa: S105 - example placeholderto API_TOKEN assignments in example files where present (e.g., examples/testing/messages.py:9).Likely an incorrect or invalid review comment.
Ihor-Bilous
requested review from
IgorDobryn,
i7an,
VladimirTaytor and
andrii-porokhnavets
VladimirTaytor
changed the title
Motivation
In this PR I added AttachmentsApi, related models, tests, examples
Changes
How to test
Summary by CodeRabbit
New Features
Refactor
Documentation