Repository navigation
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 15 minutes. View limit details
WalkthroughThis PR adds Changescredit_refund support across API, UI, and backfill script
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant main
participant reconcileLegacyRows
participant backfillMissingRefunds
participant Stripe
participant Database
main->>reconcileLegacyRows: run shared filters
reconcileLegacyRows->>Database: query legacy credit_refund rows
reconcileLegacyRows->>Stripe: list succeeded refunds
reconcileLegacyRows->>Database: reconcile and insert refund rows
main->>backfillMissingRefunds: run shared filters
backfillMissingRefunds->>Database: scan completed DevPass originals
backfillMissingRefunds->>Stripe: resolve payment intents and list refunds
backfillMissingRefunds->>Database: insert missing credit_refund rows
main-->>main: print phase summaries
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1
✨ Finishing Touches 💡 1
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.
Pull request overview
Surfaces DevPass refund transactions (credit_refund) as first-class line items in both the admin org detail view and the user DevPass billing history, and enhances the one-time backfill script to also insert missing refund rows by reconciling Stripe refund data against the DB.
Changes:
- Include
credit_refundin the API transaction history queries for both admin (/admin/devpass/{orgId}) and user (/dev-plans/invoices) views. - Update admin + user UI to render refunds with destructive styling and negative displayed amounts, and allow downloading a credit note PDF for refund line items.
- Extend the refund backfill script with a “Phase 2” that discovers Stripe refunds missing from the DB and inserts the corresponding
credit_refundtransactions.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
packages/scripts/src/backfill-stripe-refunds.ts |
Adds Phase 2 to discover/insert missing DevPass refund rows; adds org scoping and Stripe invoice→PI resolution logic. |
ee/admin/src/app/devpass/[orgId]/page.tsx |
Renders refund transactions with destructive badges and negative displayed amounts in admin DevPass history. |
apps/code/src/app/dashboard/components/DevPassInvoices.tsx |
Shows refunds in user billing history, displays negative amounts, and labels refund PDFs as credit notes. |
apps/api/src/routes/dev-plans.ts |
Includes credit_refund in /dev-plans/invoices response schema and query filtering. |
apps/api/src/routes/admin.ts |
Includes credit_refund in admin DevPass org transaction list filtering. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Map each unique payment intent to the earliest original it settled (the | ||
| // first invoice can back both a dev_plan_start and dev_plan_renewal — either | ||
| // is a valid refund anchor for revenue attribution, so pick deterministically). | ||
| const originalByPI = new Map<string, TransactionRow>(); | ||
| let skippedNoPI = 0; | ||
| for (const original of originalRows) { | ||
| let paymentIntentIds: string[] = []; | ||
| if (original.stripePaymentIntentId) { | ||
| paymentIntentIds = [original.stripePaymentIntentId]; | ||
| } else if (original.stripeInvoiceId) { | ||
| try { | ||
| paymentIntentIds = await resolveInvoicePaymentIntents( | ||
| stripe, | ||
| original.stripeInvoiceId, | ||
| ); | ||
| } catch (err) { | ||
| console.warn( | ||
| ` Failed to resolve PI for invoice ${original.stripeInvoiceId}: ${(err as Error).message}`, | ||
| ); | ||
| } | ||
| } |
| const missing: Stripe.Refund[] = []; | ||
| for (const r of refunds) { | ||
| const already = await db.query.transaction.findFirst({ | ||
| where: { | ||
| stripeRefundId: { eq: r.id }, | ||
| type: { eq: "credit_refund" }, | ||
| }, | ||
| }); | ||
| if (!already) { | ||
| missing.push(r); | ||
| } | ||
| } |
| "dev_plan_start", | ||
| "dev_plan_renewal", | ||
| "dev_plan_upgrade", | ||
| "credit_refund", | ||
| ]), |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 374049e4bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const already = await db.query.transaction.findFirst({ | ||
| where: { | ||
| stripeRefundId: { eq: r.id }, | ||
| type: { eq: "credit_refund" }, | ||
| }, |
There was a problem hiding this comment.
When a malformed credit_refund row has NULL stripe_refund_id and no stripe_payment_intent_id, phase 1 explicitly skips it and leaves it in place, but phase 2 still scans the related original transaction and this check only dedupes rows that already have the Stripe refund id. In --commit mode that inserts a second credit_refund for the same Stripe refund while the legacy row remains, so billing histories and refund totals double-count that refund; exclude originals with unreconciled legacy rows or treat those NULL-refund rows as existing before adding to missing.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/scripts/src/backfill-stripe-refunds.ts (1)
376-405: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winApply
scopePIearlier to avoid unnecessary Stripe calls when scoping to one PI.When
--payment-intent=pi_xis passed, this loop still callsresolveInvoicePaymentIntents(a Stripe API request) for every invoice-based DevPass original before thescopePIfilter at Line 397 discards them. For a direct-PI original you can skip non-matching rows before doing any work, and you only need invoice resolution for the scoped case. This meaningfully cuts Stripe API traffic (and rate-limit risk) when debugging a single PI.♻️ Proposed early scoping
for (const original of originalRows) { let paymentIntentIds: string[] = []; if (original.stripePaymentIntentId) { + if (scopePI && original.stripePaymentIntentId !== scopePI) { + continue; + } paymentIntentIds = [original.stripePaymentIntentId]; } else if (original.stripeInvoiceId) {🤖 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 `@packages/scripts/src/backfill-stripe-refunds.ts` around lines 376 - 405, Apply the scopePI filter earlier in the originalRows loop in backfill-stripe-refunds.ts so non-matching Payment Intents are skipped before any Stripe work. In the loop that builds originalByPI, check the direct stripePaymentIntentId case against scopePI before assigning or resolving anything, and avoid calling resolveInvoicePaymentIntents for invoice-backed rows unless the scoped PI could actually come from that row. Keep the existing originalByPI population logic in place, but move the scopePI guard ahead of unnecessary API calls to reduce Stripe traffic.
🤖 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 `@packages/scripts/src/backfill-stripe-refunds.ts`:
- Around line 376-405: Apply the scopePI filter earlier in the originalRows loop
in backfill-stripe-refunds.ts so non-matching Payment Intents are skipped before
any Stripe work. In the loop that builds originalByPI, check the direct
stripePaymentIntentId case against scopePI before assigning or resolving
anything, and avoid calling resolveInvoicePaymentIntents for invoice-backed rows
unless the scoped PI could actually come from that row. Keep the existing
originalByPI population logic in place, but move the scopePI guard ahead of
unnecessary API calls to reduce Stripe traffic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 806bb213-3d87-4f5e-9ec2-944798630094
📒 Files selected for processing (5)
apps/api/src/routes/admin.tsapps/api/src/routes/dev-plans.tsapps/code/src/app/dashboard/components/DevPassInvoices.tsxee/admin/src/app/devpass/[orgId]/page.tsxpackages/scripts/src/backfill-stripe-refunds.ts
374049e to
70f55d3
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
70f55d3 to
4588a71
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
4588a71 to
fe79671
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Refunds against DevPass payments are recorded as `credit_refund`
transactions and already net out of the reported revenue/margin, but
both billing-history surfaces filtered them out of the transaction list:
- Admin per-org detail (`GET /admin/devpass/{orgId}`)
- User DevPass billing (`GET /dev-plans/invoices`)
The result was a subscriber showing $0 net revenue with no visible
refund line. Add `credit_refund` to both queries and render it as a
negative "Refund" line item (the API already supports a downloadable
credit note for it).
Also enhance the backfill script so it can insert DevPass refunds that
exist in Stripe but have no `credit_refund` row at all (webhook
missed/undelivered), in addition to reconciling legacy rows with a
NULL `stripe_refund_id`. Adds an `--org=` scope flag. Both passes are
idempotent on `stripe_refund_id`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fe79671 to
2c6fe40
Compare
smakosh
left a comment
smakosh
left a comment
There was a problem hiding this comment.
Rebased onto upstream/main (7d17e5f). I resolved conflicts in apps/api/src/routes/admin.ts, apps/api/src/routes/dev-plans.ts, apps/code/.../DevPassInvoices.tsx and ee/admin/src/app/devpass/[orgId]/page.tsx by keeping main's version. Main already lists credit_refund in both histories (DEV_PLAN_HISTORY_TYPES, admin type list), shows refunds as negative amounts and labels refund downloads "Credit note". After the rebase, this PR only changes packages/scripts/src/backfill-stripe-refunds.ts, so the title and body are now stale.
Findings
packages/scripts/src/backfill-stripe-refunds.ts:355— P1 (Codex): Phase 2 deduped only bystripe_refund_id. Phase 1 deliberately leaves some legacycredit_refundrows in place: rows withNULL stripe_refund_idand no PI, and rows where the Stripe lookup failed. For those,--commitinserted a second row for the same refund and double-counted it. In a dry run, every legacy PI was also reported twice (once by Phase 1, once by Phase 2). Fixed in 2c6fe40: Phase 2 now skips any PI or original that still has an unresolved legacy row, and logs which ones it skipped.packages/scripts/src/backfill-stripe-refunds.ts:479— N+1findFirstper refund (Copilot). Fixed in 2c6fe40: oneinArraylookup per PI. It no longer filters ontype, becausetransaction_stripe_refund_id_uniquecovers every row type, so a match of any type would block the insert.packages/scripts/src/backfill-stripe-refunds.ts:290—s + r.amount / 100failed lint (no-mixed-operators), and adding parentheses conflicts with prettier. Fixed in 2c6fe40: sums integer cents, then divides once.packages/scripts/src/backfill-stripe-refunds.ts:402— Copilot asked to cache invoice→PI lookups. Not needed:transaction_stripe_invoice_id_unique(packages/db/src/schema.ts:728) means each invoice belongs to one row. The comment here about one invoice backing both start and renewal is stale. Non-blocking.packages/scripts/src/backfill-stripe-refunds.ts:36,50—dev_plan_reset_passis refundable inhandleChargeRefunded(apps/api/src/stripe.ts:3641) but is missing from both type lists, so missed Reset Pass refunds are not backfilled. Adding it would record the refund without the webhook's pass clawback. Needs author decision, non-blocking.apps/api/src/routes/dev-plans.tsroute comment (Copilot) — moot, since main's code replaces it.
Verification
eslint packages/scripts/src/backfill-stripe-refunds.ts: pass.prettier --check: pass.tsc --noEmit -p packages/scripts(after buildingdb,shared,modelsandactions): 0 errors in this file. The package still has errors in other files that also exist on main (api-ping.ts,export-models-dev.ts,poc-end-user-bonus.ts).- No vitest spec covers this one-off script, and I did not run it against Stripe or a database.
Problem
A DevPass subscriber was refunded in prod, but the refund was invisible in the billing history — the admin org-detail page and the user's DevPass billing page both showed only the original charge, while the subscriber's all-time revenue read $0.00 (
$29 − $29 refund).Root cause: the refund is recorded as a
credit_refundtransaction and already nets out of the reported revenue/margin, but both transaction-list queries filteredcredit_refundout:GET /admin/devpass/{orgId}(apps/api/src/routes/admin.ts)GET /dev-plans/invoices(apps/api/src/routes/dev-plans.ts)So refunds affected the aggregates but never appeared as a line item. (The API already had credit-note PDF support for refunds —
isInvoiceableTransaction/isRefundTransaction— confirming refunds were always meant to be listed.)Changes
Show refunds in both histories
credit_refundto both transaction-list queries.ee/admin): refund renders with a destructive badge and a negative amount.apps/code): refund renders as a Refund line with a negative amount and a downloadable credit note.Backfill missing refunds (
packages/scripts/src/backfill-stripe-refunds.ts)credit_refundrow at all (webhook missed/undelivered) and inserts them — resolving the invoice → payment-intent → succeeded-refunds chain.NULL stripe_refund_id) is preserved.--org=<id>scope flag. Both phases are idempotent onstripe_refund_id.Screenshots
DevPass billing page (
apps/code) — the refund now appears as a Refund line with a negative amount (-$15.00) and a downloadable credit note, alongside the original renewal charge. Light and dark:Verification (local, against the running stack)
credit_refundrow; re-run is a no-op (idempotent).GET /admin/devpass/{orgId}andGET /dev-plans/invoicesnow return thecredit_refundrow.application/pdf).pnpm format+ fullpnpm build(17/17) pass.Follow-up (not in this PR)
Run the backfill in prod (dry-run →
--commit), e.g. scoped first:--org=2t6ZtQyGpTEohmrJXEgX, with the liveSTRIPE_SECRET_KEYand prodDATABASE_URL.🤖 Generated with Claude Code
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes