diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/DisableEnableStrategy/DisableEnableStrategy.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/DisableEnableStrategy/DisableEnableStrategy.tsx index 7b2dd4b143..e7e457fa66 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/DisableEnableStrategy/DisableEnableStrategy.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/DisableEnableStrategy/DisableEnableStrategy.tsx @@ -11,14 +11,16 @@ import { useChangeRequestsEnabled } from 'hooks/useChangeRequestsEnabled'; import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; import { FeatureStrategyChangeRequestAlert } from 'component/feature/FeatureStrategy/FeatureStrategyForm/FeatureStrategyChangeRequestAlert/FeatureStrategyChangeRequestAlert'; import { IDisableEnableStrategyProps } from './IDisableEnableStrategyProps'; +import { useFeature } from 'hooks/api/getters/useFeature/useFeature'; const DisableStrategy: VFC = ({ ...props }) => { - const { projectId, environmentId } = props; + const { projectId, environmentId, featureId } = props; const [isDialogueOpen, setDialogueOpen] = useState(false); const { onDisable } = useEnableDisable({ ...props }); const { onSuggestDisable } = useSuggestEnableDisable({ ...props }); const { isChangeRequestConfigured } = useChangeRequestsEnabled(projectId); const isChangeRequest = isChangeRequestConfigured(environmentId); + const { refetchFeature } = useFeature(projectId, featureId); const onClick = (event: React.FormEvent) => { event.preventDefault(); @@ -26,6 +28,7 @@ const DisableStrategy: VFC = ({ ...props }) => { onSuggestDisable(); } else { onDisable(); + refetchFeature(); } setDialogueOpen(false); }; diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewSidePanel/FeatureOverviewSidePanelEnvironmentSwitches/FeatureOverviewSidePanelEnvironmentSwitch/EnableEnvironmentDialog.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewSidePanel/FeatureOverviewSidePanelEnvironmentSwitches/FeatureOverviewSidePanelEnvironmentSwitch/EnableEnvironmentDialog.tsx new file mode 100644 index 0000000000..983e51bab4 --- /dev/null +++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewSidePanel/FeatureOverviewSidePanelEnvironmentSwitches/FeatureOverviewSidePanelEnvironmentSwitch/EnableEnvironmentDialog.tsx @@ -0,0 +1,73 @@ +import { FC } from 'react'; +import { Typography } from '@mui/material'; +import { Dialogue } from 'component/common/Dialogue/Dialogue'; +import { useRequiredPathParam } from 'hooks/useRequiredPathParam'; +import PermissionButton from 'component/common/PermissionButton/PermissionButton'; +import { UPDATE_FEATURE } from 'component/providers/AccessProvider/permissions'; + +interface IEnableEnvironmentDialogProps { + isOpen: boolean; + onActivateDisabledStrategies: () => void; + onAddDefaultStrategy: () => void; + onClose: () => void; + environment?: string; + showBanner?: boolean; + disabledStrategiesCount: number; +} + +export const EnableEnvironmentDialog: FC = ({ + isOpen, + onAddDefaultStrategy, + onActivateDisabledStrategies, + onClose, + environment, + disabledStrategiesCount = 0, +}) => { + const projectId = useRequiredPathParam('projectId'); + + return ( + + + Add default strategy + + + Enable all strategies + + + } + onClose={onClose} + title="Enable feature toggle" + fullWidth + > + theme.spacing(2) }} + > + The feature toggle has {disabledStrategiesCount} disabled + {disabledStrategiesCount === 1 ? ' strategy' : ' strategies'}. + + + You can choose to enable all the disabled strategies or you can + add the default strategy to enable this feature toggle. + + + ); +}; diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewSidePanel/FeatureOverviewSidePanelEnvironmentSwitches/FeatureOverviewSidePanelEnvironmentSwitch/FeatureOverviewSidePanelEnvironmentSwitch.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewSidePanel/FeatureOverviewSidePanelEnvironmentSwitches/FeatureOverviewSidePanelEnvironmentSwitch/FeatureOverviewSidePanelEnvironmentSwitch.tsx index 2bd6b700f9..a7d740c8b5 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewSidePanel/FeatureOverviewSidePanelEnvironmentSwitches/FeatureOverviewSidePanelEnvironmentSwitch/FeatureOverviewSidePanelEnvironmentSwitch.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewSidePanel/FeatureOverviewSidePanelEnvironmentSwitches/FeatureOverviewSidePanelEnvironmentSwitch/FeatureOverviewSidePanelEnvironmentSwitch.tsx @@ -14,6 +14,8 @@ import { useChangeRequestsEnabled } from 'hooks/useChangeRequestsEnabled'; import { styled } from '@mui/material'; import StringTruncator from 'component/common/StringTruncator/StringTruncator'; import { FeatureOverviewSidePanelEnvironmentHider } from './FeatureOverviewSidePanelEnvironmentHider'; +import { useState } from 'react'; +import { EnableEnvironmentDialog } from './EnableEnvironmentDialog'; const StyledContainer = styled('div')(({ theme }) => ({ marginLeft: theme.spacing(-1.5), @@ -53,7 +55,7 @@ export const FeatureOverviewSidePanelEnvironmentSwitch = ({ const featureId = useRequiredPathParam('featureId'); const { toggleFeatureEnvironmentOn, toggleFeatureEnvironmentOff } = useFeatureApi(); - const { refetchFeature } = useFeature(projectId, featureId); + const { feature, refetchFeature } = useFeature(projectId, featureId); const { setToastData, setToastApiError } = useToast(); const { isChangeRequestConfigured } = useChangeRequestsEnabled(projectId); const { @@ -63,9 +65,20 @@ export const FeatureOverviewSidePanelEnvironmentSwitch = ({ changeRequestDialogDetails, } = useChangeRequestToggle(projectId); - const handleToggleEnvironmentOn = async () => { + const [showEnabledDialog, setShowEnabledDialog] = useState(false); + const disabledStrategiesCount = environment.strategies.filter( + strategy => strategy.disabled + ).length; + const handleToggleEnvironmentOn = async ( + shouldActivateDisabled = false + ) => { try { - await toggleFeatureEnvironmentOn(projectId, featureId, name); + await toggleFeatureEnvironmentOn( + projectId, + featureId, + name, + shouldActivateDisabled + ); setToastData({ type: 'success', title: `Available in ${name}`, @@ -114,7 +127,23 @@ export const FeatureOverviewSidePanelEnvironmentSwitch = ({ await handleToggleEnvironmentOff(); return; } - await handleToggleEnvironmentOn(); + + if (featureHasOnlyDisabledStrategies()) { + setShowEnabledDialog(true); + } else { + await handleToggleEnvironmentOn(); + } + }; + + const featureHasOnlyDisabledStrategies = () => { + const featureEnvironment = feature?.environments?.find( + env => env.name === name + ); + return ( + featureEnvironment?.strategies && + featureEnvironment?.strategies?.length > 0 && + featureEnvironment?.strategies?.every(strategy => strategy.disabled) + ); }; const defaultContent = ( @@ -126,6 +155,16 @@ export const FeatureOverviewSidePanelEnvironmentSwitch = ({ ); + const onActivateStrategies = async () => { + await handleToggleEnvironmentOn(true); + setShowEnabledDialog(false); + }; + + const onAddDefaultStrategy = async () => { + await handleToggleEnvironmentOn(); + setShowEnabledDialog(false); + }; + return ( @@ -161,6 +200,14 @@ export const FeatureOverviewSidePanelEnvironmentSwitch = ({ /> } /> + setShowEnabledDialog(false)} + environment={name} + disabledStrategiesCount={disabledStrategiesCount} + onActivateDisabledStrategies={onActivateStrategies} + onAddDefaultStrategy={onAddDefaultStrategy} + /> ); }; diff --git a/frontend/src/component/project/Project/ProjectSettings/ProjectDefaultStrategySettings/ProjectEnvironment/ProjectEnvironmentDefaultStrategy/EditDefaultStrategy.tsx b/frontend/src/component/project/Project/ProjectSettings/ProjectDefaultStrategySettings/ProjectEnvironment/ProjectEnvironmentDefaultStrategy/EditDefaultStrategy.tsx index 1480682aba..1e5e0e550d 100644 --- a/frontend/src/component/project/Project/ProjectSettings/ProjectDefaultStrategySettings/ProjectEnvironment/ProjectEnvironmentDefaultStrategy/EditDefaultStrategy.tsx +++ b/frontend/src/component/project/Project/ProjectSettings/ProjectDefaultStrategySettings/ProjectEnvironment/ProjectEnvironmentDefaultStrategy/EditDefaultStrategy.tsx @@ -21,7 +21,7 @@ import { CreateFeatureStrategySchema } from 'openapi'; import useProject from 'hooks/api/getters/useProject/useProject'; interface EditDefaultStrategyProps { - strategy: IFeatureStrategy | CreateFeatureStrategySchema; + strategy: CreateFeatureStrategySchema; } const EditDefaultStrategy = ({ strategy }: EditDefaultStrategyProps) => { @@ -30,9 +30,8 @@ const EditDefaultStrategy = ({ strategy }: EditDefaultStrategyProps) => { const { refetch: refetchProject } = useProject(projectId); - const [defaultStrategy, setDefaultStrategy] = useState< - Partial | CreateFeatureStrategySchema - >(strategy); + const [defaultStrategy, setDefaultStrategy] = + useState(strategy); const [segments, setSegments] = useState([]); const { updateDefaultStrategy, loading } = useProjectApi(); @@ -161,7 +160,7 @@ const EditDefaultStrategy = ({ strategy }: EditDefaultStrategyProps) => { @@ -81,7 +81,7 @@ const ProjectEnvironmentDefaultStrategy = ({ } > - + - + } /> diff --git a/frontend/src/hooks/api/actions/useFeatureApi/useFeatureApi.ts b/frontend/src/hooks/api/actions/useFeatureApi/useFeatureApi.ts index e6918827ba..c07538bb04 100644 --- a/frontend/src/hooks/api/actions/useFeatureApi/useFeatureApi.ts +++ b/frontend/src/hooks/api/actions/useFeatureApi/useFeatureApi.ts @@ -51,8 +51,13 @@ const useFeatureApi = () => { }; const toggleFeatureEnvironmentOn = useCallback( - async (projectId: string, featureId: string, environmentId: string) => { - const path = `api/admin/projects/${projectId}/features/${featureId}/environments/${environmentId}/on`; + async ( + projectId: string, + featureId: string, + environmentId: string, + shouldActivateDisabledStrategies = false + ) => { + const path = `api/admin/projects/${projectId}/features/${featureId}/environments/${environmentId}/on?shouldActivateDisabledStrategies=${shouldActivateDisabledStrategies}`; const req = createRequest( path, { method: 'POST' }, diff --git a/src/lib/db/feature-toggle-client-store.ts b/src/lib/db/feature-toggle-client-store.ts index 9051d0cbbc..2870449680 100644 --- a/src/lib/db/feature-toggle-client-store.ts +++ b/src/lib/db/feature-toggle-client-store.ts @@ -55,7 +55,6 @@ export default class FeatureToggleClientStore archived, isAdmin, includeStrategyIds, - includeDisabledStrategies, userId, }: IGetAllFeatures): Promise { const environment = featureQuery?.environment || DEFAULT_ENV; @@ -169,7 +168,7 @@ export default class FeatureToggleClientStore let feature: PartialDeep = acc[r.name] ?? { strategies: [], }; - if (this.isUnseenStrategyRow(feature, r)) { + if (this.isUnseenStrategyRow(feature, r) && !r.strategy_disabled) { feature.strategies?.push( FeatureToggleClientStore.rowToStrategy(r), ); @@ -211,12 +210,6 @@ export default class FeatureToggleClientStore FeatureToggleClientStore.removeIdsFromStrategies(features); } - if (!includeDisabledStrategies) { - // We should not send disabled strategies from the client API, - // as this breaks the way SDKs evaluate the status of the feature. - return this.removeDisabledStrategies(features); - } - return features; } @@ -225,7 +218,6 @@ export default class FeatureToggleClientStore id: row.strategy_id, name: row.strategy_name, title: row.strategy_title, - disabled: row.strategy_disabled, constraints: row.constraints || [], parameters: mapValues(row.parameters || {}, ensureStringValue), }; @@ -246,27 +238,6 @@ export default class FeatureToggleClientStore }); } - private removeDisabledStrategies( - features: IFeatureToggleClient[], - ): IFeatureToggleClient[] { - const filtered: IFeatureToggleClient[] = []; - features.forEach((feature) => { - let { enabled } = feature; - const filteredStrategies = feature.strategies.filter( - (strategy) => !strategy.disabled, - ); - if (!filteredStrategies.length) { - enabled = false; - } - filtered.push({ - ...feature, - enabled, - strategies: filteredStrategies, - }); - }); - return filtered; - } - private isUnseenStrategyRow( feature: PartialDeep, row: Record, diff --git a/src/lib/routes/admin-api/project/project-features.ts b/src/lib/routes/admin-api/project/project-features.ts index 718ade1543..e6e81ca87a 100644 --- a/src/lib/routes/admin-api/project/project-features.ts +++ b/src/lib/routes/admin-api/project/project-features.ts @@ -49,6 +49,10 @@ interface FeatureStrategyParams { sortOrder?: number; } +interface FeatureStrategyQuery { + shouldActivateDisabledStrategies: string; +} + interface FeatureParams extends ProjectParam { featureName: string; } @@ -641,10 +645,16 @@ export default class ProjectFeaturesController extends Controller { } async toggleFeatureEnvironmentOn( - req: IAuthRequest, + req: IAuthRequest< + FeatureStrategyParams, + any, + any, + FeatureStrategyQuery + >, res: Response, ): Promise { const { featureName, environment, projectId } = req.params; + const { shouldActivateDisabledStrategies } = req.query; await this.featureService.updateEnabled( projectId, featureName, @@ -652,6 +662,7 @@ export default class ProjectFeaturesController extends Controller { true, extractUsername(req), req.user, + shouldActivateDisabledStrategies === 'true', ); res.status(200).end(); } @@ -762,6 +773,7 @@ export default class ProjectFeaturesController extends Controller { const userName = extractUsername(req); const patch = req.body; const strategy = await this.featureService.getStrategy(strategyId); + const { newDocument } = applyPatch(strategy, patch); const updatedStrategy = await this.featureService.updateStrategy( strategyId, @@ -770,6 +782,21 @@ export default class ProjectFeaturesController extends Controller { userName, req.user, ); + const feature = await this.featureService.getFeature({ featureName }); + + const env = feature.environments.find((e) => e.name === environment); + const hasOnlyDisabledStrategies = env!.strategies.every( + (strat) => strat.disabled, + ); + if (hasOnlyDisabledStrategies) { + await this.featureService.updateEnabled( + projectId, + featureName, + environment, + false, + userName, + ); + } res.status(200).json(updatedStrategy); } diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts index 6c0f311469..f786b8930d 100644 --- a/src/lib/services/feature-toggle-service.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -648,6 +648,28 @@ class FeatureToggleService { await this.featureStrategiesStore.delete(id); + const featureStrategies = + await this.featureStrategiesStore.getStrategiesForFeatureEnv( + projectId, + featureName, + environment, + ); + + const hasOnlyDisabledStrategies = featureStrategies.every( + (strategy) => strategy.disabled, + ); + + if (hasOnlyDisabledStrategies) { + // Disable the feature in the environment if it only has disabled strategies + await this.updateEnabled( + projectId, + featureName, + environment, + false, + createdBy, + ); + } + const tags = await this.tagStore.getAllTagsForFeature(featureName); const preData = this.featureStrategyToPublic(existingStrategy); @@ -1074,7 +1096,7 @@ class FeatureToggleService { environment, enabled: envMetadata.enabled, strategies, - defaultStrategy: defaultStrategy, + defaultStrategy, }; } @@ -1242,6 +1264,7 @@ class FeatureToggleService { enabled: boolean, createdBy: string, user?: User, + shouldActivateDisabledStrategies = false, ): Promise { await this.stopWhenChangeRequestsEnabled(project, environment, user); if (enabled) { @@ -1259,6 +1282,7 @@ class FeatureToggleService { environment, enabled, createdBy, + shouldActivateDisabledStrategies, ); } @@ -1268,6 +1292,7 @@ class FeatureToggleService { environment: string, enabled: boolean, createdBy: string, + shouldActivateDisabledStrategies: boolean, ): Promise { const hasEnvironment = await this.featureEnvironmentStore.featureHasEnvironment( @@ -1287,22 +1312,51 @@ class FeatureToggleService { featureName, environment, ); - const projectEnvironmentDefaultStrategy = - await this.projectStore.getDefaultStrategy( - project, - environment, - ); + const hasDisabledStrategies = strategies.some( + (strategy) => strategy.disabled, + ); - const strategy = + if ( this.flagResolver.isEnabled('strategyImprovements') && - projectEnvironmentDefaultStrategy != null - ? getProjectDefaultStrategy( - projectEnvironmentDefaultStrategy, - featureName, - ) - : getDefaultStrategy(featureName); + hasDisabledStrategies && + shouldActivateDisabledStrategies + ) { + strategies.map(async (strategy) => { + return this.updateStrategy( + strategy.id, + { disabled: false }, + { + environment, + projectId: project, + featureName, + }, + createdBy, + ); + }); + } + + const hasOnlyDisabledStrategies = strategies.every( + (strategy) => strategy.disabled, + ); + + const shouldCreate = + hasOnlyDisabledStrategies && !shouldActivateDisabledStrategies; + + if (strategies.length === 0 || shouldCreate) { + const projectEnvironmentDefaultStrategy = + await this.projectStore.getDefaultStrategy( + project, + environment, + ); + const strategy = + this.flagResolver.isEnabled('strategyImprovements') && + projectEnvironmentDefaultStrategy != null + ? getProjectDefaultStrategy( + projectEnvironmentDefaultStrategy, + featureName, + ) + : getDefaultStrategy(featureName); - if (strategies.length === 0) { await this.unprotectedCreateStrategy( strategy, { diff --git a/src/lib/types/model.ts b/src/lib/types/model.ts index ef49e46996..3486563c8b 100644 --- a/src/lib/types/model.ts +++ b/src/lib/types/model.ts @@ -73,7 +73,7 @@ export interface IFeatureToggleClient { stale: boolean; variants: IVariant[]; enabled: boolean; - strategies: IStrategyConfig[]; + strategies: Omit[]; impressionData?: boolean; lastSeenAt?: Date; createdAt?: Date; @@ -86,7 +86,7 @@ export interface IFeatureEnvironmentInfo { environment: string; enabled: boolean; strategies: IFeatureStrategy[]; - defaultStrategy?: CreateFeatureStrategySchema | null; + defaultStrategy: CreateFeatureStrategySchema | null; } export interface FeatureToggleWithEnvironment extends FeatureToggle { diff --git a/src/lib/util/feature-evaluator/helpers.ts b/src/lib/util/feature-evaluator/helpers.ts index 12190db247..aff23e7d0a 100644 --- a/src/lib/util/feature-evaluator/helpers.ts +++ b/src/lib/util/feature-evaluator/helpers.ts @@ -1,4 +1,4 @@ -import { IStrategyConfig } from '../../types/model'; +import { IStrategyConfig } from '../../types'; import { FeatureStrategiesEvaluationResult } from './client'; import { Context } from './context'; @@ -44,6 +44,7 @@ export function getDefaultStrategy(featureName: string): IStrategyConfig { return { name: 'flexibleRollout', constraints: [], + disabled: false, parameters: { rollout: '100', stickiness: 'default', diff --git a/src/test/e2e/api/proxy/proxy.e2e.test.ts b/src/test/e2e/api/proxy/proxy.e2e.test.ts index 7baa9ba5c4..df4cc569fa 100644 --- a/src/test/e2e/api/proxy/proxy.e2e.test.ts +++ b/src/test/e2e/api/proxy/proxy.e2e.test.ts @@ -1168,7 +1168,7 @@ test('should NOT evaluate disabled strategies when returning toggles', async () }); await createFeatureToggle({ name: 'disabledFeature3', - enabled: true, + enabled: false, strategies: [ { name: 'flexibleRollout',