refactor(billing): extract transaction reporting into its own service - #3499
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughTransaction reporting is extracted from ChangesTransaction reporting extraction
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Comment |
baktun14
approved these changes
Jul 21, 2026
Base automatically changed from
refactor/billing-stripe-client-composition
to
main
July 22, 2026 08:59
ygrishajev
force-pushed
the
refactor/billing-transaction-reporting-service
branch
from
July 22, 2026 09:05
2911f99 to
402d42a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/api/src/billing/services/stripe/stripe.service.ts (1)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a type-only import.
PaymentIntentResultis only used in type positions here, so switch toimport typeto keep the import list consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/billing/services/stripe/stripe.service.ts` at line 13, Change the PaymentIntentResult import in stripe.service.ts to a type-only import, preserving its existing module and usage.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/api/src/billing/services/transaction-reporting/transaction-reporting.service.ts`:
- Around line 73-78: Update the transaction-reporting return object so nextPage
is null unless charges.has_more is true, and coalesce the selected cursor IDs to
null when charges.data is empty. Preserve prevPage’s existing startingAfter
condition while ensuring both pagination fields always satisfy the
string-or-null contract.
- Around line 146-162: Update the catch block in the transaction-reporting
generator to stop exposing `(error as Error).message` in the streamed CSV. Keep
detailed error logging in `loggerService.error`, and emit a generic error
identifier through `sanitizeForCsv` (or the established sanitized-row path) so
internal details are hidden and CSV formula injection is prevented; preserve the
existing empty fields and `hasMore = false` behavior.
---
Nitpick comments:
In `@apps/api/src/billing/services/stripe/stripe.service.ts`:
- Line 13: Change the PaymentIntentResult import in stripe.service.ts to a
type-only import, preserving its existing module and usage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fc9bece9-55e7-4621-8d47-9b80fa3be916
📒 Files selected for processing (6)
apps/api/src/billing/controllers/stripe/stripe.controller.spec.tsapps/api/src/billing/controllers/stripe/stripe.controller.tsapps/api/src/billing/services/stripe/stripe.service.spec.tsapps/api/src/billing/services/stripe/stripe.service.tsapps/api/src/billing/services/transaction-reporting/transaction-reporting.service.spec.tsapps/api/src/billing/services/transaction-reporting/transaction-reporting.service.ts
ygrishajev
force-pushed
the
refactor/billing-transaction-reporting-service
branch
from
July 22, 2026 09:43
402d42a to
06f85e2
Compare
ygrishajev
enabled auto-merge
July 22, 2026 09:46
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/billing/controllers/stripe/stripe.controller.spec.ts`:
- Around line 405-406: Update setup() to include the existing
transactionReporting mock in its returned fixtures object, alongside the other
injected dependencies, so tests can configure
getCustomerTransactions/exportTransactionsCsvStream and verify calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ea2d7258-58e1-4fe0-8ae9-701baaf9ee6a
📒 Files selected for processing (6)
apps/api/src/billing/controllers/stripe/stripe.controller.spec.tsapps/api/src/billing/controllers/stripe/stripe.controller.tsapps/api/src/billing/services/stripe/stripe.service.spec.tsapps/api/src/billing/services/stripe/stripe.service.tsapps/api/src/billing/services/transaction-reporting/transaction-reporting.service.spec.tsapps/api/src/billing/services/transaction-reporting/transaction-reporting.service.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/api/src/billing/controllers/stripe/stripe.controller.ts
- apps/api/src/billing/services/stripe/stripe.service.spec.ts
- apps/api/src/billing/services/transaction-reporting/transaction-reporting.service.spec.ts
- apps/api/src/billing/services/stripe/stripe.service.ts
- apps/api/src/billing/services/transaction-reporting/transaction-reporting.service.ts
ygrishajev
disabled auto-merge
July 22, 2026 11:48
ygrishajev
enabled auto-merge
July 22, 2026 15:01
ygrishajev
force-pushed
the
refactor/billing-transaction-reporting-service
branch
from
July 22, 2026 15:15
06f85e2 to
6098af3
Compare
Move getCustomerTransactions and the CSV export stream out of StripeService into a dedicated TransactionReportingService, and delegate to it from the controller. Behavior-preserving: the reporting tests move alongside the new service and StripeService sheds its CSV/reporting dependencies.
ygrishajev
force-pushed
the
refactor/billing-transaction-reporting-service
branch
from
July 22, 2026 15:16
6098af3 to
3de4313
Compare
ygrishajev
disabled auto-merge
July 22, 2026 15:44
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3499 +/- ##
=======================================
Coverage 72.41% 72.42%
=======================================
Files 1044 1045 +1
Lines 27257 27264 +7
Branches 6956 6957 +1
=======================================
+ Hits 19739 19746 +7
Misses 6622 6622
Partials 896 896
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
StripeServiceis still a god object — charging cards, coupons, customers, payment methods, and transaction reporting all live in one class. Reporting (listing a customer's charges for the UI, streaming the CSV export) is a self-contained concern with its own dependencies —csv-stringify, a paging generator, timezone/CSV formatting — that has nothing to do with taking a payment.This PR carves reporting out into a dedicated
TransactionReportingService.StripeServicesheds ~200 lines and its CSV/stream imports; reporting gets a single-responsibility, independently testable home. It's a pure move — no behavior changes — and the next step in the layeredStripeServicesplit.Closes CON-721 · Part of CON-718. Stacked on #3498 (the
StripeServiceinjected-client refactor) — review and merge that first; this PR's diff should be read on top of it.What
apps/api— behavior-preserving, no new runtime behavior, no migrations.TransactionReportingService(injectedSTRIPE_CLIENT+StripeTransactionRepository+ logger). It ownsgetCustomerTransactionsandexportTransactionsCsvStream, plus their private helpers (paging generator, CSV-row transform, timezone normalization, CSV-injection sanitization) — all moved verbatim fromStripeService.StripeControllernow delegates the two reporting endpoints toTransactionReportingService; every other endpoint still goes throughStripeService.StripeServicedrops the six reporting methods and the now-deadcsv-stringify/stream/Transaction/TransactionCsvRowimports.stripe.servicespec into a newtransaction-reporting.servicespec, unchanged.Verification
tsc: no new errors in the changed files (pre-existing apps/api baseline unchanged).Summary by CodeRabbit