From 577fdddb5d9eace3d7792b3e6ad9696eeb80624d Mon Sep 17 00:00:00 2001 From: wadii Date: Fri, 26 Jun 2026 10:39:18 +0200 Subject: [PATCH 1/4] feat: freeze flag editing when flag is in active experiment --- .../useFeatureExperimentFreeze.test.ts | 151 ++++++++++++++++++ .../hooks/useFeatureExperimentFreeze.ts | 35 ++++ .../modals/create-feature/index.tsx | 10 ++ .../tabs/FeatureSettingsTab.tsx | 21 ++- .../create-feature/tabs/FeatureValueTab.tsx | 26 ++- .../tabs/SegmentOverridesTab.tsx | 48 ++++-- 6 files changed, 265 insertions(+), 26 deletions(-) create mode 100644 frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts create mode 100644 frontend/common/hooks/useFeatureExperimentFreeze.ts diff --git a/frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts b/frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts new file mode 100644 index 000000000000..26373e4f2fc7 --- /dev/null +++ b/frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts @@ -0,0 +1,151 @@ +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, +}) + +describe('useFeatureExperimentFreeze', () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + it('returns isFrozen true when a running experiment exists for the feature', () => { + mockUseGetExperimentsQuery.mockReturnValue({ + data: { + results: [makeExperiment({ featureId: 42, status: 'running' })], + }, + isLoading: false, + } as any) + + 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 true when a paused experiment exists for the feature', () => { + mockUseGetExperimentsQuery.mockReturnValue({ + data: { + results: [makeExperiment({ featureId: 42, status: 'paused' })], + }, + isLoading: false, + } as any) + + const result = useFeatureExperimentFreeze(42, 'env-123') + + expect(result.isFrozen).toBe(true) + }) + + it('returns isFrozen false when experiment is created (not yet started)', () => { + mockUseGetExperimentsQuery.mockReturnValue({ + data: { + results: [makeExperiment({ featureId: 42, status: 'created' })], + }, + isLoading: false, + } as any) + + const result = useFeatureExperimentFreeze(42, 'env-123') + + expect(result.isFrozen).toBe(false) + expect(result.experiment).toBeNull() + }) + + it('returns isFrozen false when experiment is completed', () => { + mockUseGetExperimentsQuery.mockReturnValue({ + data: { + results: [makeExperiment({ featureId: 42, status: 'completed' })], + }, + isLoading: false, + } as any) + + const result = useFeatureExperimentFreeze(42, 'env-123') + + expect(result.isFrozen).toBe(false) + expect(result.experiment).toBeNull() + }) + + it('returns isFrozen false when no experiments exist', () => { + mockUseGetExperimentsQuery.mockReturnValue({ + data: { results: [] }, + isLoading: false, + } as any) + + 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({ + data: { + results: [makeExperiment({ featureId: 99, status: 'running' })], + }, + isLoading: false, + } as any) + + const result = useFeatureExperimentFreeze(42, 'env-123') + + expect(result.isFrozen).toBe(false) + }) + + it('returns isLoading true while experiments are loading', () => { + mockUseGetExperimentsQuery.mockReturnValue({ + data: undefined, + isLoading: true, + } as any) + + 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({ + data: undefined, + isLoading: false, + } as any) + + const result = useFeatureExperimentFreeze(undefined, 'env-123') + + expect(result.isFrozen).toBe(false) + expect(mockUseGetExperimentsQuery).toHaveBeenCalledWith( + expect.objectContaining({ environmentId: 'env-123' }), + expect.objectContaining({ skip: true }), + ) + }) +}) diff --git a/frontend/common/hooks/useFeatureExperimentFreeze.ts b/frontend/common/hooks/useFeatureExperimentFreeze.ts new file mode 100644 index 000000000000..1e771fdc7c41 --- /dev/null +++ b/frontend/common/hooks/useFeatureExperimentFreeze.ts @@ -0,0 +1,35 @@ +import { useMemo } from 'react' +import { useGetExperimentsQuery } from 'common/services/useExperiment' +import type { Experiment, ExperimentStatus } from 'common/types/responses' + +const FREEZE_STATUSES: ExperimentStatus[] = ['running', 'paused'] + +export function useFeatureExperimentFreeze( + featureId: number | undefined, + environmentId: string, +): { + isFrozen: boolean + experiment: Experiment | null + isLoading: boolean +} { + const { data, isLoading } = useGetExperimentsQuery( + { environmentId, page_size: 100 }, + { skip: !featureId }, + ) + + const experiment = useMemo(() => { + if (!featureId || !data?.results) return null + return ( + data.results.find( + (e) => + e.feature?.id === featureId && FREEZE_STATUSES.includes(e.status), + ) ?? null + ) + }, [data?.results, featureId]) + + return { + experiment, + isFrozen: experiment !== null, + isLoading, + } +} diff --git a/frontend/web/components/modals/create-feature/index.tsx b/frontend/web/components/modals/create-feature/index.tsx index 92c05e5fc20d..ff0b32bc69ae 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,9 @@ const CreateFeatureModal: FC = (props) => { } = props const flagId = props.environmentFlag?.id + const { experiment: freezingExperiment, isFrozen: experimentFrozen } = + useFeatureExperimentFreeze(props.projectFlag?.id, environmentId) + const [projectFlag, setProjectFlag] = useState(() => props.projectFlag ? cloneDeep(props.projectFlag) @@ -655,6 +659,8 @@ const CreateFeatureModal: FC = (props) => { error={error} projectId={projectId} noPermissions={!!noPermissions} + experimentFrozen={experimentFrozen} + freezingExperiment={freezingExperiment} featureState={environmentFlag} projectFlag={projectFlag} environmentFlag={props.environmentFlag} @@ -692,6 +698,8 @@ const CreateFeatureModal: FC = (props) => { = (props) => { = ({ + experimentFrozen, + freezingExperiment, groupOwnerIds, hasMetadataRequired, identity, @@ -121,6 +125,14 @@ const FeatureSettingsTab: FC = ({ return (
+ {experimentFrozen && freezingExperiment && ( + + This flag is part of the experiment{' '} + {freezingExperiment.name} which is currently{' '} + {freezingExperiment.status}. Some settings are restricted to prevent + skewing experiment results. + + )} {!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 +291,7 @@ const FeatureSettingsTab: FC = ({ onChange={(is_server_key_only) => onChange({ ...projectFlag, is_server_key_only }) } + disabled={!!experimentFrozen} className='ml-0' /> = ({ onChange={(is_archived) => onChange({ ...projectFlag, is_archived }) } + disabled={!!experimentFrozen} className='ml-0' /> = ({ environmentName, error, existingChangeRequest, + experimentFrozen, featureState, + freezingExperiment, identity, is4Eyes, isSaving, @@ -77,6 +82,7 @@ const FeatureValueTab: FC = ({ projectId, }) => { const isEdit = !!projectFlag?.id + const isDisabled = !!noPermissions || !!experimentFrozen const { permission: createFeature } = useHasPermission({ id: projectId, @@ -294,6 +300,14 @@ const FeatureValueTab: FC = ({ return (
+ {experimentFrozen && freezingExperiment && ( + + This flag is part of the experiment{' '} + {freezingExperiment.name} which is currently{' '} + {freezingExperiment.status}. Editing is restricted to prevent skewing + experiment results. + + )} = ({ onEnvironmentFlagChange({ enabled })} className='ml-0' @@ -338,7 +352,7 @@ const FeatureValueTab: FC = ({ ) onEnvironmentFlagChange({ feature_state_value }) }} - disabled={noPermissions} + disabled={isDisabled} placeholder="e.g. 'big' " /> } @@ -444,7 +458,7 @@ const FeatureValueTab: FC = ({ Constants.projectPermissions(ProjectPermission.CREATE_FEATURE), , )} @@ -454,7 +468,7 @@ const FeatureValueTab: FC = ({ {(!!environmentVariations || !isEdit) && ( = ({ Constants.projectPermissions(ProjectPermission.CREATE_FEATURE), , )} @@ -493,7 +507,7 @@ const FeatureValueTab: FC = ({
)} - {environmentId && onSaveFeatureValue && ( + {environmentId && onSaveFeatureValue && !experimentFrozen && ( <> = ({ environmentId, error, existingChangeRequest, + experimentFrozen, + freezingExperiment, highlightSegmentId, invalid, isSaving, @@ -159,7 +163,8 @@ const SegmentOverridesTab: FC = ({ {!isComparing && !showCreateSegment && manageSegmentOverrides && - !disableCreate && ( + !disableCreate && + !experimentFrozen && (
)} - {!isComparing && !showCreateSegment && !noPermissions && ( - - )} + {!isComparing && + !showCreateSegment && + !noPermissions && + !experimentFrozen && ( + + )} + {experimentFrozen && freezingExperiment && ( + + This flag is part of the experiment{' '} + {freezingExperiment.name} which is currently{' '} + {freezingExperiment.status}. Editing is restricted to prevent + skewing experiment results. + + )}

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

)} - {!isComparing && !showCreateSegment && } - {!isComparing && !showCreateSegment && ( + {!isComparing && !showCreateSegment && !experimentFrozen && ( + + )} + {!isComparing && !showCreateSegment && !experimentFrozen && (

Re-order overrides to adjust priority. From a34c774a70d67de43fcc129b95ee20cfa62d9ef0 Mon Sep 17 00:00:00 2001 From: wadii Date: Fri, 26 Jun 2026 14:05:12 +0200 Subject: [PATCH 2/4] refactor: improve experiment freeze with status-filtered queries and consolidated props Use two status-filtered queries (running/paused) instead of one unfiltered page_size=100 query. Consolidate three freeze props into a single FeatureExperimentFreeze object. Extract shared ExperimentFreezeNotice component with link to experiment. Disable controls during loading to prevent unlocked flash. --- .../useFeatureExperimentFreeze.test.ts | 124 ++++++----- .../hooks/useFeatureExperimentFreeze.ts | 40 ++-- .../ExperimentFreezeNotice.tsx | 35 +++ .../ExperimentFreezeNotice/index.ts | 1 + .../modals/create-feature/index.tsx | 16 +- .../tabs/FeatureSettingsTab.tsx | 29 +-- .../create-feature/tabs/FeatureValueTab.tsx | 92 ++++---- .../tabs/SegmentOverridesTab.tsx | 201 +++++++++--------- 8 files changed, 294 insertions(+), 244 deletions(-) create mode 100644 frontend/web/components/modals/create-feature/components/ExperimentFreezeNotice/ExperimentFreezeNotice.tsx create mode 100644 frontend/web/components/modals/create-feature/components/ExperimentFreezeNotice/index.ts diff --git a/frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts b/frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts index 26373e4f2fc7..0a9f095ed8b3 100644 --- a/frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts +++ b/frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts @@ -36,18 +36,23 @@ const makeExperiment = ( ...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({ - data: { - results: [makeExperiment({ featureId: 42, status: 'running' })], - }, - isLoading: false, - } as any) + mockUseGetExperimentsQuery + .mockReturnValueOnce( + withResults([makeExperiment({ featureId: 42, status: 'running' })]), + ) + .mockReturnValueOnce(empty) const result = useFeatureExperimentFreeze(42, 'env-123') @@ -57,51 +62,21 @@ describe('useFeatureExperimentFreeze', () => { }) it('returns isFrozen true when a paused experiment exists for the feature', () => { - mockUseGetExperimentsQuery.mockReturnValue({ - data: { - results: [makeExperiment({ featureId: 42, status: 'paused' })], - }, - isLoading: false, - } as any) + mockUseGetExperimentsQuery + .mockReturnValueOnce(empty) + .mockReturnValueOnce( + withResults([makeExperiment({ featureId: 42, status: 'paused' })]), + ) const result = useFeatureExperimentFreeze(42, 'env-123') expect(result.isFrozen).toBe(true) }) - it('returns isFrozen false when experiment is created (not yet started)', () => { - mockUseGetExperimentsQuery.mockReturnValue({ - data: { - results: [makeExperiment({ featureId: 42, status: 'created' })], - }, - isLoading: false, - } as any) - - const result = useFeatureExperimentFreeze(42, 'env-123') - - expect(result.isFrozen).toBe(false) - expect(result.experiment).toBeNull() - }) - - it('returns isFrozen false when experiment is completed', () => { - mockUseGetExperimentsQuery.mockReturnValue({ - data: { - results: [makeExperiment({ featureId: 42, status: 'completed' })], - }, - isLoading: false, - } as any) - - const result = useFeatureExperimentFreeze(42, 'env-123') - - expect(result.isFrozen).toBe(false) - expect(result.experiment).toBeNull() - }) - it('returns isFrozen false when no experiments exist', () => { - mockUseGetExperimentsQuery.mockReturnValue({ - data: { results: [] }, - isLoading: false, - } as any) + mockUseGetExperimentsQuery + .mockReturnValueOnce(empty) + .mockReturnValueOnce(empty) const result = useFeatureExperimentFreeze(42, 'env-123') @@ -110,23 +85,21 @@ describe('useFeatureExperimentFreeze', () => { }) it('returns isFrozen false when experiment belongs to a different feature', () => { - mockUseGetExperimentsQuery.mockReturnValue({ - data: { - results: [makeExperiment({ featureId: 99, status: 'running' })], - }, - isLoading: false, - } as any) + mockUseGetExperimentsQuery + .mockReturnValueOnce( + withResults([makeExperiment({ featureId: 99, status: 'running' })]), + ) + .mockReturnValueOnce(empty) const result = useFeatureExperimentFreeze(42, 'env-123') expect(result.isFrozen).toBe(false) }) - it('returns isLoading true while experiments are loading', () => { - mockUseGetExperimentsQuery.mockReturnValue({ - data: undefined, - isLoading: true, - } as any) + it('returns isLoading true while either query is loading', () => { + mockUseGetExperimentsQuery + .mockReturnValueOnce(empty) + .mockReturnValueOnce(loading) const result = useFeatureExperimentFreeze(42, 'env-123') @@ -134,18 +107,43 @@ describe('useFeatureExperimentFreeze', () => { expect(result.isLoading).toBe(true) }) - it('skips query when featureId is undefined', () => { - mockUseGetExperimentsQuery.mockReturnValue({ - data: undefined, - isLoading: false, - } as any) + it('skips both queries when featureId is undefined', () => { + mockUseGetExperimentsQuery + .mockReturnValueOnce(empty) + .mockReturnValueOnce(empty) const result = useFeatureExperimentFreeze(undefined, 'env-123') expect(result.isFrozen).toBe(false) - expect(mockUseGetExperimentsQuery).toHaveBeenCalledWith( - expect.objectContaining({ environmentId: 'env-123' }), - expect.objectContaining({ skip: true }), + expect(mockUseGetExperimentsQuery).toHaveBeenCalledTimes(2) + expect(mockUseGetExperimentsQuery).toHaveBeenNthCalledWith( + 1, + { environmentId: 'env-123', status: 'running' }, + { skip: true }, + ) + expect(mockUseGetExperimentsQuery).toHaveBeenNthCalledWith( + 2, + { environmentId: 'env-123', status: 'paused' }, + { skip: true }, + ) + }) + + it('passes status filter to each query', () => { + mockUseGetExperimentsQuery + .mockReturnValueOnce(empty) + .mockReturnValueOnce(empty) + + useFeatureExperimentFreeze(42, 'env-123') + + expect(mockUseGetExperimentsQuery).toHaveBeenNthCalledWith( + 1, + { environmentId: 'env-123', status: 'running' }, + { skip: false }, + ) + expect(mockUseGetExperimentsQuery).toHaveBeenNthCalledWith( + 2, + { environmentId: 'env-123', status: 'paused' }, + { skip: false }, ) }) }) diff --git a/frontend/common/hooks/useFeatureExperimentFreeze.ts b/frontend/common/hooks/useFeatureExperimentFreeze.ts index 1e771fdc7c41..21674d03b084 100644 --- a/frontend/common/hooks/useFeatureExperimentFreeze.ts +++ b/frontend/common/hooks/useFeatureExperimentFreeze.ts @@ -1,31 +1,35 @@ import { useMemo } from 'react' import { useGetExperimentsQuery } from 'common/services/useExperiment' -import type { Experiment, ExperimentStatus } from 'common/types/responses' +import type { Experiment } from 'common/types/responses' -const FREEZE_STATUSES: ExperimentStatus[] = ['running', 'paused'] +export type FeatureExperimentFreeze = { + isFrozen: boolean + experiment: Experiment | null + isLoading: boolean +} export function useFeatureExperimentFreeze( featureId: number | undefined, environmentId: string, -): { - isFrozen: boolean - experiment: Experiment | null - isLoading: boolean -} { - const { data, isLoading } = useGetExperimentsQuery( - { environmentId, page_size: 100 }, - { skip: !featureId }, +): FeatureExperimentFreeze { + const skip = !featureId + const { data: runningData, isLoading: loadingRunning } = + useGetExperimentsQuery({ environmentId, status: 'running' }, { skip }) + const { data: pausedData, isLoading: loadingPaused } = useGetExperimentsQuery( + { environmentId, status: 'paused' }, + { skip }, ) + const isLoading = loadingRunning || loadingPaused + const experiment = useMemo(() => { - if (!featureId || !data?.results) return null - return ( - data.results.find( - (e) => - e.feature?.id === featureId && FREEZE_STATUSES.includes(e.status), - ) ?? null - ) - }, [data?.results, featureId]) + if (!featureId) return null + const all = [ + ...(runningData?.results ?? []), + ...(pausedData?.results ?? []), + ] + return all.find((e) => e.feature?.id === featureId) ?? null + }, [runningData?.results, pausedData?.results, featureId]) return { experiment, 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 ff0b32bc69ae..787186cd90ef 100644 --- a/frontend/web/components/modals/create-feature/index.tsx +++ b/frontend/web/components/modals/create-feature/index.tsx @@ -110,8 +110,10 @@ const CreateFeatureModal: FC = (props) => { } = props const flagId = props.environmentFlag?.id - const { experiment: freezingExperiment, isFrozen: experimentFrozen } = - useFeatureExperimentFreeze(props.projectFlag?.id, environmentId) + const freeze = useFeatureExperimentFreeze( + props.projectFlag?.id, + environmentId, + ) const [projectFlag, setProjectFlag] = useState(() => props.projectFlag @@ -659,8 +661,7 @@ const CreateFeatureModal: FC = (props) => { error={error} projectId={projectId} noPermissions={!!noPermissions} - experimentFrozen={experimentFrozen} - freezingExperiment={freezingExperiment} + freeze={freeze} featureState={environmentFlag} projectFlag={projectFlag} environmentFlag={props.environmentFlag} @@ -698,8 +699,7 @@ const CreateFeatureModal: FC = (props) => { = (props) => { = ({ - experimentFrozen, - freezingExperiment, + environmentId, + freeze, groupOwnerIds, hasMetadataRequired, identity, @@ -125,13 +127,12 @@ const FeatureSettingsTab: FC = ({ return (

- {experimentFrozen && freezingExperiment && ( - - This flag is part of the experiment{' '} - {freezingExperiment.name} which is currently{' '} - {freezingExperiment.status}. Some settings are restricted to prevent - skewing experiment results. - + {freeze?.isFrozen && freeze.experiment && environmentId && ( + )} {!identity && projectFlag?.tags && ( @@ -291,7 +292,7 @@ const FeatureSettingsTab: FC = ({ onChange={(is_server_key_only) => onChange({ ...projectFlag, is_server_key_only }) } - disabled={!!experimentFrozen} + disabled={!!freeze?.isFrozen || !!freeze?.isLoading} className='ml-0' /> = ({ onChange={(is_archived) => onChange({ ...projectFlag, is_archived }) } - disabled={!!experimentFrozen} + disabled={!!freeze?.isFrozen || !!freeze?.isLoading} className='ml-0' /> = ({ environmentName, error, existingChangeRequest, - experimentFrozen, featureState, - freezingExperiment, + freeze, identity, is4Eyes, isSaving, @@ -82,7 +81,8 @@ const FeatureValueTab: FC = ({ projectId, }) => { const isEdit = !!projectFlag?.id - const isDisabled = !!noPermissions || !!experimentFrozen + const isDisabled = + !!noPermissions || !!freeze?.isFrozen || !!freeze?.isLoading const { permission: createFeature } = useHasPermission({ id: projectId, @@ -300,13 +300,12 @@ const FeatureValueTab: FC = ({ return (
- {experimentFrozen && freezingExperiment && ( - - This flag is part of the experiment{' '} - {freezingExperiment.name} which is currently{' '} - {freezingExperiment.status}. Editing is restricted to prevent skewing - experiment results. - + {freeze?.isFrozen && freeze.experiment && environmentId && ( + )} = ({
)} - {environmentId && onSaveFeatureValue && !experimentFrozen && ( - <> - - - - - )} + {environmentId && + onSaveFeatureValue && + !freeze?.isFrozen && + !freeze?.isLoading && ( + <> + + + + + )}
) } diff --git a/frontend/web/components/modals/create-feature/tabs/SegmentOverridesTab.tsx b/frontend/web/components/modals/create-feature/tabs/SegmentOverridesTab.tsx index c5e67bbcb3b5..c0ecc8beaa74 100644 --- a/frontend/web/components/modals/create-feature/tabs/SegmentOverridesTab.tsx +++ b/frontend/web/components/modals/create-feature/tabs/SegmentOverridesTab.tsx @@ -11,7 +11,9 @@ import WarningMessage from 'components/WarningMessage' import ModalHR from 'components/modals/ModalHR' import FeatureInPipelineGuard from 'components/release-pipelines/FeatureInPipelineGuard' import Utils from 'common/utils/utils' -import { Experiment, ProjectFlag } from 'common/types/responses' +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,8 +36,7 @@ type SegmentOverridesTabProps = { error: any existingChangeRequest?: { id: number } noPermissions?: boolean - experimentFrozen?: boolean - freezingExperiment?: Experiment | null + freeze?: FeatureExperimentFreeze disableCreate?: boolean highlightSegmentId?: number } @@ -47,8 +48,7 @@ const SegmentOverridesTab: FC = ({ environmentId, error, existingChangeRequest, - experimentFrozen, - freezingExperiment, + freeze, highlightSegmentId, invalid, isSaving, @@ -164,7 +164,8 @@ const SegmentOverridesTab: FC = ({ !showCreateSegment && manageSegmentOverrides && !disableCreate && - !experimentFrozen && ( + !freeze?.isFrozen && + !freeze?.isLoading && (
)} - {experimentFrozen && freezingExperiment && ( - - This flag is part of the experiment{' '} - {freezingExperiment.name} which is currently{' '} - {freezingExperiment.status}. Editing is restricted to prevent - skewing experiment results. - + {freeze?.isFrozen && freeze.experiment && ( + )}

@@ -222,7 +223,11 @@ const SegmentOverridesTab: FC = ({ = ({

)} - {!isComparing && !showCreateSegment && !experimentFrozen && ( - - )} - {!isComparing && !showCreateSegment && !experimentFrozen && ( -
-

- Re-order overrides to adjust priority. -

-

- {is4Eyes && isVersioned - ? 'This will create a change request with any value and segment override changes for the environment' - : 'This will update the segment overrides for the environment'}{' '} - {environmentName} -

- {is4Eyes && !isVersioned && ( - - Enable Feature Versioning to gate segment overrides with - Feature Change Requests.{' '} - - Learn more - - . - - )} -
- {isVersioned && is4Eyes - ? Utils.renderWithPermission( - savePermission, - Utils.getManageFeaturePermissionDescription(is4Eyes), - , - ) - : Utils.renderWithPermission( - manageSegmentOverrides, - Constants.environmentPermissions( - EnvironmentPermission.MANAGE_SEGMENT_OVERRIDES, - ), - <> - {!is4Eyes && isVersioned && ( - <> - - - )} + {!isComparing && + !showCreateSegment && + !freeze?.isFrozen && + !freeze?.isLoading && } + {!isComparing && + !showCreateSegment && + !freeze?.isFrozen && + !freeze?.isLoading && ( +
+

+ Re-order overrides to adjust priority. +

+

+ {is4Eyes && isVersioned + ? 'This will create a change request with any value and segment override changes for the environment' + : 'This will update the segment overrides for the environment'}{' '} + {environmentName} +

+ {is4Eyes && !isVersioned && ( + + Enable Feature Versioning to gate segment overrides with + Feature Change Requests.{' '} + + Learn more + + . + + )} +
+ {isVersioned && is4Eyes + ? Utils.renderWithPermission( + savePermission, + Utils.getManageFeaturePermissionDescription(is4Eyes), - , - )} + {getButtonText()} + , + ) + : Utils.renderWithPermission( + manageSegmentOverrides, + Constants.environmentPermissions( + EnvironmentPermission.MANAGE_SEGMENT_OVERRIDES, + ), + <> + {!is4Eyes && isVersioned && ( + <> + + + )} + + , + )} +
-
- )} + )}
From d3da974f6cb8e22944ae71d046bcc760c24abfe5 Mon Sep 17 00:00:00 2001 From: wadii Date: Fri, 26 Jun 2026 14:09:11 +0200 Subject: [PATCH 3/4] feat: freeze only running experiments --- .../useFeatureExperimentFreeze.test.ts | 68 +++++-------------- .../hooks/useFeatureExperimentFreeze.ts | 18 ++--- 2 files changed, 21 insertions(+), 65 deletions(-) diff --git a/frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts b/frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts index 0a9f095ed8b3..8f9651ee1fb5 100644 --- a/frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts +++ b/frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts @@ -48,11 +48,9 @@ describe('useFeatureExperimentFreeze', () => { }) it('returns isFrozen true when a running experiment exists for the feature', () => { - mockUseGetExperimentsQuery - .mockReturnValueOnce( - withResults([makeExperiment({ featureId: 42, status: 'running' })]), - ) - .mockReturnValueOnce(empty) + mockUseGetExperimentsQuery.mockReturnValue( + withResults([makeExperiment({ featureId: 42, status: 'running' })]), + ) const result = useFeatureExperimentFreeze(42, 'env-123') @@ -61,22 +59,8 @@ describe('useFeatureExperimentFreeze', () => { expect(result.isLoading).toBe(false) }) - it('returns isFrozen true when a paused experiment exists for the feature', () => { - mockUseGetExperimentsQuery - .mockReturnValueOnce(empty) - .mockReturnValueOnce( - withResults([makeExperiment({ featureId: 42, status: 'paused' })]), - ) - - const result = useFeatureExperimentFreeze(42, 'env-123') - - expect(result.isFrozen).toBe(true) - }) - it('returns isFrozen false when no experiments exist', () => { - mockUseGetExperimentsQuery - .mockReturnValueOnce(empty) - .mockReturnValueOnce(empty) + mockUseGetExperimentsQuery.mockReturnValue(empty) const result = useFeatureExperimentFreeze(42, 'env-123') @@ -85,21 +69,17 @@ describe('useFeatureExperimentFreeze', () => { }) it('returns isFrozen false when experiment belongs to a different feature', () => { - mockUseGetExperimentsQuery - .mockReturnValueOnce( - withResults([makeExperiment({ featureId: 99, status: 'running' })]), - ) - .mockReturnValueOnce(empty) + mockUseGetExperimentsQuery.mockReturnValue( + withResults([makeExperiment({ featureId: 99, status: 'running' })]), + ) const result = useFeatureExperimentFreeze(42, 'env-123') expect(result.isFrozen).toBe(false) }) - it('returns isLoading true while either query is loading', () => { - mockUseGetExperimentsQuery - .mockReturnValueOnce(empty) - .mockReturnValueOnce(loading) + it('returns isLoading true while the query is loading', () => { + mockUseGetExperimentsQuery.mockReturnValue(loading) const result = useFeatureExperimentFreeze(42, 'env-123') @@ -107,43 +87,27 @@ describe('useFeatureExperimentFreeze', () => { expect(result.isLoading).toBe(true) }) - it('skips both queries when featureId is undefined', () => { - mockUseGetExperimentsQuery - .mockReturnValueOnce(empty) - .mockReturnValueOnce(empty) + it('skips query when featureId is undefined', () => { + mockUseGetExperimentsQuery.mockReturnValue(empty) const result = useFeatureExperimentFreeze(undefined, 'env-123') expect(result.isFrozen).toBe(false) - expect(mockUseGetExperimentsQuery).toHaveBeenCalledTimes(2) - expect(mockUseGetExperimentsQuery).toHaveBeenNthCalledWith( - 1, + expect(mockUseGetExperimentsQuery).toHaveBeenCalledWith( { environmentId: 'env-123', status: 'running' }, { skip: true }, ) - expect(mockUseGetExperimentsQuery).toHaveBeenNthCalledWith( - 2, - { environmentId: 'env-123', status: 'paused' }, - { skip: true }, - ) }) - it('passes status filter to each query', () => { - mockUseGetExperimentsQuery - .mockReturnValueOnce(empty) - .mockReturnValueOnce(empty) + it('queries only running experiments', () => { + mockUseGetExperimentsQuery.mockReturnValue(empty) useFeatureExperimentFreeze(42, 'env-123') - expect(mockUseGetExperimentsQuery).toHaveBeenNthCalledWith( - 1, + expect(mockUseGetExperimentsQuery).toHaveBeenCalledTimes(1) + expect(mockUseGetExperimentsQuery).toHaveBeenCalledWith( { environmentId: 'env-123', status: 'running' }, { skip: false }, ) - expect(mockUseGetExperimentsQuery).toHaveBeenNthCalledWith( - 2, - { environmentId: 'env-123', status: 'paused' }, - { skip: false }, - ) }) }) diff --git a/frontend/common/hooks/useFeatureExperimentFreeze.ts b/frontend/common/hooks/useFeatureExperimentFreeze.ts index 21674d03b084..c75eb90112a2 100644 --- a/frontend/common/hooks/useFeatureExperimentFreeze.ts +++ b/frontend/common/hooks/useFeatureExperimentFreeze.ts @@ -13,23 +13,15 @@ export function useFeatureExperimentFreeze( environmentId: string, ): FeatureExperimentFreeze { const skip = !featureId - const { data: runningData, isLoading: loadingRunning } = - useGetExperimentsQuery({ environmentId, status: 'running' }, { skip }) - const { data: pausedData, isLoading: loadingPaused } = useGetExperimentsQuery( - { environmentId, status: 'paused' }, + const { data, isLoading } = useGetExperimentsQuery( + { environmentId, status: 'running' }, { skip }, ) - const isLoading = loadingRunning || loadingPaused - const experiment = useMemo(() => { - if (!featureId) return null - const all = [ - ...(runningData?.results ?? []), - ...(pausedData?.results ?? []), - ] - return all.find((e) => e.feature?.id === featureId) ?? null - }, [runningData?.results, pausedData?.results, featureId]) + if (!featureId || !data?.results) return null + return data.results.find((e) => e.feature?.id === featureId) ?? null + }, [data?.results, featureId]) return { experiment, From c23d3eef78080b5cad6ed4a2c0129631d438f489 Mon Sep 17 00:00:00 2001 From: wadii Date: Fri, 26 Jun 2026 15:34:40 +0200 Subject: [PATCH 4/4] fix: avoid layout shift during freeze loading and skip query without environmentId --- .../useFeatureExperimentFreeze.test.ts | 12 ++ .../hooks/useFeatureExperimentFreeze.ts | 2 +- .../create-feature/tabs/FeatureValueTab.tsx | 67 ++++--- .../tabs/SegmentOverridesTab.tsx | 169 +++++++++--------- 4 files changed, 129 insertions(+), 121 deletions(-) diff --git a/frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts b/frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts index 8f9651ee1fb5..096e89954ce4 100644 --- a/frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts +++ b/frontend/common/hooks/__tests__/useFeatureExperimentFreeze.test.ts @@ -99,6 +99,18 @@ describe('useFeatureExperimentFreeze', () => { ) }) + 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) diff --git a/frontend/common/hooks/useFeatureExperimentFreeze.ts b/frontend/common/hooks/useFeatureExperimentFreeze.ts index c75eb90112a2..1d603c077472 100644 --- a/frontend/common/hooks/useFeatureExperimentFreeze.ts +++ b/frontend/common/hooks/useFeatureExperimentFreeze.ts @@ -12,7 +12,7 @@ export function useFeatureExperimentFreeze( featureId: number | undefined, environmentId: string, ): FeatureExperimentFreeze { - const skip = !featureId + const skip = !featureId || !environmentId const { data, isLoading } = useGetExperimentsQuery( { environmentId, status: 'running' }, { skip }, diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx index a9b7016590a6..b8a07fa1d32c 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx @@ -506,41 +506,38 @@ const FeatureValueTab: FC = ({
)} - {environmentId && - onSaveFeatureValue && - !freeze?.isFrozen && - !freeze?.isLoading && ( - <> - - - - - )} + {environmentId && onSaveFeatureValue && !freeze?.isFrozen && ( + <> + + + + + )}
) } diff --git a/frontend/web/components/modals/create-feature/tabs/SegmentOverridesTab.tsx b/frontend/web/components/modals/create-feature/tabs/SegmentOverridesTab.tsx index c0ecc8beaa74..5675ad880b62 100644 --- a/frontend/web/components/modals/create-feature/tabs/SegmentOverridesTab.tsx +++ b/frontend/web/components/modals/create-feature/tabs/SegmentOverridesTab.tsx @@ -250,43 +250,82 @@ const SegmentOverridesTab: FC = ({
)} - {!isComparing && - !showCreateSegment && - !freeze?.isFrozen && - !freeze?.isLoading && } - {!isComparing && - !showCreateSegment && - !freeze?.isFrozen && - !freeze?.isLoading && ( -
-

- Re-order overrides to adjust priority. -

-

- {is4Eyes && isVersioned - ? 'This will create a change request with any value and segment override changes for the environment' - : 'This will update the segment overrides for the environment'}{' '} - {environmentName} -

- {is4Eyes && !isVersioned && ( - - Enable Feature Versioning to gate segment overrides with - Feature Change Requests.{' '} - - Learn more - - . - - )} -
- {isVersioned && is4Eyes - ? Utils.renderWithPermission( - savePermission, - Utils.getManageFeaturePermissionDescription(is4Eyes), + {!isComparing && !showCreateSegment && !freeze?.isFrozen && ( + + )} + {!isComparing && !showCreateSegment && !freeze?.isFrozen && ( +
+

+ Re-order overrides to adjust priority. +

+

+ {is4Eyes && isVersioned + ? 'This will create a change request with any value and segment override changes for the environment' + : 'This will update the segment overrides for the environment'}{' '} + {environmentName} +

+ {is4Eyes && !isVersioned && ( + + Enable Feature Versioning to gate segment overrides with + Feature Change Requests.{' '} + + Learn more + + . + + )} +
+ {isVersioned && is4Eyes + ? Utils.renderWithPermission( + savePermission, + Utils.getManageFeaturePermissionDescription(is4Eyes), + , + ) + : Utils.renderWithPermission( + manageSegmentOverrides, + Constants.environmentPermissions( + EnvironmentPermission.MANAGE_SEGMENT_OVERRIDES, + ), + <> + {!is4Eyes && isVersioned && ( + <> + + + )} , - ) - : Utils.renderWithPermission( - manageSegmentOverrides, - Constants.environmentPermissions( - EnvironmentPermission.MANAGE_SEGMENT_OVERRIDES, - ), - <> - {!is4Eyes && isVersioned && ( - <> - - - )} - - , - )} -
+ {isSaving ? 'Updating' : 'Update Segment Overrides'} + + , + )}
- )} +
+ )}