fix(billing): Render unlimited reserved as ∞ in usage history#113259
Open
dashed wants to merge 2 commits into
Open
fix(billing): Render unlimited reserved as ∞ in usage history#113259dashed wants to merge 2 commits into
dashed wants to merge 2 commits into
Conversation
`usagePercentage` in `static/gsApp/views/subscriptionPage/usageHistory.tsx` returned `>100%` for historical billing-period records where `prepaid` is `UNLIMITED_RESERVED` (-1), because `usage > -1` is always true for non-negative usage. The call site at the "Used (%)" cell special-cased `RESERVED_BUDGET_QUOTA` (-2) but not `UNLIMITED_RESERVED`, so unlimited categories rendered as ">100%" — the opposite of the truth — on every past-period row in the usage history table. Apply the fix at both layers: - Call site: add an `isUnlimitedReserved(metricHistory.reserved) → UNLIMITED` branch parallel to the existing `RESERVED_BUDGET_QUOTA → 'N/A'` guard. - Helper (`usagePercentage`): early-return `UNLIMITED` when `isUnlimitedReserved(prepaid)`, as defense in depth. Export the helper so the unit-test layer can pin this contract. Affected cohort: enterprise orgs promoted to unlimited via `correct_enterprise_reserved()`, and any billing-history row from a past `am3_t` / `am3_t_ent` trial period. Same cohort as #113142 and #113254. Add unit tests for the now-exported `usagePercentage` (7 cases: null, 0, unlimited, overage, under-quota, zero usage, exact match) and component regression tests for both the fully-unlimited row and the defense-in-depth `reserved > 0, prepaid = -1` case. Refs #113142 Refs #113254 Co-Authored-By: Claude <noreply@anthropic.com>
Address code-review follow-ups on #113259: - Drop the dead top-level `reserved`/`usage` fields from the new BillingHistoryFixture calls. `UsageHistoryRow` only reads `history.categories`; these were copy-pasted from the adjacent overage test and unused. - Scope the cell assertions to the Errors row via `within()`, and use `getByRole` where exactly-one ∞ is expected (implicitly pinning the count), so the tests are self-describing. - Drop the inline WHAT comments now that the assertions encode the intent. Co-Authored-By: Claude <noreply@anthropic.com>
| metricHistory.prepaid | ||
| )} | ||
| : isUnlimitedReserved(metricHistory.reserved) | ||
| ? UNLIMITED |
Member
There was a problem hiding this comment.
It makes more sense to make this 'N/A' imo. Infinite percent used could be misleading
Contributor
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
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
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.
Billing admins reviewing past periods in the usage history table see
>100%in the "Used (%)" column for any category that had unlimited reserved quota — exactly the row in the UI designed to surface overages.The bug
usageHistory.tsxrenders the per-category "Used (%)" cell with a nested ternary that special-casesRESERVED_BUDGET_QUOTA(-2) but notUNLIMITED_RESERVED(-1):Inside
usagePercentage:For
prepaid = UNLIMITED_RESERVED = -1,usage > -1is always true for any non-negative usage, so the function returns'>100%'. Opposite of the truth.The fix
Both layers, defense in depth:
1. Helper (
usagePercentage) — early-return theUNLIMITED(∞) constant for unlimited prepaid, and export the helper so the unit-test layer can pin the contract:2. Call site — add a parallel
isUnlimitedReserved(reserved) → UNLIMITEDbranch next to the existingRESERVED_BUDGET_QUOTA → 'N/A'guard:<td> {metricHistory.reserved === RESERVED_BUDGET_QUOTA ? 'N/A' + : isUnlimitedReserved(metricHistory.reserved) + ? UNLIMITED : usagePercentage(...)} </td>Why both layers: the call-site guard reads the canonical
metricHistory.reservedfield and mirrors the neighboringRESERVED_BUDGET_QUOTApattern. The helper guard protects against any caller — now or future — passingprepaid = -1, and enables pure unit tests of the helper contract.Who is affected
correct_enterprise_reserved()(getsentry/billing/plans/__init__.py:103) — the same cohort as fix(billing): Account for gifted quantities in productIsEnabled check #113142 and fix(billing): Treat UNLIMITED_RESERVED as active quota in profiling #113254.am3_t/am3_t_enttrial periods.create_metric_histories_from_updates(getsentry/models/billingmetrichistory.py:189) preserves theprepaid = UNLIMITED_RESERVEDstate on monthly rotation, so these rows persist in the usage history table indefinitely.Tests
describe('usagePercentage', ...)— 7 new pure-function unit tests of the exported helper (null, 0, unlimited, overage, under-quota, zero usage, exact match).reserved = prepaid = -1) and the defense-in-depth divergent case (reserved > 0, prepaid = -1). The existing "overage is shown as >100%" test remains green, confirming non-unlimited rows still surface overages correctly.Relationship to #113142 / #113254
Third and final PR in a set addressing the
UNLIMITED_RESERVED = -1sentinel:productIsEnabled+ usage-overview URL selectorcheckBudgetUsageFor+ profiling billing bannerusagePercentage+ usage history tableAll three now follow the same "treat
-1as active/unlimited, not as a small negative number" convention.Refs #113142
Refs #113254