Skip to content

feat(billing): force global-views flag into PlanFeatures #97148

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Aug 12, 2025
Merged

feat(billing): force global-views flag into PlanFeatures #97148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d1faec8
hack global views flag into planfeatures
cvxluo Aug 5, 2025
f1ad87f
add combo test
cvxluo Aug 5, 2025
f22b9ad
fix
cvxluo Aug 5, 2025
40aabec
Merge branch 'master' into cvxluo/hack-global-views-flag
cvxluo Aug 8, 2025
da6bd2d
fix plans with access
cvxluo Aug 8, 2025
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
40 changes: 40 additions & 0 deletions static/gsApp/components/features/planFeature.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,4 +170,44 @@ describe('PlanFeature', function () {
});
});
});

it('returns global-views in business plan even if response does not include it', async function () {
const mockFn = jest.fn(() => null);

const sub = SubscriptionFixture({organization, planTier: PlanTier.MM2});
SubscriptionStore.set(organization.slug, sub);

render(
<PlanFeature organization={organization} features={['global-views']}>
{mockFn}
</PlanFeature>
);

await waitFor(() => {
expect(mockFn).toHaveBeenCalledWith({
plan: PlanDetailsLookupFixture('am2_business'),
tierChange: 'am2',
});
});
});

it('returns business plan with other features including global-views', async function () {
const mockFn = jest.fn(() => null);

const sub = SubscriptionFixture({organization, planTier: PlanTier.MM2});
SubscriptionStore.set(organization.slug, sub);

render(
<PlanFeature organization={organization} features={['global-views', 'sso-basic']}>
{mockFn}
</PlanFeature>
);

await waitFor(() => {
expect(mockFn).toHaveBeenCalledWith({
plan: PlanDetailsLookupFixture('am2_business'),
tierChange: 'am2',
});
});
});
});
18 changes: 17 additions & 1 deletion static/gsApp/components/features/planFeature.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ import {descopeFeatureName} from 'sentry/utils';
import withSubscription from 'getsentry/components/withSubscription';
import {useBillingConfig} from 'getsentry/hooks/useBillingConfig';
import type {Plan, Subscription} from 'getsentry/types';
import {isBizPlanFamily, isDeveloperPlan} from 'getsentry/utils/billing';
import {
isAmEnterprisePlan,
isBizPlanFamily,
isDeveloperPlan,
} from 'getsentry/utils/billing';

type RenderProps = {
/**
Expand Down Expand Up @@ -105,6 +109,18 @@ function PlanFeature({subscription, features, organization, children}: Props) {
plans = billingConfig.planList;
}

// HACK: we want to remove `global-views` from getsentry and move it to flagpole,
// but since PlanFeature hooks into getsentry to determine which plan
// `global-views` is in, we need to hardcode it into the plans here
// TODO: remove this
for (const plan of plans) {
if (isBizPlanFamily(plan) || isAmEnterprisePlan(plan.id)) {
if (!plan.features.includes('global-views')) {
plan.features.push('global-views');
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Shared State Mutation Causes Feature Duplication

The code directly mutates the plan.features array by pushing 'global-views'. Since these plan objects from billingConfig.planList are shared state, this causes side effects. Other components relying on the original plan data may see an incorrect features list or accumulate duplicate 'global-views' entries across renders.

Fix in Cursor Fix in Web


// Locate the first plan that offers these features
const requiredPlan = plans.find(plan =>
features.map(descopeFeatureName).every(f => plan.features.includes(f))
Expand Down
1 change: 0 additions & 1 deletion tests/js/getsentry-test/fixtures/am1Plans.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ const AM1_BUSINESS_FEATURES = [
'data-forwarding',
'discard-groups',
'discover-query',
'global-views',
'integrations-codeowners',
'integrations-enterprise-alert-rule',
'integrations-enterprise-incident-management',
Expand Down
1 change: 0 additions & 1 deletion tests/js/getsentry-test/fixtures/am2Plans.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ const AM2_BUSINESS_FEATURES = [
'data-forwarding',
'discard-groups',
'discover-query',
'global-views',
'integrations-codeowners',
'integrations-enterprise-alert-rule',
'integrations-enterprise-incident-management',
Expand Down
1 change: 0 additions & 1 deletion tests/js/getsentry-test/fixtures/am3Plans.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ const AM3_BUSINESS_FEATURES = [
'data-forwarding',
'discard-groups',
'discover-query',
'global-views',
'indexed-spans-extraction',
'insights-initial-modules',
'insights-addon-modules',
Expand Down
4 changes: 0 additions & 4 deletions tests/js/getsentry-test/fixtures/featureList.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,6 @@ export function FeatureListFixture(): Record<string, Feature> {
name: 'Discover Query Builder',
description: 'Build and save custom queries using Discover.',
},
'global-views': {
name: 'Cross project visibility',
description: 'View data across all projects in your organization.',
},
invoices: {
name: 'Invoicing',
description: 'Standard invoicing for your accounting department.',
Expand Down
7 changes: 0 additions & 7 deletions tests/js/getsentry-test/fixtures/mm2Plans.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ const MM2_PLANS: Record<string, Plan> = {
'custom-inbound-filters',
'data-forwarding',
'discover',
'global-views',
'rate-limits',
'sso-saml2',
'integrations-event-hooks',
Expand Down Expand Up @@ -113,7 +112,6 @@ const MM2_PLANS: Record<string, Plan> = {
'custom-inbound-filters',
'data-forwarding',
'discover',
'global-views',
'rate-limits',
'sso-saml2',
'integrations-event-hooks',
Expand Down Expand Up @@ -169,7 +167,6 @@ const MM2_PLANS: Record<string, Plan> = {
'custom-inbound-filters',
'data-forwarding',
'discover',
'global-views',
'rate-limits',
'sso-saml2',
'integrations-event-hooks',
Expand Down Expand Up @@ -225,7 +222,6 @@ const MM2_PLANS: Record<string, Plan> = {
'custom-inbound-filters',
'data-forwarding',
'discover',
'global-views',
'rate-limits',
'sso-saml2',
'integrations-event-hooks',
Expand Down Expand Up @@ -281,7 +277,6 @@ const MM2_PLANS: Record<string, Plan> = {
'custom-inbound-filters',
'data-forwarding',
'discover',
'global-views',
'rate-limits',
'sso-saml2',
'integrations-event-hooks',
Expand Down Expand Up @@ -337,7 +332,6 @@ const MM2_PLANS: Record<string, Plan> = {
'custom-inbound-filters',
'data-forwarding',
'discover',
'global-views',
'rate-limits',
'sso-saml2',
'integrations-event-hooks',
Expand Down Expand Up @@ -685,7 +679,6 @@ const MM2_PLANS: Record<string, Plan> = {
'discard-groups',
'dashboards-basic',
'discover-query',
'global-views',
'integrations-codeowners',
'integrations-event-hooks',
'integrations-stacktrace-link',
Expand Down
Loading