1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-05-31 01:16:01 +02:00

feat: add warning hints on potential misconfiguration (#2948)

## About the changes
Add warnings when we detect something might be wrong with the customer
configuration, in particular with regard to variants configuration

## Rationale
Moving from variants per feature to variants per environment will allow
users to have fine-grained permissions and more control over variants on
different environments: #2254

But because this requires an additional step of copying variants to
other environments, we identified the potential risk of users forgetting
to follow this step. To keep them informed about this, we're introducing
a warning sign after a toggle is enabled when we detect that:
1. The environment is enabled without variants
2. Other enabled environments have variants

This situation would be a problem if you rely on `getVariant` method
from the SDK, because without variants you'll receive the default
variant. Probably, not what you'd expect after enabling the toggle, but
there are situations where this might be correct. Because of the latter,
we thought that adding a warning and letting the user handle the
situation was the best solution.

## UI sketches
![image
(6)](https://user-images.githubusercontent.com/455064/213676353-112639f0-7781-42c0-8c9d-8c7eba316bae.png)
![Screenshot from 2023-01-19
08-55-10](https://user-images.githubusercontent.com/455064/213664639-7b11ff4b-048a-4a36-aa71-7df2f889adff.png)

Co-authored-by: Nuno Góis <github@nunogois.com>
This commit is contained in:
Gastón Fournier 2023-01-20 18:09:01 +01:00 committed by GitHub
parent 287d28e91d
commit 70d8f9e58a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 165 additions and 48 deletions

View File

@ -7,7 +7,13 @@ const StyledHtmlTooltipBody = styled('div')(({ theme }) => ({
}));
const StyledHtmlTooltip = styled(
({ className, maxWidth, maxHeight, ...props }: IHtmlTooltipProps) => (
({
className,
maxWidth,
maxHeight,
fontSize,
...props
}: IHtmlTooltipProps) => (
<Tooltip
{...props}
title={<StyledHtmlTooltipBody>{props.title}</StyledHtmlTooltipBody>}
@ -15,11 +21,21 @@ const StyledHtmlTooltip = styled(
/>
),
{
shouldForwardProp: prop => prop !== 'maxWidth' && prop !== 'maxHeight',
shouldForwardProp: prop =>
prop !== 'maxWidth' && prop !== 'maxHeight' && prop !== 'fontSize',
}
)<{ maxWidth?: SpacingArgument; maxHeight?: SpacingArgument }>(
({ theme, maxWidth, maxHeight }) => ({
maxWidth: maxWidth || theme.spacing(37.5),
)<{
maxWidth?: SpacingArgument;
maxHeight?: SpacingArgument;
fontSize?: string;
}>(
({
theme,
maxWidth = theme.spacing(37.5),
maxHeight = theme.spacing(37.5),
fontSize = theme.fontSizes.smallerBody,
}) => ({
maxWidth,
[`& .${tooltipClasses.tooltip}`]: {
display: 'flex',
flexDirection: 'column',
@ -31,7 +47,8 @@ const StyledHtmlTooltip = styled(
fontWeight: theme.fontWeight.medium,
maxWidth: 'inherit',
border: `1px solid ${theme.palette.lightBorder}`,
maxHeight: maxHeight || theme.spacing(37.5),
maxHeight,
fontSize,
},
[`& .${tooltipClasses.arrow}`]: {
'&:before': {
@ -45,6 +62,7 @@ const StyledHtmlTooltip = styled(
export interface IHtmlTooltipProps extends TooltipProps {
maxWidth?: SpacingArgument;
maxHeight?: SpacingArgument;
fontSize?: string;
}
export const HtmlTooltip = (props: IHtmlTooltipProps) => (

View File

@ -4,6 +4,8 @@ import { useState } from 'react';
import { FeatureOverviewSidePanelEnvironmentSwitch } from 'component/feature/FeatureView/FeatureOverview/FeatureOverviewSidePanel/FeatureOverviewSidePanelEnvironmentSwitches/FeatureOverviewSidePanelEnvironmentSwitch/FeatureOverviewSidePanelEnvironmentSwitch';
import { Link, styled, Tooltip } from '@mui/material';
import { Link as RouterLink } from 'react-router-dom';
import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
import VariantsWarningTooltip from 'component/feature/FeatureView/FeatureVariants/VariantsTooltipWarning';
const StyledContainer = styled('div')(({ theme }) => ({
padding: theme.spacing(3),
@ -21,6 +23,15 @@ const StyledLabel = styled('p')(({ theme }) => ({
const StyledSubLabel = styled('p')(({ theme }) => ({
fontSize: theme.fontSizes.smallBody,
color: theme.palette.text.secondary,
display: 'flex',
alignItems: 'center',
}));
const StyledSeparator = styled('span')(({ theme }) => ({
padding: theme.spacing(0, 0.5),
'::after': {
content: '"-"',
},
}));
const StyledLink = styled(Link<typeof RouterLink | 'a'>)(() => ({
@ -44,7 +55,9 @@ export const FeatureOverviewSidePanelEnvironmentSwitches = ({
}: IFeatureOverviewSidePanelEnvironmentSwitchesProps) => {
const [showInfoBox, setShowInfoBox] = useState(false);
const [environmentName, setEnvironmentName] = useState('');
const someEnabledEnvironmentHasVariants = feature.environments.some(
environment => environment.enabled && environment.variants?.length
);
return (
<StyledContainer>
{header}
@ -58,7 +71,7 @@ export const FeatureOverviewSidePanelEnvironmentSwitches = ({
const variantsLink = variants.length > 0 && (
<>
{' - '}
<StyledSeparator />
<Tooltip title="View variants" arrow describeChild>
<StyledLink
component={RouterLink}
@ -73,6 +86,10 @@ export const FeatureOverviewSidePanelEnvironmentSwitches = ({
</>
);
const hasWarning =
environment.enabled &&
variants.length == 0 &&
someEnabledEnvironmentHasVariants;
return (
<FeatureOverviewSidePanelEnvironmentSwitch
key={environment.name}
@ -89,6 +106,15 @@ export const FeatureOverviewSidePanelEnvironmentSwitches = ({
<StyledSubLabel>
{strategiesLabel}
{variantsLink}
<ConditionallyRender
condition={hasWarning}
show={
<>
<StyledSeparator />
<VariantsWarningTooltip />
</>
}
/>
</StyledSubLabel>
</StyledSwitchLabel>
</FeatureOverviewSidePanelEnvironmentSwitch>

View File

@ -0,0 +1,33 @@
import { HtmlTooltip } from 'component/common/HtmlTooltip/HtmlTooltip';
import { WarningAmber } from '@mui/icons-material';
import { styled } from '@mui/material';
const StyledWarningAmber = styled(WarningAmber)(({ theme }) => ({
color: theme.palette.warning.main,
fontSize: theme.fontSizes.bodySize,
}));
const VariantsWarningTooltip = () => {
return (
<HtmlTooltip
arrow
title={
<>
This environment has no variants enabled. If you check this
feature's variants in this environment, you will get the{' '}
<a
href="https://docs.getunleash.io/reference/feature-toggle-variants#the-disabled-variant"
target="_blank"
>
disabled variant
</a>
.
</>
}
>
<StyledWarningAmber />
</HtmlTooltip>
);
};
export default VariantsWarningTooltip;

View File

@ -45,11 +45,28 @@ import { useFavoriteFeaturesApi } from 'hooks/api/actions/useFavoriteFeaturesApi
import { FeatureTagCell } from 'component/common/Table/cells/FeatureTagCell/FeatureTagCell';
import { useGlobalLocalStorage } from 'hooks/useGlobalLocalStorage';
import { useConditionallyHiddenColumns } from 'hooks/useConditionallyHiddenColumns';
import { flexRow } from 'themes/themeStyles';
import VariantsWarningTooltip from 'component/feature/FeatureView/FeatureVariants/VariantsTooltipWarning';
const StyledResponsiveButton = styled(ResponsiveButton)(() => ({
whiteSpace: 'nowrap',
}));
const StyledSwitchContainer = styled('div', {
shouldForwardProp: prop => prop !== 'hasWarning',
})<{ hasWarning?: boolean }>(({ theme, hasWarning }) => ({
flexGrow: 0,
...flexRow,
justifyContent: 'center',
...(hasWarning && {
'::before': {
content: '""',
display: 'block',
width: theme.spacing(2),
},
}),
}));
interface IProjectFeatureTogglesProps {
features: IProject['features'];
environments: IProject['environments'];
@ -64,8 +81,10 @@ type ListItemType = Pick<
[key in string]: {
name: string;
enabled: boolean;
variantCount: number;
};
};
someEnabledEnvironmentHasVariants: boolean;
};
const staticColumns = ['Actions', 'name', 'favorite'];
@ -273,15 +292,28 @@ export const ProjectFeatureToggles = ({
}: {
value: boolean;
row: { original: ListItemType };
}) => (
<FeatureToggleSwitch
value={value}
projectId={projectId}
featureName={feature?.name}
environmentName={name}
onToggle={onToggle}
/>
),
}) => {
const hasWarning =
feature.someEnabledEnvironmentHasVariants &&
feature.environments[name].variantCount === 0 &&
feature.environments[name].enabled;
return (
<StyledSwitchContainer hasWarning={hasWarning}>
<FeatureToggleSwitch
value={value}
projectId={projectId}
featureName={feature?.name}
environmentName={name}
onToggle={onToggle}
/>
<ConditionallyRender
condition={hasWarning}
show={<VariantsWarningTooltip />}
/>
</StyledSwitchContainer>
);
},
sortType: 'boolean',
filterName: name,
filterParsing: (value: boolean) =>
@ -311,38 +343,31 @@ export const ProjectFeatureToggles = ({
const featuresData = useMemo(
() =>
features.map(
({
name,
lastSeenAt,
createdAt,
type,
stale,
tags,
favorite,
environments: featureEnvironments,
}) => ({
name,
lastSeenAt,
createdAt,
type,
stale,
tags,
favorite,
environments: Object.fromEntries(
environments.map(env => [
features.map(feature => ({
...feature,
environments: Object.fromEntries(
environments.map(env => {
const thisEnv = feature?.environments.find(
featureEnvironment =>
featureEnvironment?.name === env
);
return [
env,
{
name: env,
enabled:
featureEnvironments?.find(
feature => feature?.name === env
)?.enabled || false,
enabled: thisEnv?.enabled || false,
variantCount: thisEnv?.variantCount || 0,
},
])
),
})
),
];
})
),
someEnabledEnvironmentHasVariants:
feature.environments?.some(
featureEnvironment =>
featureEnvironment.variantCount > 0 &&
featureEnvironment.enabled
) || false,
})),
[features, environments]
);

View File

@ -15,6 +15,7 @@ export interface IFeatureToggleListItem {
export interface IEnvironments {
name: string;
enabled: boolean;
variantCount: number;
}
export interface IFeatureToggle {

View File

@ -392,6 +392,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
enabled: r.enabled,
type: r.environment_type,
sortOrder: r.environment_sort_order,
variantCount: r.variants?.length || 0,
};
}
@ -469,6 +470,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
'features.stale as stale',
'feature_environments.enabled as enabled',
'feature_environments.environment as environment',
'feature_environments.variants as variants',
'environments.type as environment_type',
'environments.sort_order as environment_sort_order',
'ft.tag_value as tag_value',

View File

@ -27,6 +27,9 @@ export const featureEnvironmentSchema = {
sortOrder: {
type: 'number',
},
variantCount: {
type: 'number',
},
strategies: {
type: 'array',
items: {

View File

@ -91,7 +91,7 @@ export interface FeatureToggleLegacy extends FeatureToggle {
enabled: boolean;
}
export interface IEnvironmentDetail extends IEnvironmentOverview {
export interface IEnvironmentDetail extends IEnvironmentBase {
strategies: IStrategyConfig[];
variants: IVariant[];
}
@ -152,13 +152,17 @@ export interface IEnvironmentClone {
clonePermissions?: boolean;
}
export interface IEnvironmentOverview {
export interface IEnvironmentBase {
name: string;
enabled: boolean;
type: string;
sortOrder: number;
}
export interface IEnvironmentOverview extends IEnvironmentBase {
variantCount: number;
}
export interface IFeatureOverview {
name: string;
type: string;

View File

@ -1161,6 +1161,9 @@ exports[`should serve the OpenAPI spec 1`] = `
"type": {
"type": "string",
},
"variantCount": {
"type": "number",
},
},
"required": [
"name",

View File

@ -61,6 +61,7 @@ test('Can connect environment to project', async () => {
enabled: false,
sortOrder: 9999,
type: 'production',
variantCount: 0,
},
]);
});
@ -87,6 +88,7 @@ test('Can remove environment from project', async () => {
enabled: false,
sortOrder: 9999,
type: 'production',
variantCount: 0,
},
]);
});

View File

@ -39,7 +39,7 @@ export default class FakeFeatureEnvironmentStore
.filter(
(fe) =>
fe.featureName === featureName &&
environments.indexOf(fe.environment) !== -1,
environments.includes(fe.environment),
)
.map((fe) => (fe.variants = variants));
}