-
Notifications
You must be signed in to change notification settings - Fork 8
Fix issue #22: Add ContactImportsApi, related models, tests, examples #38
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 import support: new contact import models and params, a ContactImportsApi resource with POST/GET, a ContactsBaseApi accessor, package export, example usage script, and unit tests covering API and model behaviors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer Script
participant Client as MailtrapClient
participant Contacts as ContactsBaseApi
participant Imports as ContactImportsApi
participant HTTP as HttpClient
participant API as Mailtrap REST API
participant Model as ContactImport
Dev->>Client: Initialize(API_TOKEN, ACCOUNT_ID)
Dev->>Contacts: client.contacts_api
Contacts->>Imports: contact_imports (property)
note right of Imports: new accessor
Dev->>Imports: import_contacts([ImportContactParams])
Imports->>HTTP: POST /api/accounts/{account_id}/contacts/imports
HTTP->>API: request
API-->>HTTP: 201 Created + import JSON
HTTP-->>Imports: response JSON
Imports->>Model: Construct ContactImport
Imports-->>Dev: ContactImport(id, status=STARTED)
Dev->>Imports: get_by_id(import_id)
Imports->>HTTP: GET /api/accounts/{account_id}/contacts/imports/{id}
HTTP->>API: request
API-->>HTTP: 200 OK + import JSON
HTTP-->>Imports: response JSON
Imports->>Model: Construct ContactImport
Imports-->>Dev: ContactImport(status=STARTED/FINISHED, counts?)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
📜 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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mailtrap/models/contacts.py (1)
126-129
: ContactImportRequest should use ImportContactParams, not CreateContactParamsThe import endpoint builds payloads from
ImportContactParams
. UsingCreateContactParams
here is misleading and risks downstream misuse.Apply:
@dataclass class ContactImportRequest(RequestParams): - contacts: list[CreateContactParams] + contacts: list[ImportContactParams]
🧹 Nitpick comments (11)
mailtrap/__init__.py (1)
7-12
: Optionally export ContactImport and ContactImportStatus for parityAvoids users importing from
mailtrap.models.contacts
directly.Apply:
from .models.contacts import CreateContactParams from .models.contacts import ImportContactParams +from .models.contacts import ContactImport +from .models.contacts import ContactImportStatus from .models.contacts import UpdateContactFieldParamstests/unit/models/test_contacts.py (1)
1-150
: Consider silencing Ruff S101 in testsIf S101 is enabled globally, configure per-file-ignores for tests (preferred) instead of changing assertions.
Add to pyproject.toml:
[tool.ruff.lint.per-file-ignores] "tests/**" = ["S101"]examples/contacts/contact_imports.py (3)
4-5
: Avoid hardcoded secrets; use environment variablesPrevents accidental leaks and aligns with best practices.
Apply:
+import os import mailtrap as mt from mailtrap.models.contacts import ContactImport -API_TOKEN = "YOU_API_TOKEN" -ACCOUNT_ID = "YOU_ACCOUNT_ID" +API_TOKEN = os.environ.get("MAILTRAP_API_TOKEN") +ACCOUNT_ID = os.environ.get("MAILTRAP_ACCOUNT_ID")
7-16
: Remove top-level client instantiation to avoid side effects on importInitialize the client within functions or under
__main__
to keep the module import-safe.Apply:
-client = mt.MailtrapClient(token=API_TOKEN, account_id=ACCOUNT_ID) -contact_imports_api = client.contacts_api.contact_imports - - -def import_contacts(contacts: list[mt.ImportContactParams]) -> ContactImport: - return contact_imports_api.import_contacts(contacts=contacts) +def import_contacts(contacts: list[mt.ImportContactParams]) -> ContactImport: + client = mt.MailtrapClient(token=API_TOKEN, account_id=ACCOUNT_ID) + return client.contacts_api.contact_imports.import_contacts(contacts=contacts) - -def get_contact_import(import_id: int) -> ContactImport: - return contact_imports_api.get_by_id(import_id) +def get_contact_import(import_id: int) -> ContactImport: + client = mt.MailtrapClient(token=API_TOKEN, account_id=ACCOUNT_ID) + return client.contacts_api.contact_imports.get_by_id(import_id)And guard creds before running:
if __name__ == "__main__": + if not API_TOKEN or not ACCOUNT_ID: + raise RuntimeError("Set MAILTRAP_API_TOKEN and MAILTRAP_ACCOUNT_ID environment variables")
4-5
: Nit: Fix placeholders spellingPrefer “YOUR_API_TOKEN” / “YOUR_ACCOUNT_ID” in comments or docs if you keep constants.
mailtrap/models/contacts.py (1)
126-129
: Remove unused ContactImportRequest dataclassThe
ContactImportRequest
class inmailtrap/models/contacts.py
(lines 125–129) isn’t referenced anywhere else in the repository; drop it to eliminate dead code and clarify the API surface.mailtrap/api/resources/contact_imports.py (2)
8-18
: Accept broader iterables and validate non-empty payloads (optional)Allowing Sequence enables tuples/generators; a quick guard prevents accidental empty imports.
+from collections.abc import Sequence @@ - def import_contacts(self, contacts: list[ImportContactParams]) -> ContactImport: + def import_contacts(self, contacts: Sequence[ImportContactParams]) -> ContactImport: + if not contacts: + raise ValueError("contacts must contain at least one item")
8-12
: Consider adding short docstringsBrief docstrings for the class and methods will help users discover the endpoints and expected payload shape.
tests/unit/api/test_contact_imports.py (3)
1-6
: Silence Ruff S101 for pytest asserts in testsUsing bare asserts is idiomatic in pytest; silence S101 at file scope or tune ruff config for tests.
+# ruff: noqa: S101 from typing import Any import pytest import responses +from responses import matchers
118-133
: Assert request JSON to catch payload regressionsMatch the exact JSON body so issues like missing api_data() are caught early.
expected_response = { "id": IMPORT_ID, "status": "started", } + expected_payload = { + "contacts": [ + { + "email": "[email protected]", + "fields": {"first_name": "John", "last_name": "Smith"}, + "list_ids_included": [1], + "list_ids_excluded": [2], + }, + { + "email": "[email protected]", + "fields": {"first_name": "John", "last_name": "Doe"}, + "list_ids_included": [3], + "list_ids_excluded": [4], + }, + ] + } responses.post( BASE_CONTACT_IMPORTS_URL, json=expected_response, status=201, + match=[matchers.json_params_matcher(expected_payload)], )
98-117
: Optionally also validate POST body in error-path testAdding the same JSON matcher here ensures payload validation even when the server returns an error.
📜 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_imports.py
(1 hunks)mailtrap/__init__.py
(1 hunks)mailtrap/api/contacts.py
(2 hunks)mailtrap/api/resources/contact_imports.py
(1 hunks)mailtrap/models/contacts.py
(1 hunks)tests/unit/api/test_contact_imports.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/__init__.py (2)
mailtrap/api/contacts.py (1)
contacts
(26-27)mailtrap/models/contacts.py (1)
ImportContactParams
(117-123)
tests/unit/models/test_contacts.py (2)
mailtrap/models/contacts.py (1)
ImportContactParams
(117-123)mailtrap/models/common.py (1)
api_data
(15-19)
mailtrap/api/contacts.py (1)
mailtrap/api/resources/contact_imports.py (1)
ContactImportsApi
(8-28)
mailtrap/models/contacts.py (1)
mailtrap/models/common.py (1)
RequestParams
(13-19)
mailtrap/api/resources/contact_imports.py (4)
mailtrap/http.py (3)
HttpClient
(13-96)post
(31-33)get
(25-29)mailtrap/api/contacts.py (1)
contacts
(26-27)mailtrap/models/contacts.py (2)
ContactImport
(108-113)ImportContactParams
(117-123)mailtrap/models/common.py (1)
api_data
(15-19)
examples/contacts/contact_imports.py (4)
mailtrap/api/contacts.py (2)
contacts
(26-27)contact_imports
(22-23)mailtrap/models/contacts.py (2)
ContactImport
(108-113)ImportContactParams
(117-123)mailtrap/client.py (1)
MailtrapClient
(24-142)mailtrap/api/resources/contact_imports.py (2)
import_contacts
(13-18)get_by_id
(20-22)
tests/unit/api/test_contact_imports.py (4)
mailtrap/api/resources/contact_imports.py (3)
ContactImportsApi
(8-28)import_contacts
(13-18)get_by_id
(20-22)mailtrap/exceptions.py (1)
APIError
(10-15)mailtrap/http.py (3)
HttpClient
(13-96)post
(31-33)get
(25-29)mailtrap/models/contacts.py (3)
ContactImport
(108-113)ContactImportStatus
(100-104)ImportContactParams
(117-123)
🪛 Ruff (0.12.2)
tests/unit/models/test_contacts.py
132-132: Use of assert
detected
(S101)
144-144: Use of assert
detected
(S101)
examples/contacts/contact_imports.py
4-4: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
tests/unit/api/test_contact_imports.py
116-116: Use of assert
detected
(S101)
136-136: Use of assert
detected
(S101)
137-137: Use of assert
detected
(S101)
138-138: Use of assert
detected
(S101)
177-177: Use of assert
detected
(S101)
191-191: Use of assert
detected
(S101)
192-192: Use of assert
detected
(S101)
193-193: Use of assert
detected
(S101)
209-209: Use of assert
detected
(S101)
210-210: Use of assert
detected
(S101)
211-211: Use of assert
detected
(S101)
212-212: Use of assert
detected
(S101)
213-213: Use of assert
detected
(S101)
214-214: Use of assert
detected
(S101)
🔇 Additional comments (7)
mailtrap/__init__.py (1)
10-10
: Good public export of ImportContactParamsThis aligns with the example usage via
mailtrap.ImportContactParams
.tests/unit/models/test_contacts.py (1)
126-149
: ImportContactParams tests look correctCovers exclude-none and populated payload cases as intended.
mailtrap/api/contacts.py (1)
21-24
: Property accessor for contact_imports matches existing API designConsistent with
contact_fields
,contact_lists
, and prior learnings that these are properties, not methods.mailtrap/models/contacts.py (1)
100-114
: Solid modeling for import status and resultEnum values and optional counters look good and future-proof.
mailtrap/api/resources/contact_imports.py (1)
24-28
: API path builder looks correct and consistent with HttpClient._urlThe path format and leading slash handling are spot on.
tests/unit/api/test_contact_imports.py (2)
64-139
: Nice coverage of success and error pathsGood use of parametrization and error assertions across 401/403/422; enums validated properly.
179-215
: Finished import assertions look goodValidates counters and FINISHED status; this mirrors the response schema well.
Motivation
In this PR I added ContactImportsApi.
Changes
How to test
Summary by CodeRabbit
New Features
Tests