-
Notifications
You must be signed in to change notification settings - Fork 16
Create contact event #99
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
…r contact event management
…ontactEventsApi class
WalkthroughAdds Contact Events: new resource and wrapper classes, MailtrapClient getter, TypeScript types, unit tests, an example script, and README updates linking the new Contact Events API. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client as MailtrapClient
participant Wrapper as ContactEventsBaseAPI
participant Resource as ContactEventsApi
participant API as HTTP API
Note over Client,Wrapper: (getter) validate accountId
User->>Client: access contactEvents
Client->>Wrapper: new ContactEventsBaseAPI(axios, accountId)
Client-->>User: ContactEventsBaseAPI
User->>Wrapper: create(contactIdentifier, data)
Wrapper->>Resource: resource.create(contactIdentifier, data)
Resource->>API: POST /accounts/{accountId}/contacts/{contactIdentifier}/events
API-->>Resource: 200 / 4xx
Resource-->>Wrapper: response or throw
Wrapper-->>User: event response or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)src/lib/MailtrapClient.ts (1)
🔇 Additional comments (2)
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 |
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 (4)
examples/contact-events/everything.ts (1)
11-34: Consider improving error type safety and adding explanatory comments.The get/create logic with 409 conflict handling is appropriate, but could be enhanced:
Line 24: Using
anytype loses type safety. Consider usingunknowninstead:- } catch (err: any) { + } catch (err: unknown) {The nested error handling handles race conditions where a contact might be created between the get and create calls. Consider adding a comment to explain this:
// Handle race condition: contact may have been created between get and create if (status === 409) { const existing = await client.contacts.get(email); contactId = existing.data.id; }src/lib/api/resources/ContactEvents.ts (3)
17-20: Consider validating accountId parameter.While the API server will reject invalid account IDs, adding client-side validation (e.g., checking for positive integers) could provide earlier feedback and prevent unnecessary network requests.
constructor(client: AxiosInstance, accountId: number) { + if (accountId <= 0 || !Number.isInteger(accountId)) { + throw new Error("accountId must be a positive integer"); + } this.client = client; this.contactsURL = `${GENERAL_ENDPOINT}/api/accounts/${accountId}/contacts`; }
25-28: Add explicit return type annotation for the public API method.TypeScript best practice for public APIs is to explicitly declare return types, which improves type safety, IDE autocompletion, and API documentation clarity.
public async create( contactIdentifier: number | string, data: ContactEventOptions -) { +): Promise<ContactEventResponse> {
25-35: Consider adding input validation for better developer experience.Adding client-side validation for
contactIdentifieranddataparameters could catch errors earlier and provide clearer error messages before making API calls.public async create( contactIdentifier: number | string, data: ContactEventOptions -) { +): Promise<ContactEventResponse> { + // Validate contactIdentifier + if (typeof contactIdentifier === "string" && contactIdentifier.trim() === "") { + throw new Error("contactIdentifier cannot be an empty string"); + } + if (typeof contactIdentifier === "number" && contactIdentifier <= 0) { + throw new Error("contactIdentifier must be a positive number"); + } + + // Validate data + if (!data.name || typeof data.name !== "string" || data.name.trim() === "") { + throw new Error("Event name is required and must be a non-empty string"); + } + if (!data.params || typeof data.params !== "object") { + throw new Error("Event params must be an object"); + } + const url = `${this.contactsURL}/${contactIdentifier}/events`; return this.client.post<ContactEventResponse, ContactEventResponse>( url, data ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
README.md(2 hunks)examples/contact-events/everything.ts(1 hunks)src/__tests__/lib/api/ContactEvents.test.ts(1 hunks)src/__tests__/lib/api/resources/ContactEvents.test.ts(1 hunks)src/__tests__/lib/mailtrap-client.test.ts(2 hunks)src/lib/MailtrapClient.ts(2 hunks)src/lib/api/ContactEvents.ts(1 hunks)src/lib/api/resources/ContactEvents.ts(1 hunks)src/types/api/contact-events.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
src/lib/MailtrapClient.ts (1)
src/lib/api/ContactEvents.ts (1)
ContactEventsBaseAPI(5-12)
src/__tests__/lib/mailtrap-client.test.ts (2)
src/lib/MailtrapError.ts (1)
MailtrapError(1-1)src/lib/api/ContactEvents.ts (1)
ContactEventsBaseAPI(5-12)
src/lib/api/resources/ContactEvents.ts (1)
src/types/api/contact-events.ts (2)
ContactEventOptions(1-4)ContactEventResponse(6-11)
src/lib/api/ContactEvents.ts (2)
src/lib/api/resources/ContactEvents.ts (1)
ContactEventsApi(12-36)src/lib/MailtrapClient.ts (1)
contactEvents(137-140)
src/__tests__/lib/api/ContactEvents.test.ts (1)
src/lib/api/ContactEvents.ts (1)
ContactEventsBaseAPI(5-12)
src/__tests__/lib/api/resources/ContactEvents.test.ts (3)
src/lib/api/resources/ContactEvents.ts (1)
ContactEventsApi(12-36)src/lib/axios-logger.ts (1)
handleSendingError(20-69)src/lib/MailtrapError.ts (1)
MailtrapError(1-1)
🔇 Additional comments (12)
README.md (1)
36-36: LGTM!Documentation updates are clear and follow the existing structure. The Contact Events feature is properly listed and referenced.
Also applies to: 130-132
src/__tests__/lib/api/ContactEvents.test.ts (1)
1-17: LGTM!The test appropriately verifies that the ContactEventsBaseAPI wrapper exposes the create method. For a thin wrapper class, checking property existence is sufficient coverage.
src/types/api/contact-events.ts (1)
1-11: LGTM!The type definitions are clean and appropriate. The flexible
Recordtype forparamsallows for custom event data, which aligns with the goal of tracking various custom events.src/__tests__/lib/mailtrap-client.test.ts (2)
17-17: LGTM!Import added for the new ContactEventsBaseAPI class.
905-929: LGTM!The tests for the
contactEventsgetter follow the established pattern used by other API getters in this file. Coverage includes both the error case (missing accountId) and success case (returns correct instance type).src/lib/MailtrapClient.ts (2)
13-13: LGTM!Import added for the new Contact Events API wrapper.
134-140: LGTM!The
contactEventsgetter implementation perfectly follows the established pattern used by other API getters in this class. It validatesaccountIdpresence and returns a properly configured API instance.src/__tests__/lib/api/resources/ContactEvents.test.ts (1)
1-106: LGTM!Comprehensive test coverage for the ContactEventsApi resource class. The tests appropriately cover:
- Successful event creation with correct endpoint construction
- Error handling for invalid account (404)
- Error handling for invalid payload (422)
The test structure follows existing patterns and uses proper mocking with axios-mock-adapter.
examples/contact-events/everything.ts (2)
1-9: LGTM!Example setup is clear with properly marked placeholder values that users need to replace.
36-54: LGTM!The event creation demonstrates proper usage of the Contact Events API with appropriate error handling and logging. The payload structure matches the
ContactEventOptionstype definition.src/lib/api/ContactEvents.ts (1)
1-12: LGTM!The ContactEventsBaseAPI wrapper class follows the established pattern used by other base API classes in the codebase. The implementation correctly:
- Type-safely exposes the
createmethod using indexed access types- Binds the method to preserve proper
thiscontext- Delegates to the underlying ContactEventsApi resource class
src/lib/api/resources/ContactEvents.ts (1)
1-10: LGTM! Imports and configuration are properly structured.The imports and configuration extraction follow standard patterns and correctly reference the new Contact Events types.
Co-authored-by: Marcin Klocek <marcin.klocek@railsware.com>
Co-authored-by: Marcin Klocek <marcin.klocek@railsware.com>
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)
src/lib/MailtrapClient.ts (1)
11-16: Remove duplicate ContactListsBaseAPI import.Line 12 duplicates the import on line 16, causing a TypeScript compilation failure (TS2300: Duplicate identifier). This is blocking the build pipeline.
Apply this diff to remove the duplicate:
import ContactsBaseAPI from "./api/Contacts"; import ContactEventsBaseAPI from "./api/ContactEvents"; -import ContactListsBaseAPI from "./api/ContactLists"; import ContactExportsBaseAPI from "./api/ContactExports"; import ContactFieldsBaseAPI from "./api/ContactFields"; import ContactImportsBaseAPI from "./api/ContactImports"; import ContactListsBaseAPI from "./api/ContactLists";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(2 hunks)src/__tests__/lib/mailtrap-client.test.ts(2 hunks)src/lib/MailtrapClient.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/tests/lib/mailtrap-client.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/MailtrapClient.ts (1)
src/lib/api/ContactEvents.ts (1)
ContactEventsBaseAPI(5-12)
🪛 ESLint
src/lib/MailtrapClient.ts
[error] 12-12: '/home/jailuser/git/src/lib/api/ContactLists.ts' imported multiple times.
(import/no-duplicates)
[error] 144-144: Delete ···
(prettier/prettier)
[error] 145-145: Delete ···
(prettier/prettier)
🪛 GitHub Actions: test
src/lib/MailtrapClient.ts
[error] 12-12: TypeScript compile error TS2300: Duplicate identifier 'ContactListsBaseAPI'. (during 'yarn build' step)
🪛 GitHub Check: lint
src/lib/MailtrapClient.ts
[failure] 12-12:
Duplicate identifier 'ContactListsBaseAPI'.
🔇 Additional comments (3)
README.md (2)
33-38: Documentation properly surfaces Contact Events API.The reorganization of contact-related features is clear and logical. "Contact Events" is appropriately positioned as the first contact management feature, followed by other contact-related APIs. This makes the new functionality more discoverable.
120-142: Documentation sections are well-structured and consistent.The new API sections (Contact Events, Contact Exports, Contact Lists, Contacts) follow consistent formatting and clearly link to their respective example files. The documentation structure properly reflects the API reorganization described in the PR objectives.
Please verify that the referenced example files exist at the documented paths:
examples/contact-events/everything.tsexamples/contact-exports/everything.tsexamples/contact-lists/everything.tsexamples/contacts/everything.tsSince these are out of scope for this README review, you may want to cross-reference with the actual file structure when reviewing other parts of the PR.
src/lib/MailtrapClient.ts (1)
138-143: LGTM!The
contactEventsgetter correctly follows the established pattern used by other API getters in this class. It validatesaccountIdand returns a properly initializedContactEventsBaseAPIinstance.
…nts in MailtrapClient
Motivation
Add support for Contact Events API to enable tracking custom events for contacts. This allows users to create and track events (e.g., purchase_completed, page_viewed) associated with contacts, enabling better engagement tracking and personalization.
Fixes #87
Changes
src/lib/api/resources/ContactEvents.ts)src/lib/api/ContactEvents.ts)src/types/api/contact-events.ts)contactEventsgetter to MailtrapClientexamples/contact-events/everything.ts)src/__tests__/lib/api/resources/ContactEvents.test.ts)src/__tests__/lib/api/ContactEvents.test.ts)src/__tests__/lib/mailtrap-client.test.ts)How to test
npm test -- ContactEvents.test.tsto verify all Contact Events API tests passnpm test -- mailtrap-client.test.ts --testNamePattern="contactEvents"to verify MailtrapClient getter tests passexamples/contact-events/everything.tswith valid credentials and runnpx ts-node examples/contact-events/everything.tsSummary by CodeRabbit
New Features
Documentation
Examples
Types
Tests