Sending domains API #93
Conversation
…etList, get, create, delete, and sendSetupInstructions
…ith methods for listing, retrieving, creating, deleting, and sending setup instructions
…te-sending-domain
WalkthroughAdds Sending Domains support: new API resource and wrapper, MailtrapClient getter, types, tests, and an example script demonstrating list/get/create/delete/sendSetupInstructions flows with error handling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant MC as MailtrapClient
participant SDBase as SendingDomainsBaseAPI
participant SDApi as SendingDomainsApi
participant AX as Axios
participant API as Mailtrap HTTP API
Dev->>MC: new MailtrapClient({ token, accountId })
Dev->>MC: access client.sendingDomains
MC-->>Dev: SendingDomainsBaseAPI (validates accountId)
rect rgba(230,245,255,0.5)
Dev->>SDBase: getList()
SDBase->>SDApi: getList()
SDApi->>AX: GET /api/accounts/{accountId}/sending_domains
AX->>API: request
API-->>AX: 200 JSON
AX-->>SDApi: data
SDApi-->>SDBase: SendingDomain[]
SDBase-->>Dev: SendingDomain[]
end
rect rgba(240,255,240,0.5)
Dev->>SDBase: get(id)
SDBase->>SDApi: get(id)
SDApi->>AX: GET /api/accounts/{accountId}/sending_domains/{id}
API-->>AX: 200 JSON
AX-->>SDApi: data
SDApi-->>Dev: SendingDomain
end
rect rgba(255,250,230,0.5)
Dev->>SDBase: create(params)
SDBase->>SDApi: create(params)
SDApi->>AX: POST /api/accounts/{accountId}/sending_domains
API-->>AX: 201 JSON
AX-->>SDApi: data
SDApi-->>Dev: SendingDomain
Dev->>SDBase: delete(id)
SDBase->>SDApi: delete(id)
SDApi->>AX: DELETE /api/accounts/{accountId}/sending_domains/{id}
API-->>AX: 204
AX-->>SDApi: response
SDApi-->>Dev: undefined
end
rect rgba(255,235,245,0.5)
Dev->>SDBase: sendSetupInstructions(id, email)
SDBase->>SDApi: sendSetupInstructions(id, email)
SDApi->>AX: POST /api/accounts/{accountId}/sending_domains/{id}/setup_instructions
API-->>AX: 200 JSON
AX-->>SDApi: { message }
SDApi-->>Dev: SetupInstructionsResponse
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
…s to clarify their purpose and structure
…f sending domains response data
…gDomainsResponse data structure
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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)
src/lib/MailtrapClient.ts (1)
93-98: Fix accountId falsy check (0 should be valid)
if (!this.accountId)incorrectly rejectsaccountId = 0. Use a nullish check instead.- private validateAccountIdPresence(): number { - if (!this.accountId) { - throw new MailtrapError(ACCOUNT_ID_MISSING); - } - return this.accountId; - } + private validateAccountIdPresence(): number { + if (this.accountId == null) { + throw new MailtrapError(ACCOUNT_ID_MISSING); + } + return this.accountId; + }
🧹 Nitpick comments (10)
src/types/mailtrap.ts (1)
128-134: Align ContactFields with existing Record<> patterns; reconsider undefined in union
- For consistency with CustomVariables/TemplateVariables, prefer a Record type alias.
- Since
fieldsis optional,undefinedin the value union may be unnecessary unless you explicitly need “present-but-unset” semantics.Apply:
- export interface ContactFields { - [key: string]: string | number | boolean | undefined; - } +export type ContactFields = Record<string, string | number | boolean>;If you intentionally need “unset” values, keep
undefinedbut document that behavior in the docblock.Also applies to: 136-140
src/lib/api/resources/SendingDomains.ts (2)
17-18: Mark immutable URL as readonlyThis field doesn’t change after construction.
- private sendingDomainsURL: string; + private readonly sendingDomainsURL: string;
28-37: Add explicit return types and keep method contracts interceptor-agnosticDeclare return types to make contracts clear regardless of interceptor behavior.
- public async getList() { + public async getList(): Promise<SendingDomain[]> { … return response.data; } - public async get(id: number) { + public async get(id: number): Promise<SendingDomain> { … return this.client.get<SendingDomain, SendingDomain>(url); } - public async create(params: CreateSendingDomainParams) { + public async create( + params: CreateSendingDomainParams + ): Promise<SendingDomain> { … return this.client.post<SendingDomain, SendingDomain>(url, data); } - public async delete(id: number) { + public async delete(id: number): Promise<void> { … return this.client.delete(url); } - public async sendSetupInstructions(id: number, email: string) { + public async sendSetupInstructions( + id: number, + email: string + ): Promise<SetupInstructionsResponse> { … return this.client.post< SetupInstructionsResponse, SetupInstructionsResponse >(url, { email }); }Also applies to: 44-48, 53-58, 64-68, 76-83
src/__tests__/lib/mailtrap-client.test.ts (1)
878-902: Use shared error constant; adjust message to avoid copy/paste confusion
- Prefer
ACCOUNT_ID_MISSINGover a hardcoded string for consistency.- The message mentions “testing API” while testing sendingDomains. Use the constant to avoid mismatch.
- expect(() => client.sendingDomains).toThrow( - "accountId is missing, some features of testing API may not work properly." - ); + expect(() => client.sendingDomains).toThrow(ACCOUNT_ID_MISSING);Optional: add a test ensuring
accountId = 0is treated as present (per PR objective):it("allows accountId = 0", () => { const client = new MailtrapClient({ token: "test-token", accountId: 0 }); expect(client.sendingDomains).toBeInstanceOf(SendingDomainsBaseAPI); });src/lib/MailtrapClient.ts (1)
165-172: Avoid non-null assertion; reuse validated accountIdUse the value returned by the validator for clarity.
get sendingDomains() { - this.validateAccountIdPresence(); - - return new SendingDomainsBaseAPI(this.axios, this.accountId!); + const accountId = this.validateAccountIdPresence(); + return new SendingDomainsBaseAPI(this.axios, accountId); }src/__tests__/lib/api/resources/SendingDomains.test.ts (2)
181-194: Assert request payload for create()Validate the envelope
{ domain: { domain_name } }is sent.const result = await sendingDomainsAPI.create(createParams); expect(result).toEqual(mockSendingDomain); + expect(mock.history.post[0].data).toEqual( + JSON.stringify({ domain: createParams }) + );
227-233: Assert request payload for sendSetupInstructions()Ensure
{ email }is posted.const result = await sendingDomainsAPI.sendSetupInstructions( sendingDomainId, email ); expect(result).toEqual(mockResponse); + expect(mock.history.post[0].data).toEqual( + JSON.stringify({ email }) + );src/lib/api/SendingDomains.ts (1)
5-7: Remove redundant client fieldThe class-level
clientisn’t used outside the constructor; use the parameter directly.export default class SendingDomainsBaseAPI { - private client: AxiosInstance; + // no-op public get: SendingDomainsApi["get"]; @@ - constructor(client: AxiosInstance, accountId: number) { - this.client = client; - const sendingDomains = new SendingDomainsApi(this.client, accountId); + constructor(client: AxiosInstance, accountId: number) { + const sendingDomains = new SendingDomainsApi(client, accountId);Also applies to: 18-21
src/types/api/sending-domains.ts (2)
20-20: Consider using string literal unions for status fields.The
compliance_status(line 20) andstatus(line 6) fields use genericstringtypes. If these fields have a known set of values (e.g.,'verified' | 'pending' | 'failed'), consider using string literal unions for better type safety.Also applies to: 6-6
1-44: Consider adding JSDoc for developer experience.Adding JSDoc comments to interfaces would improve developer experience by providing context about field meanings, valid values, and edge cases.
Example:
+/** + * Represents a DNS record for domain verification + */ export interface DnsRecord { + /** Unique identifier for the record */ key: string; + /** Domain name this record applies to */ domain: string; // ... etc }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
examples/sending-domains/everything.ts(1 hunks)src/__tests__/lib/api/resources/SendingDomains.test.ts(1 hunks)src/__tests__/lib/mailtrap-client.test.ts(2 hunks)src/lib/MailtrapClient.ts(2 hunks)src/lib/api/SendingDomains.ts(1 hunks)src/lib/api/resources/SendingDomains.ts(1 hunks)src/types/api/contact-fields.ts(1 hunks)src/types/api/sending-domains.ts(1 hunks)src/types/mailtrap.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/lib/api/SendingDomains.ts (2)
src/lib/api/resources/SendingDomains.ts (1)
SendingDomainsApi(14-84)src/lib/MailtrapClient.ts (1)
sendingDomains(168-172)
src/__tests__/lib/mailtrap-client.test.ts (1)
src/lib/api/SendingDomains.ts (1)
SendingDomainsBaseAPI(5-28)
src/__tests__/lib/api/resources/SendingDomains.test.ts (2)
src/lib/api/SendingDomains.ts (1)
SendingDomainsBaseAPI(5-28)src/types/api/sending-domains.ts (4)
DnsRecord(1-8)SendingDomainPermissions(10-14)SendingDomain(16-32)SetupInstructionsResponse(38-40)
src/lib/MailtrapClient.ts (1)
src/lib/api/SendingDomains.ts (1)
SendingDomainsBaseAPI(5-28)
src/lib/api/resources/SendingDomains.ts (1)
src/types/api/sending-domains.ts (4)
SendingDomainsResponse(42-44)SendingDomain(16-32)CreateSendingDomainParams(34-36)SetupInstructionsResponse(38-40)
🔇 Additional comments (2)
src/types/api/contact-fields.ts (1)
1-5: Docblock addition looks goodClear, concise, consistent with the surrounding types.
src/types/api/sending-domains.ts (1)
22-22: No changes required for dns_verified_at and alert_recipient_email types
Tests showdns_verified_atas an ISO string when verified and an empty string when unverified, andalert_recipient_emailalways as a string—so the currentstringtype covers all cases.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (9)
src/lib/MailtrapClient.ts (1)
168-172: Keep the getter consistent with the other APIsAll other getters capture the validated
accountIdand pass it through directly. Doing the same here eliminates the!assertion and keeps the pattern uniform.- get sendingDomains() { - this.validateAccountIdPresence(); - - return new SendingDomainsBaseAPI(this.axios, this.accountId!); - } + get sendingDomains() { + const accountId = this.validateAccountIdPresence(); + + return new SendingDomainsBaseAPI(this.axios, accountId); + }src/lib/api/SendingDomains.ts (2)
6-7: Remove unused private field.
clientis only used to constructSendingDomainsApi; it isn’t used afterward. Drop it to reduce state.export default class SendingDomainsBaseAPI { - private client: AxiosInstance; + // no instance fields needed public get: SendingDomainsApi["get"]; @@ - constructor(client: AxiosInstance, accountId: number) { - this.client = client; - const sendingDomains = new SendingDomainsApi(this.client, accountId); + constructor(client: AxiosInstance, accountId: number) { + const sendingDomains = new SendingDomainsApi(client, accountId);
18-27: Bind pattern LGTM; consider readonly to lock API surface.Mark bound methods as
readonlyto prevent reassignment.- public get: SendingDomainsApi["get"]; + public readonly get: SendingDomainsApi["get"]; - public getList: SendingDomainsApi["getList"]; + public readonly getList: SendingDomainsApi["getList"]; - public create: SendingDomainsApi["create"]; + public readonly create: SendingDomainsApi["create"]; - public delete: SendingDomainsApi["delete"]; + public readonly delete: SendingDomainsApi["delete"]; - public sendSetupInstructions: SendingDomainsApi["sendSetupInstructions"]; + public readonly sendSetupInstructions: SendingDomainsApi["sendSetupInstructions"];src/__tests__/lib/api/resources/SendingDomains.test.ts (2)
13-23: Test hygiene: reset mocks between tests; init per-test.Avoid cross-test leakage by resetting handlers/history and creating fresh API per test.
- const axiosInstance: AxiosInstance = axios.create(); - const mock = new MockAdapter(axiosInstance); - // Add the response interceptor that returns response.data - axiosInstance.interceptors.response.use((response) => response.data); - - const testAccountId = 100; - const sendingDomainsAPI = new SendingDomainsBaseAPI( - axiosInstance, - testAccountId - ); + let axiosInstance: AxiosInstance; + let mock: MockAdapter; + const testAccountId = 100; + let sendingDomainsAPI: SendingDomainsBaseAPI; + + beforeEach(() => { + axiosInstance = axios.create(); + mock = new MockAdapter(axiosInstance); + axiosInstance.interceptors.response.use((response) => response.data); + sendingDomainsAPI = new SendingDomainsBaseAPI(axiosInstance, testAccountId); + }); + + afterEach(() => { + mock.reset(); + // mock.resetHistory(); // uncomment if asserting history + });
185-193: Assert POST payload shape for create().Validate that body is wrapped under
domainkey as expected.- mock - .onPost( - `https://mailtrap.io/api/accounts/${testAccountId}/sending_domains` - ) - .reply(200, mockSendingDomain); + let capturedBody: unknown; + mock + .onPost( + `https://mailtrap.io/api/accounts/${testAccountId}/sending_domains` + ) + .reply((config) => { + capturedBody = JSON.parse(config.data as string); + return [200, mockSendingDomain]; + }); @@ const result = await sendingDomainsAPI.create(createParams); expect(result).toEqual(mockSendingDomain); + expect(capturedBody).toEqual({ domain: createParams });src/lib/api/resources/SendingDomains.ts (2)
15-22: Make fields readonly.They are assigned once and never mutated.
- private client: AxiosInstance; + private readonly client: AxiosInstance; @@ - private sendingDomainsURL: string; + private readonly sendingDomainsURL: string;
28-37: Add explicit return types and tighten delete() return.Clarifies public contract and avoids accidental changes.
- public async getList() { + public async getList(): Promise<SendingDomain[]> { @@ - return response.data; + return response.data; } @@ - public async get(id: number) { + public async get(id: number): Promise<SendingDomain> { @@ - return this.client.get<SendingDomain, SendingDomain>(url); + return this.client.get<SendingDomain, SendingDomain>(url); } @@ - public async create(params: CreateSendingDomainParams) { + public async create( + params: CreateSendingDomainParams + ): Promise<SendingDomain> { @@ - return this.client.post<SendingDomain, SendingDomain>(url, data); + return this.client.post<SendingDomain, SendingDomain>(url, data); } @@ - public async delete(id: number) { + public async delete(id: number): Promise<void> { const url = `${this.sendingDomainsURL}/${id}`; - return this.client.delete(url); + await this.client.delete(url); + // implicit undefined } @@ - public async sendSetupInstructions(id: number, email: string) { + public async sendSetupInstructions( + id: number, + email: string + ): Promise<SetupInstructionsResponse> { @@ - return this.client.post< + return this.client.post< SetupInstructionsResponse, SetupInstructionsResponse >(url, { email }); }Also applies to: 44-48, 53-58, 64-68, 76-83
src/types/api/sending-domains.ts (2)
16-32: Consider nullability on some fields.APIs often return
nullfor timestamps/emails when unset. If that’s the case here, widen types tostring | nullfordns_verified_at(and possiblyalert_recipient_email).- dns_verified_at: string; + dns_verified_at: string | null; @@ - alert_recipient_email: string; + alert_recipient_email: string | null;If API docs guarantee strings, feel free to ignore. Please confirm against upstream docs or real responses.
1-8: Optional: narrow enums for stronger typing.If values are constrained, replace
stringwith unions (e.g.,status: "pass" | "fail" | "pending";compliance_statuswith its allowed set).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
examples/sending-domains/everything.ts(1 hunks)src/__tests__/lib/api/resources/SendingDomains.test.ts(1 hunks)src/__tests__/lib/mailtrap-client.test.ts(2 hunks)src/lib/MailtrapClient.ts(2 hunks)src/lib/api/SendingDomains.ts(1 hunks)src/lib/api/resources/SendingDomains.ts(1 hunks)src/types/api/contact-fields.ts(1 hunks)src/types/api/sending-domains.ts(1 hunks)src/types/mailtrap.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/lib/MailtrapClient.ts (1)
src/lib/api/SendingDomains.ts (1)
SendingDomainsBaseAPI(5-28)
src/lib/api/resources/SendingDomains.ts (1)
src/types/api/sending-domains.ts (4)
SendingDomainsResponse(42-44)SendingDomain(16-32)CreateSendingDomainParams(34-36)SetupInstructionsResponse(38-40)
src/__tests__/lib/api/resources/SendingDomains.test.ts (2)
src/lib/api/SendingDomains.ts (1)
SendingDomainsBaseAPI(5-28)src/types/api/sending-domains.ts (4)
DnsRecord(1-8)SendingDomainPermissions(10-14)SendingDomain(16-32)SetupInstructionsResponse(38-40)
src/lib/api/SendingDomains.ts (2)
src/lib/api/resources/SendingDomains.ts (1)
SendingDomainsApi(14-84)src/lib/MailtrapClient.ts (1)
sendingDomains(168-172)
src/__tests__/lib/mailtrap-client.test.ts (1)
src/lib/api/SendingDomains.ts (1)
SendingDomainsBaseAPI(5-28)
🔇 Additional comments (2)
src/__tests__/lib/api/resources/SendingDomains.test.ts (1)
83-91: LGTM overall; endpoints and shapes align with API.Mocks, interceptor setup, and assertions look correct. Nice coverage of all methods.
Also applies to: 132-141, 197-210, 221-233
src/lib/api/resources/SendingDomains.ts (1)
28-37: Interceptor unwrap verified
MailtrapClient constructor installs a response interceptor returningresponse.data, sogetList()’s use of.datais safe.
narekhovhannisyan
requested review from
VladimirTaytor,
leonid-shevtsov and
mklocek
…rified_at and alert_recipient_email
…rect data structure for sending domain
…re and correct HTTP status code
…t in SendingDomainsApi
…ions endpoint and response handling
…g and structured logging
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
examples/sending-domains/everything.ts (2)
17-24: Consider extracting the first domain to a variable.The code accesses
all.data[0]multiple times (lines 23, 28). Extracting it to a variable improves readability and avoids repeated array access.Apply this diff:
if (!all.data || all.data.length === 0) { console.log("No sending domains found for this account."); return; } + const firstDomain = all.data[0]; + // Get a specific sending domain - const one = await client.sendingDomains.get(all.data[0].id); + const one = await client.sendingDomains.get(firstDomain.id); console.log("One sending domain:", JSON.stringify(one, null, 2)); // Send setup instructions const setupResponse = await client.sendingDomains.sendSetupInstructions( - all.data[0].id, + firstDomain.id, "[email protected]" );
27-31: Consider making the setup email configurable.The hardcoded email
"[email protected]"is inconsistent with the pattern of using configurable constants forTOKENandACCOUNT_ID. Making it a constant improves the example's usability.Apply this diff:
const TOKEN = process.env.MAILTRAP_TOKEN || "your-token-here"; const ACCOUNT_ID = Number(process.env.MAILTRAP_ACCOUNT_ID || "1234567"); +const SETUP_EMAIL = process.env.SETUP_EMAIL || "[email protected]"; const client = new MailtrapClient({// Send setup instructions const setupResponse = await client.sendingDomains.sendSetupInstructions( firstDomain.id, - "[email protected]" + SETUP_EMAIL );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
examples/sending-domains/everything.ts(1 hunks)src/__tests__/lib/api/resources/SendingDomains.test.ts(1 hunks)src/lib/api/resources/SendingDomains.ts(1 hunks)src/types/api/sending-domains.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/tests/lib/api/resources/SendingDomains.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/api/resources/SendingDomains.ts
- src/types/api/sending-domains.ts
🔇 Additional comments (1)
examples/sending-domains/everything.ts (1)
11-46: LGTM: Well-structured example flow with proper error handling.The async flow function demonstrates all key sending domain operations in a logical sequence with comprehensive error handling. The try/catch pattern and error logging are appropriate for an example script.
3 participants
Motivation
This branch implements several improvements to the Mailtrap Node.js SDK:
accountIdoptional in GeneralAPI constructor to enable account discovery without requiring an account ID upfrontfixes #73
Changes
accountIdparameter from required to optional (accountId?: number) to enable account discoveryaccountAccesses,permissions) via getters||with??operator to properly handleaccountIdvalue of0accountIdrequired where appropriateHow to test
accounts.getAllAccounts()worksaccountAccessesandpermissionsgetters with and without accountIdaccountIdvalue of0is handled correctlynpm testto ensure all 234 tests passSummary by CodeRabbit