-
Notifications
You must be signed in to change notification settings - Fork 8
Fix issue #20: Add ContactListsApi, related models, tests, examples #36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds Contact Lists: new models, a ContactListsApi resource with full CRUD, a ContactsBaseApi accessor, package re-export of ContactListParams, an examples script, and unit tests covering success and error cases for /api/accounts/{account_id}/contacts/lists endpoints. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant MC as MailtrapClient
participant CBA as ContactsBaseApi
participant CLA as ContactListsApi
participant HTTP as HttpClient
participant API as Mailtrap REST API
Dev->>MC: instantiate(client_config)
Dev->>CBA: mc.contacts(account_id)
CBA-->>Dev: ContactsBaseApi
Dev->>CLA: contacts.contact_lists (accessor)
CLA-->>Dev: ContactListsApi
rect rgba(200,230,255,0.25)
Dev->>CLA: create/list/get/update/delete(...)
CLA->>HTTP: HTTP request to /api/accounts/{account_id}/contacts/lists[/id]
HTTP->>API: network call
API-->>HTTP: JSON response / status
HTTP-->>CLA: parsed response
CLA-->>Dev: ContactList / list / DeletedObject
end
alt Error (4xx)
API-->>HTTP: error payload
HTTP-->>CLA: raises APIError
CLA-->>Dev: APIError (propagated)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 (8)
🚧 Files skipped from review as they are similar to previous changes (6)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-08-25T13:17:32.889Z
Applied to files:
🧬 Code graph analysis (1)mailtrap/__init__.py (1)
🪛 Ruff (0.12.2)mailtrap/__init__.py8-8: (F401) ⏰ 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)
🔇 Additional comments (1)
✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (10)
tests/unit/models/test_contacts.py (1)
56-63
: Fix test name typo (ContactField → ContactList)The test name references ContactField but validates
ContactListParams
. Rename for clarity.class TestContactListParams: - def test_create_contact_field_params_api_data_should_return_correct_dict( + def test_contact_list_params_api_data_should_return_correct_dict( self, ) -> None: params = ContactListParams(name="Test List") api_data = params.api_data assert api_data == {"name": "Test List"}mailtrap/models/contacts.py (1)
33-36
: Validate name is non-empty for ContactListParamsGuard against accidental empty/whitespace-only names at the model layer. With pydantic v2 dataclasses,
Field(min_length=1)
is lightweight and consistent with existingRequestParams.api_data
behavior.-from pydantic.dataclasses import dataclass +from pydantic.dataclasses import dataclass +from pydantic import Field @@ @dataclass class ContactListParams(RequestParams): - name: str + name: str = Field(min_length=1)If you also want to trim input, you can later switch to:
from typing_extensions import Annotated
andfrom pydantic import StringConstraints
withname: Annotated[str, StringConstraints(strip_whitespace=True, min_length=1)]
.tests/unit/api/test_contact_lists.py (2)
189-206
: Assert outbound JSON payload for create()Strengthen this test by asserting the exact body sent to the API.
-from typing import Any +from typing import Any +import json @@ contact_list = contact_lists_api.create(create_contact_list_params) assert isinstance(contact_list, ContactList) assert contact_list.id == LIST_ID assert contact_list.name == "My Contact List" + # Verify payload matches params + assert json.loads(responses.calls[-1].request.body) == create_contact_list_params.api_data
251-268
: Assert outbound JSON payload for update()Mirror the create() test by validating the request body for update as well.
- contact_list = contact_lists_api.update(LIST_ID, update_contact_list_params) + contact_list = contact_lists_api.update(LIST_ID, update_contact_list_params) assert isinstance(contact_list, ContactList) assert contact_list.id == LIST_ID assert contact_list.name == "Updated Contact List" + # Verify payload matches params + import json + assert json.loads(responses.calls[-1].request.body) == update_contact_list_params.api_dataexamples/contacts/contact_lists.py (2)
5-6
: Polish placeholders for clarityUse “YOUR_...” in placeholders to avoid confusion.
-API_TOKEN = "YOU_API_TOKEN" -ACCOUNT_ID = "YOU_ACCOUNT_ID" +API_TOKEN = "YOUR_API_TOKEN" +ACCOUNT_ID = "YOUR_ACCOUNT_ID"
34-35
: Optional: show safe configuration pattern via environment variablesExamples commonly read secrets from env vars to reduce accidental token leaks when users copy-paste.
-if __name__ == "__main__": - print(list_contact_lists()) +if __name__ == "__main__": + import os + token = os.getenv("MAILTRAP_API_TOKEN", API_TOKEN) + account = os.getenv("MAILTRAP_ACCOUNT_ID", ACCOUNT_ID) + if token != "YOUR_API_TOKEN" and account != "YOUR_ACCOUNT_ID": + print(list_contact_lists()) + else: + print("Set MAILTRAP_API_TOKEN and MAILTRAP_ACCOUNT_ID to run this example.")mailtrap/api/resources/contact_lists.py (4)
40-44
: Guard against list_id=0 edge case in path builderTruthiness check could skip 0. While IDs are typically positive, prefer explicit None check.
- if list_id: + if list_id is not None: return f"{path}/{list_id}"
14-16
: Handle empty/None responses defensively in get_listHttpClient returns None for empty content. Be defensive and coerce to an empty list to keep the return type stable.
- response = self._client.get(self._api_path()) - return [ContactList(**field) for field in response] + response = self._client.get(self._api_path()) + return [ContactList(**field) for field in (response or [])]
9-13
: Optional: add a class docstring for discoverabilityA brief docstring helps users of the SDK quickly understand scope and usage.
class ContactListsApi: + """CRUD operations for contact lists within a specific Mailtrap account."""
36-38
: Use keyword argument for DeletedObject initialization for clarity and consistencyWhile the
DeletedObject
dataclass currently only has a singleid
field—so positional initialization will work—the keyword form is clearer and more robust against future changes (e.g. adding new fields or reordering). To keep all delete handlers consistent, please update the other resources as well.Locations to update:
mailtrap/api/resources/contact_lists.py
mailtrap/api/resources/templates.py
mailtrap/api/resources/contact_fields.py
Example diff for
contact_lists.py
:- def delete(self, list_id: int) -> DeletedObject: - self._client.delete(self._api_path(list_id)) - return DeletedObject(list_id) + def delete(self, list_id: int) -> DeletedObject: + self._client.delete(self._api_path(list_id)) + return DeletedObject(id=list_id)Apply the same change to the
delete
methods in:
templates.py
(line 41)contact_fields.py
(line 43)
📜 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 (7)
examples/contacts/contact_lists.py
(1 hunks)mailtrap/__init__.py
(1 hunks)mailtrap/api/contacts.py
(2 hunks)mailtrap/api/resources/contact_lists.py
(1 hunks)mailtrap/models/contacts.py
(1 hunks)tests/unit/api/test_contact_lists.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.py
🧬 Code graph analysis (7)
mailtrap/models/contacts.py (1)
mailtrap/models/common.py (1)
RequestParams
(12-18)
tests/unit/api/test_contact_lists.py (6)
mailtrap/api/contacts.py (1)
contact_lists
(16-17)mailtrap/api/resources/contact_lists.py (6)
ContactListsApi
(9-44)get_list
(14-16)get_by_id
(18-20)create
(22-27)update
(29-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
(22-23)mailtrap/models/contacts.py (2)
ContactList
(39-41)ContactListParams
(34-35)
mailtrap/api/contacts.py (1)
mailtrap/api/resources/contact_lists.py (1)
ContactListsApi
(9-44)
tests/unit/models/test_contacts.py (2)
mailtrap/models/contacts.py (1)
ContactListParams
(34-35)mailtrap/models/common.py (1)
api_data
(14-18)
mailtrap/api/resources/contact_lists.py (3)
mailtrap/http.py (4)
HttpClient
(13-96)get
(25-29)post
(31-33)patch
(39-41)mailtrap/models/common.py (2)
DeletedObject
(22-23)api_data
(14-18)mailtrap/models/contacts.py (2)
ContactList
(39-41)ContactListParams
(34-35)
mailtrap/__init__.py (1)
mailtrap/models/contacts.py (1)
ContactListParams
(34-35)
examples/contacts/contact_lists.py (6)
mailtrap/models/common.py (1)
DeletedObject
(22-23)mailtrap/models/contacts.py (2)
ContactField
(26-30)ContactListParams
(34-35)mailtrap/client.py (2)
MailtrapClient
(24-142)contacts_api
(68-73)tests/unit/api/test_contact_lists.py (1)
contact_lists_api
(23-24)mailtrap/api/contacts.py (1)
contact_lists
(16-17)mailtrap/api/resources/contact_lists.py (5)
create
(22-27)update
(29-34)get_list
(14-16)get_by_id
(18-20)delete
(36-38)
🪛 Ruff (0.12.2)
mailtrap/__init__.py
8-8: .models.contacts.ContactListParams
imported but unused; consider removing, adding to __all__
, or using a redundant alias
(F401)
🔇 Additional comments (4)
mailtrap/api/contacts.py (1)
15-17
: Property exposure of ContactListsApi — LGTM and consistent with designReturning a new
ContactListsApi
via a@property
mirrorscontact_fields
(per retrieved learnings) and keeps the surface uniform. No issues.mailtrap/models/contacts.py (1)
38-41
: ContactList dataclass — LGTMSimple response model with
id
andname
maps cleanly from API responses.tests/unit/api/test_contact_lists.py (1)
45-47
: Comprehensive CRUD and error-path coverage — nice workGood use of
responses
to exercise success and 401/403/404 branches across list/get/create/update/delete. Types and value assertions read well.examples/contacts/contact_lists.py (1)
9-9
: No changes needed:contact_lists
is a propertyVerified that in
mailtrap/api/contacts.py
thecontact_lists
accessor is decorated with@property
(lines 15–17), so using it without parentheses is correct.
Motivation
In this PR I added ContactListsApi.
Changes
How to test
Images and GIFs
Summary by CodeRabbit