diff --git a/frontend/src/scenes/experiments/ExperimentView/components.tsx b/frontend/src/scenes/experiments/ExperimentView/components.tsx index ec5c911542c7..5e0a071a5c32 100644 --- a/frontend/src/scenes/experiments/ExperimentView/components.tsx +++ b/frontend/src/scenes/experiments/ExperimentView/components.tsx @@ -37,6 +37,7 @@ import { PropertyFilterButton } from 'lib/components/PropertyFilters/components/ import { superpowersLogic } from 'lib/components/Superpowers/superpowersLogic' import { useOnMountEffect } from 'lib/hooks/useOnMountEffect' import { usePageVisibility } from 'lib/hooks/usePageVisibility' +import { LemonRadio } from 'lib/lemon-ui/LemonRadio' import { ButtonPrimitive } from 'lib/ui/Button/ButtonPrimitives' import { userHasAccess } from 'lib/utils/accessControlUtils' import { addProductIntentForCrossSell } from 'lib/utils/product-intents' @@ -619,6 +620,8 @@ export function ResumeExperimentModal(): JSX.Element { ) } +type FinishExperimentRolloutChoice = 'keep' | 'rollout' + export function FinishExperimentModal(): JSX.Element { const { experiment, isSingleVariantShipped, shippedVariantKey } = useValues(experimentLogic) const { finishExperiment, endExperimentWithoutShipping, restoreUnmodifiedExperiment } = useActions(experimentLogic) @@ -626,122 +629,144 @@ export function FinishExperimentModal(): JSX.Element { const { isFinishExperimentModalOpen } = useValues(modalsLogic) const { aggregationLabel } = useValues(groupsModel) - const [selectedVariantKey, setSelectedVariantKey] = useState() + // Default to the safe option: don't touch the feature flag. Users must explicitly + // opt in to rolling out a variant — otherwise ending an experiment can silently + // change which users see which variant, which has caused production incidents. + const [rolloutChoice, setRolloutChoice] = useState('keep') + const [selectedVariantKey, setSelectedVariantKey] = useState(null) useEffect(() => { - if (experiment.parameters?.feature_flag_variants?.length > 1) { - // First test variant selected by default - setSelectedVariantKey(experiment.parameters.feature_flag_variants[1].key) + if (isFinishExperimentModalOpen) { + setRolloutChoice('keep') + setSelectedVariantKey(null) } - }, [ - experiment.id, - experiment.parameters?.feature_flag_variants?.length, - experiment.parameters?.feature_flag_variants, - ]) + }, [isFinishExperimentModalOpen, experiment.id]) const aggregationTargetName = experiment.filters.aggregation_group_type_index != null ? aggregationLabel(experiment.filters.aggregation_group_type_index).plural : 'users' + const willModifyFlag = !isSingleVariantShipped && rolloutChoice === 'rollout' + const handleEndExperiment = (): void => { - if (isSingleVariantShipped || !selectedVariantKey) { - endExperimentWithoutShipping() - } else { + if (willModifyFlag && selectedVariantKey) { finishExperiment({ selectedVariantKey }) + } else { + endExperimentWithoutShipping() } } + const handleClose = (): void => { + restoreUnmodifiedExperiment() + closeFinishExperimentModal() + } + + const disabledReason = !experiment.conclusion + ? 'Select a conclusion' + : willModifyFlag && !selectedVariantKey + ? 'Select a variant to roll out' + : undefined + return ( - <> - { - restoreUnmodifiedExperiment() - closeFinishExperimentModal() - }} - width={600} - title="End experiment" - footer={ -
- { - restoreUnmodifiedExperiment() - closeFinishExperimentModal() - }} - > - Cancel - - - End experiment - -
- } - > -
- {isSingleVariantShipped ? ( -
- - - - {' '} - is already rolled out to 100% of {aggregationTargetName}. Ending this experiment will - mark it as complete without changing the feature flag. - -
- ) : ( -
- Variant to keep -
- The selected variant will be rolled out to 100% of {aggregationTargetName}. -
-
- { - setSelectedVariantKey(variantKey) - }} - allowClear={true} - options={ - experiment.feature_flag?.filters.multivariate?.variants?.map(({ key }) => ({ - value: key, - label: ( -
- -
- ), - })) || [] - } - /> -
-
- )} - - {!isSingleVariantShipped && ( - - For more precise control over your release, adjust the rollout percentage and release - conditions in the{' '} - - {experiment.feature_flag?.key} - {' '} - feature flag. - - )} + + + Cancel + + + End experiment +
-
- + } + > +
+ {isSingleVariantShipped ? ( + + + + {' '} + is already rolled out to 100% of {aggregationTargetName}. Ending this experiment will mark it as + complete without changing the feature flag. + + ) : ( +
+ What should happen to the feature flag? + setRolloutChoice(value)} + options={[ + { + value: 'keep', + label: 'Keep feature flag as-is', + description: `${aggregationTargetName.charAt(0).toUpperCase()}${aggregationTargetName.slice( + 1 + )} continue to see their currently assigned variant. The feature flag will not be modified.`, + 'data-attr': 'end-experiment-keep-flag', + }, + { + value: 'rollout', + label: 'Roll out a variant to 100%', + description: `The selected variant will be served to all ${aggregationTargetName}. This will modify the feature flag.`, + 'data-attr': 'end-experiment-rollout-variant', + }, + ]} + /> + {rolloutChoice === 'rollout' && ( +
+ Variant to roll out +
+ setSelectedVariantKey(variantKey)} + options={ + experiment.feature_flag?.filters.multivariate?.variants?.map(({ key }) => ({ + value: key, + label: ( +
+ +
+ ), + })) || [] + } + /> +
+
+ )} +
+ )} + + {willModifyFlag && ( + + For more precise control over your release, adjust the rollout percentage and release conditions + in the{' '} + + {experiment.feature_flag?.key} + {' '} + feature flag after ending the experiment. + + )} +
+ ) }