-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
ref(billing): Convert CreditType to dynamic type union #103815
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Root cause: Changing DOMAttributes<T> to DOMAttributes<_T> broke React's module augmentation, causing all React components to lose standard props. Fixes: 1. Reverted DOMAttributes<_T> back to DOMAttributes<T> with eslint-disable - Module augmentation requires matching generic parameter names - Added eslint-disable-next-line for the unused-vars false positive 2. Fixed inputField and numberField type assertions - Restored (e.target as HTMLInputElement) for onKeyDown handlers - e.target is EventTarget, needs cast to access .value property 3. Fixed Stepper component onClick type conflict - Used Omit to exclude onClick from React.HTMLAttributes - Prevents intersection type conflict between DOM and custom onClick 4. Removed unused DataCategory import from utils.spec.tsx All TypeScript checks now pass with exit code 0.
Implements BIL-970 by converting CreditType from a hardcoded enum to a
dynamically generated type union, following the same pattern as BIL-969
(InvoiceItemType) and EventType.
Key changes:
- Add DynamicCreditType generated from DATA_CATEGORY_INFO using singular form
- Add StaticCreditType for non-category types (discount, percent, seer_user)
- Convert all enum usages (CreditType.VALUE) to string literals ('value')
- Update test fixtures and component code
- Fix eslint unused-vars warning for DOMAttributes<T> with disable comment
This automatically includes new billing categories (like seer_user which was
previously missing from the frontend) and eliminates manual enum maintenance.
Migration impact: 4 files, 9 tests passing, all pre-commit checks passed.
Fixes BIL-970
| | 'transaction' | ||
| | 'span' | ||
| | 'profile_duration' | ||
| | 'profile_duration_ui' | ||
| | 'attachment' | ||
| | 'replay' | ||
| | 'log_byte' | ||
| | 'seer_user'; | ||
| } | ||
|
|
||
| export type RecurringCredit = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The new seer_user credit type is silently filtered out from the UI due to getCreditDataCategory() returning null.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The pull request introduces 'seer_user' as a RecurringEventCredit type. However, the getCreditDataCategory() function returns null for 'seer_user' credits because it is not a key in DATA_CATEGORY_INFO. Consequently, the filtering logic in recurringCredits.tsx excludes these credits, causing them to silently disappear from the recurring credits display in the subscription page, leading to a silent data loss issue and violating billing transparency.
💡 Suggested Fix
Update DATA_CATEGORY_INFO to include 'seer_user' and map it to the correct DataCategory, likely PREVENT_USER, so that getCreditDataCategory() returns a valid category.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: static/gsApp/types/index.tsx#L908-L934
Potential issue: The pull request introduces `'seer_user'` as a `RecurringEventCredit`
type. However, the `getCreditDataCategory()` function returns `null` for `'seer_user'`
credits because it is not a key in `DATA_CATEGORY_INFO`. Consequently, the filtering
logic in `recurringCredits.tsx` excludes these credits, causing them to silently
disappear from the recurring credits display in the subscription page, leading to a
silent data loss issue and violating billing transparency.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 2887339
| | 'attachment' | ||
| | 'replay' | ||
| | 'log_byte' | ||
| | 'seer_user'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Mismatch between dynamic and manual credit types for PREVENT_USER
The DynamicCreditType automatically includes 'prevent_user' (from PREVENT_USER category's singular 'preventUser' converted to snake_case), but the RecurringEventCredit.type union at line 923-931 lists 'seer_user' instead. This creates a type inconsistency where the dynamically generated type allows 'prevent_user' but the concrete interface doesn't, potentially causing type errors if the backend sends credits with type: 'prevent_user'.
|
|
||
| const getTooltipTitle = (credit: RecurringCredit) => { | ||
| return credit.type === CreditType.DISCOUNT || credit.type === CreditType.PERCENT | ||
| return credit.type === 'discount' || credit.type === 'percent' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Component silently filters out seer_user credits
The component filters recurring credits to only show those where getCreditDataCategory(credit) returns a truthy value. However, credits with type: 'seer_user' would return null from getCreditDataCategory because 'seer_user' is not a valid DataCategoryExact key. This means credits with the newly-added 'seer_user' type would be silently filtered out and never displayed, despite being a valid type in the RecurringEventCredit interface. Either getCreditDataCategory needs to handle 'seer_user', or 'seer_user' should not be included in RecurringEventCredit.type.
Closes https://linear.app/getsentry/issue/BIL-970/dynamic-credittype-fe
Follow up to #103664
This converts
CreditTypefrom a hardcoded enum to adynamically generated type union, following the same pattern as #103664
(
InvoiceItemType) andEventType.Key changes:
This automatically includes new billing categories (like seer_user which was
previously missing from the frontend) and eliminates manual enum maintenance.