From 516f41039948c12e2f8efc1578f4f22ad716ea62 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Fri, 25 Apr 2025 11:05:24 +0200 Subject: [PATCH 1/2] =?UTF-8?q?ref(=E2=9C=82=EF=B8=8F):=20remove=20unused?= =?UTF-8?q?=20experiment=20code?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- static/app/types/experiments.tsx | 2 - static/app/types/hooks.tsx | 18 ---- static/app/utils/useExperiment.tsx | 44 --------- static/app/utils/withExperiment.spec.tsx | 26 ------ static/app/utils/withExperiment.tsx | 107 ---------------------- static/gsApp/hooks/useExperiment.spec.tsx | 103 --------------------- static/gsApp/hooks/useExperiment.tsx | 100 -------------------- static/gsApp/registerHooks.tsx | 4 - 8 files changed, 404 deletions(-) delete mode 100644 static/app/utils/useExperiment.tsx delete mode 100644 static/app/utils/withExperiment.spec.tsx delete mode 100644 static/app/utils/withExperiment.tsx delete mode 100644 static/gsApp/hooks/useExperiment.spec.tsx delete mode 100644 static/gsApp/hooks/useExperiment.tsx diff --git a/static/app/types/experiments.tsx b/static/app/types/experiments.tsx index 6c4ecbf0f016dd..ea5a3c0e70455f 100644 --- a/static/app/types/experiments.tsx +++ b/static/app/types/experiments.tsx @@ -77,5 +77,3 @@ export type OrgExperiments = GetExperimentAssignment< export type UserExperiments = GetExperimentAssignment< TypeSelect['key'] >; - -export type ExperimentAssignment = GetExperimentAssignment; diff --git a/static/app/types/hooks.tsx b/static/app/types/hooks.tsx index 0363f29f2be737..2cdb3ec2add684 100644 --- a/static/app/types/hooks.tsx +++ b/static/app/types/hooks.tsx @@ -8,13 +8,11 @@ import type SidebarItem from 'sentry/components/sidebar/sidebarItem'; import type DateRange from 'sentry/components/timeRangeSelector/dateRange'; import type SelectorItems from 'sentry/components/timeRangeSelector/selectorItems'; import type {SVGIconProps} from 'sentry/icons/svgIcon'; -import type {UseExperiment} from 'sentry/utils/useExperiment'; import type {TitleableModuleNames} from 'sentry/views/insights/common/components/modulePageProviders'; import type {OrganizationStatsProps} from 'sentry/views/organizationStats'; import type {RouteAnalyticsContext} from 'sentry/views/routeAnalyticsContextProvider'; import type {NavigationItem, NavigationSection} from 'sentry/views/settings/types'; -import type {ExperimentKey} from './experiments'; import type {Integration, IntegrationProvider} from './integrations'; import type { Route, @@ -238,7 +236,6 @@ type CustomizationHooks = { */ type AnalyticsHooks = { 'analytics:init-user': AnalyticsInitUser; - 'analytics:log-experiment': AnalyticsLogExperiment; 'analytics:raw-track-event': AnalyticsRawTrackEvent; 'metrics:event': MetricsEvent; }; @@ -340,7 +337,6 @@ type ReactHooks = { props: RouteContextInterface ) => React.ContextType; 'react-hook:use-button-tracking': (props: ButtonProps) => () => void; - 'react-hook:use-experiment': UseExperiment; 'react-hook:use-get-max-retention-days': () => number | undefined; }; @@ -458,20 +454,6 @@ type AnalyticsRawTrackEvent = ( } ) => void; -/** - * Trigger experiment observed logging. - */ -type AnalyticsLogExperiment = (opts: { - /** - * The experiment key - */ - key: ExperimentKey; - /** - * The organization. Must be provided for organization experiments. - */ - organization?: Organization; -}) => void; - /** * Trigger recording a metric in the hook store. */ diff --git a/static/app/utils/useExperiment.tsx b/static/app/utils/useExperiment.tsx deleted file mode 100644 index eb8a28c2bd47b5..00000000000000 --- a/static/app/utils/useExperiment.tsx +++ /dev/null @@ -1,44 +0,0 @@ -import noop from 'lodash/noop'; - -import {unassignedValue} from 'sentry/data/experimentConfig'; -import HookStore from 'sentry/stores/hookStore'; -import type {ExperimentAssignment, ExperimentKey} from 'sentry/types/experiments'; - -type UseExperimentOptions = { - /** - * By default this hook will log the exposure of the experiment upon mounting - * of the component. - * - * If this is undesirable, for example if the experiment is hidden behind - * some user action beyond this component being mounted, then you will want - * to customize when exposure to the experiment has been logged. - * - * NOTE: If set to false, YOU ARE RESPONSIBLE for logging exposure of the - * experiment!! If you do not log exposure your experiment will not be - * correct!! - */ - logExperimentOnMount?: boolean; -}; - -type UseExperimentReturnValue = { - experimentAssignment: ExperimentAssignment[E] | typeof unassignedValue; - /** - * Call this method when the user has been exposed to the experiment. - * You do not need to call this unless you have disabled logging on mount. - */ - logExperiment: () => void; -}; - -export type UseExperiment = ( - experiment: E, - options?: UseExperimentOptions -) => UseExperimentReturnValue; - -export const useExperiment: UseExperiment = (...params) => { - return ( - HookStore.get('react-hook:use-experiment')[0]?.(...params) ?? { - experimentAssignment: unassignedValue, - logExperiment: noop, - } - ); -}; diff --git a/static/app/utils/withExperiment.spec.tsx b/static/app/utils/withExperiment.spec.tsx deleted file mode 100644 index e675534439347a..00000000000000 --- a/static/app/utils/withExperiment.spec.tsx +++ /dev/null @@ -1,26 +0,0 @@ -import {OrganizationFixture} from 'sentry-fixture/organization'; - -import {render, screen} from 'sentry-test/reactTestingLibrary'; - -import withExperiment from 'sentry/utils/withExperiment'; - -describe('withConfig HoC', function () { - beforeEach(function () { - jest.clearAllMocks(); - }); - - function MyComponent(props: any) { - return {props.experimentAssignment}; - } - - it('injects experiment assignment', function () { - const Container = withExperiment(MyComponent, { - // @ts-expect-error This is a test experiment that does not exist, it - // will evalulate to -1 assignment - experiment: 'orgExperiment', - }); - render(); - - expect(screen.getByText('-1')).toBeInTheDocument(); - }); -}); diff --git a/static/app/utils/withExperiment.tsx b/static/app/utils/withExperiment.tsx deleted file mode 100644 index 3f6566b04ebabe..00000000000000 --- a/static/app/utils/withExperiment.tsx +++ /dev/null @@ -1,107 +0,0 @@ -import type { - ExperimentAssignment, - ExperimentKey, - Experiments, - ExperimentType, -} from 'sentry/types/experiments'; -import type {Organization} from 'sentry/types/organization'; -import {useExperiment} from 'sentry/utils/useExperiment'; - -type Options = { - /** - * The key of the experiment that will be injected into the component - */ - experiment: E; - /** - * By default this HoC will log the exposure of the experiment upon mounting - * of the component. - * - * If this is undesirable, for example if the experiment is hidden behind - * some user action beyond this component being mounted, then you will want - * to customize when exposure to the experiment has been logged. - * - * Marking this value as true will inject a `logExperiment` function as a - * prop which takes no parameters and will log exposure of the experiment - * when called. - * - * NOTE: If set to true, YOU ARE RESPONSIBLE for logging exposure of the - * experiment!! If you do not log exposure your experiment will not be - * correct!! - */ - injectLogExperiment?: L; -}; - -type ExpectedProps = T extends 'organization' - ? {organization: Organization} - : never; - -type InjectedExperimentProps = { - /** - * The value of the injected experiment. Use this to determine behavior of - * your component depending on the value. - */ - experimentAssignment: ExperimentAssignment[E]; -} & (L extends true ? LogExperimentProps : never); - -type LogExperimentProps = { - /** - * Call this method when the user has been exposed to the experiment this - * component has been provided the value of. - */ - logExperiment: () => void; -}; - -/** - * A HoC wrapper that injects `experimentAssignment` into a component - * - * This wrapper will automatically log exposure of the experiment upon - * receiving the componentDidMount lifecycle event. - * - * For organization experiments, an organization object must be provided to the - * component. You may wish to use the withOrganization HoC for this. - * - * If exposure logging upon mount is not desirable, The `injectLogExperiment` - * option may be of use. - * - * NOTE: When using this you will have to type the `experimentAssignment` prop - * on your component. For this you should use the `ExperimentAssignment` - * mapped type. - */ -function withExperiment< - E extends ExperimentKey, - L extends boolean, - P extends InjectedExperimentProps, ->( - ExperimentComponent: React.ComponentType

, - {experiment, injectLogExperiment}: Options -) { - type Props = Omit> & - ExpectedProps; - - return function (incomingProps: Props) { - // NOTE(ts): Because of the type complexity of this HoC, typescript - // has a hard time understanding how to narrow Experiments[E]['type'] - // when we type assert on it. - // - // This means we have to do some typecasting to massage things into working - // as expected. - // - // We DO guarantee the external API of this HoC is typed accurately. - - const WrappedComponent = ExperimentComponent as React.JSXElementConstructor; - - const {experimentAssignment, logExperiment} = useExperiment(experiment, { - logExperimentOnMount: !injectLogExperiment, - }); - - const props = { - experimentAssignment, - ...(injectLogExperiment ? {logExperiment} : {}), - ...incomingProps, - } as unknown; - - return ; - }; -} - -export default withExperiment; diff --git a/static/gsApp/hooks/useExperiment.spec.tsx b/static/gsApp/hooks/useExperiment.spec.tsx deleted file mode 100644 index a25cf9dab2529c..00000000000000 --- a/static/gsApp/hooks/useExperiment.spec.tsx +++ /dev/null @@ -1,103 +0,0 @@ -import {OrganizationFixture} from 'sentry-fixture/organization'; -import {UserFixture} from 'sentry-fixture/user'; - -import {renderHook} from 'sentry-test/reactTestingLibrary'; - -import ConfigStore from 'sentry/stores/configStore'; -import type {Organization} from 'sentry/types/organization'; -import {OrganizationContext} from 'sentry/views/organizationContext'; - -import {useExperiment} from 'getsentry/hooks/useExperiment'; -import * as logExperiment from 'getsentry/utils/logExperiment'; - -jest.mock('sentry/data/experimentConfig', () => ({ - experimentConfig: { - orgExperiment: { - key: 'orgExperiment', - type: 'organization', - parameter: 'exposed', - assignments: [1, 0, -1], - }, - userExperiment: { - key: 'userExperiment', - type: 'user', - parameter: 'exposed', - assignments: [1, 0, -1], - }, - }, -})); - -const contextWrapper = (organization: Organization) => { - return function ({children}: {children: React.ReactNode}) { - return {children}; - }; -}; - -describe('useExperiment', function () { - const organization = OrganizationFixture({ - id: '1', - experiments: { - // @ts-expect-error: Using mock keys - orgExperiment: 1, - }, - }); - - beforeEach(function () { - jest.clearAllMocks(); - jest.spyOn(logExperiment, 'default').mockResolvedValue(); - }); - - it('injects org experiment assignment', function () { - const {result} = renderHook(useExperiment, { - initialProps: 'orgExperiment' as any, // Cast to any because this doesn't exist in the config types - wrapper: contextWrapper(organization), - }); - - expect(result.current).toEqual( - expect.objectContaining({ - experimentAssignment: 1, - }) - ); - }); - - it('injects user experiment assignment', function () { - ConfigStore.set('user', UserFixture({id: '123', experiments: {userExperiment: 2}})); - const {result} = renderHook(useExperiment, { - initialProps: 'userExperiment' as any, - wrapper: contextWrapper(organization), - }); - - expect(result.current).toEqual( - expect.objectContaining({ - experimentAssignment: 2, - }) - ); - }); - - it('logs experiment assignment', function () { - const logExperimentSpy = jest.spyOn(logExperiment, 'default').mockResolvedValue(); - renderHook(useExperiment, { - initialProps: 'orgExperiment' as any, - wrapper: contextWrapper(organization), - }); - - expect(logExperimentSpy).toHaveBeenCalledWith({key: 'orgExperiment', organization}); - }); - - it('defers logging when logExperimentOnMount is true', function () { - const logExperimentSpy = jest.spyOn(logExperiment, 'default').mockResolvedValue(); - const {result} = renderHook( - (args: Parameters) => useExperiment(args[0], args[1]), - { - initialProps: ['orgExperiment' as any, {logExperimentOnMount: false}], - wrapper: contextWrapper(organization), - } - ); - - expect(logExperimentSpy).not.toHaveBeenCalled(); - - result.current.logExperiment(); - - expect(logExperimentSpy).toHaveBeenCalledWith({key: 'orgExperiment', organization}); - }); -}); diff --git a/static/gsApp/hooks/useExperiment.tsx b/static/gsApp/hooks/useExperiment.tsx deleted file mode 100644 index c6040241c77da0..00000000000000 --- a/static/gsApp/hooks/useExperiment.tsx +++ /dev/null @@ -1,100 +0,0 @@ -import {useCallback, useEffect} from 'react'; -import * as Sentry from '@sentry/react'; - -import {experimentConfig, unassignedValue} from 'sentry/data/experimentConfig'; -import type { - ExperimentAssignment, - ExperimentKey, - UserExperiments, -} from 'sentry/types/experiments'; -import {ExperimentType} from 'sentry/types/experiments'; -import {defined} from 'sentry/utils'; -import type {UseExperiment} from 'sentry/utils/useExperiment'; -import useOrganization from 'sentry/utils/useOrganization'; -import {useUser} from 'sentry/utils/useUser'; - -import logExperimentAnalytics from 'getsentry/utils/logExperiment'; - -function useExperimentAssignment( - experiment: E -): ExperimentAssignment[E] | typeof unassignedValue { - const organization = useOrganization(); - const user = useUser(); - const config = experimentConfig[experiment]; - - if (!config) { - Sentry.withScope(scope => { - scope.setExtra('experiment', experiment); - Sentry.captureMessage( - 'useExperiment called with an experiment that does not exist in the config.' - ); - }); - - return unassignedValue; - } - - if (config.type === ExperimentType.ORGANIZATION) { - const key = experiment; - const assignment = organization.experiments?.[key]; - if (!defined(assignment)) { - Sentry.withScope(scope => { - scope.setExtra('experiment', experiment); - scope.setExtra('orgExperiments', organization.experiments); - Sentry.captureMessage( - 'useExperiment called with org experiment but no matching experiment exists on the org.' - ); - }); - - return unassignedValue; - } - - return assignment as ExperimentAssignment[E]; - } - - if (config.type === ExperimentType.USER) { - const key = experiment as keyof UserExperiments; - const assignment = user?.experiments?.[key]; - if (!defined(assignment)) { - Sentry.withScope(scope => { - scope.setExtra('experiment', experiment); - scope.setExtra('userExperiments', user?.experiments); - Sentry.captureMessage( - 'useExperiment called with user experiment but no matching experiment exists on the user.' - ); - }); - - return unassignedValue; - } - - return assignment as ExperimentAssignment[E]; - } - - return unassignedValue; -} - -export const useExperiment: UseExperiment = ( - experiment, - {logExperimentOnMount = true} = {} -) => { - const organization = useOrganization(); - - const logExperiment = useCallback(() => { - logExperimentAnalytics({ - key: experiment, - organization, - }); - }, [experiment, organization]); - - useEffect(() => { - if (logExperimentOnMount) { - logExperiment(); - } - }, [logExperiment, logExperimentOnMount]); - - const experimentAssignment = useExperimentAssignment(experiment); - - return { - experimentAssignment, - logExperiment, - }; -}; diff --git a/static/gsApp/registerHooks.tsx b/static/gsApp/registerHooks.tsx index c86467f1619c8a..7fef33fcbbbfef 100644 --- a/static/gsApp/registerHooks.tsx +++ b/static/gsApp/registerHooks.tsx @@ -66,8 +66,6 @@ import EnhancedOrganizationStats from 'getsentry/hooks/spendVisibility/enhancedI import SpikeProtectionProjectSettings from 'getsentry/hooks/spendVisibility/spikeProtectionProjectSettings'; import SuperuserAccessCategory from 'getsentry/hooks/superuserAccessCategory'; import TargetedOnboardingHeader from 'getsentry/hooks/targetedOnboardingHeader'; -import {useExperiment} from 'getsentry/hooks/useExperiment'; -import logExperiment from 'getsentry/utils/logExperiment'; import rawTrackAnalyticsEvent from 'getsentry/utils/rawTrackAnalyticsEvent'; import trackMetric from 'getsentry/utils/trackMetric'; @@ -115,7 +113,6 @@ const GETSENTRY_HOOKS: Partial = { */ 'analytics:raw-track-event': rawTrackAnalyticsEvent, 'analytics:init-user': hookAnalyticsInitUser, - 'analytics:log-experiment': logExperiment, 'metrics:event': trackMetric, /** @@ -248,7 +245,6 @@ const GETSENTRY_HOOKS: Partial = { 'component:crons-list-page-header': () => CronsBillingBanner, 'react-hook:route-activated': useRouteActivatedHook, 'react-hook:use-button-tracking': useButtonTracking, - 'react-hook:use-experiment': useExperiment, 'react-hook:use-get-max-retention-days': useGetMaxRetentionDays, 'component:partnership-agreement': p => ( From 8225ae9b89351f0a946b85500f204639c70a74d3 Mon Sep 17 00:00:00 2001 From: TkDodo Date: Fri, 25 Apr 2025 17:04:21 +0200 Subject: [PATCH 2/2] =?UTF-8?q?ref(=E2=9C=82=EF=B8=8F):=20remove=20more=20?= =?UTF-8?q?experiment=20things?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- static/app/data/experimentConfig.tsx | 33 -- static/app/types/experiments.tsx | 79 ----- static/app/types/organization.tsx | 2 - static/app/types/user.tsx | 2 - .../gsApp/hooks/integrationFeatures.spec.tsx | 2 +- static/gsApp/utils/logExperiment.spec.tsx | 304 ------------------ static/gsApp/utils/logExperiment.tsx | 136 -------- tests/js/fixtures/commitAuthor.ts | 1 - tests/js/fixtures/organization.ts | 3 - tests/js/fixtures/user.ts | 1 - tests/js/fixtures/userDetails.ts | 1 - 11 files changed, 1 insertion(+), 563 deletions(-) delete mode 100644 static/app/data/experimentConfig.tsx delete mode 100644 static/app/types/experiments.tsx delete mode 100644 static/gsApp/utils/logExperiment.spec.tsx delete mode 100644 static/gsApp/utils/logExperiment.tsx diff --git a/static/app/data/experimentConfig.tsx b/static/app/data/experimentConfig.tsx deleted file mode 100644 index 1185633910aa36..00000000000000 --- a/static/app/data/experimentConfig.tsx +++ /dev/null @@ -1,33 +0,0 @@ -import type {Experiments} from 'sentry/types/experiments'; -import {ExperimentType} from 'sentry/types/experiments'; -/** - * This is the value an experiment will have when the unit of assignment - * (organization, user, etc) is not part of any experiment group. - * - * This likely indicates they should see nothing, or the original version of - * what's being tested. - */ -export const unassignedValue = -1; - -/** - * Frontend experiment configuration object - */ -export const experimentList = [ - { - key: 'ExtendTrialByInvitingMemberExperiment', - type: ExperimentType.ORGANIZATION, - parameter: 'exposed', - assignments: [0, 1], - }, - { - key: 'ProjectCreationForAllExperimentV2', - type: ExperimentType.ORGANIZATION, - parameter: 'exposed', - assignments: [0, 1], - }, -] as const; - -export const experimentConfig = experimentList.reduce( - (acc, exp) => ({...acc, [exp.key]: exp}), - {} -) as Experiments; diff --git a/static/app/types/experiments.tsx b/static/app/types/experiments.tsx deleted file mode 100644 index ea5a3c0e70455f..00000000000000 --- a/static/app/types/experiments.tsx +++ /dev/null @@ -1,79 +0,0 @@ -import type {experimentList, unassignedValue} from 'sentry/data/experimentConfig'; - -/** - * The grouping of the experiment - */ -export enum ExperimentType { - ORGANIZATION = 'organization', - USER = 'user', -} - -/** - * An experiment configuration object defines an experiment in the frontend. - * This drives various logic in experiment helpers. - */ -type ExperimentConfig = { - /** - * Possible assignment values of the experiment - */ - assignments: ReadonlyArray; - /** - * The name of the organization. This maps to the key exposed by the - * organization manager on the backend. - */ - key: string; - /** - * The parameter used to access the assignment value - */ - parameter: string | 'variant' | 'exposed'; - /** - * The type of experiment. This configures what group the experiment is - * performed on. - * - * A Organization experiment assigns the whole organization. - * A User experiment assigns a single user. - */ - type: ExperimentType; -}; - -// NOTE: The code below is mostly type mechanics to provide utility types -// around experiments for use in experiment helpers. You probably don't need to -// modify this and likely just need to make changes to the experiment list [0] -// -// [0]: app/data/experimentConfig.tsx - -type ExperimentList = (typeof experimentList)[number]; - -type ExperimentSelect< - C extends ExperimentConfig, - N extends ExperimentConfig['key'], -> = C extends {key: N} ? C : never; - -type TypeSelect< - C extends ExperimentConfig, - T extends ExperimentConfig['type'], -> = C extends {type: T} ? C : never; - -/** - * A mapping of experiment key to the experiment configuration. - */ -export type Experiments = { - [E in ExperimentList['key']]: ExperimentSelect; -}; - -/** - * Represents an active experiment key - */ -export type ExperimentKey = keyof Experiments; - -type GetExperimentAssignment = { - [K in E]: Experiments[K]['assignments'][number]; -}; - -export type OrgExperiments = GetExperimentAssignment< - TypeSelect['key'] ->; - -export type UserExperiments = GetExperimentAssignment< - TypeSelect['key'] ->; diff --git a/static/app/types/organization.tsx b/static/app/types/organization.tsx index 60866877acb795..1cccd6746c97cf 100644 --- a/static/app/types/organization.tsx +++ b/static/app/types/organization.tsx @@ -7,7 +7,6 @@ import type { import type {WidgetType} from 'sentry/views/dashboards/types'; import type {Actor, Avatar, ObjectStatus, Scope} from './core'; -import type {OrgExperiments} from './experiments'; import type {ExternalTeam} from './integrations'; import type {OnboardingTaskStatus} from './onboarding'; import type {Project} from './project'; @@ -64,7 +63,6 @@ export interface Organization extends OrganizationSummary { defaultRole: string; enhancedPrivacy: boolean; eventsMemberAdmin: boolean; - experiments: Partial; genAIConsent: boolean; isDefault: boolean; isDynamicallySampled: boolean; diff --git a/static/app/types/user.tsx b/static/app/types/user.tsx index dbef142fa0b474..19c3d7f63dff1a 100644 --- a/static/app/types/user.tsx +++ b/static/app/types/user.tsx @@ -1,6 +1,5 @@ import type {UserEnrolledAuthenticator} from './auth'; import type {Avatar, Scope} from './core'; -import type {UserExperiments} from './experiments'; /** * Avatars are a more primitive version of User. @@ -36,7 +35,6 @@ export interface User extends Omit { id: string; is_verified: boolean; }>; - experiments: Partial; flags: {newsletter_consent_prompt: boolean}; has2fa: boolean; hasPasswordAuth: boolean; diff --git a/static/gsApp/hooks/integrationFeatures.spec.tsx b/static/gsApp/hooks/integrationFeatures.spec.tsx index d8706f4461a214..ff8b4a4ca357a4 100644 --- a/static/gsApp/hooks/integrationFeatures.spec.tsx +++ b/static/gsApp/hooks/integrationFeatures.spec.tsx @@ -16,7 +16,7 @@ import {PlanTier} from 'getsentry/types'; describe('hookIntegrationFeatures', function () { const {FeatureList, IntegrationFeatures} = hookIntegrationFeatures(); - const organization = OrganizationFixture({experiments: {}}); + const organization = OrganizationFixture({}); ConfigStore.set('user', UserFixture({isSuperuser: true})); beforeEach(() => { diff --git a/static/gsApp/utils/logExperiment.spec.tsx b/static/gsApp/utils/logExperiment.spec.tsx deleted file mode 100644 index d8909cc6105c59..00000000000000 --- a/static/gsApp/utils/logExperiment.spec.tsx +++ /dev/null @@ -1,304 +0,0 @@ -import {OrganizationFixture} from 'sentry-fixture/organization'; - -import {unassignedValue} from 'sentry/data/experimentConfig'; -import ConfigStore from 'sentry/stores/configStore'; -import localStorage from 'sentry/utils/localStorage'; - -import logExperiment from 'getsentry/utils/logExperiment'; - -jest.mock('sentry/utils/localStorage'); - -jest.mock('sentry/data/experimentConfig', () => ({ - experimentConfig: { - orgExperiment: { - key: 'orgExperiment', - type: 'organization', - parameter: 'exposed', - assignments: [1, 0, -1], - }, - variantExperiment: { - key: 'variantExperiment', - type: 'organization', - parameter: 'variant', - assignments: [1, 0, -1], - }, - userExperiment: { - key: 'userExperiment', - type: 'user', - parameter: 'exposed', - assignments: [1, 0, -1], - }, - }, -})); - -const mockedLocalStorageGetItem = localStorage.getItem as jest.MockedFunction< - typeof localStorage.getItem ->; - -describe('logExperiment', function () { - afterEach(function () { - jest.clearAllMocks(); - mockedLocalStorageGetItem.mockClear(); - }); - - it('logs organization experiments', async function () { - const organization = OrganizationFixture({ - id: '7', - experiments: { - // @ts-expect-error: Using mock keys - orgExperiment: 1, - }, - }); - - const data = { - experiment_name: 'orgExperiment', - unit_name: 'org_id', - unit_id: parseInt(organization.id, 10), - params: {exposed: 1}, - }; - - const mock = MockApiClient.addMockResponse({ - url: '/_experiment/log_exposure/', - method: 'POST', - body: data, - }); - - logExperiment({ - organization, - // @ts-expect-error: Using mock keys - key: 'orgExperiment', - }); - - await tick(); - - expect(mock).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({data})); - - expect(localStorage.setItem).toHaveBeenCalledWith( - 'logged-sentry-experiments', - JSON.stringify({orgExperiment: parseInt(organization.id, 10)}) - ); - }); - - it('logs user experiments', function () { - ConfigStore.set('user', { - ...ConfigStore.get('user'), - id: '123', - isSuperuser: false, - experiments: { - userExperiment: 1, - }, - }); - - const data = { - experiment_name: 'userExperiment', - unit_name: 'user_id', - unit_id: 123, - params: {exposed: 1}, - }; - - const mock = MockApiClient.addMockResponse({ - url: '/_experiment/log_exposure/', - method: 'POST', - body: data, - }); - - logExperiment({ - // @ts-expect-error: Using mock keys - key: 'userExperiment', - }); - - expect(mock).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({data})); - }); - - it('logs different parameters', function () { - const organization = OrganizationFixture({ - id: '1', - experiments: { - // @ts-expect-error: Using mock keys - variantExperiment: 1, - }, - }); - - const data = { - experiment_name: 'variantExperiment', - unit_name: 'org_id', - unit_id: parseInt(organization.id, 10), - params: {variant: 1}, - }; - - const mock = MockApiClient.addMockResponse({ - url: '/_experiment/log_exposure/', - method: 'POST', - body: data, - }); - - logExperiment({ - organization, - // @ts-expect-error: Using mock keys - key: 'variantExperiment', - }); - - expect(mock).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({data})); - }); - - it('does not log unassigned experiments', function () { - const mock = MockApiClient.addMockResponse({ - url: '/_experiment/log_exposure/', - method: 'POST', - }); - - const organization = OrganizationFixture({ - id: '1', - experiments: { - // @ts-expect-error: Using mock keys - orgExperiment: unassignedValue, - }, - }); - - logExperiment({ - organization, - // @ts-expect-error: Using mock keys - key: 'orgExperiment', - }); - - expect(mock).not.toHaveBeenCalled(); - }); - - it('does not log when missing org', function () { - const mock = MockApiClient.addMockResponse({ - url: '/_experiment/log_exposure/', - method: 'POST', - }); - - logExperiment({ - organization: undefined, - // @ts-expect-error: Using mock keys - key: 'orgExperiment', - }); - - expect(mock).not.toHaveBeenCalled(); - }); - it("if experiment stored in local storage, don't call log_exposure", async function () { - const organization = OrganizationFixture({ - id: '7', - experiments: { - // @ts-expect-error: Using mock keys - orgExperiment: 1, - }, - }); - - mockedLocalStorageGetItem.mockImplementation(() => - JSON.stringify({orgExperiment: parseInt(organization.id, 10)}) - ); - - const data = { - experiment_name: 'orgExperiment', - unit_name: 'org_id', - unit_id: parseInt(organization.id, 10), - params: {exposed: 1}, - }; - - const mock = MockApiClient.addMockResponse({ - url: '/_experiment/log_exposure/', - method: 'POST', - body: data, - }); - - logExperiment({ - organization, - // @ts-expect-error: Using mock keys - key: 'orgExperiment', - }); - - await tick(); - - expect(mock).not.toHaveBeenCalled(); - expect(localStorage.setItem).not.toHaveBeenCalled(); - }); - it('if local storage has different experiment, call log_exposure', async function () { - const organization = OrganizationFixture({ - id: '7', - experiments: { - // @ts-expect-error: Using mock keys - orgExperiment: 1, - }, - }); - - const unit_id = parseInt(organization.id, 10); - - mockedLocalStorageGetItem.mockImplementation(() => - JSON.stringify({anotherExperiment: unit_id}) - ); - - const data = { - experiment_name: 'orgExperiment', - unit_name: 'org_id', - unit_id, - params: {exposed: 1}, - }; - - const mock = MockApiClient.addMockResponse({ - url: '/_experiment/log_exposure/', - method: 'POST', - body: data, - }); - - logExperiment({ - organization, - // @ts-expect-error: Using mock keys - key: 'orgExperiment', - }); - - await tick(); - - expect(mock).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({data})); - - expect(localStorage.setItem).toHaveBeenCalledWith( - 'logged-sentry-experiments', - JSON.stringify({anotherExperiment: unit_id, orgExperiment: unit_id}) - ); - }); - it('if local storage has different unit_id, call log_exposure', async function () { - const organization = OrganizationFixture({ - id: '7', - experiments: { - // @ts-expect-error: Using mock keys - orgExperiment: 1, - }, - }); - - const unit_id = parseInt(organization.id, 10); - - mockedLocalStorageGetItem.mockImplementation(() => - JSON.stringify({orgExperiment: 12}) - ); - - const data = { - experiment_name: 'orgExperiment', - unit_name: 'org_id', - unit_id, - params: {exposed: 1}, - }; - - const mock = MockApiClient.addMockResponse({ - url: '/_experiment/log_exposure/', - method: 'POST', - body: data, - }); - - logExperiment({ - organization, - // @ts-expect-error: Using mock keys - key: 'orgExperiment', - }); - - await tick(); - - expect(mock).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({data})); - - expect(localStorage.setItem).toHaveBeenCalledWith( - 'logged-sentry-experiments', - JSON.stringify({orgExperiment: unit_id}) - ); - }); -}); diff --git a/static/gsApp/utils/logExperiment.tsx b/static/gsApp/utils/logExperiment.tsx deleted file mode 100644 index b140ef88774b5c..00000000000000 --- a/static/gsApp/utils/logExperiment.tsx +++ /dev/null @@ -1,136 +0,0 @@ -import * as Sentry from '@sentry/react'; - -import {Client} from 'sentry/api'; -import {experimentConfig, unassignedValue} from 'sentry/data/experimentConfig'; -import ConfigStore from 'sentry/stores/configStore'; -import type {ExperimentKey} from 'sentry/types/experiments'; -import {ExperimentType} from 'sentry/types/experiments'; -import type {Organization} from 'sentry/types/organization'; -import {defined} from 'sentry/utils'; -import localStorage from 'sentry/utils/localStorage'; - -// the local storage key that stores which experiments we've logged -const SENTRY_EXPERIMENTS_KEY = 'logged-sentry-experiments'; - -const unitNameMap = { - organization: 'org_id', - user: 'user_id', -} as const; - -type Options = { - /** - * The experiment key - */ - key: ExperimentKey; - /** - * The organization for org based experiments - */ - organization?: Organization; -}; - -/** - * Log exposure to an experiment. - * - * Generally you will want to call this just before the logic to determine the - * variant or exposure of a particular experiment. - */ -export default async function logExperiment({key, organization}: Options) { - // Never log experiments for super users - const user = ConfigStore.get('user'); - if (user.isSuperuser) { - return; - } - - const config = experimentConfig[key]; - - if (config === undefined) { - return; - } - - const {type, parameter} = config; - - const assignment = - type === ExperimentType.ORGANIZATION - ? organization?.experiments?.[key] - : type === ExperimentType.USER - ? // @ts-expect-error TS(7053): Element implicitly has an 'any' type because expre... Remove this comment to see the full error message - user.experiments?.[key] - : null; - - // Nothing to log if there is no assignment - if (!defined(assignment)) { - return; - } - - // Do not log if the assignment matches the unassigned group constant - if (assignment === unassignedValue) { - return; - } - - const unitId = - type === ExperimentType.ORGANIZATION - ? Number(organization?.id) - : type === ExperimentType.USER - ? Number(user.id) - : null; - - // if the value of this experiment matches the stored value, we can - // skip calling log_exposure - if (getExperimentLogged(key) === unitId) { - return; - } - - const data = { - experiment_name: key, - unit_name: unitNameMap[type], - unit_id: unitId, - params: {[parameter]: assignment}, - }; - - const client = new Client({baseUrl: ''}); - - try { - await client.requestPromise('/_experiment/log_exposure/', { - method: 'POST', - data, - }); - // update local storage with the new experiment we've logged - setExperimentLogged(key, unitId); - } catch (error) { - Sentry.withScope(scope => { - scope.setExtra('data', data); - scope.setExtra('error', error); - Sentry.captureException(new Error('Could not log experiment')); - }); - } -} - -const getExperimentsLogged = () => { - try { - const localStorageExperiments = localStorage.getItem(SENTRY_EXPERIMENTS_KEY); - const jsonData: unknown = JSON.parse(localStorageExperiments || '{}'); - if (typeof jsonData === 'object' && !Array.isArray(jsonData) && jsonData !== null) { - return jsonData; - } - } catch (err) { - // some sort of malformed json - Sentry.captureException(err); - } - // by default, return an empty config - return {}; -}; - -const getExperimentLogged = (key: string) => { - const experimentData = getExperimentsLogged(); - // @ts-expect-error TS(7053): Element implicitly has an 'any' type because expre... Remove this comment to see the full error message - return experimentData[key]; -}; - -const setExperimentLogged = (key: string, unitId: number | null) => { - // shouldn't be null but need to make TS happy - if (unitId === null) { - return; - } - const experimentData = {...getExperimentsLogged(), [key]: unitId}; - localStorage.setItem(SENTRY_EXPERIMENTS_KEY, JSON.stringify(experimentData)); -}; diff --git a/tests/js/fixtures/commitAuthor.ts b/tests/js/fixtures/commitAuthor.ts index 0ca9b13546a266..dba125e25aac55 100644 --- a/tests/js/fixtures/commitAuthor.ts +++ b/tests/js/fixtures/commitAuthor.ts @@ -46,7 +46,6 @@ export function CommitAuthorFixture(params: Partial = {}): User { }, permissions: new Set(), canReset2fa: false, - experiments: [], flags: {newsletter_consent_prompt: false}, identities: [], ...params, diff --git a/tests/js/fixtures/organization.ts b/tests/js/fixtures/organization.ts index bfccc1524eb183..a73db4ddaa0249 100644 --- a/tests/js/fixtures/organization.ts +++ b/tests/js/fixtures/organization.ts @@ -3,8 +3,6 @@ import {OrgRoleListFixture, TeamRoleListFixture} from 'sentry-fixture/roleList'; import type {Organization} from 'sentry/types/organization'; export function OrganizationFixture( params: Partial = {}): Organization { - - const slug = params.slug ?? 'org-slug'; return { id: '3', @@ -33,7 +31,6 @@ export function OrganizationFixture( params: Partial = {}): Organi id: 'active', name: 'active', }, - experiments: {}, scrapeJavaScript: true, features: [], onboardingTasks: [], diff --git a/tests/js/fixtures/user.ts b/tests/js/fixtures/user.ts index aca3e09470cd8a..4ddcb226baa1e8 100644 --- a/tests/js/fixtures/user.ts +++ b/tests/js/fixtures/user.ts @@ -27,7 +27,6 @@ export function UserFixture(params: Partial = {}): User { canReset2fa: false, dateJoined: '2020-01-01T00:00:00.000Z', emails: [], - experiments: [], has2fa: false, identities: [], isActive: false, diff --git a/tests/js/fixtures/userDetails.ts b/tests/js/fixtures/userDetails.ts index 0da3c92d69cc60..21eed341b209a2 100644 --- a/tests/js/fixtures/userDetails.ts +++ b/tests/js/fixtures/userDetails.ts @@ -42,7 +42,6 @@ export function UserDetailsFixture(params: Partial = {}): User { permissions: new Set(), email: 'billyfirefox@test.com', canReset2fa: false, - experiments: [], flags: {newsletter_consent_prompt: false}, hasPasswordAuth: false, ...params,