-
Notifications
You must be signed in to change notification settings - Fork 13
Contact imports #95
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
Contact imports #95
Conversation
…o support contact import functionality
…ting for API error messages
WalkthroughAdds a new Contact Imports feature: README docs and example, resource and wrapper API classes, TypeScript types, MailtrapClient getter, and unit tests enabling create/get contact-import operations (no other API changes). Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Client as MailtrapClient
participant Wrapper as ContactImportsBaseAPI
participant Resource as ContactImportsApi
participant API as HTTP API
User->>Client: request contactImports
Client->>Client: validateAccountIdPresence()
Client-->>User: ContactImportsBaseAPI instance
rect rgb(230,245,245)
Note over User,API: Create import (async processing)
User->>Wrapper: create(ImportContactsRequest)
Wrapper->>Resource: create(...)
Resource->>API: POST /api/accounts/{accountId}/contacts/imports
API-->>Resource: ContactImportResponse { id, status, ... }
Resource-->>Wrapper: response
Wrapper-->>User: ContactImportResponse
end
rect rgb(245,235,245)
Note over User,API: Retrieve import status
User->>Wrapper: get(importId)
Wrapper->>Resource: get(importId)
Resource->>API: GET /api/accounts/{accountId}/contacts/imports/{importId}
API-->>Resource: ContactImportResponse
Resource-->>Wrapper: response
Wrapper-->>User: ContactImportResponse
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related issues
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
…ror message formatting
…n and method properties
…ing create and get methods with various scenarios
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
🧹 Nitpick comments (8)
src/types/api/contact-imports.ts (1)
13-17: Preserve ZIP leading zeros by widening type.Use
string | numberforzip_codeto avoid truncating leading zeros (e.g., "02108").fields?: { first_name?: string; last_name?: string; - zip_code?: number; + zip_code?: string | number; [key: string]: string | number | undefined; };src/__tests__/lib/api/ContactImports.test.ts (1)
11-14: Strengthen assertions (optional).Also assert members are functions to catch mis‑binding regressions.
expect(contactImportsAPI).toHaveProperty("create"); expect(contactImportsAPI).toHaveProperty("get"); + expect(typeof (contactImportsAPI as any).create).toBe("function"); + expect(typeof (contactImportsAPI as any).get).toBe("function");README.md (1)
130-133: Add note about required accountId.Contact Imports requires
accountIdinMailtrapClientconfig. Add a one‑liner to reduce setup confusion.-### Contact Imports API +### Contact Imports API +Note: Contact Imports require providing `accountId` when initializing `MailtrapClient`. - [Contact Imports](examples/contact-imports/everything.ts)src/lib/api/ContactImports.ts (1)
6-14: Make bound methods readonly.Prevents accidental reassignment of API surface.
-export default class ContactImportsBaseAPI { - public create: ContactImportsApi["create"]; - - public get: ContactImportsApi["get"]; +export default class ContactImportsBaseAPI { + public readonly create: ContactImportsApi["create"]; + + public readonly get: ContactImportsApi["get"];src/__tests__/lib/api/resources/ContactImports.test.ts (1)
176-193: Nice coverage; consider adding base-only error case (optional).Add a test where
errors = { base: [...] }(noname) to assert logger falls back tobase.+ it("fails with base-only validation errors.", async () => { + const endpoint = `${GENERAL_ENDPOINT}/api/accounts/${accountId}/contacts/imports`; + mock.onPost(endpoint).reply(422, { errors: { base: ["Contact limit exceeded"] } }); + await expect(contactImportsAPI.create(createImportRequest)) + .rejects.toThrow("Contact limit exceeded"); + });src/lib/api/resources/ContactImports.ts (3)
36-36: Remove redundant URL variable.The
urlconstant is unnecessary here since it's just assigning the same value asthis.contactImportsURL.Apply this diff to simplify:
- const url = `${this.contactImportsURL}`; - return this.client.post<ContactImportResponse, ContactImportResponse>( - url, + this.contactImportsURL, data );
35-42: Add explicit return type annotation.Public API methods should have explicit return type annotations for better type safety and documentation.
Apply this diff:
- public async create(data: ImportContactsRequest) { + public async create(data: ImportContactsRequest): Promise<ContactImportResponse> { const url = `${this.contactImportsURL}`;
26-30: Add explicit return type annotation.Public API methods should have explicit return type annotations. The axios client is configured with a response interceptor that extracts
response.datadirectly, so the method implicitly returnsPromise<ContactImportResponse>.- public async get(importId: number) { + public async get(importId: number): Promise<ContactImportResponse> { const url = `${this.contactImportsURL}/${importId}`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
README.md(2 hunks)examples/contact-imports/everything.ts(1 hunks)src/__tests__/lib/api/ContactImports.test.ts(1 hunks)src/__tests__/lib/api/resources/ContactImports.test.ts(1 hunks)src/lib/MailtrapClient.ts(3 hunks)src/lib/api/ContactImports.ts(1 hunks)src/lib/api/resources/ContactImports.ts(1 hunks)src/types/api/contact-imports.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/__tests__/lib/api/resources/ContactImports.test.ts (4)
src/lib/api/resources/ContactImports.ts (1)
ContactImportsApi(13-43)src/types/api/contact-imports.ts (2)
ImportContactsRequest(9-21)ContactImportResponse(1-7)src/lib/axios-logger.ts (1)
handleSendingError(20-69)src/lib/MailtrapError.ts (1)
MailtrapError(1-1)
src/lib/api/ContactImports.ts (2)
src/lib/api/resources/ContactImports.ts (1)
ContactImportsApi(13-43)src/lib/MailtrapClient.ts (1)
contactImports(153-156)
src/lib/api/resources/ContactImports.ts (1)
src/types/api/contact-imports.ts (2)
ContactImportResponse(1-7)ImportContactsRequest(9-21)
src/lib/MailtrapClient.ts (2)
src/lib/api/ContactImports.ts (1)
ContactImportsBaseAPI(5-15)src/lib/api/SendingDomains.ts (1)
SendingDomainsBaseAPI(5-28)
🔇 Additional comments (4)
src/lib/MailtrapClient.ts (2)
150-156: LGTM: new contactImports getter is correctly wired.
178-180: LGTM: sendingDomains getter now consistently validates accountId.src/lib/api/resources/ContactImports.ts (2)
1-11: LGTM!The imports and configuration setup are clean and appropriate for the API resource implementation.
18-21: Validation for accountId is present but limited to presence checking.The
contactImportsgetter in MailtrapClient (line 154) callsvalidateAccountIdPresence()before instantiating ContactImportsBaseAPI. However, this validation only checks if accountId is truthy—it does not validate that it's a positive integer. Zero would be rejected even if it might be a valid account ID. Consider whether stricter range validation is needed, or if relying on the API endpoint to validate the account ID is sufficient for your use case.
| // API returns errors as an array in data.errors | ||
| mock.onPost(endpoint).reply(422, { | ||
| errors: { |
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.
So does it return data.errors as array or object? Because comment says one and test does the opposite 🤔 According to docs it should be an array of objects https://api-docs.mailtrap.io/docs/mailtrap-api-docs/b46b1fe35bdf1-import-contacts
…ormat with detailed error objects
Motivation
Add support for the Contact Imports API to enable bulk importing of contacts into Mailtrap accounts. This feature allows users to efficiently import multiple contacts at once with optional fields, contact lists inclusion/exclusion, and tracking of import status. The implementation follows the existing patterns used for other contact management APIs (Contacts, Contact Lists, Contact Fields) to maintain consistency across the SDK. Fixes #74
Changes
src/lib/api/resources/ContactImports.ts)create()method to bulk import contacts with optional fields and list assignmentsget()method to retrieve import status by ID with progress trackingsrc/lib/api/ContactImports.ts)src/types/api/contact-imports.ts)ContactImportResponsetype with status, counts, and import metadataImportContactsRequesttype supporting contacts with fields, list inclusions/exclusionssrc/lib/MailtrapClient.ts)contactImportsgetter method following the same pattern as other API getterssendingDomainsgetter to use consistent patternsrc/__tests__/lib/api/resources/ContactImports.test.ts)src/__tests__/lib/api/ContactImports.test.ts)examples/contact-imports/everything.ts)README.md)How to test
npm test(expect 247+ passing tests)npm test -- src/__tests__/lib/api/resources/ContactImports.test.ts(should pass 9 tests)npm test -- src/__tests__/lib/api/ContactImports.test.ts(should pass 1 test)npm run lintnpm run lint:tscexamples/contact-imports/everything.tsfor proper usage patternsclient.contactImportsgetter is accessibleContactImportResponsematches expected API structure with all status valuesSummary by CodeRabbit
New Features
Documentation
Examples
Tests