Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 44 additions & 23 deletions static/gsApp/types/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -865,19 +865,39 @@ export type PreviewInvoiceItem = BaseInvoiceItem & {
period_start: string;
};

// TODO(data categories): BIL-970
export enum CreditType {
ERROR = 'error',
TRANSACTION = 'transaction',
SPAN = 'span',
PROFILE_DURATION = 'profile_duration',
PROFILE_DURATION_UI = 'profile_duration_ui',
ATTACHMENT = 'attachment',
REPLAY = 'replay',
DISCOUNT = 'discount',
PERCENT = 'percent',
LOG_BYTE = 'log_byte',
}
/**
* Dynamically generate credit types from DATA_CATEGORY_INFO.
* Uses SINGULAR form (unlike InvoiceItemType which uses plural).
* This automatically includes new billing categories without manual enum updates.
*
* Follows the pattern: snake_case of singular
* Example: DATA_CATEGORY_INFO.ERROR (singular: "error") -> "error"
* Example: DATA_CATEGORY_INFO.LOG_BYTE (singular: "logByte") -> "log_byte"
*/
type DynamicCreditType = {
[K in keyof typeof DATA_CATEGORY_INFO]: (typeof DATA_CATEGORY_INFO)[K]['isBilledCategory'] extends true
? CamelToSnake<(typeof DATA_CATEGORY_INFO)[K]['singular']>
: never;
}[keyof typeof DATA_CATEGORY_INFO];

/**
* Static credit types not tied to data categories.
* These must be manually maintained but change infrequently.
*/
type StaticCreditType =
| 'discount' // Dollar-based recurring discount
| 'percent' // Percentage-based recurring discount
| 'seer_user'; // Special: maps to PREVENT_USER category (temporary until category renamed)

/**
* Complete credit type union.
* Automatically stays in sync with backend when new billing categories are added.
*
* Migration from enum: Use string literals instead of enum members.
* Before: CreditType.ERROR
* After: 'error'
*/
export type CreditType = DynamicCreditType | StaticCreditType;

type BaseRecurringCredit = {
amount: number;
Expand All @@ -888,26 +908,27 @@ type BaseRecurringCredit = {

interface RecurringDiscount extends BaseRecurringCredit {
totalAmountRemaining: number;
type: CreditType.DISCOUNT;
type: 'discount';
}

interface RecurringPercentDiscount extends BaseRecurringCredit {
percentPoints: number;
totalAmountRemaining: number;
type: CreditType.PERCENT;
type: 'percent';
}

interface RecurringEventCredit extends BaseRecurringCredit {
totalAmountRemaining: null;
type:
| CreditType.ERROR
| CreditType.TRANSACTION
| CreditType.SPAN
| CreditType.PROFILE_DURATION
| CreditType.PROFILE_DURATION_UI
| CreditType.ATTACHMENT
| CreditType.REPLAY
| CreditType.LOG_BYTE;
| 'error'
| 'transaction'
| 'span'
| 'profile_duration'
| 'profile_duration_ui'
| 'attachment'
| 'replay'
| 'log_byte'
| 'seer_user';
Copy link
Contributor

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'.

Fix in Cursor Fix in Web

}

export type RecurringCredit =
Comment on lines +924 to 934
Copy link

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

Expand Down
22 changes: 11 additions & 11 deletions static/gsApp/views/subscriptionPage/recurringCredits.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {render, screen} from 'sentry-test/reactTestingLibrary';

import {DataCategory} from 'sentry/types/core';

import {CreditType, type Plan} from 'getsentry/types';
import type {Plan} from 'getsentry/types';
import RecurringCredits from 'getsentry/views/subscriptionPage/recurringCredits';

describe('Recurring Credits', () => {
Expand All @@ -34,7 +34,7 @@ describe('Recurring Credits', () => {
periodStart: moment().format(),
periodEnd: moment().utc().add(3, 'months').format(),
amount: 1500,
type: CreditType.DISCOUNT,
type: 'discount',
totalAmountRemaining: 7500,
}),
],
Expand All @@ -61,7 +61,7 @@ describe('Recurring Credits', () => {
periodStart: moment().format(),
periodEnd: moment().utc().add(3, 'months').format(),
amount: 100_000,
type: CreditType.TRANSACTION,
type: 'transaction',
totalAmountRemaining: null,
}),
],
Expand All @@ -88,7 +88,7 @@ describe('Recurring Credits', () => {
periodStart: moment().format(),
periodEnd: moment().utc().add(3, 'months').format(),
amount: 10,
type: CreditType.PROFILE_DURATION,
type: 'profile_duration',
totalAmountRemaining: null,
}),
],
Expand Down Expand Up @@ -125,7 +125,7 @@ describe('Recurring Credits', () => {
periodStart: moment().format(),
periodEnd: moment().utc().add(3, 'months').format(),
amount: 10,
type: CreditType.PROFILE_DURATION_UI,
type: 'profile_duration_ui',
totalAmountRemaining: null,
}),
],
Expand Down Expand Up @@ -162,7 +162,7 @@ describe('Recurring Credits', () => {
periodStart: moment().format(),
periodEnd: moment().utc().add(3, 'months').format(),
amount: 1.5,
type: CreditType.ATTACHMENT,
type: 'attachment',
totalAmountRemaining: null,
}),
],
Expand All @@ -189,7 +189,7 @@ describe('Recurring Credits', () => {
periodStart: moment().format(),
periodEnd: moment().utc().add(3, 'months').format(),
amount: 3_000_000,
type: CreditType.REPLAY,
type: 'replay',
totalAmountRemaining: null,
}),
],
Expand All @@ -216,7 +216,7 @@ describe('Recurring Credits', () => {
periodStart: moment().format(),
periodEnd: moment().utc().add(3, 'months').format(),
amount: 2.5,
type: CreditType.LOG_BYTE,
type: 'log_byte',
totalAmountRemaining: null,
}),
],
Expand All @@ -243,15 +243,15 @@ describe('Recurring Credits', () => {
periodStart: moment().format(),
periodEnd: '2021-12-01',
amount: 50000,
type: CreditType.ERROR,
type: 'error',
totalAmountRemaining: null,
}),
RecurringCreditFixture({
id: 1,
periodStart: moment().format(),
periodEnd: '2022-01-01',
amount: 100000,
type: CreditType.ERROR,
type: 'error',
totalAmountRemaining: null,
}),
],
Expand Down Expand Up @@ -287,7 +287,7 @@ describe('Recurring Credits', () => {
periodStart: moment().format(),
periodEnd: moment().utc().add(3, 'months').format(),
amount: 1500,
type: CreditType.DISCOUNT,
type: 'discount',
totalAmountRemaining: 7500,
}),
],
Expand Down
9 changes: 4 additions & 5 deletions static/gsApp/views/subscriptionPage/recurringCredits.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ import {space} from 'sentry/styles/space';
import type {DataCategory} from 'sentry/types/core';

