-
Notifications
You must be signed in to change notification settings - Fork 11
Supressions api #68
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
base: main
Are you sure you want to change the base?
Supressions api #68
Conversation
…retrieval, update, and deletion
…d method presence
…error handling and initialization
…stead of allTemplates
…te error handling and initialization
… and method presence
…tList and delete methods, including error handling
…ions with error handling
…os, including methods for listing and deleting suppressions
WalkthroughThis update introduces new API modules and example scripts for managing email templates and suppressions, expands type definitions, and adds comprehensive tests for the new features. The client class is extended with new getter properties for templates and suppressions, and the documentation is updated to reflect these additions and reorganize example listings. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MailtrapClient
participant TemplatesBaseAPI
participant SuppressionsBaseAPI
participant TemplatesApi
participant SuppressionsApi
User->>MailtrapClient: new MailtrapClient(token, accountId)
User->>MailtrapClient: .templates
MailtrapClient->>TemplatesBaseAPI: return new TemplatesBaseAPI(axios, accountId)
TemplatesBaseAPI->>TemplatesApi: perform CRUD methods as called
User->>MailtrapClient: .suppressions
MailtrapClient->>SuppressionsBaseAPI: return new SuppressionsBaseAPI(axios, accountId)
SuppressionsBaseAPI->>SuppressionsApi: perform getList/delete as called
Suggested labels
Suggested reviewers
Poem
✨ 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 (
|
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: 0
🧹 Nitpick comments (8)
src/__tests__/lib/api/Suppressions.test.ts (1)
1-20
: LGTM! Consider expanding test coverage.The test structure and basic functionality checks are correct. The test properly verifies that the
SuppressionsBaseAPI
class initializes with the expectedgetList
anddelete
methods.Consider adding tests for:
- Error handling when methods are called
- Verification that methods are properly bound from the underlying
SuppressionsApi
- Edge cases like missing accountId
+ it("properly binds methods from SuppressionsApi", () => { + expect(typeof suppressionsAPI.getList).toBe("function"); + expect(typeof suppressionsAPI.delete).toBe("function"); + });examples/sending/suppressions.ts (1)
3-4
: Fix the ACCOUNT_ID type to be consistent with API expectations.The
ACCOUNT_ID
should be a number, not a string, based on the API requirements and the fact that it's used as a number in the client constructor.-const ACCOUNT_ID = "<YOUR-ACCOUNT-ID-HERE>"; +const ACCOUNT_ID = 123; // Replace with your actual account IDOr if you want to keep the placeholder format:
-const ACCOUNT_ID = "<YOUR-ACCOUNT-ID-HERE>"; +const ACCOUNT_ID = parseInt("<YOUR-ACCOUNT-ID-HERE>"); // Replace with your actual account ID numberREADME.md (1)
64-64
: Fix markdown list indentation for consistency.The static analysis tools flagged multiple unordered list indentation issues. The lists should start at column 0 for consistency with markdown standards.
- - [Contacts CRUD](examples/contacts/everything.ts) + - [Contacts CRUD](examples/contacts/everything.ts) - - [Contact Lists CRUD](examples/contact-lists/everything.ts) + - [Contact Lists CRUD](examples/contact-lists/everything.ts) - - [Templates CRUD](examples/templates/everything.ts) + - [Templates CRUD](examples/templates/everything.ts) - - [Advanced](examples/sending/everything.ts) - - [Minimal](examples/sending/minimal.ts) - - [Send email using template](examples/sending/template.ts) - - [Suppressions](examples/sending/suppressions.ts) - - [Nodemailer transport](examples/sending/transport.ts) + - [Advanced](examples/sending/everything.ts) + - [Minimal](examples/sending/minimal.ts) + - [Send email using template](examples/sending/template.ts) + - [Suppressions](examples/sending/suppressions.ts) + - [Nodemailer transport](examples/sending/transport.ts) - - [Transactional emails](examples/batch/transactional.ts) - - [Bulk emails](examples/batch/bulk.ts) - - [Sandbox emails](examples/batch/sandbox.ts) - - [Emails from template](examples/batch/template.ts) + - [Transactional emails](examples/batch/transactional.ts) + - [Bulk emails](examples/batch/bulk.ts) + - [Sandbox emails](examples/batch/sandbox.ts) + - [Emails from template](examples/batch/template.ts) - - [Transport](examples/bulk/transport.ts) + - [Transport](examples/bulk/transport.ts)Also applies to: 68-68, 72-72, 76-80, 84-87, 100-100
src/lib/api/Suppressions.ts (1)
6-8
: Consider removing unused private properties.The
client
andaccountId
properties are stored but never used directly in this class since all functionality is delegated to theSuppressionsApi
instance.export default class SuppressionsBaseAPI { - private client: AxiosInstance; - - private accountId?: number; - public getList: SuppressionsApi["getList"]; public delete: SuppressionsApi["delete"]; constructor(client: AxiosInstance, accountId?: number) { - this.client = client; - this.accountId = accountId; const suppressions = new SuppressionsApi(client, accountId); this.getList = suppressions.getList.bind(suppressions); this.delete = suppressions.delete.bind(suppressions); } }examples/templates/everything.ts (1)
13-19
: Consider using more realistic template content.The current HTML and text content is very basic. Consider using more representative template content that showcases typical use cases with dynamic placeholders.
const newTemplate = await client.templates.create({ name: "Welcome Email", subject: "Welcome to Our Service!", category: "Promotional", - body_html: "<h1>Welcome!</h1><p>Thank you for joining our service.</p>", - body_text: "Welcome! Thank you for joining our service." + body_html: "<h1>Welcome {{name}}!</h1><p>Thank you for joining {{service_name}}. Your account is now active.</p>", + body_text: "Welcome {{name}}! Thank you for joining {{service_name}}. Your account is now active." });src/lib/api/Templates.ts (1)
6-8
: Consider removing unused private properties for consistency.Similar to the
SuppressionsBaseAPI
, the private properties are stored but never used directly since all functionality is delegated.export default class TemplatesBaseAPI { - private client: AxiosInstance; - - private accountId?: number; - public get: TemplatesApi["get"]; // ... rest of propertiessrc/lib/api/resources/Suppressions.ts (1)
35-39
: Consider adding parameter validation for the delete method.While the current implementation is functional, adding basic parameter validation would improve the developer experience by providing clearer error messages for invalid inputs.
public async delete(id: string) { + if (!id || typeof id !== 'string' || id.trim() === '') { + throw new Error('Suppression ID is required and must be a non-empty string'); + } + return this.client.delete<Suppression, Suppression>( `${this.suppressionsURL}/${id}` ); }src/types/api/templates.ts (1)
29-29
: Consider using a more descriptive type name.The
TemplateList
type alias is simple but could be more descriptive.-export type TemplateList = Template[]; +export type TemplateList = Template[];Alternatively, you could use
Template[]
directly where needed instead of creating this alias, since it's a simple array type.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
README.md
(2 hunks)examples/sending/suppressions.ts
(1 hunks)examples/templates/everything.ts
(1 hunks)src/__tests__/lib/api/Suppressions.test.ts
(1 hunks)src/__tests__/lib/api/Templates.test.ts
(1 hunks)src/__tests__/lib/api/resources/Suppression.test.ts
(1 hunks)src/__tests__/lib/api/resources/Templates.test.ts
(1 hunks)src/__tests__/lib/mailtrap-client.test.ts
(2 hunks)src/lib/MailtrapClient.ts
(3 hunks)src/lib/api/Suppressions.ts
(1 hunks)src/lib/api/Templates.ts
(1 hunks)src/lib/api/resources/Suppressions.ts
(1 hunks)src/lib/api/resources/Templates.ts
(1 hunks)src/types/api/suppressions.ts
(1 hunks)src/types/api/templates.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (10)
src/__tests__/lib/api/Suppressions.test.ts (1)
src/lib/api/Suppressions.ts (1)
SuppressionsBaseAPI
(5-21)
src/__tests__/lib/api/Templates.test.ts (1)
src/lib/api/Templates.ts (1)
TemplatesBaseAPI
(5-30)
src/__tests__/lib/mailtrap-client.test.ts (3)
src/lib/MailtrapError.ts (1)
MailtrapError
(1-1)src/lib/api/Templates.ts (1)
TemplatesBaseAPI
(5-30)src/lib/api/Suppressions.ts (1)
SuppressionsBaseAPI
(5-21)
src/lib/api/Suppressions.ts (2)
src/lib/api/resources/Suppressions.ts (1)
SuppressionsApi
(9-40)src/lib/MailtrapClient.ts (1)
suppressions
(149-155)
src/lib/api/Templates.ts (2)
src/lib/api/resources/Templates.ts (1)
TemplatesApi
(13-72)src/lib/MailtrapClient.ts (1)
templates
(138-144)
src/lib/api/resources/Suppressions.ts (1)
src/types/api/suppressions.ts (1)
Suppression
(1-16)
src/lib/MailtrapClient.ts (3)
src/lib/MailtrapError.ts (1)
MailtrapError
(1-1)src/lib/api/Templates.ts (1)
TemplatesBaseAPI
(5-30)src/lib/api/Suppressions.ts (1)
SuppressionsBaseAPI
(5-21)
src/__tests__/lib/api/resources/Suppression.test.ts (4)
src/lib/api/resources/Suppressions.ts (1)
SuppressionsApi
(9-40)src/types/api/suppressions.ts (1)
Suppression
(1-16)src/lib/axios-logger.ts (1)
handleSendingError
(20-69)src/lib/MailtrapError.ts (1)
MailtrapError
(1-1)
src/__tests__/lib/api/resources/Templates.test.ts (4)
src/lib/api/resources/Templates.ts (1)
TemplatesApi
(13-72)src/types/api/templates.ts (3)
TemplateCreateParams
(13-19)Template
(1-11)TemplateUpdateParams
(21-27)src/lib/axios-logger.ts (1)
handleSendingError
(20-69)src/lib/MailtrapError.ts (1)
MailtrapError
(1-1)
src/lib/api/resources/Templates.ts (1)
src/types/api/templates.ts (3)
Template
(1-11)TemplateCreateParams
(13-19)TemplateUpdateParams
(21-27)
🪛 LanguageTool
README.md
[duplication] ~86-~86: Possible typo: you repeated a word.
Context: ...ls](examples/batch/bulk.ts) - Sandbox emails - [Emails from template](examples/batch/template....
(ENGLISH_WORD_REPEAT_RULE)
🪛 markdownlint-cli2 (0.17.2)
README.md
64-64: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
68-68: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
72-72: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
76-76: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
77-77: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
78-78: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
79-79: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
80-80: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
84-84: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
85-85: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
86-86: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
87-87: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
100-100: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
🔇 Additional comments (29)
src/__tests__/lib/mailtrap-client.test.ts (3)
14-15
: LGTM! Proper import structure.The new imports for
TemplatesBaseAPI
andSuppressionsBaseAPI
are correctly added and follow the existing import pattern.
788-812
: LGTM! Consistent test pattern for templates getter.The test suite for the
templates
getter follows the established pattern perfectly:
- Proper error handling when
accountId
is missing- Correct instance verification when
accountId
is provided- Consistent with other API getter tests
814-838
: LGTM! Consistent test pattern for suppressions getter.The test suite for the
suppressions
getter mirrors the templates test pattern and existing API getter tests:
- Proper error handling for missing
accountId
- Correct instance type verification
- Uses proper Jest expectations
src/__tests__/lib/api/Templates.test.ts (1)
1-20
: LGTM! Comprehensive test coverage for TemplatesBaseAPI.The test properly verifies that
TemplatesBaseAPI
initializes with all expected CRUD methods (create
,getList
,get
,update
,delete
). The test structure is consistent with other API wrapper tests and provides good coverage for the wrapper functionality.examples/sending/suppressions.ts (1)
11-30
: LGTM! Well-structured example demonstrating suppressions functionality.The example effectively demonstrates:
- Retrieving all suppressions
- Filtering suppressions by email
- Conditionally deleting suppressions with proper error handling
- Good use of async/await pattern and descriptive console logging
README.md (1)
70-72
: LGTM! Good documentation improvements with new API sections.The addition of the "Templates API" section and the "Suppressions" example in the "Sending API" section properly documents the new functionality. The more descriptive naming (e.g., "Contacts CRUD", "Contact Lists CRUD") improves clarity for users.
Also applies to: 79-79
src/lib/api/Suppressions.ts (1)
1-21
: LGTM! Well-structured wrapper class following established patterns.The implementation correctly follows the same architectural pattern as other API wrappers in the codebase (like
ContactsBaseAPI
andContactListsBaseAPI
). The method binding approach ensures properthis
context for the delegated methods.examples/templates/everything.ts (1)
1-44
: Excellent comprehensive example demonstrating the full template lifecycle.The example script provides a clear demonstration of all template operations in a logical sequence. The async/await usage and error handling are appropriate for an example script.
src/lib/api/Templates.ts (1)
1-30
: LGTM! Consistent implementation following established patterns.The
TemplatesBaseAPI
class correctly implements the wrapper pattern used throughout the codebase, providing a clean interface for template operations while delegating to the underlyingTemplatesApi
resource.src/lib/MailtrapClient.ts (2)
14-14
: LGTM! Clean imports following the established pattern.The imports are properly placed and follow the existing import organization in the file.
Also applies to: 25-25
135-155
: LGTM! Perfect consistency with existing getter patterns.Both new getters (
templates
andsuppressions
) follow the exact same pattern as existing getters in the class:
- Proper
accountId
validation with descriptive error throwing- Consistent return of wrapper API instances
- Clear JSDoc comments
- Proper integration with the class architecture
The implementation maintains perfect consistency with the established codebase patterns.
src/lib/api/resources/Suppressions.ts (2)
1-40
: LGTM! Solid resource implementation following established patterns.The
SuppressionsApi
class provides a clean interface to the suppressions API endpoints with proper TypeScript typing and clear method documentation. The URL construction and HTTP method usage are correct.
25-29
: Well-designed filtering capability.The optional
getList
method provides useful filtering functionality while maintaining a clean API surface. The implementation correctly passes the parameter as a query parameter.src/__tests__/lib/api/resources/Suppression.test.ts (4)
1-12
: Clean import structure and proper dependencies.The imports are well-organized and include all necessary dependencies for testing the SuppressionsApi class.
19-69
: Comprehensive mock data setup with proper typing.The mock data objects provide realistic test data that properly matches the
Suppression
type structure. The coverage includes different suppression types and both nullable and non-nullable fields.
127-135
: Robust error handling test implementation.The error handling test properly verifies that
MailtrapError
is thrown with the correct error message from the mocked API response.
111-116
: [web_search]How to mock GET requests with query parameters using axios-mock-adapter
src/types/api/templates.ts (3)
1-11
: Well-structured Template interface with comprehensive fields.The interface includes all necessary fields for a template object with proper typing. The optional
body_text
field allows for HTML-only templates.
13-19
: Appropriate create parameters interface.The create parameters correctly identify required fields, with
body_text
remaining optional for flexibility.
21-27
: Flexible update parameters with all optional fields.This design allows for partial updates, which is a good API practice for PATCH operations.
src/lib/api/resources/Templates.ts (4)
20-24
: Proper constructor implementation with URL building.The constructor correctly initializes the client, stores the accountId, and builds the base URL for template operations.
29-33
: Clean implementation of getList method.The method correctly constructs the URL and uses proper TypeScript generics for the response type.
47-52
: Correct payload wrapping for API requirements.The create method properly wraps the parameters in an
email_template
object, which appears to be the expected API format.
57-62
: Consistent update implementation with proper HTTP method.The update method uses PATCH (correct for partial updates) and maintains the same payload wrapping pattern as create.
src/__tests__/lib/api/resources/Templates.test.ts (4)
23-61
: Comprehensive mock data with realistic template objects.The mock data provides excellent test coverage with complete template objects that match the TypeScript interfaces. The separation of create/update request and response objects is well-organized.
75-85
: Proper test setup with axios interceptors and mocking.The beforeAll setup correctly configures axios interceptors for response unwrapping and error handling, matching the production configuration.
197-204
: Thorough verification of POST request payload wrapping.The test correctly verifies that the create request payload is wrapped in an
email_template
object, matching the API's expected format.
234-244
: Consistent PATCH request testing with payload verification.The update test maintains the same thorough verification pattern, ensuring the payload wrapping is consistent across create and update operations.
src/types/api/suppressions.ts (1)
1-16
: Well-designed Suppression type with comprehensive field coverage.The type definition effectively captures all suppression data with:
- Proper union types for restricted values (
type
andsending_stream
)- Appropriate nullable fields for optional message metadata
- Clear field naming that reflects the API structure
The extensive message metadata fields provide good context for understanding suppression reasons.
// Get all suppressions | ||
const allSuppressions = await client.suppressions.getList(); | ||
console.log("All suppressions:", allSuppressions); |
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.
Saying "all" in this case is unfortunate, because it doesn't actually return all suppressions. As the docs say, the endpoint returns up to 1000 suppressions per request (hard limit, and there's no pagination - to find something specific, a query filter should be used).
console.log("All suppressions:", allSuppressions); | ||
|
||
// Get suppressions filtered by email | ||
const filteredSuppressions = await client.suppressions.getList("[email protected]"); |
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.
I'd make this a named parameter, so that it's easier to add more named filters in the future.
const filteredSuppressions = await client.suppressions.getList("[email protected]"); | |
const filteredSuppressions = await client.suppressions.getList({email: "[email protected]"}); |
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.
Can you base this PR on #67, so that we only see relevant changes here?
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.
Yes, will wait for that too until I can properly review @narekhovhannisyan
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.
I wonder why the name of the file is singular (Suppression). All other files in this directory are plural
Motivation
The Mailtrap API supports suppressions functionality for managing email suppressions, but the Node.js client was missing this capability. Users need to be able to list and delete email suppressions programmatically through the client.
Changes
SuppressionsApi
class insrc/lib/api/resources/Suppressions.ts
with methods:getList()
anddelete()
SuppressionsBaseAPI
class insrc/lib/api/Suppressions.ts
to integrate with the main clientsrc/types/api/suppressions.ts
forSuppression
interfaceMailtrapClient
class with proper error handling for missingaccountId
examples/sending/suppressions.ts
demonstrating suppressions operationsHow to test
npm test
to ensure all new tests passnode examples/sending/suppressions.ts
(with valid token and account ID)accountId
is missing from client configurationSummary by CodeRabbit
New Features
Documentation
Tests