Repository navigation
fix(admin): devpass chart uses paid amount, nets refunds - #2225
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WalkthroughThe PR updates the DevPass timeseries admin endpoint to compute revenue using actual paid dollars instead of credits and adds a daily refunds aggregation. A new self-join query calculates per-day refund totals from credit_refund transactions, which are then subtracted from daily revenue totals. ChangesDevPass Timeseries Revenue and Refunds
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
🧹 Nitpick comments (1)
apps/api/src/routes/admin.ts (1)
8152-8315: 🏗️ Heavy liftAdd a focused regression test for legacy revenue plus refunds.
This query now depends on three subtle cases at once: current DevPass charges using
amount, legacy personal-org subscription charges, andcredit_refundrows linked back throughrelatedTransactionId. A route-level fixture covering those cases would make this much harder to regress.🤖 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/routes/admin.ts` around lines 8152 - 8315, Add a focused regression test that covers (1) a current DevPass charge using transaction.amount, (2) a legacy subscription charge on a personal org (LEGACY_DEV_PLAN_TX_TYPES with tables.organization.isPersonal = true), and (3) a credit_refund row that links back via relatedTransactionId so refunds are netted out; create route-level fixtures that insert one of each transaction on the same date (use DEV_PLAN_TX_TYPES, LEGACY_DEV_PLAN_TX_TYPES, and a transaction with type "credit_refund" whose relatedTransactionId points to one of the originals), call the admin revenue endpoint (the logic computing revenuePerDay/refundsPerDay and the date loop in admin.ts), and assert the returned per-day revenue equals SUM(amounts of completed dev/legacy rows) minus the credit_refund.amount, with cost unchanged—this ensures revenuePerDay, refundsPerDay, and the netting logic are covered and prevents regressions.
🤖 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.
Nitpick comments:
In `@apps/api/src/routes/admin.ts`:
- Around line 8152-8315: Add a focused regression test that covers (1) a current
DevPass charge using transaction.amount, (2) a legacy subscription charge on a
personal org (LEGACY_DEV_PLAN_TX_TYPES with tables.organization.isPersonal =
true), and (3) a credit_refund row that links back via relatedTransactionId so
refunds are netted out; create route-level fixtures that insert one of each
transaction on the same date (use DEV_PLAN_TX_TYPES, LEGACY_DEV_PLAN_TX_TYPES,
and a transaction with type "credit_refund" whose relatedTransactionId points to
one of the originals), call the admin revenue endpoint (the logic computing
revenuePerDay/refundsPerDay and the date loop in admin.ts), and assert the
returned per-day revenue equals SUM(amounts of completed dev/legacy rows) minus
the credit_refund.amount, with cost unchanged—this ensures revenuePerDay,
refundsPerDay, and the netting logic are covered and prevents regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4b766438-b706-41ab-aa5e-788666ce83e7
📒 Files selected for processing (1)
apps/api/src/routes/admin.ts
Summary
The DevPass admin "revenue & usage" chart was over-reporting revenue and silently dropping all pre-rename history. Stripe MRR shows ~$500 from Aug→Nov 2025, dipping to ~$0 through April, then jumping to $1,762 in May, while the admin chart's "All time" line was flat for months and totaled ~3× the actual revenue.
Two bugs in the timeseries handler at
apps/api/src/routes/admin.ts:SUM(transaction.creditAmount), butcreditAmountondev_plan_*rows is the credits limit (price ×DEV_PLAN_CREDITS_MULTIPLIER, default 3) — not dollars paid. Switched toSUM(transaction.amount), which is set tosession.amount_total / 100(orinvoice.amount_paid / 100) on every revenue-bearing insert.subscription_*rows had nocreditAmount. The pre-rename insert paths inapps/api/src/stripe.tsonly setamount, so even though the legacy filter matches them, they contributed $0 to the chart. Switching toamountfixes this too — that's why the chart was flat through Jan/Feb/Mar despite Stripe showing real subscribers.Also nets out refunds:
credit_refundrows are joined to their original transaction viarelatedTransactionId, scoped to dev plan / (legacy + personal-org) subscription rows, and subtracted from the matching day's revenue.Test plan
/devpassin admin — confirm the "All time" range now extends back to the earliest legacysubscription_start(personal org) and chart shows non-zero revenue in Aug–Nov 2025from/tofilters and verify the same numbers hold for narrower ranges🤖 Generated with Claude Code
Summary by CodeRabbit