feat(billing): implement charges csv export#1790
feat(billing): implement charges csv export#1790ygrishajev merged 43 commits intoakash-network:mainfrom
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughAdds a streaming CSV export for Stripe transactions: new API route and controller, a paginated streaming service producing CSV chunks, request schemas with timezone/date-range validation, HTTP SDK and UI wiring to download a Blob, seeders/tests, types, and config/dependency updates. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Web_UI as Web UI
participant SDK as HTTP SDK
participant API as API Router
participant Controller as StripeController
participant Service as StripeService
participant Stripe as Stripe API
User->>Web_UI: Click "Export CSV"
Web_UI->>SDK: exportTransactionsCsv({startDate,endDate,timezone})
SDK->>API: GET /v1/stripe/transactions/export?... (responseType=blob)
API->>Controller: route -> exportTransactionsCsvStream
Controller->>Service: exportTransactionsCsvStream(customerId, options)
par paging loop
Service->>Stripe: list transactions (limit, starting_after, date range)
Stripe-->>Service: page of transactions
Service->>Service: transform rows -> csv-stringify -> yield chunks
end
Service-->>Controller: AsyncIterable<string> (CSV chunks)
Controller-->>API: stream response (text/csv, chunked, Content-Disposition)
API-->>SDK: HTTP response -> Blob
SDK-->>Web_UI: Blob
Web_UI->>User: trigger download
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/api/src/billing/controllers/stripe/stripe.controller.ts(1 hunks)apps/api/src/billing/http-schemas/stripe.schema.ts(2 hunks)apps/api/src/billing/routes/stripe-transactions/stripe-transactions.router.ts(3 hunks)apps/api/src/billing/services/stripe/stripe.service.spec.ts(2 hunks)apps/api/src/billing/services/stripe/stripe.service.ts(1 hunks)apps/api/test/seeders/payment-method.seeder.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/billing/routes/stripe-transactions/stripe-transactions.router.tsapps/api/src/billing/controllers/stripe/stripe.controller.tsapps/api/test/seeders/payment-method.seeder.tsapps/api/src/billing/http-schemas/stripe.schema.tsapps/api/src/billing/services/stripe/stripe.service.tsapps/api/src/billing/services/stripe/stripe.service.spec.ts
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/billing/routes/stripe-transactions/stripe-transactions.router.tsapps/api/src/billing/controllers/stripe/stripe.controller.tsapps/api/test/seeders/payment-method.seeder.tsapps/api/src/billing/http-schemas/stripe.schema.tsapps/api/src/billing/services/stripe/stripe.service.tsapps/api/src/billing/services/stripe/stripe.service.spec.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/src/billing/services/stripe/stripe.service.spec.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: baktun14
PR: akash-network/console#1725
File: apps/api/src/utils/constants.ts:5-5
Timestamp: 2025-07-24T17:00:52.361Z
Learning: In the Akash Network Console project, when cross-cutting concerns or broader refactoring issues are identified during PR review, the preferred approach is to create a separate GitHub issue to track the work rather than expanding the scope of the current PR. This maintains focus and allows for proper planning of architectural improvements.
📚 Learning: applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type d...
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Applied to files:
apps/api/test/seeders/payment-method.seeder.ts
📚 Learning: applies to **/*.spec.{ts,tsx} : don't use `jest.mock()` to mock dependencies in test files. instead,...
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-07-21T08:24:24.269Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` to mock dependencies in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test.
Applied to files:
apps/api/src/billing/services/stripe/stripe.service.spec.ts
📚 Learning: applies to **/*.spec.{ts,tsx} : use `setup` function instead of `beforeeach` in test files...
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Applied to files:
apps/api/src/billing/services/stripe/stripe.service.spec.ts
📚 Learning: applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it...
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Applied to files:
apps/api/src/billing/services/stripe/stripe.service.spec.ts
🧬 Code Graph Analysis (5)
apps/api/src/billing/routes/stripe-transactions/stripe-transactions.router.ts (3)
apps/api/src/billing/http-schemas/stripe.schema.ts (1)
CustomerTransactionsCsvExportQuerySchema(180-186)apps/api/src/billing/controllers/stripe/stripe.controller.ts (1)
exportTransactionsCsv(153-159)apps/api/src/billing/services/stripe/stripe.service.ts (1)
exportTransactionsCsv(416-440)
apps/api/src/billing/controllers/stripe/stripe.controller.ts (1)
apps/api/src/auth/services/auth.service.ts (1)
Protected(39-55)
apps/api/test/seeders/payment-method.seeder.ts (1)
apps/api/src/billing/services/stripe/stripe.service.ts (1)
PaymentMethod(515-544)
apps/api/src/billing/services/stripe/stripe.service.ts (2)
apps/api/src/billing/http-schemas/stripe.schema.ts (1)
Transaction(203-203)apps/api/src/core/services/tx/tx.service.ts (1)
transaction(19-26)
apps/api/src/billing/services/stripe/stripe.service.spec.ts (1)
apps/api/test/seeders/payment-method.seeder.ts (1)
generatePaymentMethod(11-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (12)
apps/api/src/billing/http-schemas/stripe.schema.ts (1)
131-186: LGTM! Excellent refactoring for code reuse.The date range validation logic is well-structured and properly extracted for reuse. The 366-day limit is appropriate for preventing excessive data exports, and the validation correctly handles optional dates.
apps/api/src/billing/controllers/stripe/stripe.controller.ts (1)
152-159: LGTM! Consistent with existing controller patterns.The method follows established authorization and validation patterns in the controller. The error handling for missing
stripeCustomerIdis appropriate and user-friendly.apps/api/test/seeders/payment-method.seeder.ts (1)
5-17: LGTM! Clean enhancement for test data customization.The changes maintain backward compatibility while enabling more precise control over generated payment method data. The TypeScript typing is appropriate and the implementation follows good patterns.
apps/api/src/billing/routes/stripe-transactions/stripe-transactions.router.ts (2)
54-75: LGTM! Proper OpenAPI documentation for CSV export.The route definition correctly specifies the CSV content type and binary format in the OpenAPI schema. The request validation using the shared schema is appropriate.
102-116: LGTM! Correct implementation of CSV file download.The route handler properly:
- Extracts validated query parameters
- Calls the controller method
- Sets appropriate Content-Type and Content-Disposition headers
- Generates a descriptive filename based on the date range
The filename generation is safe since the dates are validated to be ISO datetime strings by the schema.
apps/api/src/billing/services/stripe/stripe.service.spec.ts (5)
368-429: LGTM! Comprehensive test coverage for single page export.The test properly mocks dependencies, verifies CSV format, and checks service method calls with correct parameters. The use of
generatePaymentMethodwith specific inputs is excellent for testing precise scenarios.
431-508: LGTM! Excellent pagination handling test.The test properly verifies that multiple pages are fetched correctly and the
startingAfterparameter is properly passed between calls. The verification of CSV content inclusion is appropriate.
540-556: LGTM! Good coverage for empty results scenario.The test correctly handles the empty transaction case and verifies the appropriate message is returned.
558-587: LGTM! Important edge case coverage for null payment methods.This test ensures the CSV export handles transactions with missing payment method data gracefully, which is a realistic scenario in production data.
1020-1077: LGTM! Proper test setup following coding guidelines.The setup function is correctly positioned at the bottom of the describe block, uses jest-mock-extended for mocking, and returns the service under test with its dependencies. No shared state is used, which follows the guidelines.
apps/api/src/billing/services/stripe/stripe.service.ts (2)
416-440: LGTM! Well-implemented pagination with appropriate safety limits.The method correctly handles pagination through all transactions while preventing resource exhaustion with the 50,000 transaction limit. The reuse of existing
getCustomerTransactionsmethod maintains consistency.
442-473: CSV formatting implementation is solid with proper escaping.The method correctly:
- Handles the empty transactions case gracefully
- Converts Unix timestamps to ISO date format (YYYY-MM-DD)
- Formats amounts from cents to dollars with proper decimal places
- Escapes CSV fields by doubling internal quotes and wrapping in quotes
- Handles optional fields safely with fallback to empty strings
The currency conversion to uppercase and amount formatting follow standard practices.
baktun14
left a comment
baktun14
left a comment
There was a problem hiding this comment.
Looks good! Just need to address the potentially memory issues
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/api/src/billing/services/stripe/stripe.service.ts (1)
418-444: Solid, memory-safe streaming pipeline.Using Readable.from(generator) piped into csv-stringify and yielding chunks keeps memory bounded and avoids buffering entire CSVs in RAM. Good job addressing the previous review’s streaming concern.
🧹 Nitpick comments (6)
apps/api/src/billing/services/stripe/stripe.service.ts (2)
470-483: Sentinel “no transactions” row could surprise downstream CSV consumers.Emitting a synthetic row with a message in the id column can break parsers expecting a consistent schema. Consider emitting only the header (no rows) when empty, or add a separate footer comment line prefixed with “#”.
Here’s a minimal change to output only headers when empty:
- if (!hasYieldedAny) { - yield { - id: "No transactions found for the specified date range", - date: "", - amount: "", - currency: "", - status: "", - paymentMethodType: "", - cardBrand: "", - cardLast4: "", - description: "", - receiptUrl: "" - }; - } + // If no records, emit nothing further. csv-stringify already emitted headers when header: true. + if (!hasYieldedAny) return;
372-416: Optional: enforce a maximum date range or record cap to protect the service.Even with streaming, extremely large ranges can cause long-lived requests and heavy Stripe pagination. Consider enforcing a maximum range (e.g., 90 days) or an overall record cap with a clear error.
Would you like me to add a guard in getCustomerTransactions and/or in the date-range schema to enforce a maximum window?
apps/api/src/billing/routes/stripe-transactions/stripe-transactions.router.ts (4)
102-111: Minor: naming clarity.csvStream is an AsyncIterable, not a Node.js stream. Consider renaming to csvChunks or csvIterable to avoid confusion with Node streams.
- const csvStream = await container.resolve(StripeController).exportTransactionsCsvStream({ + const csvChunks = await container.resolve(StripeController).exportTransactionsCsvStream({ startDate, endDate });And below:
- for await (const chunk of csvStream) { + for await (const chunk of csvChunks) { controller.enqueue(new TextEncoder().encode(chunk)); }
112-123: Avoid re-creating TextEncoder on every chunk.Instantiate TextEncoder once per request to reduce allocations during streaming.
const readableStream = new ReadableStream({ async start(controller) { + const encoder = new TextEncoder(); try { - for await (const chunk of csvStream) { - controller.enqueue(new TextEncoder().encode(chunk)); + for await (const chunk of csvStream) { + controller.enqueue(encoder.encode(chunk)); } controller.close(); } catch (error) { controller.error(error); } } });
125-131: Drop Transfer-Encoding header and declare charset.Servers set Transfer-Encoding automatically for streamed responses. Also, include charset for correctness.
return new Response(readableStream, { headers: { - "Content-Type": "text/csv", + "Content-Type": "text/csv; charset=utf-8", "Content-Disposition": `attachment; filename="${filename}"`, - "Transfer-Encoding": "chunked" } });
54-75: Optional: add an OpenAPI example and proper schema for streaming text.If your tooling supports it, prefer schema: { type: "string" } without format: "binary" for text/csv, and add an example to improve DX. Not blocking.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snappackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
apps/api/package.json(1 hunks)apps/api/src/billing/controllers/stripe/stripe.controller.ts(1 hunks)apps/api/src/billing/routes/stripe-transactions/stripe-transactions.router.ts(3 hunks)apps/api/src/billing/services/stripe/stripe.service.spec.ts(2 hunks)apps/api/src/billing/services/stripe/stripe.service.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/src/billing/services/stripe/stripe.service.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/billing/controllers/stripe/stripe.controller.tsapps/api/src/billing/services/stripe/stripe.service.tsapps/api/src/billing/routes/stripe-transactions/stripe-transactions.router.ts
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/billing/controllers/stripe/stripe.controller.tsapps/api/src/billing/services/stripe/stripe.service.tsapps/api/src/billing/routes/stripe-transactions/stripe-transactions.router.ts
🧬 Code Graph Analysis (3)
apps/api/src/billing/controllers/stripe/stripe.controller.ts (1)
apps/api/src/auth/services/auth.service.ts (1)
Protected(39-55)
apps/api/src/billing/services/stripe/stripe.service.ts (2)
apps/api/src/core/services/tx/tx.service.ts (1)
transaction(19-26)apps/api/src/billing/http-schemas/stripe.schema.ts (1)
Transaction(203-203)
apps/api/src/billing/routes/stripe-transactions/stripe-transactions.router.ts (1)
apps/api/src/billing/http-schemas/stripe.schema.ts (1)
CustomerTransactionsCsvExportQuerySchema(180-186)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: validate (apps/stats-web) / validate-unsafe
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
🔇 Additional comments (3)
apps/api/package.json (1)
74-74: Dependency addition looks good; aligns with streaming CSV use-case.Adding csv-stringify is appropriate for the streaming pipeline introduced in the service. No concerns on the version constraint.
apps/api/src/billing/controllers/stripe/stripe.controller.ts (1)
154-161: Controller delegation and auth check look correct.Authorization is enforced via @Protected, and you assert the presence of stripeCustomerId before delegating to the service streaming method. Good separation of concerns.
apps/api/src/billing/routes/stripe-transactions/stripe-transactions.router.ts (1)
54-75: OpenAPI route definition for CSV looks good.Clear summary, correct method and path, and proper content type. This will document the export endpoint well.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/api/src/billing/services/stripe/stripe.service.ts (1)
500-516: Add an explicit return type to enforce the CSV row contractThe method shape matches TransactionCsvRow; add the explicit return type to lock this down at compile time.
Apply this diff:
- private transformTransactionForCsv(transaction: Transaction) { + private transformTransactionForCsv(transaction: Transaction): TransactionCsvRow {
🧹 Nitpick comments (2)
apps/api/src/billing/services/stripe/stripe.service.ts (2)
442-445: Handle stream errors to avoid unhandled exceptions in the CSV pipelineIf csv-stringify or the source stream emits an error, this loop will throw without logging context. Wrap with try/catch and log (then rethrow) to aid observability.
Apply this diff:
- for await (const chunk of csvStream) { - yield chunk.toString(); - } + try { + for await (const chunk of csvStream) { + yield chunk.toString(); + } + } catch (err) { + logger.error({ + event: "CSV_STREAMING_ERROR", + error: err instanceof Error ? err.message : String(err) + }); + throw err; + }
447-451: Nit: Prefer void for the third AsyncGenerator generic parameterYou aren’t sending values into the generator via next(), so using void communicates intent better than unknown.
Apply this diff:
- ): AsyncGenerator<TransactionCsvRow, void, unknown> { + ): AsyncGenerator<TransactionCsvRow, void, void> {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/api/src/billing/services/stripe/stripe.service.ts(3 hunks)apps/api/src/types/transactions.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/types/transactions.tsapps/api/src/billing/services/stripe/stripe.service.ts
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/types/transactions.tsapps/api/src/billing/services/stripe/stripe.service.ts
🧬 Code Graph Analysis (1)
apps/api/src/billing/services/stripe/stripe.service.ts (2)
apps/api/src/types/transactions.ts (1)
TransactionCsvRow(21-32)apps/api/src/billing/http-schemas/stripe.schema.ts (1)
Transaction(203-203)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
🔇 Additional comments (5)
apps/api/src/types/transactions.ts (1)
21-32: LGTM: Well-scoped CSV row shapeThe type is clear, minimal, and matches the export needs across the service and tests. Using strings here keeps the streaming pipeline straightforward.
apps/api/src/billing/services/stripe/stripe.service.ts (4)
419-446: Good streaming pipeline: memory-safe and backpressure-awareUsing Readable.from over the async generator and piping through csv-stringify keeps memory bounded and respects backpressure. This addresses prior concerns about large CSV exports.
490-498: CSV injection mitigation is correctly implementedPrefixing =, +, -, @ with a single quote aligns with OWASP guidance for CSV/Excel injection. Good call applying this to user-controlled description.
474-487: Sentinel row is intentional — tests assert the messagerg shows the sentinel is emitted in the service and explicitly asserted in tests, so this is deliberate.
- apps/api/src/billing/services/stripe/stripe.service.ts — sentinel yield with id: "No transactions found for the specified date range" (around lines 474–487)
- apps/api/src/billing/services/stripe/stripe.service.spec.ts — test asserts: expect(fullCsv).toContain("No transactions found for the specified date range"); (line ~518)
Action: confirm product requirement — if you want header-only on empty results, remove the sentinel yield and update the test(s); otherwise keep as-is.
1-5: Keep importing Readable from "stream" to match the rest of apps/apiSearch in apps/api found 1 import from "stream" and 0 from "node:stream", so changing this file alone would be inconsistent. Leave the import as-is:
- apps/api/src/billing/services/stripe/stripe.service.ts — keep:
import { Readable } from "stream";If you want to standardize on
node:stream, update all occurrences in a single PR instead of this file alone.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
apps/deploy-web/src/components/billing-usage/BillingView/BillingView.spec.tsx (1)
1-112: ManygetBy*calls remain acrossapps/deploy-webtests
The guideline to preferqueryBy*overgetBy*in.spec.tsxfiles has only been applied toBillingView.spec.tsx. A search finds hundreds of remaininggetBy*/getAllBy*usages underapps/deploy-web. To fully comply:• Replace all presence assertions using
screen.getBy*(and any destructuredgetBy*) withscreen.queryBy*, e.g.:
expect(screen.getByText("…")).toBeInTheDocument()→expect(screen.queryByText("…")).toBeInTheDocument()
• For interactive queries (e.g.fireEvent.click/change), add non-null assertions:fireEvent.click(screen.getByText("Next"))→fireEvent.click(screen.queryByText("Next")!)
• Automate via a codemod or scripted find-and-replace across all*.spec.tsxunderapps/deploy-web.Let’s roll this out project-wide rather than file-by-file.
🧹 Nitpick comments (5)
packages/http-sdk/src/stripe/stripe.types.ts (1)
69-71: Duplicate interface declaration: SetupIntentResponseSetupIntentResponse is declared twice in this file (also at Line 7). While TypeScript will merge identical interface declarations, this duplication is unnecessary and can mislead future edits.
Consider removing one of the duplicate declarations to avoid confusion. Keep the one closest to related payment-method types for cohesion.
packages/http-sdk/src/stripe/stripe.service.ts (1)
67-80: Use shared type, add validation, and specify Accept headerA few non-breaking enhancements to keep this method consistent and DRY:
- Swap the inline
{ startDate: Date; endDate: Date }for the sharedExportTransactionsCsvParams.- Add a lightweight guard to mirror
getCustomerTransactions(throw ifstartDate >= endDate).- Explicitly set
Accept: "text/csv"so the intent and content negotiation are clear.- No change to
extractData—it’s available onApiHttpServiceand appropriate for a rawBlobresponse.Apply this diff in
packages/http-sdk/src/stripe/stripe.service.ts:@@ -67,14 +67,23 @@ export class StripeService extends ApiHttpService { - async exportTransactionsCsv(params: { startDate: Date; endDate: Date }): Promise<Blob> { + async exportTransactionsCsv(params: ExportTransactionsCsvParams): Promise<Blob> { if (params.startDate >= params.endDate) { throw new Error("startDate must be less than endDate"); } const queryParams = new URLSearchParams({ startDate: params.startDate.toISOString(), endDate: params.endDate.toISOString() }); const url = `/v1/stripe/transactions/export?${queryParams}`; return this.extractData( - await this.get(url, { responseType: "blob" }) + await this.get(url, { + responseType: "blob", + headers: { Accept: "text/csv" } + }) ); }And at the top of the file:
import type { ExportTransactionsCsvParams } from "./stripe.types";apps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx (1)
63-67: Strengthen the export test by asserting the mutation is invoked with dates.Currently, the test only checks that the
onExportprop is called. Given the container now triggers a mutation when a valid date range exists, it’s helpful to assert thatmutateis called with{ startDate, endDate }.You already control the mocked hook; capture its
mutateand assert args:- const mockedUseExportTransactionsCsvMutation = jest.fn(() => ({ - mutate: jest.fn(), + const mutateMock = jest.fn(); + const mockedUseExportTransactionsCsvMutation = jest.fn(() => ({ + mutate: mutateMock, isLoading: false, isError: false, error: null })) as unknown as jest.MockedFunction<typeof useExportTransactionsCsvMutation>;Then extend the test:
- it("calls onExport", async () => { - const { child, onExport } = await setup(); - child.onExport(); - expect(onExport).toHaveBeenCalled(); - }); + it("calls onExport and triggers export mutation with date range", async () => { + const { child, onExport } = await setup(); + child.onExport(); + expect(onExport).toHaveBeenCalled(); + // also check mutation invocation: + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + const { mutate } = (mockedUseExportTransactionsCsvMutation as unknown as jest.Mock).mock.results[0].value; + expect(mutate).toHaveBeenCalledWith( + expect.objectContaining({ startDate: expect.any(Date), endDate: expect.any(Date) }) + ); + });Also applies to: 102-108
apps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.tsx (1)
126-132: Surface export errors to the UI by handling mutation failures.Right now, export errors are silent. Consider setting
errorwhen the export mutation fails, and optionally clearing stale errors when a new export starts.Proposed change:
const exportCsv = () => { if (startDate && endDate) { - exportMutation.mutate({ startDate, endDate }); + setError(null); + exportMutation.mutate( + { startDate, endDate }, + { + onError: (e: Error) => setError(e) + } + ); } else { setError(new Error("Please select a valid date range before exporting.")); } };apps/deploy-web/src/queries/usePaymentQueries.ts (1)
117-139: LGTM: export CSV mutation triggers a safe, revocable download.The mutation is correctly typed, calls the service, and the
onSuccesshandler creates, clicks, and revokes the object URL.Optional polish:
- Consider a
mutationKey(e.g., using a helper likeQueryKeys.getExportTransactionsCsvKey(...)if available) to improve DevTools visibility.- If desired UX-wise, add an
onErrorin callers (as suggested in the container) rather than here, to keep side-effects localized to the component that owns error UI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
apps/api/package.json(1 hunks)apps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx(6 hunks)apps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.tsx(5 hunks)apps/deploy-web/src/components/billing-usage/BillingView/BillingView.spec.tsx(3 hunks)apps/deploy-web/src/components/billing-usage/BillingView/BillingView.tsx(4 hunks)apps/deploy-web/src/queries/queryKeys.ts(1 hunks)apps/deploy-web/src/queries/usePaymentQueries.ts(2 hunks)packages/http-sdk/src/stripe/stripe.service.ts(1 hunks)packages/http-sdk/src/stripe/stripe.types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/package.json
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/deploy-web/src/queries/queryKeys.tspackages/http-sdk/src/stripe/stripe.service.tsapps/deploy-web/src/components/billing-usage/BillingView/BillingView.spec.tsxpackages/http-sdk/src/stripe/stripe.types.tsapps/deploy-web/src/components/billing-usage/BillingView/BillingView.tsxapps/deploy-web/src/queries/usePaymentQueries.tsapps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.tsxapps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/deploy-web/src/queries/queryKeys.tspackages/http-sdk/src/stripe/stripe.service.tsapps/deploy-web/src/components/billing-usage/BillingView/BillingView.spec.tsxpackages/http-sdk/src/stripe/stripe.types.tsapps/deploy-web/src/components/billing-usage/BillingView/BillingView.tsxapps/deploy-web/src/queries/usePaymentQueries.tsapps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.tsxapps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx
**/*.spec.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/deploy-web/src/components/billing-usage/BillingView/BillingView.spec.tsxapps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx
apps/{deploy-web,provider-console}/**/*.spec.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/query-by-in-tests.mdc)
Use
queryBymethods instead ofgetBymethods in test expectations in.spec.tsxfiles
Files:
apps/deploy-web/src/components/billing-usage/BillingView/BillingView.spec.tsxapps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx
🧬 Code Graph Analysis (2)
apps/deploy-web/src/queries/usePaymentQueries.ts (2)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(27-29)packages/http-sdk/src/stripe/stripe.types.ts (1)
ExportTransactionsCsvParams(101-104)
apps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx (1)
apps/deploy-web/src/queries/usePaymentQueries.ts (1)
useExportTransactionsCsvMutation(117-139)
🔇 Additional comments (13)
packages/http-sdk/src/stripe/stripe.types.ts (1)
101-104: Type addition is sensible and consistentExportTransactionsCsvParams with Date fields is a good fit for the client-side API and aligns with the rest of your date handling (ISO string conversion at call sites).
apps/deploy-web/src/queries/queryKeys.ts (1)
108-108: Cache key factory looks goodKey is stable (uses ISO strings) and specific to the date range. No issues.
apps/deploy-web/src/components/billing-usage/BillingView/BillingView.tsx (1)
51-52: Prop-driven export wiring LGTMShifting export into a parent-provided onExport keeps the view dumb and improves testability/composability. Button disablement when no data is also sensible.
Also applies to: 67-68, 174-177
apps/deploy-web/src/components/billing-usage/BillingView/BillingView.spec.tsx (1)
107-112: New onExport test is appropriateAsserts the contract of the prop at the view layer. After switching to queryBy*, the intent remains clear.
apps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx (5)
49-54: LGTM: pagination handler is correctly exercised.The test cleanly verifies that the pagination callback is wired and invoked with the new state.
56-61: LGTM: date range handler is correctly exercised.The test verifies wiring for date range changes; aligns with the container’s
onDateRangeChange.
110-112: LGTM: dependency injection for hooks keeps tests isolated.Passing both query and export mutation hooks via
dependenciesavoidsjest.mock()and aligns with the repo’s testing guidelines.
128-133: LGTM: event forwarding pattern is consistent.Forwarding to both the local spy and the child-provided prop maintains observability in tests while preserving intended behavior.
140-140: LGTM: setup returns the new onExport handle.Returning
onExportfromsetupmatches the added behavior under test and keeps tests cohesive.apps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.tsx (3)
6-12: LGTM: dependency wiring includes the new export mutation.Importing and exposing
useExportTransactionsCsvMutationinDEPENDENCIESkeeps the component testable and aligns with the DI pattern already in use.
52-67: LGTM: consistent date-range flow into queries.Renaming to
startDate/endDateand passing through tousePaymentTransactionsQuerymatches the new API and types in UsePaymentTransactionsOptions.
21-27: LGTM: onExport added to ChildrenProps and properly exposed.The new
onExportprop is included in the contract and wired toexportCsv, fitting the intended UX.Also applies to: 134-149
apps/deploy-web/src/queries/usePaymentQueries.ts (1)
1-8: LGTM: imports expanded for export params.Adding
ExportTransactionsCsvParamsaligns with the new hook signature and keeps types explicit.
apps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx
Outdated
Show resolved
Hide resolved
apps/api/src/billing/routes/stripe-transactions/stripe-transactions.router.ts
Outdated
Show resolved
Hide resolved
|
|
||
| const csvContent = [headers, ...rows].map(row => row.map(field => `"${String(field).replace(/"/g, '""')}"`).join(",")).join("\n"); | ||
| private transformTransactionForCsv(transaction: Transaction) { | ||
| const date = new Date(transaction.created * 1000).toISOString().split("T")[0]; |
There was a problem hiding this comment.
question(non-blocking): is it expected for the api caller that this date will be in UTC?
There was a problem hiding this comment.
I think this is an important note. The user could call this endpoint from any part of the globe by picking the date range on the picker (in local timezone). The endpoint receives both startDate and endDate with timezone data. I should use it to properly filter the transactions.
There was a problem hiding this comment.
I'm now extracting the timezone offset from UTC inside *createTransactionGenerator(), passing it to transformTransactionForCsv() here and re-applying to the date it when returning a transaction item. This should provide expected dates in the report.
Transactions are already properly filtered on the Stripe side since I'm passing timestamps of start and end dates that I'm getting from Date.getTime(), which is UTC-based, inside getCustomerTransactions()
There was a problem hiding this comment.
You could add timezone as a required parameter to controller. So, when frontend calls it, it will need to specify timezone. because all dates which are sent over the wire are in UTC. toISOString() converts date to ISO8601 in UTC timezone
There was a problem hiding this comment.
Required timezone on the query schema side with custom refinement + passed it down from the frontend 👍
apps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/api/src/billing/routes/stripe-transactions/stripe-transactions.router.ts (1)
103-122: Nice switch to Node.js stream helpers; tweak headers and charsetUsing
Readable.from+Readable.toWebis a solid simplification (addresses prior review). Minor header/encoding tweaks:
- Add
charset=utf-8to Content-Type (CSV often contains non-ASCII).- You can omit
Transfer-Encoding; the runtime will set it appropriately.Apply this diff:
const readableStream = Readable.toWeb(Readable.from(csvStream)) as ReadableStream; return new Response(readableStream, { headers: { - "Content-Type": "text/csv", + "Content-Type": "text/csv; charset=utf-8", "Content-Disposition": `attachment; filename="${filename}"`, - "Transfer-Encoding": "chunked" + // Let the runtime decide transfer-encoding for streaming } });apps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx (1)
9-9: Fix invalid typeof usage on type-only imports (will fail type-check).
typeofcannot reference symbols imported viaimport type. Use inline type queries withtypeof import("...").symbolfor your jest mocked function types.-import type { useExportTransactionsCsvQuery, usePaymentTransactionsQuery } from "@src/queries"; +import type { useExportTransactionsCsvQuery, usePaymentTransactionsQuery } from "@src/queries"; +// Type aliases using inline type queries to work with consistent-type-imports +type UsePaymentTransactionsQuery = typeof import("@src/queries").usePaymentTransactionsQuery; +type UseExportTransactionsCsvQuery = typeof import("@src/queries").useExportTransactionsCsvQuery; @@ - })) as unknown as jest.MockedFunction<typeof usePaymentTransactionsQuery>; + })) as unknown as jest.MockedFunction<UsePaymentTransactionsQuery>; @@ - })) as unknown as jest.MockedFunction<typeof useExportTransactionsCsvQuery>; + })) as unknown as jest.MockedFunction<UseExportTransactionsCsvQuery>;Also applies to: 94-99, 101-108
🧹 Nitpick comments (15)
apps/api/tsconfig.json (1)
9-9: Review TSlibconfiguration in apps/api/tsconfig.json• No bleeding-edge ESNext-only APIs detected (per search for
Object.groupBy,Array.toReversed, etc.).
• You do use Fetch API (fetch,Response,Headers) and Web-Streams (ReadableStreamviaReadable.toWeb), so narrowinglibwill drop those type definitions unless you explicitly add a DOM-or-Web-Streams lib.
• apps/api/package.json doesn’t declare its ownengines.node(falls back to root’s 22.14.0). Ensure your runtime supports native fetch and streams-to-web.Optional refactor—if you want to tighten what TS exposes, you can switch to:
- "lib": ["esnext"] + "lib": [ + "ES2022", + "ESNext.AsyncIterable", + "DOM" + ]This gives you modern ES2022 + async-iterable support plus web-API types for fetch/Response/ReadableStream.
If you genuinely need stage-3+ ESNext features, you may keepesnext—just verify your Node version or polyfills match the surfaced types.packages/ui/components/custom/date-range-picker.tsx (1)
19-20: Prop type change is breaking; consider a union with DateRange and keep state in sync with propSwitching
dateandonChangeto strictly{ from: Date; to: Date }is fine, but it’s a breaking change. If not all call-sites were updated, consider a transitional union, and also sync internal state whendatechanges.Apply this diff to loosen
datetype:interface DateRangePickerProps { className?: string; - date?: { from: Date; to: Date }; + date?: DateRange | { from: Date; to: Date } | null; onChange?: (date: { from: Date; to: Date }) => void; minDate?: Date | null;Additionally, ensure prop updates reflect in internal state (outside this hunk):
// Sync internal state when `date` prop changes React.useEffect(() => { if (date?.from && date?.to) { setSelectedRange({ from: date.from, to: date.to }); setCalendarMonth(date.from); } }, [date?.from, date?.to]);Please confirm all usages were updated across the codebase to the new shape.
packages/http-sdk/src/stripe/stripe.service.ts (1)
68-81: Add client-side date range validation to mirror getCustomerTransactions
getCustomerTransactionsvalidatesstartDate < endDate;exportTransactionsCsvshould too for consistency and early feedback.Apply this diff:
async exportTransactionsCsv(params: ExportTransactionsCsvParams): Promise<Blob> { + if (params.startDate >= params.endDate) { + throw new Error("startDate must be less than endDate"); + } const queryParams = new URLSearchParams({ startDate: params.startDate.toISOString(), endDate: params.endDate.toISOString() });apps/deploy-web/src/components/billing-usage/BillingView/BillingView.spec.tsx (3)
95-97: Avoid non-determinism from new Date() in testsUsing
new Date()in expectations/setup can cause flakiness across time zones and execution times. Prefer fixed dates in test data.Example:
- to: new Date() + to: new Date("2025-06-30T00:00:00.000Z")
143-143: Stabilize DateRangePicker stub: avoid falling back to new Date()Falling back to
new Date()in the stubbedonChangeintroduces flakiness whendate?.to/date?.fromare absent. Use the existing bound’s value (which defaultProps guarantees) rather than the current time.Apply this diff:
- onChange={e => onChange?.({ from: new Date(e.target.value), to: date?.to || new Date() })} + onChange={e => onChange?.({ from: new Date(e.target.value), to: date?.to as Date })}- onChange={e => onChange?.({ from: date?.from || new Date(), to: new Date(e.target.value) })} + onChange={e => onChange?.({ from: date?.from as Date, to: new Date(e.target.value) })}Also applies to: 151-151
21-24: Follow repo guideline: prefer queryBy over getBy in expectations**Per project testing guidelines for apps/deploy-web, use
queryBy*in expectations to avoid throwy selectors.Example changes (apply similarly across this file):
- expect(screen.getByText("Error fetching billing data")).toBeInTheDocument(); - expect(screen.getByText("fail!")).toBeInTheDocument(); + expect(screen.queryByText("Error fetching billing data")).toBeInTheDocument(); + expect(screen.queryByText("fail!")).toBeInTheDocument();Note: It’s fine to keep
getBy*for actions (finding elements to click/type), but swap toqueryBy*in assertions.apps/api/src/billing/routes/stripe-transactions/stripe-transactions.router.ts (2)
2-2: Prefernode:streamimport pathUsing Node’s scheme improves clarity and avoids tooling/bundler ambiguity.
Apply this diff:
-import { Readable } from "stream"; +import { Readable } from "node:stream";
55-76: OpenAPI response looks good; consider documenting validation errorThe 200 text/csv response is defined correctly. Optionally, add a 400/422 response documenting date-range validation errors for better API UX.
Example:
responses: { description: "CSV file with transaction data", content: { "text/csv": { schema: { type: "string", format: "binary" } } } }, + 400: { description: "Invalid date range" } }apps/deploy-web/src/queries/usePaymentQueries.ts (1)
117-127: Avoid caching large Blob responses; annotate generics for useQuerySince this is an on-demand export and returns a potentially large Blob, avoid caching it in React Query to save memory, and annotate the query’s types explicitly.
Apply this diff:
-export const useExportTransactionsCsvQuery = (options: ExportTransactionsCsvParams): UseQueryResult<Blob, Error> => { +export const useExportTransactionsCsvQuery = (options: ExportTransactionsCsvParams): UseQueryResult<Blob, Error> => { const { stripe } = useServices(); - return useQuery({ - enabled: false, + return useQuery<Blob, Error>({ + enabled: false, + // Avoid retaining blobs in cache; pick the key that matches your @tanstack/react-query version + // cacheTime (v4) | gcTime (v5) + cacheTime: 0 as unknown as number, queryKey: QueryKeys.getExportTransactionsCsvKey(options), queryFn: async (): Promise<Blob> => { return await stripe.exportTransactionsCsv(options); } }); };If you’re on React Query v5, replace
cacheTimewithgcTime. Otherwise keepcacheTime. This prevents holding Blob data after the component unmounts.Please confirm the installed
@tanstack/react-querymajor version so we can finalizecacheTimevsgcTime.apps/deploy-web/src/components/billing-usage/BillingView/BillingView.tsx (2)
174-174: Remove redundant dateRange checks in button disabled predicate.With the new strict type
dateRange: { from: Date; to: Date },fromandtoare always defined. The checks are dead code; simplify to rely on data presence.- <Button variant="secondary" onClick={onExport} className="h-12 gap-4" disabled={!data.length || !dateRange.from || !dateRange.to}> + <Button variant="secondary" onClick={onExport} className="h-12 gap-4" disabled={!data.length}>
50-57: Consider normalizing errorMessage type to a plain string for consistency.BillingContainer always passes a string (default ""), while BillingView allows
string | null. Unifying tostringavoids unnecessary nullability and simplifies rendering logic.- errorMessage: string | null; + errorMessage: string;apps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx (2)
32-39: Rename test to reflect current assertion (it checks a message, not an error object).- it("passes error object if queryError is axios error", async () => { + it("passes errorMessage when queryError is axios error", async () => {
101-107: Remove unused/incorrect property from mocked query return.
useExportTransactionsCsvQueryis a query (not a mutation) and doesn’t exposemutate. Keeping only relevant keys clarifies intent and aligns withUseQueryResult.- const mockedUseExportTransactionsCsvQuery = jest.fn(() => ({ - mutate: jest.fn(), - isLoading: false, - isError: false, - error: null, - refetch: jest.fn() - })) as unknown as jest.MockedFunction<typeof useExportTransactionsCsvQuery>; + const mockedUseExportTransactionsCsvQuery = jest.fn(() => ({ + isLoading: false, + isError: false, + error: null, + refetch: jest.fn() + })) as unknown as jest.MockedFunction<UseExportTransactionsCsvQuery>;apps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.tsx (2)
73-77: Clear stale errorMessage when queryError resolves.Currently, once set,
errorMessagemay persist even after a successful fetch. Reset it when the error goes away.React.useEffect(() => { if (axios.isAxiosError(queryError)) { setErrorMessage(queryError.response?.data.message || "An error occurred while fetching payment transactions."); + } else if (!queryError) { + setErrorMessage(""); } }, [queryError]);
134-140: Simplify exportCsv guard; startDate/endDate are guaranteed by type.
dateRangeis always{ from: Date; to: Date }, so the truthiness check is redundant.- const exportCsv = async () => { - if (startDate && endDate) { - await exportCsvQuery.refetch(); - } else { - setErrorMessage("Please select a valid date range before exporting."); - } - }; + const exportCsv = async () => { + await exportCsvQuery.refetch(); + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
apps/api/package.json(1 hunks)apps/api/src/billing/routes/stripe-transactions/stripe-transactions.router.ts(3 hunks)apps/api/src/billing/services/stripe/stripe.service.spec.ts(2 hunks)apps/api/src/billing/services/stripe/stripe.service.ts(3 hunks)apps/api/tsconfig.json(1 hunks)apps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx(7 hunks)apps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.tsx(6 hunks)apps/deploy-web/src/components/billing-usage/BillingView/BillingView.spec.tsx(6 hunks)apps/deploy-web/src/components/billing-usage/BillingView/BillingView.tsx(5 hunks)apps/deploy-web/src/queries/queryKeys.ts(1 hunks)apps/deploy-web/src/queries/usePaymentQueries.ts(2 hunks)packages/http-sdk/src/stripe/stripe.service.ts(2 hunks)packages/ui/components/custom/date-range-picker.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/api/package.json
- apps/api/src/billing/services/stripe/stripe.service.spec.ts
- apps/deploy-web/src/queries/queryKeys.ts
- apps/api/src/billing/services/stripe/stripe.service.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
packages/ui/components/custom/date-range-picker.tsxapps/deploy-web/src/components/billing-usage/BillingView/BillingView.tsxapps/deploy-web/src/queries/usePaymentQueries.tsapps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.tsxpackages/http-sdk/src/stripe/stripe.service.tsapps/api/src/billing/routes/stripe-transactions/stripe-transactions.router.tsapps/deploy-web/src/components/billing-usage/BillingView/BillingView.spec.tsxapps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
packages/ui/components/custom/date-range-picker.tsxapps/deploy-web/src/components/billing-usage/BillingView/BillingView.tsxapps/deploy-web/src/queries/usePaymentQueries.tsapps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.tsxpackages/http-sdk/src/stripe/stripe.service.tsapps/api/src/billing/routes/stripe-transactions/stripe-transactions.router.tsapps/deploy-web/src/components/billing-usage/BillingView/BillingView.spec.tsxapps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx
**/*.spec.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/deploy-web/src/components/billing-usage/BillingView/BillingView.spec.tsxapps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx
apps/{deploy-web,provider-console}/**/*.spec.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/query-by-in-tests.mdc)
Use
queryBymethods instead ofgetBymethods in test expectations in.spec.tsxfiles
Files:
apps/deploy-web/src/components/billing-usage/BillingView/BillingView.spec.tsxapps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx
🧠 Learnings (2)
📚 Learning: 2025-07-21T08:24:27.953Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-07-21T08:24:27.953Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Applied to files:
apps/deploy-web/src/queries/usePaymentQueries.tsapps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx
📚 Learning: 2025-07-21T08:24:24.269Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-07-21T08:24:24.269Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` to mock dependencies in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test.
Applied to files:
apps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx
🧬 Code Graph Analysis (5)
apps/deploy-web/src/queries/usePaymentQueries.ts (2)
packages/http-sdk/src/stripe/stripe.types.ts (1)
ExportTransactionsCsvParams(101-104)apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(27-29)
apps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.tsx (2)
apps/deploy-web/src/utils/dateUtils.ts (1)
createDateRange(46-58)apps/deploy-web/src/utils/domUtils.ts (1)
downloadCsv(60-75)
packages/http-sdk/src/stripe/stripe.service.ts (1)
packages/http-sdk/src/stripe/stripe.types.ts (1)
ExportTransactionsCsvParams(101-104)
apps/api/src/billing/routes/stripe-transactions/stripe-transactions.router.ts (1)
packages/http-sdk/src/stripe/stripe.service.ts (1)
exportTransactionsCsv(68-81)
apps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.spec.tsx (1)
apps/deploy-web/src/queries/usePaymentQueries.ts (1)
useExportTransactionsCsvQuery(117-127)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
🔇 Additional comments (6)
apps/api/tsconfig.json (1)
8-8: NodeNext module setting looks appropriate for ESM in Node.This aligns with modern Node ESM/TS interop and should work fine with service code and imports.
packages/ui/components/custom/date-range-picker.tsx (1)
187-197: Apply only when both ends are selected — good guardOnly emitting
onChangewhen bothfromandtoexist avoids partial ranges leaking upstream and matches the stricter prop typing.packages/http-sdk/src/stripe/stripe.service.ts (1)
68-81: extractData correctly returns raw Blob for blob responses
Theprotected extractDatamethod inHttpServiceis implemented as:protected extractData<T>(response: AxiosResponse<T>): T { return response.data; }At runtime, when you call
this.get(..., { responseType: "blob" }),response.datais aBlob, andextractDatareturns thatBlobdirectly. No changes are required.apps/deploy-web/src/components/billing-usage/BillingView/BillingView.spec.tsx (2)
108-113: LGTM: verifies export flow is wired through onExportThe test correctly ensures the "Export as CSV" control triggers the provided callback.
163-165: LGTM: props updated to new API (errorMessage, onExport)Defaults align with the component’s new public API and keep tests isolated.
apps/deploy-web/src/components/billing-usage/BillingContainer/BillingContainer.tsx (1)
55-58: Nice wiring of the export flow with dependency injection and automatic CSV download.
- Using a disabled query with
refetch()is a clean trigger for one-off exports.- The DI pattern (D.useExportTransactionsCsvQuery) keeps the component testable.
- Download side-effect is correctly isolated in a useEffect.
Also applies to: 79-81, 134-140, 148-149
apps/deploy-web/src/components/billing-usage/BillingView/BillingView.tsx
Outdated
Show resolved
Hide resolved
| async exportTransactionsCsvStream(options: { startDate: string; endDate: string; timezone: string }): Promise<AsyncIterable<string>> { | ||
| const { currentUser } = this.authService; | ||
|
|
||
| assert(currentUser.stripeCustomerId, 500, "Payment account not properly configured. Please contact support."); |
There was a problem hiding this comment.
| assert(currentUser.stripeCustomerId, 500, "Payment account not properly configured. Please contact support."); | |
| assert(currentUser.stripeCustomerId, 403, "Payments are not configured. Please start with a trial first"); |
There was a problem hiding this comment.
This assertion was taken from this.getCustomerTransactions above, which is now replaced with gracefully returning empty data there. I'm wondering if something similar should be done here or if assertion is better suited still?
if (!currentUser.stripeCustomerId) {
return 'No transactions found';
}
There was a problem hiding this comment.
I used this placeholder iterator:
if (!currentUser.stripeCustomerId) {
return (async function* () {
yield "Payments are not configured. Please start with a trial first";
})();
}
|
|
||
| if (!currentUser.stripeCustomerId) { | ||
| return (async function* () { | ||
| yield "Payments are not configured. Please start with a trial first"; |
There was a problem hiding this comment.
I'm not sure i follow. this should be an error. basically this call is forbidden 403 as user gotta do other stuff. we can also use 428 Precondition Required
There was a problem hiding this comment.
@ygrishajev I used this assertion that you proposed earlier:
assert(currentUser.stripeCustomerId, 403, "Payments are not configured. Please start with a trial first");
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/api/src/billing/controllers/stripe/stripe.controller.ts (1)
158-158: Status/message consistency for missing billing setup403 is acceptable here, but we’re inconsistent across endpoints (some return 500 with “contact support”, others return empty arrays). Either standardize on 403 for “action forbidden until billing is set up” or consider 428 Precondition Required as previously suggested. Also, the “Please start with a trial first” phrase may be misleading for users who actually need to set up billing, not a trial.
Option A (keep 403, clarify message):
- assert(currentUser.stripeCustomerId, 403, "Payments are not configured. Please start with a trial first"); + assert(currentUser.stripeCustomerId, 403, "Payments are not configured. Please set up billing to export transactions.");Option B (switch to 428):
- assert(currentUser.stripeCustomerId, 403, "Payments are not configured. Please start with a trial first"); + assert(currentUser.stripeCustomerId, 428, "Billing account is required to export transactions.");
🧹 Nitpick comments (2)
apps/api/src/billing/controllers/stripe/stripe.controller.ts (2)
155-161: Drop unnecessary async; return the AsyncIterable directlyThe method doesn’t await anything. Returning the AsyncIterable directly avoids an extra Promise wrapper and simplifies the signature.
Apply this diff:
- async exportTransactionsCsvStream(options: { startDate: string; endDate: string; timezone: string }): Promise<AsyncIterable<string>> { + exportTransactionsCsvStream(options: { startDate: string; endDate: string; timezone: string }): AsyncIterable<string> { const { currentUser } = this.authService; assert(currentUser.stripeCustomerId, 403, "Payments are not configured. Please start with a trial first"); return this.stripe.exportTransactionsCsvStream(currentUser.stripeCustomerId, options); }
154-154: Consider throttling heavy exportsCSV export can be IO-heavy. If abuse or load spikes are a concern, consider adding the Semaphore decorator as you did for other actions. If rate limiting is handled elsewhere (e.g., at the router/gateway), feel free to ignore.
- @Protected([{ action: "read", subject: "StripePayment" }]) + @Semaphore() + @Protected([{ action: "read", subject: "StripePayment" }])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/api/src/billing/controllers/stripe/stripe.controller.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/billing/controllers/stripe/stripe.controller.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/billing/controllers/stripe/stripe.controller.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: domhhv
PR: akash-network/console#1790
File: apps/deploy-web/src/queries/queryKeys.ts:107-120
Timestamp: 2025-08-25T15:19:15.429Z
Learning: In the Akash Console billing system, timezone is only required for the CSV export functionality (getExportTransactionsCsvKey), not for regular transaction list queries (getPaymentTransactionsKey). The CSV export needs server-side timezone handling for proper date formatting in the exported file, while the regular transaction list handles dates differently.
🧬 Code graph analysis (1)
apps/api/src/billing/controllers/stripe/stripe.controller.ts (1)
apps/api/src/auth/services/auth.service.ts (1)
Protected(39-55)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: validate (packages) / validate-unsafe
- GitHub Check: validate (apps/provider-proxy) / validate-unsafe
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (1)
apps/api/src/billing/controllers/stripe/stripe.controller.ts (1)
154-161: Overall LGTM on route guard + delegationGood use of the Protected decorator and clean delegation to StripeService for streaming. The controller stays thin, which is desirable here.
| async exportTransactionsCsvStream(options: { startDate: string; endDate: string; timezone: string }): Promise<AsyncIterable<string>> { | ||
| const { currentUser } = this.authService; | ||
|
|
||
| assert(currentUser.stripeCustomerId, 403, "Payments are not configured. Please start with a trial first"); | ||
|
|
||
| return this.stripe.exportTransactionsCsvStream(currentUser.stripeCustomerId, options); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Type the options via the request schema type and verify timezone validation
Per the retrieved learning, timezone is required only for the CSV export. If there’s a Zod schema/type (e.g., CustomerTransactionsCsvExportQuery or similar), prefer using that type here to avoid drift. Also verify the schema enforces a valid IANA timezone and that the service normalizes it.
Run this script to confirm the schema/type and headers:
If a concrete type exists, update the signature accordingly:
// Example if a type exists:
// exportTransactionsCsvStream(options: CustomerTransactionsCsvExportQuery): AsyncIterable<string>Note: I used the retrieved learning about timezone being required for CSV export to guide this verification.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Locate CSV export query schema/type (name guesses; show context)
rg -n --type=ts -C3 'CustomerTransactionsCsvExport(Query|Request)(Schema|Type)?|TransactionsCsvExport' || true
# 2) Check that timezone is required and validated (IANA or normalization)
rg -n --type=ts -C2 '(timezone).*z\.string|timezone.*refine|IANA|normalize(Time|T)?Zone' || true
# 3) Confirm service method and any timezone normalization usage
rg -n --type=ts -C3 'exportTransactionsCsvStream\(' || true
rg -n --type=ts -C2 'normalize(Time|T)?Zone' || true
# 4) Ensure router sets CSV headers and filename
rg -n --type=ts -C2 'text/csv|Content-Disposition|attachment; filename' apps || trueLength of output: 19755
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show normalizeTimeZone method definition
rg -n --type=ts 'private normalizeTimeZone' -C10 apps/api/src/billing/services/stripe/stripe.service.ts || true
# Show exportTransactionsCsvStream implementation context
sed -n '400,450p' apps/api/src/billing/services/stripe/stripe.service.ts || trueLength of output: 2770
Use the Zod-inferred request schema type in your controller
To keep your controller signature in sync with the HTTP schema (and avoid type drift), replace the inline { startDate: string; endDate: string; timezone: string } type with the inferred type from CustomerTransactionsCsvExportQuerySchema. Your timezone validation is already handled by the schema’s .refine(...), and the service’s normalizeTimeZone(...) will fallback to UTC when needed.
• File: apps/api/src/billing/controllers/stripe/stripe.controller.ts (around line 155)
Replace:
async exportTransactionsCsvStream(
options: { startDate: string; endDate: string; timezone: string }
): Promise<AsyncIterable<string>> { … }With something like:
import { CustomerTransactionsCsvExportQuerySchema } from "@src/billing/http-schemas/stripe.schema";
import type { infer as ZodInfer } from "zod";
async exportTransactionsCsvStream(
options: ZodInfer<typeof CustomerTransactionsCsvExportQuerySchema>
): Promise<AsyncIterable<string>> { … }• This change ensures any updates to the export query schema (including IANA-timezone validation) flow through to the controller automatically.
• Timezone is validated by the Zod schema:
timezone: z
.string()
.refine(tz => Intl.supportedValuesOf("timeZone").includes(tz), {
message: "Invalid IANA timezone",
})and normalized in the service:
private normalizeTimeZone(tz: string): string {
return Intl.supportedValuesOf("timeZone").includes(tz) ? tz : "UTC";
}🤖 Prompt for AI Agents
In apps/api/src/billing/controllers/stripe/stripe.controller.ts around lines
155-161, replace the inline options type { startDate: string; endDate: string;
timezone: string } with the Zod-inferred type from
CustomerTransactionsCsvExportQuerySchema so the controller signature stays in
sync with the HTTP schema; import the schema and use Zod's infer type (e.g.
import the schema from @src/billing/http-schemas/stripe.schema and use type
ZodInfer<typeof CustomerTransactionsCsvExportQuerySchema> for the options
parameter) so timezone validation comes from the schema and the service can
still normalize it as needed.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
5 participants
Closes #1788
Summary by CodeRabbit