Skip to content

Commit a904373

Browse files
authored
feat(billing): force global-views flag into PlanFeatures (#97148)
`PlanFeatures` takes a list of features and determines what plan has every one of those features. This is used in `powerFeatureHovercard`, which for a given feature, it shows a hovercard that says "Requires x plan", where x is the plan name from `PlanFeatures`. We wanted to move `global-views` to flagpole controlled, rather than getsentry (see getsentry/getsentry#17944), but since `PlanFeatures` reads from getsentry to determine what plan name to show, we ran into an issue where the plan name showed as 'unavailable' instead of 'business' (see #96947) To move `global-views` to flagpole controlled without having the hovercard break, we need `PlanFeatures` to know that `global-views` is a business feature. This _temporarily_ hard codes the feature into every business plan that `PlanFeatures` retrieves, so that we can safely remove the flag from getsentry. The motivation to have the flag in flagpole is so [this product change](https://sentry.slack.com/archives/C01N4L6SPT2/p1754414626890039) can be rolled out via the automator.
1 parent 1d46292 commit a904373

File tree

7 files changed

+57
-15
lines changed

7 files changed

+57
-15
lines changed

static/gsApp/components/features/planFeature.spec.tsx

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,4 +170,44 @@ describe('PlanFeature', function () {
170170
});
171171
});
172172
});
173+
174+
it('returns global-views in business plan even if response does not include it', async function () {
175+
const mockFn = jest.fn(() => null);
176+
177+
const sub = SubscriptionFixture({organization, planTier: PlanTier.MM2});
178+
SubscriptionStore.set(organization.slug, sub);
179+
180+
render(
181+
<PlanFeature organization={organization} features={['global-views']}>
182+
{mockFn}
183+
</PlanFeature>
184+
);
185+
186+
await waitFor(() => {
187+
expect(mockFn).toHaveBeenCalledWith({
188+
plan: PlanDetailsLookupFixture('am2_business'),
189+
tierChange: 'am2',
190+
});
191+
});
192+
});
193+
194+
it('returns business plan with other features including global-views', async function () {
195+
const mockFn = jest.fn(() => null);
196+
197+
const sub = SubscriptionFixture({organization, planTier: PlanTier.MM2});
198+
SubscriptionStore.set(organization.slug, sub);
199+
200+
render(
201+
<PlanFeature organization={organization} features={['global-views', 'sso-basic']}>
202+
{mockFn}
203+
</PlanFeature>
204+
);
205+
206+
await waitFor(() => {
207+
expect(mockFn).toHaveBeenCalledWith({
208+
plan: PlanDetailsLookupFixture('am2_business'),
209+
tierChange: 'am2',
210+
});
211+
});
212+
});
173213
});

static/gsApp/components/features/planFeature.tsx

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,11 @@ import {descopeFeatureName} from 'sentry/utils';
66
import withSubscription from 'getsentry/components/withSubscription';
77
import {useBillingConfig} from 'getsentry/hooks/useBillingConfig';
88
import type {Plan, Subscription} from 'getsentry/types';
9-
import {isBizPlanFamily, isDeveloperPlan} from 'getsentry/utils/billing';
9+
import {
10+
isAmEnterprisePlan,
11+
isBizPlanFamily,
12+
isDeveloperPlan,
13+
} from 'getsentry/utils/billing';
1014