import {useRecurringCredits} from 'getsentry/hooks/useRecurringCredits';
import type {Plan, RecurringCredit} from 'getsentry/types';
import {CreditType} from 'getsentry/types';
import type {CreditType, Plan, RecurringCredit} from 'getsentry/types';
import {formatReservedWithUnits} from 'getsentry/utils/billing';
import {getCreditDataCategory, getPlanCategoryName} from 'getsentry/utils/dataCategory';
import {displayPrice} from 'getsentry/views/amCheckout/utils';
Expand All @@ -25,7 +24,7 @@ const isExpired = (date: moment.MomentInput) => {
const getActiveDiscounts = (recurringCredits: RecurringCredit[]) =>
recurringCredits.filter(
credit =>
(credit.type === CreditType.DISCOUNT || credit.type === CreditType.PERCENT) &&
(credit.type === 'discount' || credit.type === 'percent') &&
credit.totalAmountRemaining > 0 &&
!isExpired(credit.periodEnd)
);
Expand Down Expand Up @@ -58,7 +57,7 @@ function RecurringCredits({displayType, planDetails}: Props) {
}

const getTooltipTitle = (credit: RecurringCredit) => {
return credit.type === CreditType.DISCOUNT || credit.type === CreditType.PERCENT
return credit.type === 'discount' || credit.type === 'percent'
Copy link
Contributor

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.

Fix in Cursor Fix in Web

? tct('[amount] per month or [annualAmount] remaining towards an annual plan.', {
amount: displayPrice({cents: credit.amount}),
annualAmount: displayPrice({
Expand All @@ -69,7 +68,7 @@ function RecurringCredits({displayType, planDetails}: Props) {
};

const getAmount = (credit: RecurringCredit, category: DataCategory | CreditType) => {
if (credit.type === CreditType.DISCOUNT || credit.type === CreditType.PERCENT) {
if (credit.type === 'discount' || credit.type === 'percent') {
return (
<Fragment>
{tct('[amount]/mo', {
Expand Down
3 changes: 1 addition & 2 deletions tests/js/getsentry-test/fixtures/recurringCredit.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
import moment from 'moment-timezone';

import type {RecurringCredit as TRecurringCredit} from 'getsentry/types';
import {CreditType} from 'getsentry/types';

export function RecurringCreditFixture(params?: TRecurringCredit): TRecurringCredit {
return {
id: 1,
periodStart: moment().format(),
periodEnd: moment().utc().add(3, 'months').format(),
amount: 50000,
type: CreditType.ERROR,
type: 'error',
totalAmountRemaining: null,
...params,
};
Expand Down
Loading