Fix issue #26: Add MessagesApi, releated models, examples, tests #43
Conversation
WalkthroughAdds message models and a MessagesApi resource, integrates it into TestingApi, exposes UpdateEmailMessageParams from package root, improves HttpClient JSON/empty-response handling, provides an example script for message helpers, and adds unit tests covering all message endpoints. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer Code
participant Client as MailtrapClient
participant Testing as TestingApi
participant Messages as MessagesApi
participant HTTP as HttpClient
participant API as Mailtrap Testing API
Dev->>Client: client.testing_api.messages.get_list(inbox_id, search, last_id, page)
Client->>Testing: testing_api
Testing->>Messages: messages (account_id, client)
Messages->>HTTP: GET /api/accounts/{account}/inboxes/{inbox}/messages{?q,last_id,page}
HTTP->>API: Request
API-->>HTTP: 200 OK (JSON list)
HTTP-->>Messages: Parsed list[EmailMessage]
Messages-->>Dev: list[EmailMessage]
note right of HTTP #ffeead: On non-JSON 200 OK → return text
note right of HTTP #ffe7d6: On failed empty body → APIError ("Not Found"/"Empty response body")
sequenceDiagram
autonumber
actor Dev as Developer Code
participant Messages as MessagesApi
participant HTTP as HttpClient
participant API as Mailtrap Testing API
Dev->>Messages: update(inbox_id, message_id, UpdateEmailMessageParams)
Messages->>HTTP: PATCH /.../messages/{message_id} body: {"is_read":"true|false"}
HTTP->>API: Request
API-->>HTTP: 200 OK (EmailMessage JSON)
HTTP-->>Messages: EmailMessage
Messages-->>Dev: EmailMessage
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, 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: 2
🧹 Nitpick comments (6)
mailtrap/http.py (1)
66-70: Treat whitespace-only bodies as emptySome servers return a single newline/space on errors; consider stripping before the emptiness check to avoid misleading “Invalid JSON”.
- if not response.content: + if not response.content or not response.content.strip(): if status_code == 404: raise APIError(status_code, errors=["Not Found"]) raise APIError(status_code, errors=["Empty response body"])tests/unit/api/testing/test_messages.py (2)
17-17: Unused constantATTACHMENT_ID isn’t referenced.
-ATTACHMENT_ID = 67
190-207: Doc vs behavior mismatch for paginationYour MessagesApi docstring says last_id overrides page, but tests assert both are sent. Decide one: either (a) enforce override in code and adjust this test, or (b) update the docstring.
I can provide a patch for (a) in MessagesApi if you choose that route.
examples/testing/messages.py (2)
9-12: Placeholders: tweak INBOX_ID type; suppress S105 for token
- Use an int for INBOX_ID to match function signatures.
- Keep placeholder token but silence the linter as this is an example.
-API_TOKEN = "YOU_API_TOKEN" +API_TOKEN = "YOU_API_TOKEN" # noqa: S105 (example placeholder) ACCOUNT_ID = "YOU_ACCOUNT_ID" -INBOX_ID = "YOUR_INBOX_ID" +INBOX_ID = 123456 # example inbox id
48-77: Fix type hints: message_id should be int; headers return type is dictAlign wrappers with MessagesApi signatures and actual return types.
-from mailtrap.models.messages import AnalysisReport +from typing import Any +from mailtrap.models.messages import AnalysisReport @@ -def get_spam_report(inbox_id: int, message_id: str) -> SpamReport: +def get_spam_report(inbox_id: int, message_id: int) -> SpamReport: @@ -def get_html_analysis(inbox_id: int, message_id: str) -> AnalysisReport: +def get_html_analysis(inbox_id: int, message_id: int) -> AnalysisReport: @@ -def get_text_body(inbox_id: int, message_id: str) -> str: +def get_text_body(inbox_id: int, message_id: int) -> str: @@ -def get_raw_body(inbox_id: int, message_id: str) -> str: +def get_raw_body(inbox_id: int, message_id: int) -> str: @@ -def get_html_source(inbox_id: int, message_id: str) -> str: +def get_html_source(inbox_id: int, message_id: int) -> str: @@ -def get_html_body(inbox_id: int, message_id: str) -> str: +def get_html_body(inbox_id: int, message_id: int) -> str: @@ -def get_eml_body(inbox_id: int, message_id: str) -> str: +def get_eml_body(inbox_id: int, message_id: int) -> str: @@ -def get_mail_headers(inbox_id: int, message_id: str) -> str: - return messages_api.get_mail_headers(inbox_id=inbox_id, message_id=message_id) +def get_mail_headers(inbox_id: int, message_id: int) -> dict[str, Any]: + return messages_api.get_mail_headers(inbox_id=inbox_id, message_id=message_id)mailtrap/api/resources/messages.py (1)
148-152: Avoid falsy‑id pitfallUse an explicit None check so id=0 (if ever valid) isn’t dropped.
- if message_id: + if message_id is not None: return f"{path}/{message_id}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
examples/testing/messages.py(1 hunks)mailtrap/__init__.py(1 hunks)mailtrap/api/resources/messages.py(1 hunks)mailtrap/api/testing.py(2 hunks)mailtrap/http.py(2 hunks)mailtrap/models/messages.py(1 hunks)tests/unit/api/testing/test_messages.py(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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.py
📚 Learning: 2025-09-05T23:31:55.179Z
Learnt from: Ihor-Bilous
PR: railsware/mailtrap-python#40
File: mailtrap/api/resources/inboxes.py:40-67
Timestamp: 2025-09-05T23:31:55.179Z
Learning: The Mailtrap API does not return 204 No Content responses for inbox management endpoints (delete, clean, mark_as_read, reset_credentials, enable_email_address, reset_email_username). All these endpoints return JSON data that can be used to construct Inbox objects.
Applied to files:
examples/testing/messages.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:
mailtrap/api/testing.py
🧬 Code graph analysis (7)
examples/testing/messages.py (4)
mailtrap/api/testing.py (1)
messages(26-27)mailtrap/models/messages.py (5)
AnalysisReport(126-127)EmailMessage(47-69)ForwardedMessage(84-85)SpamReport(96-103)UpdateEmailMessageParams(73-80)mailtrap/client.py (1)
testing_api(52-58)mailtrap/api/resources/messages.py (13)
show_message(19-22)update(24-35)delete(37-40)get_list(42-88)forward(90-98)get_spam_report(100-103)get_html_analysis(105-108)get_text_body(110-114)get_raw_body(116-120)get_html_source(122-127)get_html_body(129-133)get_eml_body(135-139)get_mail_headers(141-146)
tests/unit/api/testing/test_messages.py (4)
mailtrap/api/resources/messages.py (14)
MessagesApi(14-152)get_list(42-88)show_message(19-22)update(24-35)delete(37-40)forward(90-98)get_spam_report(100-103)get_html_analysis(105-108)get_text_body(110-114)get_html_body(129-133)get_raw_body(116-120)get_html_source(122-127)get_eml_body(135-139)get_mail_headers(141-146)mailtrap/exceptions.py (1)
APIError(10-15)mailtrap/http.py (5)
HttpClient(14-106)get(26-30)patch(40-42)delete(44-46)post(32-34)mailtrap/models/messages.py (2)
EmailMessage(47-69)UpdateEmailMessageParams(73-80)
mailtrap/api/testing.py (2)
mailtrap/api/resources/messages.py (1)
MessagesApi(14-152)tests/unit/api/testing/test_messages.py (1)
client(24-25)
mailtrap/__init__.py (2)
mailtrap/api/testing.py (1)
messages(26-27)mailtrap/models/messages.py (1)
UpdateEmailMessageParams(73-80)
mailtrap/api/resources/messages.py (3)
mailtrap/http.py (5)
HttpClient(14-106)get(26-30)patch(40-42)delete(44-46)post(32-34)mailtrap/api/testing.py (1)
messages(26-27)mailtrap/models/messages.py (7)
AnalysisReport(126-127)AnalysisReportResponse(141-142)EmailMessage(47-69)ForwardedMessage(84-85)SpamReport(96-103)UpdateEmailMessageParams(73-80)api_data(77-80)
mailtrap/http.py (1)
mailtrap/exceptions.py (1)
APIError(10-15)
mailtrap/models/messages.py (1)
mailtrap/models/common.py (1)
RequestParams(13-19)
🪛 Ruff (0.12.2)
examples/testing/messages.py
9-9: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
🔇 Additional comments (9)
mailtrap/http.py (2)
58-61: OK to fall back to raw text on non‑JSON successReturning response.text on JSON parse errors for 2xx is reasonable for the body endpoints.
Please confirm this matches Mailtrap’s successful non‑JSON endpoints (text/html, message/rfc822).
71-81: Error mapping looks good401 → AuthorizationError and generic APIError with flattened messages is appropriate.
mailtrap/models/messages.py (3)
95-104: Alias-based parsing into dataclasses: confirm Pydantic behaviorYou’re instantiating SpamReport/SpamDetail with aliased keys (e.g., “ResponseCode”, “Pts”). With pydantic.dataclasses, alias handling on init depends on v2 behavior. If aliases aren’t honored at init, switch these to BaseModel or parse via TypeAdapter before constructing.
Would you like me to provide a drop-in BaseModel variant if verification fails?
72-81: Lowercasing bools for API payloadsConverting booleans to "true"/"false" strings matches the documented API; impl is fine.
46-70: EmailMessage shape looks consistent with testsField types and nesting align with responses used in tests.
mailtrap/api/testing.py (1)
25-27: Good addition — resource wiring mirrors existing patternExposes MessagesApi via the testing surface with the correct host/client.
mailtrap/__init__.py (1)
21-21: Re‑export of UpdateEmailMessageParamsPublic API ergonomics improved; matches example usage.
tests/unit/api/testing/test_messages.py (1)
550-571: Mixed content-types in error responses are handledGood coverage for text/plain and empty bodies; aligns with HttpClient changes.
mailtrap/api/resources/messages.py (1)
42-78: Docstring is clear and helpfulNice endpoint notes and parameter docs.
Ihor-Bilous
requested review from
IgorDobryn,
i7an,
VladimirTaytor and
andrii-porokhnavets
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
tests/unit/test_http.py (6)
1-9: Use Response-spec mocks for stronger contracts.Add
from requests import Responseand construct mocks withspec=Responseto catch accidental attribute drift.Example update in tests:
+from requests import Response ... - mock_response = Mock() + mock_response = Mock(spec=Response)You can also DRY this via a tiny helper/fixture that sets
status_code,content, andjson/side_effect.
13-27: Parametrize to reduce duplication.Group the “singular/string/list/unknown” cases into one parametrized test for brevity and easier additions.
Sketch:
@pytest.mark.parametrize( "payload,expected", [ ({"error": "Simple error message"}, ["Simple error message"]), ({"errors": "Error message"}, ["Error message"]), ({"errors": ["Error 1", "Error 2"]}, ["Error 1", "Error 2"]), ({"unknown_key": "Some error"}, ["Unknown error"]), ], ) def test_extract_errors_flat(payload, expected): assert HttpClient._extract_errors(payload) == expectedAlso applies to: 34-37
28-33: Confirm desired behavior for deeper nesting.
flatten_errorsonly flattens one level; nested dicts will be stringified (e.g.,"field1: {'nested': 'err'}"). If recursive flattening is desired, add a test and adjust implementation; otherwise, consider a comment to document intent.Possible extra test:
def test_extract_errors_with_deeply_nested_dict(): data = {"errors": {"field1": {"nested": "err"}}} errors = HttpClient._extract_errors(data) assert errors == ["field1: {'nested': 'err'}"] # current behavior
39-51: Also assert AuthorizationError carries status=401.Add a status assertion to lock API contract.
@@ - assert "Unauthorized" in str(exc_info.value) + assert "Unauthorized" in str(exc_info.value) + assert exc_info.value.status == 401
52-77: Solid handling of empty-content branches.Covers 404 vs generic empty body. Consider adding a whitespace-only content case to pin current behavior (“Invalid JSON”).
Example:
def test_handle_failed_response_404_with_whitespace_content(): client = HttpClient("test.mailtrap.com") mock_response = Mock() mock_response.status_code = 404 mock_response.content = b" " mock_response.json.side_effect = json.JSONDecodeError("Invalid JSON", "", 0) with pytest.raises(APIError) as exc_info: client._handle_failed_response(mock_response) assert exc_info.value.status == 404 assert "Invalid JSON" in exc_info.value.errors
1-105: Add direct tests for _process_response happy paths.Given recent changes, add small tests for:
- 2xx + empty body → None
- 2xx + invalid JSON → returns
response.text- 2xx + valid JSON → parsed object
Appendions:
def test_process_response_200_empty_returns_none(): client = HttpClient("test.mailtrap.com") from requests import Response r = Mock(spec=Response) r.ok = True r.content = b"" assert client._process_response(r) is None def test_process_response_200_invalid_json_returns_text(): client = HttpClient("test.mailtrap.com") from requests import Response r = Mock(spec=Response) r.ok = True r.content = b"not json" r.text = "not json" r.json.side_effect = json.JSONDecodeError("Invalid JSON", "", 0) assert client._process_response(r) == "not json" def test_process_response_200_valid_json_returns_parsed(): client = HttpClient("test.mailtrap.com") from requests import Response r = Mock(spec=Response) r.ok = True r.content = b'{"ok": true}' r.json.return_value = {"ok": True} assert client._process_response(r) == {"ok": True}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/unit/test_http.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/unit/test_http.py (2)
mailtrap/exceptions.py (2)
APIError(10-15)AuthorizationError(18-20)mailtrap/http.py (3)
HttpClient(14-106)_extract_errors(84-106)_handle_failed_response(63-81)
⏰ 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). (6)
- GitHub Check: Test python3.10 on windows-latest
- GitHub Check: Test python3.9 on windows-latest
- GitHub Check: Test python3.12 on windows-latest
- GitHub Check: Test python3.9 on macos-latest
- GitHub Check: Test python3.13 on windows-latest
- GitHub Check: Test python3.11 on windows-latest
🔇 Additional comments (3)
tests/unit/test_http.py (3)
13-27: Good coverage for _extract_errors basic shapes.
78-91: LGTM: robust simulation of JSON decode failure.
92-104: LGTM: generic server error path is asserted well.
Motivation
In this PR I added MessagesApi, related models, tests, examples
Changes
How to test
Summary by CodeRabbit