diff --git a/frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts b/frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts new file mode 100644 index 000000000000..096e89954ce4 --- /dev/null +++ b/frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts @@ -0,0 +1,125 @@ +jest.mock('common/services/useExperiment', () => ({ + useGetExperimentsQuery: jest.fn(), +})) + +jest.mock('react', () => ({ + ...jest.requireActual('react'), + useMemo: (fn: () => any) => fn(), +})) + +import { useFeatureExperimentFreeze } from 'common/hooks/useFeatureExperimentFreeze' +import { useGetExperimentsQuery } from 'common/services/useExperiment' +import { Experiment } from 'common/types/responses' + +const mockUseGetExperimentsQuery = + useGetExperimentsQuery as jest.MockedFunction + +const makeExperiment = ( + overrides: Partial & { featureId: number }, +): Experiment => ({ + created_at: '', + ended_at: null, + feature: { + id: overrides.featureId, + initial_value: null, + multivariate_options: [], + name: 'test-flag', + type: 'MULTIVARIATE', + }, + hypothesis: '', + id: 1, + metrics: [], + name: 'Test Experiment', + started_at: null, + status: overrides.status ?? 'running', + updated_at: '', + ...overrides, +}) + +const empty = { data: { results: [] }, isLoading: false } as any +const loading = { data: undefined, isLoading: true } as any + +const withResults = (experiments: Experiment[]) => + ({ data: { results: experiments }, isLoading: false } as any) + +describe('useFeatureExperimentFreeze', () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + it('returns isFrozen true when a running experiment exists for the feature', () => { + mockUseGetExperimentsQuery.mockReturnValue( + withResults([makeExperiment({ featureId: 42, status: 'running' })]), + ) + + const result = useFeatureExperimentFreeze(42, 'env-123') + + expect(result.isFrozen).toBe(true) + expect(result.experiment?.id).toBe(1) + expect(result.isLoading).toBe(false) + }) + + it('returns isFrozen false when no experiments exist', () => { + mockUseGetExperimentsQuery.mockReturnValue(empty) + + const result = useFeatureExperimentFreeze(42, 'env-123') + + expect(result.isFrozen).toBe(false) + expect(result.experiment).toBeNull() + }) + + it('returns isFrozen false when experiment belongs to a different feature', () => { + mockUseGetExperimentsQuery.mockReturnValue( + withResults([makeExperiment({ featureId: 99, status: 'running' })]), + ) + + const result = useFeatureExperimentFreeze(42, 'env-123') + + expect(result.isFrozen).toBe(false) + }) + + it('returns isLoading true while the query is loading', () => { + mockUseGetExperimentsQuery.mockReturnValue(loading) + + const result = useFeatureExperimentFreeze(42, 'env-123') + + expect(result.isFrozen).toBe(false) + expect(result.isLoading).toBe(true) + }) + + it('skips query when featureId is undefined', () => { + mockUseGetExperimentsQuery.mockReturnValue(empty) + + const result = useFeatureExperimentFreeze(undefined, 'env-123') + + expect(result.isFrozen).toBe(false) + expect(mockUseGetExperimentsQuery).toHaveBeenCalledWith( + { environmentId: 'env-123', status: 'running' }, + { skip: true }, + ) + }) + + it('skips query when environmentId is empty', () => { + mockUseGetExperimentsQuery.mockReturnValue(empty) + + const result = useFeatureExperimentFreeze(42, '') + + expect(result.isFrozen).toBe(false) + expect(mockUseGetExperimentsQuery).toHaveBeenCalledWith( + { environmentId: '', status: 'running' }, + { skip: true }, + ) + }) + + it('queries only running experiments', () => { + mockUseGetExperimentsQuery.mockReturnValue(empty) + + useFeatureExperimentFreeze(42, 'env-123') + + expect(mockUseGetExperimentsQuery).toHaveBeenCalledTimes(1) + expect(mockUseGetExperimentsQuery).toHaveBeenCalledWith( + { environmentId: 'env-123', status: 'running' }, + { skip: false }, + ) + }) +}) diff --git a/frontend/common/hooks/useFeatureExperimentFreeze.ts b/frontend/common/hooks/useFeatureExperimentFreeze.ts new file mode 100644 index 000000000000..1d603c077472 --- /dev/null +++ b/frontend/common/hooks/useFeatureExperimentFreeze.ts @@ -0,0 +1,31 @@ +import { useMemo } from 'react' +import { useGetExperimentsQuery } from 'common/services/useExperiment' +import type { Experiment } from 'common/types/responses' + +export type FeatureExperimentFreeze = { + isFrozen: boolean + experiment: Experiment | null + isLoading: boolean +} + +export function useFeatureExperimentFreeze( + featureId: number | undefined, + environmentId: string, +): FeatureExperimentFreeze { + const skip = !featureId || !environmentId + const { data, isLoading } = useGetExperimentsQuery( + { environmentId, status: 'running' }, + { skip }, + ) + + const experiment = useMemo(() => { + if (!featureId || !data?.results) return null + return data.results.find((e) => e.feature?.id === featureId) ?? null + }, [data?.results, featureId]) + + return { + experiment, + isFrozen: experiment !== null, + isLoading, + } +} diff --git a/frontend/web/components/modals/create-feature/components/ExperimentFreezeNotice/ExperimentFreezeNotice.tsx b/frontend/web/components/modals/create-feature/components/ExperimentFreezeNotice/ExperimentFreezeNotice.tsx new file mode 100644 index 000000000000..bb99336416b5 --- /dev/null +++ b/frontend/web/components/modals/create-feature/components/ExperimentFreezeNotice/ExperimentFreezeNotice.tsx @@ -0,0 +1,35 @@ +import { FC } from 'react' +import InfoMessage from 'components/InfoMessage' + +type FreezeExperiment = { + id: number + name: string + status: string +} + +type ExperimentFreezeNoticeProps = { + experiment: FreezeExperiment + projectId: number | string + environmentId: string +} + +const ExperimentFreezeNotice: FC = ({ + environmentId, + experiment, + projectId, +}) => { + const experimentUrl = `/project/${projectId}/environment/${environmentId}/experiments/${experiment.id}` + + return ( + + This flag is part of the experiment{' '} + + {experiment.name} + {' '} + which is currently {experiment.status}. Editing is restricted to prevent + skewing experiment results. + + ) +} + +export default ExperimentFreezeNotice diff --git a/frontend/web/components/modals/create-feature/components/ExperimentFreezeNotice/index.ts b/frontend/web/components/modals/create-feature/components/ExperimentFreezeNotice/index.ts new file mode 100644 index 000000000000..eaf9e8a43e0a --- /dev/null +++ b/frontend/web/components/modals/create-feature/components/ExperimentFreezeNotice/index.ts @@ -0,0 +1 @@ +export { default } from './ExperimentFreezeNotice' diff --git a/frontend/web/components/modals/create-feature/index.tsx b/frontend/web/components/modals/create-feature/index.tsx index 92c05e5fc20d..787186cd90ef 100644 --- a/frontend/web/components/modals/create-feature/index.tsx +++ b/frontend/web/components/modals/create-feature/index.tsx @@ -11,6 +11,7 @@ import moment from 'moment' import { useProjectEnvironments } from 'common/hooks/useProjectEnvironments' import { useHasGithubIntegration } from 'common/hooks/useHasGithubIntegration' import { useHasGitLabIntegration } from 'common/hooks/useHasGitLabIntegration' +import { useFeatureExperimentFreeze } from 'common/hooks/useFeatureExperimentFreeze' import FeatureListStore from 'common/stores/feature-list-store' import IdentityProvider from 'common/providers/IdentityProvider' import FeatureListProvider from 'common/providers/FeatureListProvider' @@ -109,6 +110,11 @@ const CreateFeatureModal: FC = (props) => { } = props const flagId = props.environmentFlag?.id + const freeze = useFeatureExperimentFreeze( + props.projectFlag?.id, + environmentId, + ) + const [projectFlag, setProjectFlag] = useState(() => props.projectFlag ? cloneDeep(props.projectFlag) @@ -655,6 +661,7 @@ const CreateFeatureModal: FC = (props) => { error={error} projectId={projectId} noPermissions={!!noPermissions} + freeze={freeze} featureState={environmentFlag} projectFlag={projectFlag} environmentFlag={props.environmentFlag} @@ -692,6 +699,7 @@ const CreateFeatureModal: FC = (props) => { = (props) => { = ({ + environmentId, + freeze, groupOwnerIds, hasMetadataRequired, identity, @@ -121,6 +127,13 @@ const FeatureSettingsTab: FC = ({ return (
+ {freeze?.isFrozen && freeze.experiment && environmentId && ( + + )} {!identity && projectFlag?.tags && ( = ({ }) .unwrap() .catch((e) => - toast( - e?.data?.[0] || 'Failed to remove owner.', - 'danger', - ), + toast(e?.data?.[0] || 'Failed to remove owner.', 'danger'), ) } /> @@ -282,6 +292,7 @@ const FeatureSettingsTab: FC = ({ onChange={(is_server_key_only) => onChange({ ...projectFlag, is_server_key_only }) } + disabled={!!freeze?.isFrozen || !!freeze?.isLoading} className='ml-0' /> = ({ onChange={(is_archived) => onChange({ ...projectFlag, is_archived }) } + disabled={!!freeze?.isFrozen || !!freeze?.isLoading} className='ml-0' /> = ({ error, existingChangeRequest, featureState, + freeze, identity, is4Eyes, isSaving, @@ -77,6 +81,8 @@ const FeatureValueTab: FC = ({ projectId, }) => { const isEdit = !!projectFlag?.id + const isDisabled = + !!noPermissions || !!freeze?.isFrozen || !!freeze?.isLoading const { permission: createFeature } = useHasPermission({ id: projectId, @@ -294,6 +300,13 @@ const FeatureValueTab: FC = ({ return (
+ {freeze?.isFrozen && freeze.experiment && environmentId && ( + + )} = ({ onEnvironmentFlagChange({ enabled })} className='ml-0' @@ -338,7 +351,7 @@ const FeatureValueTab: FC = ({ ) onEnvironmentFlagChange({ feature_state_value }) }} - disabled={noPermissions} + disabled={isDisabled} placeholder="e.g. 'big' " /> } @@ -444,7 +457,7 @@ const FeatureValueTab: FC = ({ Constants.projectPermissions(ProjectPermission.CREATE_FEATURE), , )} @@ -454,7 +467,7 @@ const FeatureValueTab: FC = ({ {(!!environmentVariations || !isEdit) && ( = ({ Constants.projectPermissions(ProjectPermission.CREATE_FEATURE), , )} @@ -493,7 +506,7 @@ const FeatureValueTab: FC = ({
)} - {environmentId && onSaveFeatureValue && ( + {environmentId && onSaveFeatureValue && !freeze?.isFrozen && ( <> = ({ projectFlag={projectFlag} environmentId={environmentId} environmentName={environmentName || ''} - isSaving={!!isSaving} + isSaving={!!isSaving || !!freeze?.isLoading} featureName={projectFlag.name} isInvalid={!!invalid} existingChangeRequest={!!existingChangeRequest} diff --git a/frontend/web/components/modals/create-feature/tabs/SegmentOverridesTab.tsx b/frontend/web/components/modals/create-feature/tabs/SegmentOverridesTab.tsx index 77f72d5bbcdb..5675ad880b62 100644 --- a/frontend/web/components/modals/create-feature/tabs/SegmentOverridesTab.tsx +++ b/frontend/web/components/modals/create-feature/tabs/SegmentOverridesTab.tsx @@ -12,6 +12,8 @@ import ModalHR from 'components/modals/ModalHR' import FeatureInPipelineGuard from 'components/release-pipelines/FeatureInPipelineGuard' import Utils from 'common/utils/utils' import { ProjectFlag } from 'common/types/responses' +import { FeatureExperimentFreeze } from 'common/hooks/useFeatureExperimentFreeze' +import ExperimentFreezeNotice from 'components/modals/create-feature/components/ExperimentFreezeNotice' import { EnvironmentPermission } from 'common/types/permissions.types' export type SegmentOverrideValue = { @@ -34,6 +36,7 @@ type SegmentOverridesTabProps = { error: any existingChangeRequest?: { id: number } noPermissions?: boolean + freeze?: FeatureExperimentFreeze disableCreate?: boolean highlightSegmentId?: number } @@ -45,6 +48,7 @@ const SegmentOverridesTab: FC = ({ environmentId, error, existingChangeRequest, + freeze, highlightSegmentId, invalid, isSaving, @@ -159,7 +163,9 @@ const SegmentOverridesTab: FC = ({ {!isComparing && !showCreateSegment && manageSegmentOverrides && - !disableCreate && ( + !disableCreate && + !freeze?.isFrozen && + !freeze?.isLoading && (
)} - {!isComparing && !showCreateSegment && !noPermissions && ( - - )} + {!isComparing && + !showCreateSegment && + !noPermissions && + !freeze?.isFrozen && + !freeze?.isLoading && ( + + )} + {freeze?.isFrozen && freeze.experiment && ( + + )}

Segment Overrides apply when the identity traits match the segment @@ -206,7 +223,11 @@ const SegmentOverridesTab: FC = ({ = ({

)} - {!isComparing && !showCreateSegment && } - {!isComparing && !showCreateSegment && ( + {!isComparing && !showCreateSegment && !freeze?.isFrozen && ( + + )} + {!isComparing && !showCreateSegment && !freeze?.isFrozen && (

Re-order overrides to adjust priority. @@ -269,7 +292,8 @@ const SegmentOverridesTab: FC = ({ isSaving || !projectFlag.name || invalid || - !savePermission + !savePermission || + !!freeze?.isLoading } > {getButtonText()} @@ -294,7 +318,8 @@ const SegmentOverridesTab: FC = ({ isSaving || !projectFlag.name || invalid || - !savePermission + !savePermission || + !!freeze?.isLoading } > {getScheduleButtonText()} @@ -310,7 +335,8 @@ const SegmentOverridesTab: FC = ({ isSaving || !projectFlag.name || invalid || - !manageSegmentOverrides + !manageSegmentOverrides || + !!freeze?.isLoading } > {isSaving ? 'Updating' : 'Update Segment Overrides'}