Fix issue #27: Add InboxesApi, related models, tests, examples. #40
Conversation
…ectsApi with parameters classes, add docstring to all resources methods.
WalkthroughAdds account-scoped InboxesApi and exposes it via TestingApi.inboxes; introduces CreateInboxParams, UpdateInboxParams, and ProjectParams models (with validation), re-exports them, updates ProjectsApi signatures to accept ProjectParams, adds example usage and tests, and adds docstrings to several resource modules. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant MC as MailtrapClient
participant T as TestingApi
participant I as InboxesApi
participant HTTP as Mailtrap HTTP API
Dev->>MC: instantiate(api_token)
Dev->>T: MC.testing(account_id)
T-->>Dev: TestingApi
Dev->>I: T.inboxes (property)
I-->>Dev: InboxesApi
rect rgba(220,235,255,0.35)
note right of I: List inboxes
Dev->>I: get_list()
I->>HTTP: GET /api/accounts/{account_id}/inboxes
HTTP-->>I: 200 OK (list)
I-->>Dev: list[Inbox]
end
rect rgba(220,255,220,0.35)
note right of I: Create inbox
Dev->>I: create(project_id, CreateInboxParams)
I->>HTTP: POST /api/accounts/{account_id}/projects/{project_id}/inboxes {"inbox": {...}}
HTTP-->>I: 201 Created (Inbox)
I-->>Dev: Inbox
end
rect rgba(255,245,210,0.35)
note right of I: Update & Actions
Dev->>I: update(inbox_id, UpdateInboxParams)
I->>HTTP: PATCH /api/accounts/{account_id}/inboxes/{inbox_id}
HTTP-->>I: 200 OK (Inbox)
I-->>Dev: Inbox
Dev->>I: clean / all_read / reset_credentials / toggle_email_username / reset_email_username
I->>HTTP: PATCH .../{action}
HTTP-->>I: 200 OK (Inbox)
I-->>Dev: Inbox
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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. 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mailtrap/api/resources/contact_fields.py (1)
29-33: api_data used as attribute; should be calledThis will otherwise send a method object in JSON.
- json=field_params.api_data, + json=field_params.api_data(),- json=field_params.api_data, + json=field_params.api_data(),Also applies to: 42-45
🧹 Nitpick comments (21)
mailtrap/api/resources/templates.py (2)
50-51: Truthiness check may skip template_id=0Be explicit to avoid accidental falsy IDs.
- if template_id: + if template_id is not None: return f"{path}/{template_id}"
16-16: Nit: tighten docstring phrasingPrefer imperative, consistent style across resources.
- """Get all email templates existing in your account.""" + """List email templates in the account."""mailtrap/api/resources/contact_fields.py (1)
16-16: Polish docstrings for grammar/consistencyLowercase “contact field”, use singular where appropriate, and remove comma splices.
- """Get all Contact Fields existing in your account.""" + """List contact fields in the account."""- """Get a contact Field by ID.""" + """Get a contact field by ID."""- """Create new Contact Fields. Please note, you can have up to 40 fields.""" + """Create a contact field. You can have up to 40 custom fields per account."""- """ - Update existing Contact Field. Please note, - you cannot change data_type of the field. - """ + """Update a contact field. Note: data_type cannot be changed."""- """ - Delete existing Contact Field Please, note, you cannot delete a Contact Field - which is used in Automations, Email Campaigns (started or scheduled), and in - conditions of Contact Segments (you'll see the corresponding error) - """ + """Delete a contact field. + + You cannot delete a field used in Automations, Email Campaigns (started or scheduled), + or in Contact Segment conditions; the API will return an error."""Also applies to: 21-21, 28-28, 38-41, 49-53
mailtrap/api/resources/contact_lists.py (1)
15-15: Unify docstring styleUse imperative mood and singular nouns.
- """Get all contact lists existing in your account.""" + """List contact lists in the account."""- """Get a contact list by ID.""" + """Get a contact list by ID."""- """Create new Contact Lists.""" + """Create a contact list."""- """Update existing Contact List.""" + """Update a contact list."""- """Delete existing Contact List.""" + """Delete a contact list."""Also applies to: 20-20, 25-25, 33-33, 41-41
mailtrap/api/resources/contacts.py (1)
18-18: Clarify that URL encoding is handled internally; standardize docstring style.
The current phrasing may imply users must pre-encode. Also consider adding Parameters/Returns/Raises for consistency.Apply this doc tweak:
- """Get contact using id or email (URL encoded).""" + """Get a contact by id or email (URL encoding handled automatically).""" - """Create a new contact.""" + """Create a new contact.""" - """Update contact using id or email (URL encoded).""" + """Update a contact by id or email (URL encoding handled automatically).""" - """Delete contact using id or email (URL encoded).""" + """Delete a contact by id or email (URL encoding handled automatically)."""Also applies to: 23-23, 33-33, 41-41
tests/unit/models/test_projects.py (1)
7-7: Ruff S101 in tests — either ignore or configure per-file.
If the repo doesn’t already ignore S101 for tests, add a local noqa.- assert entity.api_data == {"name": "test"} + assert entity.api_data == {"name": "test"} # noqa: S101Alternatively, configure Ruff: [tool.ruff.per-file-ignores] "tests/**" = ["S101"].
mailtrap/models/projects.py (1)
25-27: Add a brief docstring for discoverability.
Optional but keeps parity with other request param models.@dataclass class ProjectParams(RequestParams): - name: str + """Payload for project create/update.""" + name: strmailtrap/api/testing.py (1)
20-22: Optional: add a short docstring to the inboxes property.
Improves IDE/tooling hints and matches docstring push in this PR.@property def inboxes(self) -> InboxesApi: - return InboxesApi(account_id=self._account_id, client=self._client) + """Access the Inboxes API for the current account.""" + return InboxesApi(account_id=self._account_id, client=self._client)tests/unit/models/test_inboxes.py (1)
1-31: Optional: silence Ruff S101 for testsIf Ruff is enforced in CI, either add per-file ignores for tests or annotate individual asserts with noqa to avoid noise.
Example pyproject setting:
[tool.ruff.lint.per-file-ignores] "tests/**" = ["S101"]tests/unit/api/testing/test_projects.py (2)
236-239: Typo in test input stringMinor nit: “Update Project Namet” → “Updated Project Name” for clarity. No behavior change.
- _ = client.update( - PROJECT_ID, project_params=ProjectParams(name="Update Project Namet") - ) + _ = client.update( + PROJECT_ID, project_params=ProjectParams(name="Updated Project Name") + )
190-197: Optionally assert request JSON payloadsTo harden tests, assert the exact JSON body sent on create/update using responses matchers.
+from responses import matchers @@ - responses.post( - BASE_PROJECTS_URL, - json=sample_project_dict, - status=201, - ) + responses.post( + BASE_PROJECTS_URL, + json=sample_project_dict, + status=201, + match=[matchers.json_params_matcher({"project": {"name": "New Project"}})], + ) @@ - responses.patch( - url, - json=updated_project_dict, - status=200, - ) + responses.patch( + url, + json=updated_project_dict, + status=200, + match=[matchers.json_params_matcher({"project": {"name": updated_name}})], + )Also applies to: 252-260
mailtrap/models/inboxes.py (1)
45-57: Keep ValueError, but centralize the message and simplify the checkBehavior is fine; consider de-duplicating the error string (helps tests) and simplifying the condition. Also addresses TRY003 hint.
-from typing import Optional +from typing import Optional, ClassVar @@ @dataclass class UpdateInboxParams(RequestParams): name: Optional[str] = None email_username: Optional[str] = None + ERROR_MSG: ClassVar[str] = "At least one field must be provided for update action" def __post_init__(self) -> None: - if all( - value is None - for value in [ - self.name, - self.email_username, - ] - ): - raise ValueError("At least one field must be provided for update action") + if self.name is None and self.email_username is None: + raise ValueError(self.ERROR_MSG)mailtrap/api/resources/projects.py (1)
51-55: Guard against falsy project_id values in _api_pathUse an explicit None check to avoid misrouting if 0 ever surfaces.
- if project_id: + if project_id is not None: return f"{path}/{project_id}"tests/unit/api/testing/test_inboxes.py (2)
200-211: Optionally assert JSON payloads for create/updateLike projects tests, add matchers to verify exact request bodies sent to the API.
+from responses import matchers @@ - responses.post( + responses.post( BASE_PROJECT_INBOXES_URL, status=status_code, json=response_json, - ) + ) @@ - responses.post( + responses.post( BASE_PROJECT_INBOXES_URL, json=sample_inbox_dict, status=201, + match=[matchers.json_params_matcher({"inbox": {"name": "New Inbox"}})], ) @@ - responses.patch( + responses.patch( url, status=status_code, json=response_json, - ) + ) @@ - responses.patch( + responses.patch( url, json=updated_inbox_dict, status=200, + match=[matchers.json_params_matcher({"inbox": {"name": updated_name}})], )Also applies to: 223-229, 258-269, 287-290
536-553: Naming nit: “enable_email_address” toggles stateMethod name implies one-way “enable”, but endpoint is toggle_email_username. Consider adding a toggle_email_username alias in the API for clarity while keeping the current name for backward compatibility.
Would you like a follow-up PR to add a method alias and docstring clarifications?
Also applies to: 556-571
examples/testing/projects.py (1)
4-4: Silence the S105 false positive for example creds.Keep placeholders for consistency across examples, but add a localized Ruff ignore to avoid noisy alerts.
-API_TOKEN = "YOU_API_TOKEN" +API_TOKEN = "YOU_API_TOKEN" # noqa: S105examples/testing/inboxes.py (3)
49-51: Rename to reflect toggle behavior.Endpoint toggles the state; consider naming the wrapper accordingly to avoid confusion.
-def enable_inbox_email_address(inbox_id: int) -> Inbox: - return inboxes_api.enable_email_address(inbox_id) +def toggle_inbox_email_address(inbox_id: int) -> Inbox: + return inboxes_api.enable_email_address(inbox_id)
57-58: Add return type for consistency.Resources return Inbox on delete; mirror that in the example.
-def delete_inbox(inbox_id: int): +def delete_inbox(inbox_id: int) -> Inbox: return inboxes_api.delete(inbox_id)
6-6: Silence the S105 false positive for example creds.Same rationale as other examples.
-API_TOKEN = "YOU_API_TOKEN" +API_TOKEN = "YOU_API_TOKEN" # noqa: S105mailtrap/api/resources/inboxes.py (2)
70-74: Robust None-check for ID.Use an explicit None check to avoid edge cases if 0 ever appears (defensive, low risk).
- def _api_path(self, inbox_id: Optional[int] = None) -> str: + def _api_path(self, inbox_id: int | None = None) -> str: path = f"/api/accounts/{self._account_id}/inboxes" - if inbox_id: + if inbox_id is not None: return f"{path}/{inbox_id}" return pathAlso remove the now-unused
Optionalimport at Line 1:-from typing import Optional +from __future__ import annotations
60-64: Method name vs. behavior (toggle).Endpoint toggles the email address; consider an alias with clearer naming to reduce ambiguity without breaking the current API.
def enable_email_address(self, inbox_id: int) -> Inbox: """Turn the email address of the inbox on/off.""" response = self._client.patch(f"{self._api_path(inbox_id)}/toggle_email_username") return Inbox(**response) + + # Alias for clarity; preserves existing name. + def toggle_email_address(self, inbox_id: int) -> Inbox: + """Alias for enable_email_address; toggles the inbox email address.""" + return self.enable_email_address(inbox_id)
📜 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 (18)
examples/testing/inboxes.py(1 hunks)examples/testing/projects.py(1 hunks)mailtrap/__init__.py(1 hunks)mailtrap/api/resources/contact_fields.py(2 hunks)mailtrap/api/resources/contact_imports.py(1 hunks)mailtrap/api/resources/contact_lists.py(1 hunks)mailtrap/api/resources/contacts.py(2 hunks)mailtrap/api/resources/inboxes.py(1 hunks)mailtrap/api/resources/projects.py(2 hunks)mailtrap/api/resources/templates.py(2 hunks)mailtrap/api/sending.py(1 hunks)mailtrap/api/testing.py(2 hunks)mailtrap/models/inboxes.py(3 hunks)mailtrap/models/projects.py(2 hunks)tests/unit/api/testing/test_inboxes.py(1 hunks)tests/unit/api/testing/test_projects.py(5 hunks)tests/unit/models/test_inboxes.py(1 hunks)tests/unit/models/test_projects.py(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 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 inherits from RequestParams, which defines api_data as a property that returns a filtered dictionary, so it should be accessed without parentheses like contact.api_data.
Applied to files:
mailtrap/models/inboxes.pytests/unit/models/test_projects.pymailtrap/__init__.pymailtrap/models/projects.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:
mailtrap/__init__.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.pytests/unit/api/testing/test_projects.pyexamples/testing/projects.pymailtrap/api/resources/projects.pyexamples/testing/inboxes.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:
mailtrap/api/resources/contact_fields.py
📚 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/inboxes.py
🧬 Code graph analysis (17)
mailtrap/models/inboxes.py (1)
mailtrap/models/common.py (1)
RequestParams(13-19)
tests/unit/models/test_projects.py (2)
mailtrap/models/projects.py (1)
ProjectParams(26-27)mailtrap/models/common.py (1)
api_data(15-19)
mailtrap/__init__.py (3)
mailtrap/api/testing.py (2)
inboxes(21-22)projects(17-18)mailtrap/models/inboxes.py (2)
CreateInboxParams(40-41)UpdateInboxParams(45-57)mailtrap/models/projects.py (1)
ProjectParams(26-27)
mailtrap/api/testing.py (4)
mailtrap/api/resources/inboxes.py (1)
InboxesApi(9-74)tests/unit/api/testing/test_inboxes.py (1)
client(25-26)tests/unit/api/testing/test_projects.py (1)
client(21-22)tests/unit/api/email_templates/test_email_templates.py (1)
client(22-23)
mailtrap/api/resources/templates.py (3)
mailtrap/http.py (3)
get(25-29)patch(39-41)delete(43-45)mailtrap/models/templates.py (2)
EmailTemplate(40-49)CreateEmailTemplateParams(9-14)mailtrap/models/common.py (2)
api_data(15-19)DeletedObject(23-24)
mailtrap/api/resources/contact_imports.py (3)
mailtrap/http.py (1)
post(31-33)mailtrap/models/common.py (1)
api_data(15-19)mailtrap/models/contacts.py (1)
ContactImport(108-113)
mailtrap/api/resources/contact_fields.py (3)
mailtrap/http.py (2)
get(25-29)patch(39-41)mailtrap/models/contacts.py (2)
ContactField(28-32)CreateContactFieldParams(11-14)mailtrap/models/common.py (2)
api_data(15-19)DeletedObject(23-24)
mailtrap/api/resources/contacts.py (3)
mailtrap/http.py (2)
get(25-29)patch(39-41)mailtrap/models/contacts.py (3)
ContactResponse(96-97)CreateContactParams(52-57)Contact(85-92)mailtrap/models/common.py (1)
DeletedObject(23-24)
mailtrap/api/resources/contact_lists.py (4)
mailtrap/api/resources/contact_fields.py (1)
_api_path(57-61)mailtrap/api/resources/contacts.py (1)
_api_path(45-49)mailtrap/models/contacts.py (2)
ContactList(41-43)ContactListParams(36-37)mailtrap/models/common.py (2)
api_data(15-19)DeletedObject(23-24)
tests/unit/api/testing/test_inboxes.py (5)
mailtrap/api/testing.py (1)
inboxes(21-22)mailtrap/api/resources/inboxes.py (11)
InboxesApi(9-74)get_list(14-17)get_by_id(19-22)create(24-30)update(32-38)delete(40-43)clean(45-48)mark_as_read(50-53)reset_credentials(55-58)enable_email_address(60-63)reset_email_username(65-68)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/inboxes.py (3)
CreateInboxParams(40-41)Inbox(10-36)UpdateInboxParams(45-57)
tests/unit/models/test_inboxes.py (2)
mailtrap/models/inboxes.py (2)
CreateInboxParams(40-41)UpdateInboxParams(45-57)mailtrap/models/common.py (1)
api_data(15-19)
mailtrap/models/projects.py (1)
mailtrap/models/common.py (1)
RequestParams(13-19)
mailtrap/api/resources/inboxes.py (4)
mailtrap/http.py (4)
HttpClient(13-96)get(25-29)post(31-33)patch(39-41)mailtrap/api/testing.py (1)
inboxes(21-22)mailtrap/models/inboxes.py (3)
CreateInboxParams(40-41)Inbox(10-36)UpdateInboxParams(45-57)mailtrap/models/common.py (1)
api_data(15-19)
tests/unit/api/testing/test_projects.py (3)
mailtrap/api/testing.py (1)
projects(17-18)mailtrap/models/projects.py (1)
ProjectParams(26-27)mailtrap/api/resources/projects.py (2)
create(24-33)update(35-44)
examples/testing/projects.py (3)
mailtrap/models/projects.py (2)
Project(17-22)ProjectParams(26-27)mailtrap/client.py (2)
MailtrapClient(24-142)testing_api(51-57)mailtrap/api/resources/projects.py (4)
get_list(14-17)get_by_id(19-22)create(24-33)update(35-44)
mailtrap/api/resources/projects.py (3)
mailtrap/api/testing.py (1)
projects(17-18)mailtrap/models/projects.py (2)
ProjectParams(26-27)Project(17-22)mailtrap/models/common.py (1)
api_data(15-19)
examples/testing/inboxes.py (4)
mailtrap/api/testing.py (1)
inboxes(21-22)mailtrap/models/inboxes.py (3)
Inbox(10-36)CreateInboxParams(40-41)UpdateInboxParams(45-57)mailtrap/client.py (2)
MailtrapClient(24-142)testing_api(51-57)mailtrap/api/resources/inboxes.py (10)
get_list(14-17)create(24-30)get_by_id(19-22)update(32-38)clean(45-48)mark_as_read(50-53)reset_credentials(55-58)enable_email_address(60-63)reset_email_username(65-68)delete(40-43)
🪛 Ruff (0.12.2)
mailtrap/models/inboxes.py
57-57: Avoid specifying long messages outside the exception class
(TRY003)
tests/unit/models/test_projects.py
7-7: Use of assert detected
(S101)
tests/unit/api/testing/test_inboxes.py
98-98: Use of assert detected
(S101)
112-112: Use of assert detected
(S101)
113-113: Use of assert detected
(S101)
114-114: Use of assert detected
(S101)
154-154: Use of assert detected
(S101)
169-169: Use of assert detected
(S101)
170-170: Use of assert detected
(S101)
211-211: Use of assert detected
(S101)
227-227: Use of assert detected
(S101)
228-228: Use of assert detected
(S101)
270-270: Use of assert detected
(S101)
289-289: Use of assert detected
(S101)
290-290: Use of assert detected
(S101)
330-330: Use of assert detected
(S101)
345-345: Use of assert detected
(S101)
346-346: Use of assert detected
(S101)
386-386: Use of assert detected
(S101)
401-401: Use of assert detected
(S101)
402-402: Use of assert detected
(S101)
442-442: Use of assert detected
(S101)
457-457: Use of assert detected
(S101)
458-458: Use of assert detected
(S101)
498-498: Use of assert detected
(S101)
513-513: Use of assert detected
(S101)
514-514: Use of assert detected
(S101)
554-554: Use of assert detected
(S101)
569-569: Use of assert detected
(S101)
570-570: Use of assert detected
(S101)
610-610: Use of assert detected
(S101)
625-625: Use of assert detected
(S101)
626-626: Use of assert detected
(S101)
tests/unit/models/test_inboxes.py
11-11: Use of assert detected
(S101)
19-19: Use of assert detected
(S101)
23-23: Use of assert detected
(S101)
27-27: Use of assert detected
(S101)
30-30: Use of assert detected
(S101)
examples/testing/projects.py
4-4: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/testing/inboxes.py
6-6: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
⏰ 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). (1)
- GitHub Check: Test python3.13 on macos-latest
🔇 Additional comments (20)
mailtrap/api/sending.py (1)
21-21: Docstring addition looks fineShort, clear, and consistent with nearby resources.
mailtrap/api/resources/templates.py (1)
26-31: Incorrect suggestion —api_datais a property, not a method
RequestParamsdeclaresapi_datawith@property, so.api_dataalready returns the payload dict. Adding()would attempt to call a dict and raise a TypeError. No changes needed.Likely an incorrect or invalid review comment.
mailtrap/api/resources/contact_imports.py (2)
14-19: Docstring is solidClear limits and async behavior described.
20-23: api_data used as attribute in comprehensionCall the method to build a serializable payload.
- json={"contacts": [contact.api_data for contact in contacts]}, + json={"contacts": [contact.api_data() for contact in contacts]},⛔ Skipped due to learnings
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.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 inherits from RequestParams, which defines api_data as a property that returns a filtered dictionary, so it should be accessed without parentheses like contact.api_data.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 inherits from RequestParams, which defines api_data as a property that returns a filtered dictionary, so it should be accessed without parentheses like contact.api_data.mailtrap/api/resources/contact_lists.py (1)
26-29: api_data used as attribute; should be calledFix payload construction in create/update.
- json=list_params.api_data, + json=list_params.api_data(),- json=list_params.api_data, + json=list_params.api_data(),Also applies to: 34-37
⛔ Skipped due to learnings
Learnt from: Ihor-Bilous PR: railsware/mailtrap-python#34 File: mailtrap/api/resources/templates.py:33-36 Timestamp: 2025-08-21T10:37:54.227Z Learning: In the mailtrap-python codebase, api_data is a property on ParametersObject (and classes that inherit from it), not a method. It should be accessed without parentheses: template_params.api_data, not template_params.api_data().mailtrap/api/resources/contacts.py (1)
18-18: Docstrings added — nice coverage across CRUD.
Clearer public API docs; consistent with other resources.Also applies to: 23-23, 33-33, 41-41
mailtrap/models/projects.py (1)
25-27: ProjectParams looks correct and aligns with RequestParams usage.
Matches how api_data is produced elsewhere; minimal, type-safe payload object.mailtrap/__init__.py (1)
13-14: Re-exports LGTM.
Expands the public surface predictably (CreateInboxParams, UpdateInboxParams, ProjectParams).Also applies to: 21-21
mailtrap/api/testing.py (1)
3-3: New inboxes property mirrors projects — LGTM.
Straightforward exposure of InboxesApi via TestingApi.Also applies to: 20-22
tests/unit/models/test_inboxes.py (3)
10-11: LGTM: api_data for CreateInboxParamsShape and property access look correct and consistent with RequestParams.api_data semantics.
21-24: LGTM: api_data for UpdateInboxParams with both fieldsOutput dict matches inputs and excludes None as intended.
25-31: LGTM: api_data for UpdateInboxParams with single-field inputsBoth cases are correct and validate exclude_none behavior.
tests/unit/api/testing/test_projects.py (1)
182-182: LGTM: switched to ProjectParams for createSignature and payload usage align with ProjectsApi.create expecting params.api_data.
mailtrap/models/inboxes.py (2)
26-26: LGTM: Inbox.api_domain addedField addition looks consistent with sample payloads and Inbox mapping.
39-42: LGTM: CreateInboxParamsMinimal, correct, and leverages RequestParams.api_data to serialize.
mailtrap/api/resources/projects.py (2)
24-33: LGTM: create now accepts ProjectParamsSignature and payload switch to params.api_data is clean and future-proof.
35-44: LGTM: update now accepts ProjectParamsConsistent with create; good docstring additions.
tests/unit/api/testing/test_inboxes.py (1)
29-61: LGTM: sample inbox fixture mirrors model schema wellCovers all required fields, including api_domain and permissions.
examples/testing/projects.py (1)
15-25: Good adoption of ProjectParams and ID-based ops.API usage aligns with ProjectsApi surface (get_by_id/create/update) and new ProjectParams model. Looks consistent with the broader shift in this PR.
examples/testing/inboxes.py (1)
13-55: Example wrappers read well and mirror the InboxesApi.Signatures and payloads (CreateInboxParams/UpdateInboxParams) match the API; type hints are clear.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/unit/models/test_inboxes.py (1)
15-19: Fix exception assertion — usematchand correct the message typoSame issue as noted previously: rely on
matchand fixactio→action. Also drop the unused assignment.- def test_raise_error_when_all_fields_are_missing(self) -> None: - with pytest.raises(ValueError) as exc: - _ = UpdateInboxParams() - - assert "At least one field must be provided for update actio" in str(exc) + def test_raise_error_when_all_fields_are_missing(self) -> None: + with pytest.raises( + ValueError, + match="At least one field must be provided for update action", + ): + UpdateInboxParams()
🧹 Nitpick comments (1)
tests/unit/models/test_inboxes.py (1)
10-10: Silence Ruff S101 for pytestassertin testsPytest idiomatically uses
assert; to keep the current style and satisfy Ruff, add a per-file ignore.Option A — file-level directive (quick fix):
+ # ruff: noqa: S101 import pytestOption B — repo-wide config (preferred, if consistent with the codebase):
# pyproject.toml [tool.ruff.lint.per-file-ignores] "tests/**" = ["S101"]Also applies to: 18-18, 22-22, 26-26, 29-29
📜 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 (2)
tests/unit/models/test_inboxes.py(1 hunks)tests/unit/models/test_templates.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/unit/models/test_inboxes.py (2)
mailtrap/models/inboxes.py (2)
CreateInboxParams(40-41)UpdateInboxParams(45-57)mailtrap/models/common.py (1)
api_data(15-19)
🪛 Ruff (0.12.2)
tests/unit/models/test_inboxes.py
10-10: Use of assert detected
(S101)
18-18: Use of assert detected
(S101)
22-22: Use of assert detected
(S101)
26-26: Use of assert detected
(S101)
29-29: Use of assert detected
(S101)
⏰ 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). (5)
- GitHub Check: Test python3.13 on windows-latest
- GitHub Check: Test python3.11 on windows-latest
- GitHub Check: Test python3.12 on windows-latest
- GitHub Check: Test python3.9 on windows-latest
- GitHub Check: Test python3.10 on windows-latest
Ihor-Bilous
force-pushed
the
ISSUE-27
branch
from
81b783f to
c4d85e5
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
tests/unit/models/test_inboxes.py (4)
8-11: Rename “props” → “fields” in test namesMinor clarity/consistency with Python terminology.
-def test_api_data_should_return_dict_with_all_props(self) -> None: +def test_api_data_returns_all_fields(self) -> None:- def test_api_data_should_return_dict_with_all_props(self) -> None: + def test_api_data_returns_all_fields(self) -> None:Also applies to: 20-23
24-29: Clarify test name: it returns only provided fields, not “required props”The params are optional; suggest reflecting that in the name.
- def test_api_data_should_return_dict_with_required_props_only(self) -> None: + def test_api_data_returns_only_provided_fields(self) -> None:
18-18: Drop unused assignmentThe temporary
_isn’t needed insidepytest.raises.- _ = UpdateInboxParams() + UpdateInboxParams()
10-10: Ruff S101 in tests — prefer config suppression over code changes
assertis idiomatic in pytest; suppress S101 for tests instead of rewriting assertions.Add to pyproject.toml:
[tool.ruff.lint.per-file-ignores] "tests/**/*" = ["S101"]Also applies to: 22-22, 26-26, 29-29
📜 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 (2)
tests/unit/models/test_inboxes.py(1 hunks)tests/unit/models/test_templates.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/models/test_templates.py
🧰 Additional context used
🧬 Code graph analysis (1)
tests/unit/models/test_inboxes.py (2)
mailtrap/models/inboxes.py (2)
CreateInboxParams(40-41)UpdateInboxParams(45-57)mailtrap/models/common.py (1)
api_data(15-19)
🪛 Ruff (0.12.2)
tests/unit/models/test_inboxes.py
10-10: Use of assert detected
(S101)
22-22: Use of assert detected
(S101)
26-26: Use of assert detected
(S101)
29-29: Use of assert detected
(S101)
⏰ 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). (5)
- GitHub Check: Test python3.10 on windows-latest
- GitHub Check: Test python3.12 on windows-latest
- GitHub Check: Test python3.9 on windows-latest
- GitHub Check: Test python3.13 on windows-latest
- GitHub Check: Test python3.11 on windows-latest
🔇 Additional comments (2)
tests/unit/models/test_inboxes.py (2)
14-19: Exception expectation fixed — LGTMNow correctly asserts
ValueErrorwithmatch=.... This resolves the earlier failure mode.
20-23: api_data with both fields — LGTMCovers the happy path and validates
exclude_none=Truebehavior.
Ihor-Bilous
requested review from
IgorDobryn,
i7an,
VladimirTaytor and
andrii-porokhnavets
Motivation
In this PR I added InboxesApi, related models, tests, examples, improved ProjectsApi with parameters classes, added docstrings to all resources methods.
Changes
How to test
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests