Fix issue #21: Add ContactsApi, related models, tests, examples #37
Conversation
WalkthroughAdds a Contacts API with account-scoped CRUD resource (ContactsApi), contact models and params, package export updates, example usage script, tests for contacts plus expanded test error fixtures, and allows string IDs on DeletedObject.id. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant EX as Example script
participant MC as MailtrapClient
participant CBASE as ContactsBaseApi
participant CAPI as ContactsApi
participant HTTP as HttpClient
participant SRV as Mailtrap API
EX->>MC: initialize(API_TOKEN, ACCOUNT_ID)
MC->>CBASE: access .contacts
CBASE-->>CAPI: return ContactsApi(account_id, client)
rect rgba(200,240,255,0.12)
note right of EX: Create contact
EX->>CAPI: create(CreateContactParams)
CAPI->>HTTP: POST /api/accounts/{acct}/contacts {contact: ...}
HTTP->>SRV: request
SRV-->>HTTP: 201 ContactResponse
HTTP-->>CAPI: ContactResponse
CAPI-->>EX: Contact
end
rect rgba(220,255,220,0.12)
note right of EX: Get / Update
EX->>CAPI: get_by_id(idOrEmail)
CAPI->>HTTP: GET /api/accounts/{acct}/contacts/{idOrEmail}
SRV-->>HTTP: 200 ContactResponse
HTTP-->>CAPI: ContactResponse
CAPI-->>EX: Contact
EX->>CAPI: update(idOrEmail, UpdateContactParams)
CAPI->>HTTP: PATCH /api/accounts/{acct}/contacts/{idOrEmail} {contact: ...}
SRV-->>HTTP: 200 ContactResponse
HTTP-->>CAPI: ContactResponse
CAPI-->>EX: Contact
end
rect rgba(255,235,235,0.12)
note right of EX: Delete
EX->>CAPI: delete(idOrEmail)
CAPI->>HTTP: DELETE /api/accounts/{acct}/contacts/{idOrEmail}
SRV-->>HTTP: 204
HTTP-->>CAPI: success
CAPI-->>EX: DeletedObject(id)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (7)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Ihor-Bilous
requested review from
andrii-porokhnavets,
IgorDobryn,
i7an and
VladimirTaytor
![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 (16)
mailtrap/models/common.py (1)
22-25: Make DeletedObject immutableIt’s a value object returned from delete; freezing prevents accidental mutation.
Apply:
-@dataclass +@dataclass(frozen=True) class DeletedObject: - id: Union[int, str] + id: Union[int, str]tests/conftest.py (1)
13-22: Centralize validation error payloads for reuseConsider moving shared 422 validation payload/message used in test_contact_fields into conftest for reuse in other suites.
tests/unit/api/test_contact_fields.py (2)
21-32: Avoid brittle hard-coded validation message stringsFlatten the errors dict to a message at assertion time to reduce duplication and sensitivity to formatting changes, or move these constants to conftest for reuse.
62-62: Rename test class for accuracyThis suite targets ContactFields; rename for clarity.
Apply:
-class TestContactsApi: +class TestContactFieldsApi:mailtrap/__init__.py (1)
9-11: Define all to formalize public API and silence F401 for re-exports.Ruff flags these imports as unused. Listing them in all both documents the public surface and resolves F401.
from .models.templates import UpdateEmailTemplateParams - + +__all__ = [ + # client + "MailtrapClient", + "SEND_ENDPOINT_RESPONSE", + # exceptions + "MailtrapError", + "APIError", + "AuthorizationError", + "ClientConfigurationError", + # mail models + "Address", + "Attachment", + "BaseMail", + "Disposition", + "Mail", + "MailFromTemplate", + "CreateEmailTemplateParams", + "UpdateEmailTemplateParams", + # contacts models + "ContactListParams", + "CreateContactFieldParams", + "UpdateContactFieldParams", + "CreateContactParams", + "UpdateContactParams", +]Also applies to: 20-20
tests/unit/models/test_contacts.py (1)
59-65: Rename test to reflect subject under test.Function name mentions “contact_field” but it tests ContactListParams.
- def test_create_contact_field_params_api_data_should_return_correct_dict( + def test_contact_list_params_api_data_should_return_correct_dict(tests/unit/api/test_contacts.py (2)
59-59: Fix test class typo for clarity.Rename to TestContactsApi to avoid confusion.
-class TestContactstApi: +class TestContactsApi:
110-129: Optional: add a test for get_by_id using an email identifier.Since the API accepts either ID or email, consider a case like get_by_id("[email protected]").
mailtrap/models/contacts.py (4)
54-57: Remove duplicate type in Union for fields values.The Union repeats str; tighten the type.
- fields: Optional[dict[str, Union[str, int, float, bool, str]]] = ( + fields: Optional[dict[str, Union[str, int, float, bool]]] = ( None # field_merge_tag: value )
63-66: Same Union cleanup for UpdateContactParams.fields.- fields: Optional[dict[str, Union[str, int, float, bool, str]]] = ( + fields: Optional[dict[str, Union[str, int, float, bool]]] = ( None # field_merge_tag: value )
88-88: Same Union cleanup for Contact.fields.- fields: dict[str, Union[str, int, float, bool, str]] # field_merge_tag: value + fields: dict[str, Union[str, int, float, bool]] # field_merge_tag: value
70-81: Consider guarding mutually exclusive list operations.If API semantics disallow providing both list_ids_included and list_ids_excluded simultaneously, add a check to prevent ambiguous updates.
examples/contacts/contacts.py (1)
8-9: Typo in placeholders.Use YOUR_* to match common convention.
-API_TOKEN = "YOU_API_TOKEN" -ACCOUNT_ID = "YOU_ACCOUNT_ID" +API_TOKEN = "YOUR_API_TOKEN" +ACCOUNT_ID = "YOUR_ACCOUNT_ID"mailtrap/api/resources/contacts.py (3)
12-14: Accept int or str account_id; normalize to str to avoid type friction.-from typing import Optional +from typing import Optional, Union- def __init__(self, client: HttpClient, account_id: str) -> None: - self._account_id = account_id + def __init__(self, client: HttpClient, account_id: Union[int, str]) -> None: + self._account_id = str(account_id)Also applies to: 1-1
16-18: Name suggests ID-only; consider aliasing to get() for clarity.Since emails are supported, either rename to get() or add a get() alias that delegates here to better reflect accepted identifiers.
36-38: Prefer named argument; optionally propagate server-returned id if present.Use a named field for clarity and future extensibility. If DELETE ever returns a body with id, prefer that over the input identifier.
- self._client.delete(self._api_path(contact_id_or_email)) - return DeletedObject(contact_id_or_email) + self._client.delete(self._api_path(contact_id_or_email)) + return DeletedObject(id=contact_id_or_email)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
examples/contacts/contacts.py(1 hunks)mailtrap/__init__.py(1 hunks)mailtrap/api/contacts.py(2 hunks)mailtrap/api/resources/contacts.py(1 hunks)mailtrap/models/common.py(2 hunks)mailtrap/models/contacts.py(2 hunks)tests/conftest.py(1 hunks)tests/unit/api/test_contact_fields.py(6 hunks)tests/unit/api/test_contact_lists.py(5 hunks)tests/unit/api/test_contacts.py(1 hunks)tests/unit/models/test_contacts.py(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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:
mailtrap/api/contacts.pymailtrap/api/resources/contacts.py
🧬 Code graph analysis (7)
mailtrap/api/contacts.py (1)
mailtrap/api/resources/contacts.py (1)
ContactsApi(11-44)
tests/unit/models/test_contacts.py (2)
mailtrap/models/contacts.py (2)
CreateContactParams(52-57)UpdateContactParams(61-81)mailtrap/models/common.py (1)
api_data(15-19)
tests/unit/api/test_contacts.py (5)
mailtrap/api/resources/contacts.py (5)
ContactsApi(11-44)get_by_id(16-18)create(20-25)update(27-34)delete(36-38)mailtrap/exceptions.py (1)
APIError(10-15)mailtrap/http.py (4)
HttpClient(13-96)get(25-29)post(31-33)patch(39-41)mailtrap/models/common.py (1)
DeletedObject(23-24)mailtrap/models/contacts.py (4)
Contact(85-92)ContactStatus(46-48)CreateContactParams(52-57)UpdateContactParams(61-81)
mailtrap/api/resources/contacts.py (4)
mailtrap/http.py (4)
HttpClient(13-96)get(25-29)post(31-33)patch(39-41)mailtrap/models/common.py (2)
DeletedObject(23-24)api_data(15-19)mailtrap/api/contacts.py (1)
contacts(21-22)mailtrap/models/contacts.py (4)
Contact(85-92)ContactResponse(96-97)CreateContactParams(52-57)UpdateContactParams(61-81)
mailtrap/__init__.py (1)
mailtrap/models/contacts.py (3)
CreateContactParams(52-57)UpdateContactFieldParams(18-24)UpdateContactParams(61-81)
mailtrap/models/contacts.py (1)
mailtrap/models/common.py (1)
RequestParams(13-19)
examples/contacts/contacts.py (3)
mailtrap/models/common.py (1)
DeletedObject(23-24)mailtrap/models/contacts.py (3)
Contact(85-92)CreateContactParams(52-57)UpdateContactParams(61-81)mailtrap/api/resources/contacts.py (4)
create(20-25)update(27-34)get_by_id(16-18)delete(36-38)
🪛 Ruff (0.12.2)
mailtrap/__init__.py
9-9: .models.contacts.CreateContactParams imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
10-10: .models.contacts.UpdateContactFieldParams imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
11-11: .models.contacts.UpdateContactParams imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
🔇 Additional comments (14)
mailtrap/models/common.py (1)
24-24: Widening DeletedObject.id to support string IDs — LGTMMatches the API behavior where contact IDs can be strings or ints.
tests/conftest.py (1)
13-22: New error constants (429/500/422) — LGTMConsistent with existing patterns and used across tests.
tests/unit/api/test_contact_lists.py (1)
60-69: Expanded error coverage to 429/500 across endpoints — LGTMGood parity with other API resources.
Also applies to: 127-136, 187-196, 260-268, 332-340
tests/unit/api/test_contact_fields.py (2)
77-86: Added 429/500 and validation error scenarios — LGTMImproves negative-path coverage consistently across endpoints.
Also applies to: 142-151, 204-218, 285-299, 366-399
377-397: Placeholder format inconsistency in expected error textFirst string uses "%{automation names}" while the second uses "{segment names}". Verify this mirrors the real API payload and isn’t a typo.
mailtrap/api/contacts.py (2)
3-3: Import of ContactsApi — LGTMKeeps resource imports co-located with contact_fields/lists.
20-22: New contacts property — LGTMAligns with existing property pattern; simple entry point to ContactsApi.
tests/unit/models/test_contacts.py (2)
67-86: CreateContactParams tests cover happy-path and None exclusion well.Assertions align with RequestParams.api_data behavior. LGTM.
89-122: UpdateContactParams tests validate guard and payload shaping correctly.Coverage for validation error and full dict looks solid. LGTM.
tests/unit/api/test_contacts.py (1)
247-250: Validation error aggregation assertion matches HttpClient._extract_errors semantics.Good check on nested list flattening and joined error message. LGTM.
mailtrap/models/contacts.py (1)
46-49: New models and response wrapper look consistent and minimal.Enums, dataclasses, and response mapping align with API tests. LGTM.
Also applies to: 84-97
examples/contacts/contacts.py (1)
15-26: Example usage is clear and idiomatic.Good demonstration of CreateContactParams and API call flow. LGTM.
mailtrap/api/resources/contacts.py (2)
11-15: Clean, minimal wrapper — LGTM.Constructor and field assignments are straightforward and consistent with the rest of the client patterns.
20-25: No changes required for api_data usageVerified that
api_datais defined as a@propertyonRequestParamsinmailtrap/models/common.py(lines 14–17). Therefore, callingcontact_params.api_datacorrectly returns the serialized data dictionary, and no updates toapi_data()are needed.
Motivation
In this PR I added ContactsApi.
Changes
How to test
Summary by CodeRabbit
New Features
Documentation
Tests