Repository navigation
Conversation
Usage corrections (charge delete/shrink, e.g. after subscription edits) were invisible in the credit transaction listing: they increase FBO, so the consumed loader skipped them, and the funded loader dropped them because they are not credit purchases. Balances jumped between rows. Advance cancellations written by corrections (FBO -x, advance +x) also leaked into the consumed loader as phantom consumption. - add `refunded` credit transaction type - list corrections as refunded rows, netting balance impact per ledger group, booked time and currency; zero sums are hidden - exclude correction-direction transactions from consumed - extract shared ledger candidate paging from the funded loader Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
| txType = customerbalance.CreditTransactionTypeVoided | ||
| case api.BillingCreditTransactionTypeRefunded: | ||
| txType = customerbalance.CreditTransactionTypeRefunded |
There was a problem hiding this comment.
Public filter lacks coverage The new
refunded filter is tested through CustomerBalanceSvc.ListCreditTransactions, which bypasses the HTTP request and this API-to-domain conversion. An API-level test of filter[type]=refunded and the returned transaction type would catch a public API regression that the current service tests cannot.
Prompt To Fix With AI
This is a comment left during a code review.
Path: api/v3/handlers/customers/credits/convert.go
Line: 535-537
Comment:
**Public filter lacks coverage** The new `refunded` filter is tested through `CustomerBalanceSvc.ListCreditTransactions`, which bypasses the HTTP request and this API-to-domain conversion. An API-level test of `filter[type]=refunded` and the returned transaction type would catch a public API regression that the current service tests cannot.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Problem
Editing a subscription recreates subscription items, which shrinks or deletes the old flat-fee charges and corrects their credit allocations. The credit transaction listing showed the original
consumedrows but not the corrections, so the history had prorated usage that seemingly didn't affect the balance.consumedloader (negative FBO movement) never saw them. Thefundedloader looks at positive movement too, but only emits credit purchases, so it dropped them.available_balancejumped between rows.FBO -x, advance+x). It has zero balance impact but negative FBO, so it showed up as a fakeconsumed -xrow with an inventedbefore.Change
refunded(TypeSpec + generated SDKs + v3 handler mapping).ledger.transaction.direction=correctionon FBO/receivable. Each row nets the customer balance impact (FBO + advance) per ledger group, booked time and currency, and hides zero sums. This is needed because one backfilled-advance correction writes offsetting postings (attribution unwind −8, purchased credit re-issue +8, collection reversal +8, advance cancellation 0).charge_id, subscription/feature references), so clients can relate refunds to the consumption they undo. Collapsing them in the UI is left for later.created_atis the correction time.consumednow excludesdirection=correctiontransactions, which removes the fake rows.voided.candidate_loader.go) that both loaders use. Funded behavior is unchanged.Tests
test/credits/sanity_credit_transactions_test.gochecks exact rows (type, amount, before/after) and a gap-free balance chain for:type=refundedfilter and page size 1Before the fix, the same scenarios listed no refund row and a fake
consumedrow wherever an advance was involved.Notes
typeshould handlerefunded.make gen-apifails locally in the legacy Python client step (missing venv, unrelated). I ran the AIP generation steps fromapi/spec/Makefilepluspnpm run build/format. The legacy spec is untouched.🤖 Generated with Claude Code
The PR appears safe to merge, with a non-blocking gap in public API test coverage.
Fix with agent prompt
Summary
The PR adds
refundedcredit transactions for corrected usage and fees, nets related ledger postings into balance-impact rows, and excludes corrections fromconsumedrows. It updates the API types, generated clients, documentation, and service-level scenarios for deletion, proration, backfill, filtering, and paging.Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Usage or fee consumed] --> B[Charge corrected] B --> C[Correction ledger group] C --> D[Net FBO and advance by booked time and currency] D --> E{Nonzero impact?} E -- Yes --> F[refunded transaction] E -- No --> G[No visible row] C --> H[Exclude correction postings from consumed]Reviews (1) · Last reviewed commit: "fix(credits): list usage corrections as ..."