-
Notifications
You must be signed in to change notification settings - Fork 16
Billing api #103
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
Billing api #103
Conversation
WalkthroughAdds Billing usage management: a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant General
participant BillingApi
participant Axios
participant Server
rect rgb(240,248,255)
Note over General: Lazy initialization of billing
User->>General: access .billing
General->>General: checkAccountIdPresence()
alt accountId present
General->>BillingApi: instantiate (axios, accountId)
General-->>User: return BillingApi
else accountId missing
General-->>User: throw Error("accountId required")
end
end
rect rgb(245,255,240)
Note over BillingApi: Fetch billing usage
User->>BillingApi: getCurrentBillingCycleUsage()
BillingApi->>Axios: GET /api/accounts/{accountId}/billing/usage
Axios->>Server: HTTP GET
alt 200 Success
Server-->>Axios: 200 + data
Axios-->>BillingApi: response.data
BillingApi-->>User: BillingCycleUsage
else 4xx/5xx Error
Server-->>Axios: error response
Axios->>Axios: handleSendingError()
Axios-->>BillingApi: MailtrapError
BillingApi-->>User: throw MailtrapError
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
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 (1)
src/lib/api/resources/Billing.ts (1)
23-26: Optional: Simplify by usingthis.billingURLdirectly.The local variable
urlis only used once and could be eliminated for brevity.Apply this diff if you prefer a more concise implementation:
public async getCurrentBillingCycleUsage() { - const url = this.billingURL; - - return this.client.get<BillingCycleUsage, BillingCycleUsage>(url); + return this.client.get<BillingCycleUsage, BillingCycleUsage>(this.billingURL); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md(2 hunks)examples/general/billing.ts(1 hunks)src/__tests__/lib/api/General.test.ts(4 hunks)src/__tests__/lib/api/resources/Billing.test.ts(1 hunks)src/lib/api/General.ts(6 hunks)src/lib/api/resources/Billing.ts(1 hunks)src/types/api/billing.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/lib/api/resources/Billing.ts (1)
src/types/api/billing.ts (1)
BillingCycleUsage(1-32)
src/lib/api/General.ts (1)
src/lib/api/resources/Billing.ts (1)
BillingApi(10-28)
src/__tests__/lib/api/resources/Billing.test.ts (2)
src/lib/axios-logger.ts (1)
handleSendingError(20-69)src/lib/MailtrapError.ts (1)
MailtrapError(1-1)
🔇 Additional comments (16)
README.md (2)
44-44: LGTM!The documentation update appropriately lists the new billing usage management feature.
182-182: LGTM!The example link is properly formatted and follows the existing documentation pattern.
examples/general/billing.ts (1)
1-20: LGTM!The example clearly demonstrates billing API usage and follows the established pattern of other examples in the codebase. The error handling and result logging are appropriate.
src/lib/api/resources/Billing.ts (1)
10-18: LGTM!The constructor properly initializes the Billing API with the required dependencies and correctly constructs the billing endpoint URL using the account ID.
src/__tests__/lib/api/resources/Billing.test.ts (4)
13-56: LGTM!The test setup properly initializes the Billing API and defines a comprehensive response data fixture that matches the
BillingCycleUsagetype structure.
73-84: LGTM!The success case test properly verifies the endpoint URL construction and response data handling.
86-130: LGTM!The error handling tests comprehensively cover 404 and 403 scenarios, properly validating that
MailtrapErroris thrown with the correct message and that status codes are accessible viaerror.cause.response.status.
132-153: LGTM!The test properly validates error handling when the server returns a 500 error without an error body, ensuring the fallback error message and status code accessibility work correctly.
src/lib/api/General.ts (3)
5-5: LGTM!The import and instance field for BillingApi follow the established pattern used for other account-scoped APIs.
Also applies to: 19-19
36-46: LGTM!The
checkAccountIdPresence()helper method properly validates that an account ID is present before instantiating account-scoped APIs and provides a clear error message.
75-85: LGTM!The
billinggetter properly implements lazy instantiation of the BillingApi, following the established pattern used for other account-scoped APIs. The implementation includes proper account ID validation and singleton behavior.src/types/api/billing.ts (1)
1-32: LGTM!The
BillingCycleUsagetype is well-structured and clearly defines the shape of the billing API response, with properly nested objects for billing cycle information, sending plan/usage, and testing plan/usage.src/__tests__/lib/api/General.test.ts (4)
12-24: LGTM!The test updates properly validate that the
billingproperty is present and accessible when an account ID is provided, following the established pattern for other account-scoped APIs.
62-69: LGTM!The test properly validates that accessing the
billingAPI without an account ID throws the expected error, maintaining consistency with other account-scoped API behaviors.
89-100: LGTM!The backward compatibility test updates properly validate that the
billingAPI is defined and exposes the expectedgetCurrentBillingCycleUsage()method, ensuring the new API integrates seamlessly with existing functionality.
122-135: LGTM!The edge case test properly validates that the
billingAPI works correctly whenaccountIdis 0, ensuring that falsy numeric values are handled appropriately. This is an important test for preventing bugs with truthy/falsy checks.
…API in GeneralAPI
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 (1)
src/lib/api/General.ts (1)
75-85: Implementation looks good; consider JSDoc consistency.The lazy instantiation pattern and error handling are correctly implemented and consistent with the other getters.
However, the JSDoc says "Getter for Billing API" while
accountAccessesandpermissionssay "Singleton getter for..." Consider updating for consistency.Apply this diff to align the JSDoc with the existing pattern:
/** - * Getter for Billing API. + * Singleton getter for Billing API. */ public get billing(): BillingApi {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/api/General.ts(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/api/General.ts (1)
src/lib/api/resources/Billing.ts (1)
BillingApi(10-28)
🔇 Additional comments (3)
src/lib/api/General.ts (3)
5-5: LGTM!The import and field declaration follow the established patterns for other API resources in this class.
Also applies to: 19-19
36-46: LGTM!The
checkAccountIdPresence()helper effectively eliminates code duplication across the getter methods and provides a clear, actionable error message when accountId is missing.
48-50: LGTM!The JSDoc has been correctly updated to describe the
accountAccessesgetter. The past review comment has been properly addressed.
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 (1)
src/lib/api/General.ts (1)
75-85: Well-implemented billing getter.The implementation correctly follows the singleton pattern and uses the
checkAccountIdPresence()helper for validation. The JSDoc is clear and matches the pattern of other getters.Minor observation:
accountAccessesInstanceandpermissionsInstanceare instantiated eagerly in the constructor whenaccountIdis provided (lines 26-33), whilebillingInstanceis always instantiated lazily. While this doesn't affect functionality, you could consider making the pattern consistent by either:
- Instantiating
billingInstancein the constructor alongside the others, or- Moving all three to pure lazy instantiation (removing lines 26-33)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/api/General.ts(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/api/General.ts (1)
src/lib/api/resources/Billing.ts (1)
BillingApi(10-28)
🔇 Additional comments (5)
src/lib/api/General.ts (5)
5-5: LGTM!The import follows the established pattern for resource APIs and is correctly placed.
19-19: LGTM!The private field follows the singleton pattern established by other API instances in this class.
36-46: Excellent refactoring with the validation helper.The
checkAccountIdPresence()method centralizes the accountId validation logic and provides a clear, actionable error message. Thenumberreturn type improves type safety for callers by eliminating the nullable type.
51-61: LGTM!The refactoring to use
checkAccountIdPresence()improves consistency and maintainability while preserving the lazy instantiation pattern.
66-73: LGTM!Consistent refactoring that aligns with the
accountAccessesgetter pattern.
Motivation
Add support for the Billing API to allow users to retrieve current billing cycle usage information for their Mailtrap accounts. This enables developers to programmatically check billing cycle dates, plan information, and usage statistics for both sending and testing services.
Fixes #100
Changes
BillingApiclass insrc/lib/api/resources/Billing.tswithgetCurrentBillingCycleUsage()methodBillingCycleUsageTypeScript type definition insrc/types/api/billing.tscovering billing cycle, sending plan/usage, and testing plan/usageGeneralAPIclass as a lazy-instantiated getter propertyexamples/general/billing.tsdemonstrating how to use the billing APIsrc/__tests__/lib/api/resources/Billing.test.tscovering:src/__tests__/lib/api/General.test.tsto include billing API tests for:How to test
npm test -- --testPathPattern="Billing"to verify all billing API tests passnpm test -- --testPathPattern="General"to verify General API tests including billing integration passexamples/general/billing.tsfollows the same pattern as other examplesclient.general.billing.getCurrentBillingCycleUsage()with valid credentialserror.cause.response.statusSummary by CodeRabbit
New Features
Documentation