1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-01-25 00:07:47 +01:00

refactor: remove variants per environment feature flag (#3102)

https://linear.app/unleash/issue/2-428/clean-up-feature-flag-once-were-done-with-the-migration

Cleans up the variants per environment feature flag due to GA.
This commit is contained in:
Nuno Góis 2023-02-14 14:02:02 +00:00 committed by GitHub
parent 0a32647d75
commit 8729f082d2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 63 additions and 321 deletions

View File

@ -232,36 +232,29 @@ describe('feature', () => {
cy.wait('@addStrategyToFeature');
});
it('can add two variant to the feature', () => {
it('can add two variants to the development environment', () => {
cy.visit(`/projects/default/features/${featureToggleName}/variants`);
cy.intercept(
'PATCH',
`/api/admin/projects/default/features/${featureToggleName}/variants`,
`/api/admin/projects/default/features/${featureToggleName}/environments/development/variants`,
req => {
if (req.body.length === 1) {
expect(req.body[0].op).to.equal('add');
expect(req.body[0].path).to.match(/\//);
expect(req.body[0].value.name).to.equal(variant1);
} else if (req.body.length === 2) {
expect(req.body[0].op).to.equal('replace');
expect(req.body[0].path).to.match(/weight/);
expect(req.body[0].value).to.equal(500);
expect(req.body[1].op).to.equal('add');
expect(req.body[1].path).to.match(/\//);
expect(req.body[1].value.name).to.equal(variant2);
}
expect(req.body[0].op).to.equal('add');
expect(req.body[0].path).to.equal('/0');
expect(req.body[0].value.name).to.equal(variant1);
expect(req.body[0].value.weight).to.equal(500);
expect(req.body[1].op).to.equal('add');
expect(req.body[1].path).to.equal('/1');
expect(req.body[1].value.name).to.equal(variant2);
expect(req.body[1].value.weight).to.equal(500);
}
).as('variantCreation');
cy.get('[data-testid=ADD_VARIANT_BUTTON]').click();
cy.get('[data-testid=ADD_VARIANT_BUTTON]').first().click();
cy.wait(1000);
cy.get('[data-testid=VARIANT_NAME_INPUT]').type(variant1);
cy.get('[data-testid=DIALOGUE_CONFIRM_ID]').click();
cy.wait('@variantCreation');
cy.get('[data-testid=ADD_VARIANT_BUTTON]').click();
cy.wait(1000);
cy.get('[data-testid=VARIANT_NAME_INPUT]').type(variant2);
cy.get('[data-testid=MODAL_ADD_VARIANT_BUTTON]').click();
cy.get('[data-testid=VARIANT_NAME_INPUT]').last().type(variant2);
cy.get('[data-testid=DIALOGUE_CONFIRM_ID]').click();
cy.wait('@variantCreation');
});
@ -269,60 +262,61 @@ describe('feature', () => {
it('can set weight to fixed value for one of the variants', () => {
cy.visit(`/projects/default/features/${featureToggleName}/variants`);
cy.get(`[data-testid=VARIANT_EDIT_BUTTON_${variant1}]`).click();
cy.get('[data-testid=EDIT_VARIANTS_BUTTON]').click();
cy.wait(1000);
cy.get('[data-testid=VARIANT_NAME_INPUT]')
.last()
.children()
.find('input')
.should('have.attr', 'disabled');
cy.get('[data-testid=VARIANT_WEIGHT_CHECK]').find('input').check();
cy.get('[data-testid=VARIANT_WEIGHT_INPUT]').clear().type('15');
cy.get('[data-testid=VARIANT_WEIGHT_CHECK]')
.last()
.find('input')
.check();
cy.get('[data-testid=VARIANT_WEIGHT_INPUT]').last().clear().type('15');
cy.intercept(
'PATCH',
`/api/admin/projects/default/features/${featureToggleName}/variants`,
`/api/admin/projects/default/features/${featureToggleName}/environments/development/variants`,
req => {
expect(req.body[0].op).to.equal('replace');
expect(req.body[0].path).to.match(/weight/);
expect(req.body[0].value).to.equal(850);
expect(req.body[0].path).to.equal('/1/weightType');
expect(req.body[0].value).to.equal('fix');
expect(req.body[1].op).to.equal('replace');
expect(req.body[1].path).to.match(/weightType/);
expect(req.body[1].value).to.equal('fix');
expect(req.body[1].path).to.equal('/1/weight');
expect(req.body[1].value).to.equal(150);
expect(req.body[2].op).to.equal('replace');
expect(req.body[2].path).to.match(/weight/);
expect(req.body[2].value).to.equal(150);
expect(req.body[2].path).to.equal('/0/weight');
expect(req.body[2].value).to.equal(850);
}
).as('variantUpdate');
cy.get('[data-testid=DIALOGUE_CONFIRM_ID]').click();
cy.wait('@variantUpdate');
cy.get(`[data-testid=VARIANT_WEIGHT_${variant1}]`).should(
cy.get(`[data-testid=VARIANT_WEIGHT_${variant2}]`).should(
'have.text',
'15 %'
);
});
it('can delete variant', () => {
const variantName = 'to-be-deleted';
cy.visit(`/projects/default/features/${featureToggleName}/variants`);
cy.get('[data-testid=ADD_VARIANT_BUTTON]').click();
cy.get('[data-testid=EDIT_VARIANTS_BUTTON]').click();
cy.wait(1000);
cy.get('[data-testid=VARIANT_NAME_INPUT]').type(variantName);
cy.get('[data-testid=DIALOGUE_CONFIRM_ID]').click();
cy.get(`[data-testid=VARIANT_DELETE_BUTTON_${variant2}]`).click();
cy.intercept(
'PATCH',
`/api/admin/projects/default/features/${featureToggleName}/variants`,
`/api/admin/projects/default/features/${featureToggleName}/environments/development/variants`,
req => {
const patch = req.body.find(
(patch: Record<string, string>) => patch.op === 'remove'
);
expect(patch.path).to.match(/\//);
expect(req.body[0].op).to.equal('remove');
expect(req.body[0].path).to.equal('/1');
expect(req.body[1].op).to.equal('replace');
expect(req.body[1].path).to.equal('/0/weight');
expect(req.body[1].value).to.equal(1000);
}
).as('delete');
cy.get(`[data-testid=VARIANT_DELETE_BUTTON_${variantName}]`).click();
cy.get('[data-testid=DIALOGUE_CONFIRM_ID]').click();
cy.wait('@delete');
});

View File

@ -119,7 +119,11 @@ describe('imports', () => {
// cy.contains('Import completed');
cy.visit(`/projects/default/features/${randomFeatureName}`);
cy.contains('enabled in development');
cy.get(
"[data-testid='feature-toggle-status'] input[type='checkbox']:checked"
)
.closest('div')
.contains('development');
cy.contains('50%');
});
});

View File

@ -1,6 +1,5 @@
import FeatureOverviewMetaData from './FeatureOverviewMetaData/FeatureOverviewMetaData';
import FeatureOverviewEnvironments from './FeatureOverviewEnvironments/FeatureOverviewEnvironments';
import FeatureOverviewEnvSwitches from './FeatureOverviewEnvSwitches/FeatureOverviewEnvSwitches';
import { Routes, Route, useNavigate } from 'react-router-dom';
import { FeatureStrategyCreate } from 'component/feature/FeatureStrategy/FeatureStrategyCreate/FeatureStrategyCreate';
import { SidebarModal } from 'component/common/SidebarModal/SidebarModal';
@ -10,8 +9,6 @@ import {
} from 'component/feature/FeatureStrategy/FeatureStrategyEdit/FeatureStrategyEdit';
import { useRequiredPathParam } from 'hooks/useRequiredPathParam';
import { usePageTitle } from 'hooks/usePageTitle';
import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig';
import { FeatureOverviewSidePanel } from 'component/feature/FeatureView/FeatureOverview/FeatureOverviewSidePanel/FeatureOverviewSidePanel';
import { useHiddenEnvironments } from 'hooks/useHiddenEnvironments';
import { styled } from '@mui/material';
@ -34,7 +31,6 @@ const StyledMainContent = styled('div')(({ theme }) => ({
}));
const FeatureOverview = () => {
const { uiConfig } = useUiConfig();
const navigate = useNavigate();
const projectId = useRequiredPathParam('projectId');
const featureId = useRequiredPathParam('featureId');
@ -48,20 +44,9 @@ const FeatureOverview = () => {
<StyledContainer>
<div>
<FeatureOverviewMetaData />
<ConditionallyRender
condition={Boolean(uiConfig.flags.variantsPerEnvironment)}
show={
<FeatureOverviewSidePanel
hiddenEnvironments={hiddenEnvironments}
setHiddenEnvironments={setHiddenEnvironments}
/>
}
elseShow={
<FeatureOverviewEnvSwitches
hiddenEnvironments={hiddenEnvironments}
setHiddenEnvironments={setHiddenEnvironments}
/>
}
<FeatureOverviewSidePanel
hiddenEnvironments={hiddenEnvironments}
setHiddenEnvironments={setHiddenEnvironments}
/>
</div>
<StyledMainContent>

View File

@ -145,17 +145,6 @@ const FeatureOverviewMetaData = () => {
/>
</StyledBody>
</StyledPaddingContainerTop>
<ConditionallyRender
condition={
tags.length > 0 &&
!Boolean(uiConfig.flags.variantsPerEnvironment)
}
show={
<StyledPaddingContainerBottom>
<FeatureOverviewTags projectId={projectId} />
</StyledPaddingContainerBottom>
}
/>
</StyledContainer>
);
};

View File

@ -59,7 +59,7 @@ export const FeatureOverviewSidePanelEnvironmentSwitches = ({
environment => environment.enabled && environment.variants?.length
);
return (
<StyledContainer>
<StyledContainer data-testid="feature-toggle-status">
{header}
{feature.environments.map(environment => {
const strategiesLabel =

View File

@ -284,6 +284,7 @@ export const EnvironmentVariantsModal = ({
</StyledName>
</div>
<PermissionButton
data-testid="MODAL_ADD_VARIANT_BUTTON"
onClick={() =>
setVariantsEdit(variantsEdit => [
...variantsEdit,
@ -400,6 +401,7 @@ export const EnvironmentVariantsModal = ({
<StyledButtonContainer>
<Button
data-testid="DIALOGUE_CONFIRM_ID"
type="submit"
variant="contained"
color="primary"

View File

@ -304,6 +304,7 @@ export const VariantForm = ({
>
<div>
<IconButton
data-testid={`VARIANT_DELETE_BUTTON_${variant.name}`}
onClick={() => removeVariant(variant.id)}
disabled={isProtectedVariant(variant)}
>
@ -318,6 +319,7 @@ export const VariantForm = ({
This will be used to identify the variant in your code
</StyledSubLabel>
<StyledInput
data-testid="VARIANT_NAME_INPUT"
autoFocus
label="Variant name"
error={Boolean(errors.name)}
@ -336,6 +338,7 @@ export const VariantForm = ({
label="Custom percentage"
control={
<Switch
data-testid="VARIANT_WEIGHT_CHECK"
checked={customPercentage}
onChange={e =>
setCustomPercentage(
@ -346,6 +349,7 @@ export const VariantForm = ({
}
/>
<StyledWeightInput
data-testid="VARIANT_WEIGHT_INPUT"
type="number"
label="Variant weight"
error={Boolean(errors.percentage)}

View File

@ -318,6 +318,7 @@ export const FeatureEnvironmentVariants = () => {
)}
show={
<PermissionIconButton
data-testid="EDIT_VARIANTS_BUTTON"
onClick={() =>
editVariants(environment)
}
@ -332,6 +333,7 @@ export const FeatureEnvironmentVariants = () => {
}
elseShow={
<PermissionButton
data-testid="ADD_VARIANT_BUTTON"
onClick={() =>
editVariants(environment)
}

View File

@ -1,18 +1,10 @@
import { FeatureVariantsList } from './FeatureVariantsList/FeatureVariantsList';
import { usePageTitle } from 'hooks/usePageTitle';
import { FeatureEnvironmentVariants } from './FeatureEnvironmentVariants/FeatureEnvironmentVariants';
import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig';
const FeatureVariants = () => {
usePageTitle('Variants');
const { uiConfig } = useUiConfig();
if (uiConfig.flags.variantsPerEnvironment) {
return <FeatureEnvironmentVariants />;
}
return <FeatureVariantsList />;
return <FeatureEnvironmentVariants />;
};
export default FeatureVariants;

View File

@ -4,7 +4,6 @@ import { emptyFeature } from './emptyFeature';
import handleErrorResponses from '../httpErrorResponseHandler';
import { formatApiPath } from 'utils/formatPath';
import { IFeatureToggle } from 'interfaces/featureToggle';
import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig';
export interface IUseFeatureOutput {
feature: IFeatureToggle;
@ -26,14 +25,9 @@ export const useFeature = (
): IUseFeatureOutput => {
const path = formatFeatureApiPath(projectId, featureId);
const { uiConfig } = useUiConfig();
const {
flags: { variantsPerEnvironment },
} = uiConfig;
const { data, error, mutate } = useSWR<IFeatureResponse>(
['useFeature', path, variantsPerEnvironment],
() => featureFetcher(path, variantsPerEnvironment),
['useFeature', path],
() => featureFetcher(path),
options
);
@ -51,14 +45,9 @@ export const useFeature = (
};
export const featureFetcher = async (
path: string,
variantsPerEnvironment?: boolean
path: string
): Promise<IFeatureResponse> => {
const variantEnvironments = variantsPerEnvironment
? '?variantEnvironments=true'
: '';
const res = await fetch(path + variantEnvironments);
const res = await fetch(path);
if (res.status === 404) {
return { status: 404 };
@ -79,6 +68,6 @@ export const formatFeatureApiPath = (
featureId: string
): string => {
return formatApiPath(
`api/admin/projects/${projectId}/features/${featureId}`
`api/admin/projects/${projectId}/features/${featureId}?variantEnvironments=true`
);
};

View File

@ -38,7 +38,6 @@ export interface IFlags {
UG?: boolean;
ENABLE_DARK_MODE_SUPPORT?: boolean;
embedProxyFrontend?: boolean;
variantsPerEnvironment?: boolean;
networkView?: boolean;
maintenance?: boolean;
maintenanceMode?: boolean;

View File

@ -84,7 +84,6 @@ exports[`should create default config 1`] = `
"responseTimeWithAppName": false,
"showProjectApiAccess": false,
"strictSchemaValidation": false,
"variantsPerEnvironment": false,
},
},
"flagResolver": FlagResolver {
@ -106,7 +105,6 @@ exports[`should create default config 1`] = `
"responseTimeWithAppName": false,
"showProjectApiAccess": false,
"strictSchemaValidation": false,
"variantsPerEnvironment": false,
},
"externalResolver": {
"isEnabled": [Function],

View File

@ -97,13 +97,6 @@ export default class EnvironmentService {
environment,
projectId,
);
if (!this.flagResolver.isEnabled('variantsPerEnvironment')) {
await this.featureEnvironmentStore.clonePreviousVariants(
environment,
projectId,
);
}
} catch (e) {
if (e.code === UNIQUE_CONSTRAINT_VIOLATION) {
throw new NameExistsError(

View File

@ -27,7 +27,6 @@ function getSetup() {
}
async function setupV3VariantsCompatibilityScenario(
variantsPerEnvironment: boolean,
envs = [
{ name: 'env-2', enabled: true },
{ name: 'env-3', enabled: true },
@ -55,9 +54,7 @@ async function setupV3VariantsCompatibilityScenario(
env.name,
[
{
name: variantsPerEnvironment
? `${env.name}-variant`
: 'variant-name',
name: `${env.name}-variant`,
stickiness: 'default',
weight: 1000,
weightType: 'variable',
@ -69,7 +66,7 @@ async function setupV3VariantsCompatibilityScenario(
stateService: new StateService(stores, {
getLogger,
flagResolver: {
isEnabled: () => variantsPerEnvironment,
isEnabled: () => true,
getAll: () => ({}),
},
}),
@ -622,88 +619,8 @@ test('exporting to new format works', async () => {
expect(exported.featureStrategies).toHaveLength(1);
});
test('exporting with no environments should fail', async () => {
const { stateService, stores } = await setupV3VariantsCompatibilityScenario(
false,
);
await stores.environmentStore.deleteAll();
expect(stateService.export({})).rejects.toThrowError();
});
// This test should be removed as soon as variants per environment is GA
test('exporting variants to v3 format should pick variants from the correct env', async () => {
const { stateService } = await setupV3VariantsCompatibilityScenario(false);
const exported = await stateService.export({});
expect(exported.features).toHaveLength(1);
expect(exported.features[0].variants).toStrictEqual([
{
name: 'variant-name',
stickiness: 'default',
weight: 1000,
weightType: 'variable',
},
]);
exported.featureEnvironments.forEach((fe) =>
expect(fe.variants).toBeUndefined(),
);
expect(exported.environments).toHaveLength(3);
});
// This test should be removed as soon as variants per environment is GA
test('exporting variants to v3 format when some envs are disabled should export variants', async () => {
const { stateService } = await setupV3VariantsCompatibilityScenario(false, [
{ name: 'env-2', enabled: false },
{ name: 'env-3', enabled: false },
{ name: 'env-1', enabled: true },
]);
const exported = await stateService.export({});
expect(exported.features).toHaveLength(1);
expect(exported.features[0].variants).toStrictEqual([
{
name: 'variant-name',
stickiness: 'default',
weight: 1000,
weightType: 'variable',
},
]);
exported.featureEnvironments.forEach((fe) =>
expect(fe.variants).toBeUndefined(),
);
expect(exported.environments).toHaveLength(3);
});
// This test should be removed as soon as variants per environment is GA
test('exporting variants to v3 format when all envs are disabled should export variants', async () => {
const { stateService } = await setupV3VariantsCompatibilityScenario(false, [
{ name: 'env-2', enabled: false },
{ name: 'env-3', enabled: false },
{ name: 'env-1', enabled: false },
]);
const exported = await stateService.export({});
expect(exported.features).toHaveLength(1);
expect(exported.features[0].variants).toStrictEqual([
{
name: 'variant-name',
stickiness: 'default',
weight: 1000,
weightType: 'variable',
},
]);
exported.featureEnvironments.forEach((fe) =>
expect(fe.variants).toBeUndefined(),
);
expect(exported.environments).toHaveLength(3);
});
test('exporting variants to v4 format should not include variants in features', async () => {
const { stateService } = await setupV3VariantsCompatibilityScenario(true);
const { stateService } = await setupV3VariantsCompatibilityScenario();
const exported = await stateService.export({});
expect(exported.features).toHaveLength(1);

View File

@ -703,61 +703,7 @@ export default class StateService {
environments: IEnvironment[];
featureEnvironments: IFeatureEnvironment[];
}> {
if (this.flagResolver.isEnabled('variantsPerEnvironment')) {
return this.exportV4(opts);
}
// adapt v4 to v3. We need includeEnvironments set to true to filter the
// best environment from where we'll pick variants (cause now they are stored
// per environment despite being displayed as if they belong to the feature)
const v4 = await this.exportV4({ ...opts, includeEnvironments: true });
// undefined defaults to true
if (opts.includeFeatureToggles !== false) {
const enabledEnvironments = v4.environments.filter(
(env) => env.enabled !== false,
);
const featureAndEnvs = v4.featureEnvironments.map((fE) => {
const envDetails = enabledEnvironments.find(
(env) => fE.environment === env.name,
);
return { ...fE, ...envDetails, active: fE.enabled };
});
v4.features = v4.features.map((f) => {
const variants = featureAndEnvs
.sort((e1, e2) => {
if (e1.active !== e2.active) {
return e1.active ? -1 : 1;
}
if (
e1.type !== 'production' ||
e2.type !== 'production'
) {
if (e1.type === 'production') {
return -1;
} else if (e2.type === 'production') {
return 1;
}
}
return e1.sortOrder - e2.sortOrder;
})
.find((fe) => fe.featureName === f.name)?.variants;
return { ...f, variants };
});
v4.featureEnvironments = v4.featureEnvironments.map((fe) => {
delete fe.variants;
return fe;
});
}
// only if explicitly set to false (i.e. undefined defaults to true)
if (opts.includeEnvironments === false) {
delete v4.environments;
} else {
if (v4.environments.length === 0) {
throw Error('Unable to export without enabled environments');
}
}
v4.version = 3;
return v4;
return this.exportV4(opts);
}
async exportV4({

View File

@ -30,10 +30,6 @@ const flags = {
process.env.UNLEASH_EXPERIMENTAL_PROXY_RETURN_ALL_TOGGLES,
false,
),
variantsPerEnvironment: parseEnvVarBoolean(
process.env.UNLEASH_EXPERIMENTAL_VARIANTS_PER_ENVIRONMENT,
false,
),
networkView: parseEnvVarBoolean(
process.env.UNLEASH_EXPERIMENTAL_NETWORK_VIEW,
false,

View File

@ -38,7 +38,6 @@ process.nextTick(async () => {
embedProxyFrontend: true,
anonymiseEventLog: false,
responseTimeWithAppName: true,
variantsPerEnvironment: true,
maintenance: true,
featuresExportImport: true,
newProjectOverview: true,

View File

@ -26,7 +26,6 @@ export function createTestConfig(config?: IUnleashOptions): IUnleashConfig {
flags: {
embedProxy: true,
embedProxyFrontend: true,
variantsPerEnvironment: true,
},
},
};

View File

@ -4,9 +4,6 @@ import getLogger from '../../../fixtures/no-logger';
import { DEFAULT_ENV } from '../../../../lib/util/constants';
import { collectIds } from '../../../../lib/util/collect-ids';
import { ApiTokenType } from '../../../../lib/types/models/api-token';
import variantsv3 from '../../../examples/variantsexport_v3.json';
import v3WithDefaultDisabled from '../../../examples/exported3-with-default-disabled.json';
import { StateService } from '../../../../lib/services';
const importData = require('../../../examples/import.json');
@ -435,66 +432,3 @@ test(`should not show environment on feature toggle, when environment is disable
expect(result[1].name).toBe('production');
expect(result[1].enabled).toBeFalsy();
});
test(`should handle v3 export with variants in features`, async () => {
app.services.stateService = new StateService(db.stores, {
getLogger,
flagResolver: {
isEnabled: () => false,
getAll: () => ({}),
},
});
await app.request
.post('/api/admin/state/import?drop=true')
.attach('file', 'src/test/examples/variantsexport_v3.json')
.expect(202);
const exported = await app.services.stateService.export({});
let exportedFeatures = exported.features
.map((f) => {
delete f.createdAt;
return f;
})
.sort();
let importedFeatures = variantsv3.features
.map((f) => {
delete f.createdAt;
return f;
})
.sort();
expect(exportedFeatures).toStrictEqual(importedFeatures);
});
test(`should handle v3 export with variants in features and only 1 env`, async () => {
app.services.stateService = new StateService(db.stores, {
getLogger,
flagResolver: {
isEnabled: () => false,
getAll: () => ({}),
},
});
await app.request
.post('/api/admin/state/import?drop=true')
.attach(
'file',
'src/test/examples/exported3-with-default-disabled.json',
)
.expect(202);
const exported = await app.services.stateService.export({});
let exportedFeatures = exported.features
.map((f) => {
delete f.createdAt;
return f;
})
.sort();
let importedFeatures = v3WithDefaultDisabled.features
.map((f) => {
delete f.createdAt;
return f;
})
.sort();
expect(exportedFeatures).toStrictEqual(importedFeatures);
});