1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-06-18 01:18:23 +02:00

Feat: enable toggle dialog (#3686)

<!-- Thanks for creating a PR! To make it easier for reviewers and
everyone else to understand what your changes relate to, please add some
relevant content to the headings below. Feel free to ignore or delete
sections that you don't think are relevant. Thank you! ❤️ -->
- Creates a dialog when the feature has ONLY disabled strategies and the
environment in turned on
- Adds functionality to either `enable` the strategies or add the
default one (if a project specific default strategy is set, uses it)
## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->

<!-- Does it close an issue? Multiple? -->



Uploading Screen Recording 2023-05-05 at 17.40.48.mov…



Closes #

<!-- (For internal contributors): Does it relate to an issue on public
roadmap? -->
<!--
Relates to [roadmap](https://github.com/orgs/Unleash/projects/10) item:
#
-->

### Important files
<!-- PRs can contain a lot of changes, but not all changes are equally
important. Where should a reviewer start looking to get an overview of
the changes? Are any files particularly important? -->


## Discussion points
<!-- Anything about the PR you'd like to discuss before it gets merged?
Got any questions or doubts? -->

---------

Signed-off-by: andreas-unleash <andreas@getunleash.ai>
This commit is contained in:
andreas-unleash 2023-05-08 11:42:26 +03:00 committed by GitHub
parent edefa6fc7e
commit 83bb9b1656
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 244 additions and 64 deletions

View File

@ -11,14 +11,16 @@ import { useChangeRequestsEnabled } from 'hooks/useChangeRequestsEnabled';
import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
import { FeatureStrategyChangeRequestAlert } from 'component/feature/FeatureStrategy/FeatureStrategyForm/FeatureStrategyChangeRequestAlert/FeatureStrategyChangeRequestAlert'; import { FeatureStrategyChangeRequestAlert } from 'component/feature/FeatureStrategy/FeatureStrategyForm/FeatureStrategyChangeRequestAlert/FeatureStrategyChangeRequestAlert';
import { IDisableEnableStrategyProps } from './IDisableEnableStrategyProps'; import { IDisableEnableStrategyProps } from './IDisableEnableStrategyProps';
import { useFeature } from 'hooks/api/getters/useFeature/useFeature';
const DisableStrategy: VFC<IDisableEnableStrategyProps> = ({ ...props }) => { const DisableStrategy: VFC<IDisableEnableStrategyProps> = ({ ...props }) => {
const { projectId, environmentId } = props; const { projectId, environmentId, featureId } = props;
const [isDialogueOpen, setDialogueOpen] = useState(false); const [isDialogueOpen, setDialogueOpen] = useState(false);
const { onDisable } = useEnableDisable({ ...props }); const { onDisable } = useEnableDisable({ ...props });
const { onSuggestDisable } = useSuggestEnableDisable({ ...props }); const { onSuggestDisable } = useSuggestEnableDisable({ ...props });
const { isChangeRequestConfigured } = useChangeRequestsEnabled(projectId); const { isChangeRequestConfigured } = useChangeRequestsEnabled(projectId);
const isChangeRequest = isChangeRequestConfigured(environmentId); const isChangeRequest = isChangeRequestConfigured(environmentId);
const { refetchFeature } = useFeature(projectId, featureId);
const onClick = (event: React.FormEvent) => { const onClick = (event: React.FormEvent) => {
event.preventDefault(); event.preventDefault();
@ -26,6 +28,7 @@ const DisableStrategy: VFC<IDisableEnableStrategyProps> = ({ ...props }) => {
onSuggestDisable(); onSuggestDisable();
} else { } else {
onDisable(); onDisable();
refetchFeature();
} }
setDialogueOpen(false); setDialogueOpen(false);
}; };

View File

@ -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<IEnableEnvironmentDialogProps> = ({
isOpen,
onAddDefaultStrategy,
onActivateDisabledStrategies,
onClose,
environment,
disabledStrategiesCount = 0,
}) => {
const projectId = useRequiredPathParam('projectId');
return (
<Dialogue
open={isOpen}
secondaryButtonText="Cancel"
permissionButton={
<>
<PermissionButton
type="button"
permission={UPDATE_FEATURE}
projectId={projectId}
environmentId={environment}
onClick={onAddDefaultStrategy}
>
Add default strategy
</PermissionButton>
<PermissionButton
type="button"
variant={'text'}
permission={UPDATE_FEATURE}
projectId={projectId}
environmentId={environment}
onClick={onActivateDisabledStrategies}
>
Enable all strategies
</PermissionButton>
</>
}
onClose={onClose}
title="Enable feature toggle"
fullWidth
>
<Typography
variant="body1"
color="text.primary"
sx={{ mb: theme => theme.spacing(2) }}
>
The feature toggle has {disabledStrategiesCount} disabled
{disabledStrategiesCount === 1 ? ' strategy' : ' strategies'}.
</Typography>
<Typography variant="body1" color="text.primary">
You can choose to enable all the disabled strategies or you can
add the default strategy to enable this feature toggle.
</Typography>
</Dialogue>
);
};

View File

@ -14,6 +14,8 @@ import { useChangeRequestsEnabled } from 'hooks/useChangeRequestsEnabled';
import { styled } from '@mui/material'; import { styled } from '@mui/material';
import StringTruncator from 'component/common/StringTruncator/StringTruncator'; import StringTruncator from 'component/common/StringTruncator/StringTruncator';
import { FeatureOverviewSidePanelEnvironmentHider } from './FeatureOverviewSidePanelEnvironmentHider'; import { FeatureOverviewSidePanelEnvironmentHider } from './FeatureOverviewSidePanelEnvironmentHider';
import { useState } from 'react';
import { EnableEnvironmentDialog } from './EnableEnvironmentDialog';
const StyledContainer = styled('div')(({ theme }) => ({ const StyledContainer = styled('div')(({ theme }) => ({
marginLeft: theme.spacing(-1.5), marginLeft: theme.spacing(-1.5),
@ -53,7 +55,7 @@ export const FeatureOverviewSidePanelEnvironmentSwitch = ({
const featureId = useRequiredPathParam('featureId'); const featureId = useRequiredPathParam('featureId');
const { toggleFeatureEnvironmentOn, toggleFeatureEnvironmentOff } = const { toggleFeatureEnvironmentOn, toggleFeatureEnvironmentOff } =
useFeatureApi(); useFeatureApi();
const { refetchFeature } = useFeature(projectId, featureId); const { feature, refetchFeature } = useFeature(projectId, featureId);
const { setToastData, setToastApiError } = useToast(); const { setToastData, setToastApiError } = useToast();
const { isChangeRequestConfigured } = useChangeRequestsEnabled(projectId); const { isChangeRequestConfigured } = useChangeRequestsEnabled(projectId);
const { const {
@ -63,9 +65,20 @@ export const FeatureOverviewSidePanelEnvironmentSwitch = ({
changeRequestDialogDetails, changeRequestDialogDetails,
} = useChangeRequestToggle(projectId); } = 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 { try {
await toggleFeatureEnvironmentOn(projectId, featureId, name); await toggleFeatureEnvironmentOn(
projectId,
featureId,
name,
shouldActivateDisabled
);
setToastData({ setToastData({
type: 'success', type: 'success',
title: `Available in ${name}`, title: `Available in ${name}`,
@ -114,7 +127,23 @@ export const FeatureOverviewSidePanelEnvironmentSwitch = ({
await handleToggleEnvironmentOff(); await handleToggleEnvironmentOff();
return; 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 = ( 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 ( return (
<StyledContainer> <StyledContainer>
<StyledLabel> <StyledLabel>
@ -161,6 +200,14 @@ export const FeatureOverviewSidePanelEnvironmentSwitch = ({
/> />
} }
/> />
<EnableEnvironmentDialog
isOpen={showEnabledDialog}
onClose={() => setShowEnabledDialog(false)}
environment={name}
disabledStrategiesCount={disabledStrategiesCount}
onActivateDisabledStrategies={onActivateStrategies}
onAddDefaultStrategy={onAddDefaultStrategy}
/>
</StyledContainer> </StyledContainer>
); );
}; };

View File

@ -21,7 +21,7 @@ import { CreateFeatureStrategySchema } from 'openapi';
import useProject from 'hooks/api/getters/useProject/useProject'; import useProject from 'hooks/api/getters/useProject/useProject';
interface EditDefaultStrategyProps { interface EditDefaultStrategyProps {
strategy: IFeatureStrategy | CreateFeatureStrategySchema; strategy: CreateFeatureStrategySchema;
} }
const EditDefaultStrategy = ({ strategy }: EditDefaultStrategyProps) => { const EditDefaultStrategy = ({ strategy }: EditDefaultStrategyProps) => {
@ -30,9 +30,8 @@ const EditDefaultStrategy = ({ strategy }: EditDefaultStrategyProps) => {
const { refetch: refetchProject } = useProject(projectId); const { refetch: refetchProject } = useProject(projectId);
const [defaultStrategy, setDefaultStrategy] = useState< const [defaultStrategy, setDefaultStrategy] =
Partial<IFeatureStrategy> | CreateFeatureStrategySchema useState<CreateFeatureStrategySchema>(strategy);
>(strategy);
const [segments, setSegments] = useState<ISegment[]>([]); const [segments, setSegments] = useState<ISegment[]>([]);
const { updateDefaultStrategy, loading } = useProjectApi(); const { updateDefaultStrategy, loading } = useProjectApi();
@ -161,7 +160,7 @@ const EditDefaultStrategy = ({ strategy }: EditDefaultStrategyProps) => {
<ProjectDefaultStrategyForm <ProjectDefaultStrategyForm
projectId={projectId} projectId={projectId}
strategy={defaultStrategy as any} strategy={defaultStrategy as any}
setStrategy={setDefaultStrategy} setStrategy={setDefaultStrategy as any}
segments={segments} segments={segments}
setSegments={setSegments} setSegments={setSegments}
environmentId={environmentId} environmentId={environmentId}

View File

@ -61,7 +61,7 @@ const ProjectEnvironmentDefaultStrategy = ({
return ( return (
<> <>
<StrategyItemContainer <StrategyItemContainer
strategy={(strategy || DEFAULT_STRATEGY) as any} strategy={strategy as any}
description={description} description={description}
actions={ actions={
<> <>
@ -81,7 +81,7 @@ const ProjectEnvironmentDefaultStrategy = ({
</> </>
} }
> >
<StrategyExecution strategy={strategy || DEFAULT_STRATEGY} /> <StrategyExecution strategy={strategy} />
</StrategyItemContainer> </StrategyItemContainer>
<Routes> <Routes>
<Route <Route
@ -92,7 +92,7 @@ const ProjectEnvironmentDefaultStrategy = ({
onClose={onSidebarClose} onClose={onSidebarClose}
open open
> >
<EditDefaultStrategy strategy={strategy as any} /> <EditDefaultStrategy strategy={strategy} />
</SidebarModal> </SidebarModal>
} }
/> />

View File

@ -51,8 +51,13 @@ const useFeatureApi = () => {
}; };
const toggleFeatureEnvironmentOn = useCallback( const toggleFeatureEnvironmentOn = useCallback(
async (projectId: string, featureId: string, environmentId: string) => { async (
const path = `api/admin/projects/${projectId}/features/${featureId}/environments/${environmentId}/on`; projectId: string,
featureId: string,
environmentId: string,
shouldActivateDisabledStrategies = false
) => {
const path = `api/admin/projects/${projectId}/features/${featureId}/environments/${environmentId}/on?shouldActivateDisabledStrategies=${shouldActivateDisabledStrategies}`;
const req = createRequest( const req = createRequest(
path, path,
{ method: 'POST' }, { method: 'POST' },

View File

@ -55,7 +55,6 @@ export default class FeatureToggleClientStore
archived, archived,
isAdmin, isAdmin,
includeStrategyIds, includeStrategyIds,
includeDisabledStrategies,
userId, userId,
}: IGetAllFeatures): Promise<IFeatureToggleClient[]> { }: IGetAllFeatures): Promise<IFeatureToggleClient[]> {
const environment = featureQuery?.environment || DEFAULT_ENV; const environment = featureQuery?.environment || DEFAULT_ENV;
@ -169,7 +168,7 @@ export default class FeatureToggleClientStore
let feature: PartialDeep<IFeatureToggleClient> = acc[r.name] ?? { let feature: PartialDeep<IFeatureToggleClient> = acc[r.name] ?? {
strategies: [], strategies: [],
}; };
if (this.isUnseenStrategyRow(feature, r)) { if (this.isUnseenStrategyRow(feature, r) && !r.strategy_disabled) {
feature.strategies?.push( feature.strategies?.push(
FeatureToggleClientStore.rowToStrategy(r), FeatureToggleClientStore.rowToStrategy(r),
); );
@ -211,12 +210,6 @@ export default class FeatureToggleClientStore
FeatureToggleClientStore.removeIdsFromStrategies(features); 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; return features;
} }
@ -225,7 +218,6 @@ export default class FeatureToggleClientStore
id: row.strategy_id, id: row.strategy_id,
name: row.strategy_name, name: row.strategy_name,
title: row.strategy_title, title: row.strategy_title,
disabled: row.strategy_disabled,
constraints: row.constraints || [], constraints: row.constraints || [],
parameters: mapValues(row.parameters || {}, ensureStringValue), 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( private isUnseenStrategyRow(
feature: PartialDeep<IFeatureToggleClient>, feature: PartialDeep<IFeatureToggleClient>,
row: Record<string, any>, row: Record<string, any>,

View File

@ -49,6 +49,10 @@ interface FeatureStrategyParams {
sortOrder?: number; sortOrder?: number;
} }
interface FeatureStrategyQuery {
shouldActivateDisabledStrategies: string;
}
interface FeatureParams extends ProjectParam { interface FeatureParams extends ProjectParam {
featureName: string; featureName: string;
} }
@ -641,10 +645,16 @@ export default class ProjectFeaturesController extends Controller {
} }
async toggleFeatureEnvironmentOn( async toggleFeatureEnvironmentOn(
req: IAuthRequest<FeatureStrategyParams, any, any, any>, req: IAuthRequest<
FeatureStrategyParams,
any,
any,
FeatureStrategyQuery
>,
res: Response<void>, res: Response<void>,
): Promise<void> { ): Promise<void> {
const { featureName, environment, projectId } = req.params; const { featureName, environment, projectId } = req.params;
const { shouldActivateDisabledStrategies } = req.query;
await this.featureService.updateEnabled( await this.featureService.updateEnabled(
projectId, projectId,
featureName, featureName,
@ -652,6 +662,7 @@ export default class ProjectFeaturesController extends Controller {
true, true,
extractUsername(req), extractUsername(req),
req.user, req.user,
shouldActivateDisabledStrategies === 'true',
); );
res.status(200).end(); res.status(200).end();
} }
@ -762,6 +773,7 @@ export default class ProjectFeaturesController extends Controller {
const userName = extractUsername(req); const userName = extractUsername(req);
const patch = req.body; const patch = req.body;
const strategy = await this.featureService.getStrategy(strategyId); const strategy = await this.featureService.getStrategy(strategyId);
const { newDocument } = applyPatch(strategy, patch); const { newDocument } = applyPatch(strategy, patch);
const updatedStrategy = await this.featureService.updateStrategy( const updatedStrategy = await this.featureService.updateStrategy(
strategyId, strategyId,
@ -770,6 +782,21 @@ export default class ProjectFeaturesController extends Controller {
userName, userName,
req.user, 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); res.status(200).json(updatedStrategy);
} }

View File

@ -648,6 +648,28 @@ class FeatureToggleService {
await this.featureStrategiesStore.delete(id); 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 tags = await this.tagStore.getAllTagsForFeature(featureName);
const preData = this.featureStrategyToPublic(existingStrategy); const preData = this.featureStrategyToPublic(existingStrategy);
@ -1074,7 +1096,7 @@ class FeatureToggleService {
environment, environment,
enabled: envMetadata.enabled, enabled: envMetadata.enabled,
strategies, strategies,
defaultStrategy: defaultStrategy, defaultStrategy,
}; };
} }
@ -1242,6 +1264,7 @@ class FeatureToggleService {
enabled: boolean, enabled: boolean,
createdBy: string, createdBy: string,
user?: User, user?: User,
shouldActivateDisabledStrategies = false,
): Promise<FeatureToggle> { ): Promise<FeatureToggle> {
await this.stopWhenChangeRequestsEnabled(project, environment, user); await this.stopWhenChangeRequestsEnabled(project, environment, user);
if (enabled) { if (enabled) {
@ -1259,6 +1282,7 @@ class FeatureToggleService {
environment, environment,
enabled, enabled,
createdBy, createdBy,
shouldActivateDisabledStrategies,
); );
} }
@ -1268,6 +1292,7 @@ class FeatureToggleService {
environment: string, environment: string,
enabled: boolean, enabled: boolean,
createdBy: string, createdBy: string,
shouldActivateDisabledStrategies: boolean,
): Promise<FeatureToggle> { ): Promise<FeatureToggle> {
const hasEnvironment = const hasEnvironment =
await this.featureEnvironmentStore.featureHasEnvironment( await this.featureEnvironmentStore.featureHasEnvironment(
@ -1287,22 +1312,51 @@ class FeatureToggleService {
featureName, featureName,
environment, environment,
); );
const projectEnvironmentDefaultStrategy = const hasDisabledStrategies = strategies.some(
await this.projectStore.getDefaultStrategy( (strategy) => strategy.disabled,
project, );
environment,
);
const strategy = if (
this.flagResolver.isEnabled('strategyImprovements') && this.flagResolver.isEnabled('strategyImprovements') &&
projectEnvironmentDefaultStrategy != null hasDisabledStrategies &&
? getProjectDefaultStrategy( shouldActivateDisabledStrategies
projectEnvironmentDefaultStrategy, ) {
featureName, strategies.map(async (strategy) => {
) return this.updateStrategy(
: getDefaultStrategy(featureName); 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( await this.unprotectedCreateStrategy(
strategy, strategy,
{ {

View File

@ -73,7 +73,7 @@ export interface IFeatureToggleClient {
stale: boolean; stale: boolean;
variants: IVariant[]; variants: IVariant[];
enabled: boolean; enabled: boolean;
strategies: IStrategyConfig[]; strategies: Omit<IStrategyConfig, 'disabled'>[];
impressionData?: boolean; impressionData?: boolean;
lastSeenAt?: Date; lastSeenAt?: Date;
createdAt?: Date; createdAt?: Date;
@ -86,7 +86,7 @@ export interface IFeatureEnvironmentInfo {
environment: string; environment: string;
enabled: boolean; enabled: boolean;
strategies: IFeatureStrategy[]; strategies: IFeatureStrategy[];
defaultStrategy?: CreateFeatureStrategySchema | null; defaultStrategy: CreateFeatureStrategySchema | null;
} }
export interface FeatureToggleWithEnvironment extends FeatureToggle { export interface FeatureToggleWithEnvironment extends FeatureToggle {

View File

@ -1,4 +1,4 @@
import { IStrategyConfig } from '../../types/model'; import { IStrategyConfig } from '../../types';
import { FeatureStrategiesEvaluationResult } from './client'; import { FeatureStrategiesEvaluationResult } from './client';
import { Context } from './context'; import { Context } from './context';
@ -44,6 +44,7 @@ export function getDefaultStrategy(featureName: string): IStrategyConfig {
return { return {
name: 'flexibleRollout', name: 'flexibleRollout',
constraints: [], constraints: [],
disabled: false,
parameters: { parameters: {
rollout: '100', rollout: '100',
stickiness: 'default', stickiness: 'default',

View File

@ -1168,7 +1168,7 @@ test('should NOT evaluate disabled strategies when returning toggles', async ()
}); });
await createFeatureToggle({ await createFeatureToggle({
name: 'disabledFeature3', name: 'disabledFeature3',
enabled: true, enabled: false,
strategies: [ strategies: [
{ {
name: 'flexibleRollout', name: 'flexibleRollout',