Contacts api #64
Contacts api #64
Conversation
…ejs into contacts-api
…using MailtrapClient
…actData type and structure payloads
…ionality into a single example
WalkthroughThis update introduces a new Contacts API feature, including type definitions, API classes, and example scripts for managing contacts and contact lists. It adds related documentation, test files, and integrates the Contacts API into the main client. No breaking changes or modifications to existing exported entities were made. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MailtrapClient
participant ContactsBaseAPI
participant ContactsApi
participant Mailtrap API
User->>MailtrapClient: Initialize with token/accountId
User->>MailtrapClient: Access contacts property
MailtrapClient->>ContactsBaseAPI: Instantiate with axios/accountId
User->>ContactsBaseAPI: create/update/delete/get contact
ContactsBaseAPI->>ContactsApi: Forward method call
ContactsApi->>Mailtrap API: HTTP request (GET/POST/PATCH/DELETE)
Mailtrap API-->>ContactsApi: API response
ContactsApi-->>ContactsBaseAPI: Return data
ContactsBaseAPI-->>User: Return result
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
src/lib/api/resources/contacts/ContactLists.ts (1)
8-24: ContactListsApi implementation looks good but could be enhancedThe ContactListsApi class has a clean implementation for listing contact lists. However, there are a few enhancements to consider:
- The account ID is optional in the constructor but the URL construction doesn't handle the case when it's undefined.
- There's no type definition for the response structure.
- The API doesn't support pagination which might be needed for accounts with many contact lists.
Consider adding these improvements:
import { AxiosInstance } from "axios"; import CONFIG from "../../../../config"; + import { ContactListsResponse } from "../../../../../types/mailtrap"; const { CLIENT_SETTINGS } = CONFIG; const { GENERAL_ENDPOINT } = CLIENT_SETTINGS; export default class ContactListsApi { private client: AxiosInstance; private contactListsURL: string; constructor(client: AxiosInstance, accountId?: number) { this.client = client; + if (!accountId) { + throw new Error("Account ID is required for ContactListsApi"); + } this.contactListsURL = `${GENERAL_ENDPOINT}/api/accounts/${accountId}/contacts/lists`; } /** * Lists all contact lists for the account. + * @returns Promise with contact lists data */ - public async list() { - return this.client.get(this.contactListsURL); + public async list(params?: { page?: number; per_page?: number }) { + return this.client.get<ContactListsResponse>(this.contactListsURL, { params }); } }src/lib/api/resources/contacts/Contacts.ts (1)
21-23: Add return type annotations for better code clarityAdding explicit return type annotations to the async methods would improve code clarity and provide better TypeScript type checking.
- public async create(data: ContactData) { + public async create(data: ContactData): Promise<AxiosResponse> { return this.client.post(this.contactsURL, { contact: data }); } - public async update(id: number, data: ContactData) { + public async update(id: number, data: ContactData): Promise<AxiosResponse> { const url = `${this.contactsURL}/${id}`; return this.client.patch(url, { contact: data }); } - public async delete(id: number) { + public async delete(id: number): Promise<AxiosResponse> { const url = `${this.contactsURL}/${id}`; return this.client.delete(url); }Don't forget to also import the AxiosResponse type:
- import { AxiosInstance } from "axios"; + import { AxiosInstance, AxiosResponse } from "axios";Also applies to: 28-31, 36-39
src/lib/MailtrapClient.ts (1)
155-179: Minor clean-ups insidebatchSend
preparedRequestsstill serialises properties that areundefined; removing them trims the payload and matches classic Mailtrap examples.- const preparedRequests = request.requests.map((req) => ({ - to: req.to, - cc: req.cc, - bcc: req.bcc, - custom_variables: req.custom_variables, - })); + const preparedRequests = request.requests.map( + ({ to, cc, bcc, custom_variables }) => + Object.fromEntries( + Object.entries({ to, cc, bcc, custom_variables }).filter( + ([, v]) => v !== undefined + ) + ) + );
- A short JSDoc block describing the supported shapes (
InlineBatchSendBase | TemplateBatchSendBase) would help IDEs and consumers.src/types/mailtrap.ts (1)
79-83:BaseAddressduplicatesAddress– keep a single source of truth
BaseAddressand the earlier-declaredAddressshare the exact shape. Re-useAddressinstead of introducing a parallel type to avoid drift.-interface BaseAddress { - email: string; - name?: string; -} +type BaseAddress = Address;src/__tests__/lib/mailtrap-client.test.ts (2)
331-675: Isolate Axios mocks between test suitesThe
AxiosMockAdapterinstance is created only inside thesend()suite.
batch sendingtests run inside that suite (✓) but latercontactsandcontact listssuites rely on the samemockwithout their ownbeforeAll/afterEach, risking leftover interceptors if additional suites are appended in the future.A minimal fix is to move the adapter initialisation & reset one level up:
describe("lib/mailtrap-client: ", () => { - let mock: AxiosMockAdapter; + let mock: AxiosMockAdapter; + + beforeAll(() => { + mock = new AxiosMockAdapter(axios); + }); + + afterEach(() => { + mock.reset(); + });…and remove the nested
beforeAll / afterEachblocks.
This guarantees every test starts with a clean slate.
733-871: Expand coverage & negative cases for Contacts / Lists APIsNice positive-path assertions! Two quick wins:
- Add a test that accessing
client.contacts/client.contactListswithout anaccountIdthrowsMailtrapErroronce you implement the guard recommended above.- Verify that
fieldsandlist_idsround-trip correctly when present in responses, ensuring JSON serialisation remains symmetric.Would you like help drafting these extra tests?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (15)
README.md(2 hunks)examples/batch/bulk.ts(1 hunks)examples/batch/sandbox.ts(1 hunks)examples/batch/template.ts(1 hunks)examples/batch/transactional.ts(1 hunks)examples/contacts/contact-lists.ts(1 hunks)examples/contacts/everything.ts(1 hunks)examples/testing/send-mail.ts(1 hunks)src/__tests__/lib/mailtrap-client.test.ts(3 hunks)src/lib/MailtrapClient.ts(3 hunks)src/lib/api/Contacts.ts(1 hunks)src/lib/api/resources/contacts/ContactLists.ts(1 hunks)src/lib/api/resources/contacts/Contacts.ts(1 hunks)src/lib/mail-buffer-encoder.ts(1 hunks)src/types/mailtrap.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/lib/mail-buffer-encoder.ts (1)
src/types/mailtrap.ts (1)
src/lib/api/Contacts.ts (2)
src/lib/api/resources/contacts/Contacts.ts (1)
ContactsApi(8-40)src/lib/api/resources/contacts/ContactLists.ts (1)
ContactListsApi(8-24)
src/lib/api/resources/contacts/Contacts.ts (1)
src/types/mailtrap.ts (1)
ContactData(121-125)
src/__tests__/lib/mailtrap-client.test.ts (1)
src/lib/MailtrapError.ts (1)
MailtrapError(1-1)
src/lib/MailtrapClient.ts (4)
src/lib/api/resources/contacts/Contacts.ts (1)
ContactsApi(8-40)src/lib/api/resources/contacts/ContactLists.ts (1)
ContactListsApi(8-24)src/types/mailtrap.ts (2)
BatchSendRequest(105-113)BatchSendResponse(74-77)src/lib/mail-buffer-encoder.ts (1)
encodeMailBuffers(8-33)
🪛 LanguageTool
README.md
[grammar] ~61-~61: You’ve repeated a verb. Did you mean to only write one of them?
Context: ...s](examples/general/permissions.ts) - Contacts - [Contact Lists](examples/contacts/contact-lists....
(REPEATED_VERBS)
[grammar] ~74-~74: Possible verb agreement error. Did you mean “sends”? (Some collective nouns can be treated as both singular and plural, so ‘Send’ is not always incorrect.)
Context: ...ails](examples/batch/send-batch.ts) - [Send a batch of transactional emails](exampl...
(COLLECTIVE_NOUN_VERB_AGREEMENT_VBP)
🪛 markdownlint-cli2 (0.17.2)
README.md
61-61: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
62-62: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
73-73: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
74-74: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
75-75: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
76-76: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
🔇 Additional comments (10)
examples/testing/send-mail.ts (1)
7-7: Documentation enhancement looks good!Adding a more specific link to the Demo Domain setup section is helpful for users following this example. This improves the documentation quality.
examples/contacts/contact-lists.ts (1)
1-20: Well-structured example for the new contactLists APIThis example effectively demonstrates how to use the newly added contactLists API, providing a clear pattern for users to follow. The code showcases:
- Proper initialization of the client with token and accountId
- Direct access to the contactLists getter
- Appropriate error handling
This example aligns well with the PR objective of integrating contact lists APIs into the Mailtrap client as direct getters.
README.md (2)
61-62: Documentation updates for new contact APIs look goodAdding references to the new contact-related examples aligns with the PR objective of integrating contacts and contact lists APIs into the Mailtrap client.
🧰 Tools
🪛 LanguageTool
[grammar] ~61-~61: You’ve repeated a verb. Did you mean to only write one of them?
Context: ...s](examples/general/permissions.ts) - Contacts - [Contact Lists](examples/contacts/contact-lists....(REPEATED_VERBS)
🪛 markdownlint-cli2 (0.17.2)
61-61: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
62-62: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
71-77: New Batch Sending API section enhances documentationThis new section with examples for batch sending provides valuable user guidance. While batch sending isn't mentioned in the PR objectives, it complements the contacts API integration by providing comprehensive documentation.
🧰 Tools
🪛 LanguageTool
[grammar] ~74-~74: Possible verb agreement error. Did you mean “sends”? (Some collective nouns can be treated as both singular and plural, so ‘Send’ is not always incorrect.)
Context: ...ails](examples/batch/send-batch.ts) - [Send a batch of transactional emails](exampl...(COLLECTIVE_NOUN_VERB_AGREEMENT_VBP)
🪛 markdownlint-cli2 (0.17.2)
73-73: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
74-74: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
75-75: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
76-76: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
src/lib/mail-buffer-encoder.ts (1)
8-8: Good improvement to function signature flexibilityChanging from
Partial<Mail>allows the function to handle objects with any subset ofThe implementation correctly maintains its behavior while supporting this more flexible type.
examples/batch/bulk.ts (1)
1-40: Well-structured batch email example with proper error handlingThis example clearly demonstrates how to use the batch sending API for multiple recipients with custom variables. The code structure follows best practices with promise chaining and proper error handling.
examples/batch/template.ts (1)
1-39: Clear template-based batch sending exampleThe example effectively demonstrates how to use the template-based batch sending capability with different template variables for each recipient. The code is well-structured and follows consistent patterns with other examples.
examples/contacts/everything.ts (1)
1-37: Overall good example demonstrating contact lifecycle managementThis example effectively shows the complete contact lifecycle including creation, updating, and deletion. The code is well-structured with appropriate error handling.
Also applies to: 41-44
examples/batch/transactional.ts (1)
1-37: Well-structured example demonstrating batch transactional email sendingThis example clearly demonstrates how to use the batch sending API with a well-organized structure including base email configuration, multiple recipient requests with custom variables, and proper promise handling.
examples/batch/sandbox.ts (1)
1-42: Well-implemented sandbox mode exampleThis example correctly demonstrates how to configure the client for sandbox mode with a test inbox ID and structure a batch send request with custom variables. The code follows good practices for promise handling and clearly shows the configuration differences compared to standard transactional sending.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
README.md (2)
61-62: Remove leading space before list items under "General API"
The list markers for the new "Contacts" and "Contact Lists" entries are indented and trigger markdownlint MD007. They should be aligned at the margin.- - [Contacts](examples/contacts/everything.ts) + - [Contacts](examples/contacts/everything.ts) - - [Contact Lists](examples/contacts/contact-lists.ts) + - [Contact Lists](examples/contacts/contact-lists.ts)🧰 Tools
🪛 LanguageTool
[grammar] ~61-~61: You’ve repeated a verb. Did you mean to only write one of them?
Context: ...s](examples/general/permissions.ts) - Contacts - [Contact Lists](examples/contacts/contact-lists....(REPEATED_VERBS)
🪛 markdownlint-cli2 (0.17.2)
61-61: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
62-62: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
73-76: Remove leading space before list items under "Batch Sending API"
The new batch sending example entries are indented and trigger markdownlint MD007. Align the list markers at the root margin for consistency.- - [Send a batch of emails](examples/batch/send-batch.ts) + - [Send a batch of emails](examples/batch/send-batch.ts) - - [Send a batch of transactional emails](examples/batch/transactional-batch.ts) + - [Send a batch of transactional emails](examples/batch/transactional-batch.ts) - - [Send a batch of bulk emails](examples/batch/bulk-batch.ts) + - [Send a batch of bulk emails](examples/batch/bulk-batch.ts) - - [Send a batch of sandbox emails](examples/batch/sandbox-batch.ts) + - [Send a batch of sandbox emails](examples/batch/sandbox-batch.ts)🧰 Tools
🪛 LanguageTool
[grammar] ~74-~74: Possible verb agreement error. Did you mean “sends”? (Some collective nouns can be treated as both singular and plural, so ‘Send’ is not always incorrect.)
Context: ...ails](examples/batch/send-batch.ts) - [Send a batch of transactional emails](exampl...(COLLECTIVE_NOUN_VERB_AGREEMENT_VBP)
🪛 markdownlint-cli2 (0.17.2)
73-73: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
74-74: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
75-75: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
76-76: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
README.md(2 hunks)src/lib/api/Contacts.ts(1 hunks)src/types/mailtrap.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/types/mailtrap.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/api/Contacts.ts
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~61-~61: You’ve repeated a verb. Did you mean to only write one of them?
Context: ...s](examples/general/permissions.ts) - Contacts - [Contact Lists](examples/contacts/contact-lists....
(REPEATED_VERBS)
[grammar] ~74-~74: Possible verb agreement error. Did you mean “sends”? (Some collective nouns can be treated as both singular and plural, so ‘Send’ is not always incorrect.)
Context: ...ails](examples/batch/send-batch.ts) - [Send a batch of transactional emails](exampl...
(COLLECTIVE_NOUN_VERB_AGREEMENT_VBP)
🪛 markdownlint-cli2 (0.17.2)
README.md
61-61: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
62-62: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
73-73: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
74-74: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
75-75: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
76-76: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/__tests__/lib/api/resources/Contacts.test.ts (2)
31-31: Test data correctly matches the Contact interface.The string ID in the mock response correctly matches the
Contactinterface definition whereid: string. The past review comment suggesting this should be a number appears to be outdated.
39-40: Test data correctly matches the Contact interface.The numeric timestamps in the mock response correctly match the
Contactinterface definition wherecreated_at: numberandupdated_at: number. The past review comment suggesting these should be strings appears to be outdated.
🧹 Nitpick comments (1)
examples/contacts/everything.ts (1)
40-42: Consider adding more context to error logging.For better debugging in real applications, consider including the operation context in error messages.
.catch(error => { - console.error("Error in contact lifecycle:", error); + console.error("Error in contact lifecycle (create/update/delete):", error); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
examples/contacts/everything.ts(1 hunks)src/__tests__/lib/api/Contacts.test.ts(1 hunks)src/__tests__/lib/api/resources/Contacts.test.ts(1 hunks)src/lib/api/Contacts.ts(1 hunks)src/lib/api/resources/Contacts.ts(1 hunks)src/types/api/contacts.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/tests/lib/api/Contacts.test.ts
- src/lib/api/resources/Contacts.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/__tests__/lib/api/resources/Contacts.test.ts (3)
src/lib/api/resources/Contacts.ts (1)
ContactsApi(14-61)src/lib/axios-logger.ts (1)
handleSendingError(20-69)src/lib/MailtrapError.ts (1)
MailtrapError(1-1)
src/lib/api/Contacts.ts (2)
src/lib/api/resources/Contacts.ts (1)
ContactsApi(14-61)src/lib/MailtrapClient.ts (1)
contacts(109-111)
🔇 Additional comments (9)
examples/contacts/everything.ts (1)
20-42: LGTM! Consistent API usage throughout the contact lifecycle.The example correctly demonstrates the contact lifecycle using the direct
client.contactsgetter, which aligns perfectly with the PR objectives of providing straightforward access to contact APIs.src/lib/api/Contacts.ts (1)
5-27: LGTM! Clean wrapper implementation with proper method binding.The ContactsBaseAPI class provides a clean abstraction over ContactsApi with proper method binding to maintain context. The constructor correctly passes both client and accountId parameters to the underlying ContactsApi instance.
src/__tests__/lib/api/resources/Contacts.test.ts (4)
98-131: LGTM! Comprehensive test coverage for create method.The tests properly cover both success and error scenarios with correct endpoint verification and error handling validation.
133-171: LGTM! Thorough testing of update method.The update tests correctly verify the PATCH request with proper endpoint construction and error handling.
173-219: LGTM! Complete delete method test coverage.The delete tests appropriately verify the DELETE request and error scenarios.
221-268: LGTM! Proper list method testing.The list tests correctly validate the contact lists endpoint and error handling.
src/types/api/contacts.ts (3)
1-9: LGTM! Well-structured Contact interface.The Contact interface provides a clear and comprehensive structure for contact data with appropriate types for each field.
11-21: LGTM! Clean data interfaces with proper inheritance.The ContactData and ContactUpdateData interfaces are well-designed with ContactUpdateData appropriately extending ContactData to add list management fields.
23-25: LGTM! Consistent response wrapper interface.The ContactResponse interface follows the expected API response pattern by wrapping the Contact data.
README.md
Outdated
| - [Contacts](examples/contacts/contacts.ts) | ||
| - [Contact Lists](examples/contacts/contact-lists.ts) |
There was a problem hiding this comment.
And there is no link to everything.ts
| const testInboxId = 100; | ||
| const contactsAPI = new Contacts(axios, testInboxId); |
There was a problem hiding this comment.
It's not an inbox ID
| const testInboxId = 100; | |
| const contactsAPI = new Contacts(axios, testInboxId); | |
| const accountId = 100; | |
| const contactsAPI = new Contacts(axios, accountId); |
src/lib/api/resources/Contacts.ts
Outdated
| /** | ||
| * Gets a list of contact lists. | ||
| */ | ||
| public async list() { |
There was a problem hiding this comment.
If it's a list of lists, I think the method should be plural then. But also, I have seen we already exposed ContactLists API operations like create and update, so ContactLists should probably be a separate "API", not just a method on Contacts API.
https://api-docs.mailtrap.io/docs/mailtrap-api-docs/e3bba0bfa185e-create-contact-list
| /** | |
| * Gets a list of contact lists. | |
| */ | |
| public async list() { | |
| /** | |
| * Gets a list of contact lists. | |
| */ | |
| public async lists() { |
There was a problem hiding this comment.
@mklocek I guess it's an "index" action for the Contact resource.
There was a problem hiding this comment.
it's not, it requests contactListsURL (${GENERAL_ENDPOINT}/api/accounts/${accountId}/contacts/lists)
There was a problem hiding this comment.
@vittorius yes, otherwise we are having one more nesting layer, which is uncomfortable. In any case let me know which approach fits better and I will tune the implementation
There was a problem hiding this comment.
there is no such thing as index of contacts. there is such thing as index of contact lists though, and there are also endpoints for creating, updating contact lists etc. it's a separate "resource"
There was a problem hiding this comment.
@mklocek seems create and update was added recently, I can provide it with separate PR
src/types/api/contacts.ts
Outdated
| updated_at: number; | ||
| list_ids: number[]; | ||
| status: "subscribed" | "unsubscribed"; | ||
| fields: Record<string, string>; |
There was a problem hiding this comment.
The values can be: string | integer | float | boolean | date string in ISO-8601 format
src/types/mailtrap.ts
Outdated
| export interface ContactFields { | ||
| first_name?: string; | ||
| last_name?: string; | ||
| [key: string]: string | undefined; |
There was a problem hiding this comment.
The values can be: string | integer | float | boolean | date string in ISO-8601 format
src/types/mailtrap.ts
Outdated
| first_name?: string; | ||
| last_name?: string; |
There was a problem hiding this comment.
Those are default fields but are not required (users can delete them). I think they shouldn't be part of the type definition
README.md
Outdated
| - [Contacts](examples/contacts/contacts.ts) | ||
| - [Contact Lists](examples/contacts/contact-lists.ts) |
There was a problem hiding this comment.
And there is no link to everything.ts
src/lib/api/resources/Contacts.ts
Outdated
| /** | ||
| * Gets a list of contact lists. | ||
| */ | ||
| public async list() { |
There was a problem hiding this comment.
@mklocek I guess it's an "index" action for the Contact resource.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
62-65: Fix list indentation for Contacts API.The list item under the Contacts API section has an extra leading space and violates MD007 (ul-indent). It should start at column 0 for consistency with the lint rules.
- - [Contacts](examples/contacts/everything.ts) +- [Contacts](examples/contacts/everything.ts)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
64-64: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(1 hunks)examples/contacts/everything.ts(1 hunks)src/__tests__/lib/api/Contacts.test.ts(1 hunks)src/lib/api/Contacts.ts(1 hunks)src/lib/api/resources/Contacts.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/tests/lib/api/Contacts.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/contacts/everything.ts
- src/lib/api/Contacts.ts
- src/lib/api/resources/Contacts.ts
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
64-64: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
🔇 Additional comments (1)
README.md (1)
64-64:✅ Verification successful
Verify example file presence.
Ensure that
examples/contacts/everything.tsexists in the repository and is correctly linked in the README.
🏁 Script executed:
#!/bin/bash # Verify the presence of the Contacts example script. if [ ! -f examples/contacts/everything.ts ]; then echo "Error: examples/contacts/everything.ts not found" exit 1 fi echo "OK: examples/contacts/everything.ts exists."Length of output: 145
Example file verified in README
Theexamples/contacts/everything.tsfile exists in the repository and is correctly referenced inREADME.md. No further action is needed.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
64-64: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
…creating a new one
…ccessful retrieval and error handling
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/__tests__/lib/api/resources/Contacts.test.ts (1)
31-31: Data type inconsistencies persist from past reviews.The mock data still contains type inconsistencies that were flagged in previous reviews:
- ID fields: Using strings (e.g.,
"018dd5e3-f6d2-7c00-8f9b-e5c3f2d8a132") but should be numbers according to past review feedback- Timestamp fields: Using numbers (e.g.,
1742820600230) but should be strings according to past review feedbackThese inconsistencies could cause type validation issues and test failures.
Apply these fixes to align with the Contact interface:
- id: "018dd5e3-f6d2-7c00-8f9b-e5c3f2d8a132", + id: 123456, - created_at: 1742820600230, - updated_at: 1742820600230, + created_at: "2025-01-27T14:30:00.230Z", + updated_at: "2025-01-27T14:30:00.230Z",Apply similar fixes to all other mock responses in the file.
Also applies to: 39-40, 60-60, 62-63, 104-104, 106-107, 229-229, 237-238
🧹 Nitpick comments (2)
src/lib/api/Contacts.ts (1)
5-27: Consider simplifying the wrapper pattern or adding value.The
ContactsBaseAPIclass is essentially a delegation wrapper aroundContactsApi. While functional, this pattern adds complexity without clear benefits. Consider either:
- Directly exposing
ContactsApifrom the client- Adding value in this wrapper (caching, validation, etc.)
- Using a factory function instead of a class
Current implementation works but could be streamlined:
-export default class ContactsBaseAPI { - private client: AxiosInstance; - private accountId?: number; - public get: ContactsApi["get"]; - public create: ContactsApi["create"]; - public update: ContactsApi["update"]; - public delete: ContactsApi["delete"]; - - constructor(client: AxiosInstance, accountId?: number) { - this.client = client; - this.accountId = accountId; - const contacts = new ContactsApi(this.client, this.accountId); - this.get = contacts.get.bind(contacts); - this.create = contacts.create.bind(contacts); - this.update = contacts.update.bind(contacts); - this.delete = contacts.delete.bind(contacts); - } -} +export default function createContactsAPI(client: AxiosInstance, accountId?: number) { + return new ContactsApi(client, accountId); +}src/__tests__/lib/api/resources/Contacts.test.ts (1)
75-80: Add missing assertion for the 'get' method.The initialization test checks for
create,update, anddeletemethods but omits thegetmethod, which is also part of the ContactsApi interface.it("initializes with all necessary params.", () => { + expect(contactsAPI).toHaveProperty("get"); expect(contactsAPI).toHaveProperty("create"); expect(contactsAPI).toHaveProperty("update"); expect(contactsAPI).toHaveProperty("delete"); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/__tests__/lib/api/Contacts.test.ts(1 hunks)src/__tests__/lib/api/resources/Contacts.test.ts(1 hunks)src/lib/api/Contacts.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/tests/lib/api/Contacts.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/lib/api/Contacts.ts (2)
src/lib/api/resources/Contacts.ts (1)
ContactsApi(13-59)src/lib/MailtrapClient.ts (1)
contacts(109-111)
src/__tests__/lib/api/resources/Contacts.test.ts (3)
src/lib/api/resources/Contacts.ts (1)
ContactsApi(13-59)src/lib/axios-logger.ts (1)
handleSendingError(20-69)src/lib/MailtrapError.ts (1)
MailtrapError(1-1)
🔇 Additional comments (1)
src/__tests__/lib/api/resources/Contacts.test.ts (1)
83-96: Excellent test coverage and structure.The test suite demonstrates good practices:
- Proper setup/teardown with axios interceptors
- Comprehensive coverage of all CRUD operations
- Both success and error scenarios tested
- Correct use of axios-mock-adapter
- Proper error handling verification with MailtrapError
Also applies to: 98-269
…g and updating it
Contacts API Integration
Motivation
The contacts and contact lists APIs were not properly integrated into the Mailtrap client. We needed to add these APIs as direct getters on the
MailtrapClientclass to maintain consistency with the API design and provide easy access to contact management functionality.Changes
contactsandcontactListsgetters toMailtrapClientclassgeneralAPIHow to test
yarn testSummary by CodeRabbit