1115
type RenderProps = {
1216
/**
@@ -105,6 +109,18 @@ function PlanFeature({subscription, features, organization, children}: Props) {
105109
plans = billingConfig.planList;
106110
}
107111

112+
// HACK: we want to remove `global-views` from getsentry and move it to flagpole,
113+
// but since PlanFeature hooks into getsentry to determine which plan
114+
// `global-views` is in, we need to hardcode it into the plans here
115+
// TODO: remove this
116+
for (const plan of plans) {
117+
if (isBizPlanFamily(plan) || isAmEnterprisePlan(plan.id)) {
118+
if (!plan.features.includes('global-views')) {
119+
plan.features.push('global-views');
120+
}
121+
}
122+
}
123+
108124
// Locate the first plan that offers these features
109125
const requiredPlan = plans.find(plan =>
110126
features.map(descopeFeatureName).every(f => plan.features.includes(f))

tests/js/getsentry-test/fixtures/am1Plans.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ const AM1_BUSINESS_FEATURES = [
8383
'data-forwarding',
8484
'discard-groups',
8585
'discover-query',
86-
'global-views',
8786
'integrations-codeowners',
8887
'integrations-enterprise-alert-rule',
8988
'integrations-enterprise-incident-management',

tests/js/getsentry-test/fixtures/am2Plans.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,6 @@ const AM2_BUSINESS_FEATURES = [
9494
'data-forwarding',
9595
'discard-groups',
9696
'discover-query',
97-
'global-views',
9897
'integrations-codeowners',
9998
'integrations-enterprise-alert-rule',
10099
'integrations-enterprise-incident-management',

tests/js/getsentry-test/fixtures/am3Plans.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ const AM3_BUSINESS_FEATURES = [
115115
'data-forwarding',
116116
'discard-groups',
117117
'discover-query',
118-
'global-views',
119118
'indexed-spans-extraction',
120119
'insights-initial-modules',
121120
'insights-addon-modules',

tests/js/getsentry-test/fixtures/featureList.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,6 @@ export function FeatureListFixture(): Record<string, Feature> {
7474
name: 'Discover Query Builder',
7575
description: 'Build and save custom queries using Discover.',
7676
},
77-
'global-views': {
78-
name: 'Cross project visibility',
79-
description: 'View data across all projects in your organization.',
80-
},
8177
invoices: {
8278
name: 'Invoicing',
8379
description: 'Standard invoicing for your accounting department.',

tests/js/getsentry-test/fixtures/mm2Plans.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ const MM2_PLANS: Record<string, Plan> = {
5757
'custom-inbound-filters',
5858
'data-forwarding',
5959
'discover',
60-
'global-views',
6160
'rate-limits',
6261
'sso-saml2',
6362
'integrations-event-hooks',
@@ -113,7 +112,6 @@ const MM2_PLANS: Record<string, Plan> = {
113112
'custom-inbound-filters',
114113
'data-forwarding',
115114
'discover',
116-
'global-views',
117115
'rate-limits',
118116
'sso-saml2',
119117
'integrations-event-hooks',
@@ -169,7 +167,6 @@ const MM2_PLANS: Record<string, Plan> = {
169167
'custom-inbound-filters',
170168
'data-forwarding',
171169
'discover',
172-
'global-views',
173170
'rate-limits',
174171
'sso-saml2',
175172
'integrations-event-hooks',
@@ -225,7 +222,6 @@ const MM2_PLANS: Record<string, Plan> = {
225222
'custom-inbound-filters',
226223
'data-forwarding',
227224
'discover',
228-
'global-views',
229225
'rate-limits',
230226
'sso-saml2',
231227
'integrations-event-hooks',
@@ -281,7 +277,6 @@ const MM2_PLANS: Record<string, Plan> = {
281277
'custom-inbound-filters',
282278
'data-forwarding',
283279
'discover',
284-
'global-views',
285280
'rate-limits',
286281
'sso-saml2',
287282
'integrations-event-hooks',
@@ -337,7 +332,6 @@ const MM2_PLANS: Record<string, Plan> = {
337332
'custom-inbound-filters',
338333
'data-forwarding',
339334
'discover',
340-
'global-views',
341335
'rate-limits',
342336
'sso-saml2',
343337
'integrations-event-hooks',
@@ -685,7 +679,6 @@ const MM2_PLANS: Record<string, Plan> = {
685679
'discard-groups',
686680
'dashboards-basic',
687681
'discover-query',
688-
'global-views',
689682
'integrations-codeowners',
690683
'integrations-event-hooks',
691684
'integrations-stacktrace-link',

0 commit comments

Comments
 (0)