1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-09-24 17:51:14 +02:00

Fix/variants: Fix delete one variant + remove switch when add first variant (#466)

Co-authored-by: Ivar Conradi Østhus <ivarconr@gmail.com>
Co-authored-by: Fredrik Oseberg <fredrik.no@gmail.com>
This commit is contained in:
Youssef Khedher 2021-10-28 12:32:29 +01:00 committed by GitHub
parent df9c5c9b30
commit ed6efff643
11 changed files with 175 additions and 58 deletions

View File

@ -276,8 +276,8 @@ describe('feature toggle', () => {
cy.wait('@variantcreation'); cy.wait('@variantcreation');
}); });
it('Can set weight to fixed value for one of the variants', () => { it('Can set weight to fixed value for one of the variants', () => {
const variantName = 'my-new-variant';
cy.wait(500); cy.wait(500);
cy.visit(`/projects/default/features2/${featureToggleName}/variants`); cy.visit(`/projects/default/features2/${featureToggleName}/variants`);
cy.get('[data-test=VARIANT_EDIT_BUTTON]').first().click(); cy.get('[data-test=VARIANT_EDIT_BUTTON]').first().click();
cy.get('[data-test=VARIANT_NAME_INPUT]') cy.get('[data-test=VARIANT_NAME_INPUT]')

View File

@ -50,6 +50,9 @@ export const trim = value => {
}; };
export function updateWeight(variants, totalWeight) { export function updateWeight(variants, totalWeight) {
if (variants.length === 0){
return [];
}
const variantMetadata = variants.reduce( const variantMetadata = variants.reduce(
({ remainingPercentage, variableVariantCount }, variant) => { ({ remainingPercentage, variableVariantCount }, variant) => {
if (variant.weight && variant.weightType === weightTypes.FIX) { if (variant.weight && variant.weightType === weightTypes.FIX) {

View File

@ -40,6 +40,8 @@ import { projectFilterGenerator } from '../../../utils/project-filter-generator'
import { getToggleCopyPath } from '../../../utils/route-path-helpers'; import { getToggleCopyPath } from '../../../utils/route-path-helpers';
import useFeatureApi from '../../../hooks/api/actions/useFeatureApi/useFeatureApi'; import useFeatureApi from '../../../hooks/api/actions/useFeatureApi/useFeatureApi';
import useToast from '../../../hooks/useToast'; import useToast from '../../../hooks/useToast';
import useUiConfig from '../../../hooks/api/getters/useUiConfig/useUiConfig';
import { getTogglePath } from '../../../utils/route-path-helpers';
const FeatureView = ({ const FeatureView = ({
activeTab, activeTab,
@ -68,6 +70,16 @@ const FeatureView = ({
const { changeFeatureProject } = useFeatureApi(); const { changeFeatureProject } = useFeatureApi();
const { toast, setToastData } = useToast(); const { toast, setToastData } = useToast();
const archive = !Boolean(isFeatureView); const archive = !Boolean(isFeatureView);
const { uiConfig } = useUiConfig();
useEffect(() => {
if(uiConfig.flags.E && featureToggle && featureToggle.project) {
const newTogglePAth = getTogglePath(featureToggle.project, featureToggleName, true);
history.push(newTogglePAth);
}
}, [featureToggleName, uiConfig, featureToggle, history])
useEffect(() => { useEffect(() => {
scrollToTop(); scrollToTop();

View File

@ -1,4 +1,4 @@
import React, { useEffect, useState } from 'react'; import { useEffect, useState } from 'react';
import PropTypes from 'prop-types'; import PropTypes from 'prop-types';
import { import {
FormControl, FormControl,
@ -21,6 +21,11 @@ import Dialogue from '../../../../../common/Dialogue';
import { trim, modalStyles } from '../../../../../common/util'; import { trim, modalStyles } from '../../../../../common/util';
import PermissionSwitch from '../../../../../common/PermissionSwitch/PermissionSwitch'; import PermissionSwitch from '../../../../../common/PermissionSwitch/PermissionSwitch';
import { UPDATE_FEATURE } from '../../../../../providers/AccessProvider/permissions'; import { UPDATE_FEATURE } from '../../../../../providers/AccessProvider/permissions';
import useFeature from '../../../../../../hooks/api/getters/useFeature/useFeature';
import { useParams } from 'react-router-dom';
import { IFeatureViewParams } from '../../../../../../interfaces/params';
import { IFeatureVariant } from '../../../../../../interfaces/featureToggle';
import cloneDeep from 'lodash.clonedeep';
const payloadOptions = [ const payloadOptions = [
{ key: 'string', label: 'string' }, { key: 'string', label: 'string' },
@ -34,8 +39,8 @@ const AddVariant = ({
showDialog, showDialog,
closeDialog, closeDialog,
save, save,
validateName,
editVariant, editVariant,
validateName,
title, title,
editing, editing,
}) => { }) => {
@ -44,6 +49,9 @@ const AddVariant = ({
const [overrides, setOverrides] = useState([]); const [overrides, setOverrides] = useState([]);
const [error, setError] = useState({}); const [error, setError] = useState({});
const commonStyles = useCommonStyles(); const commonStyles = useCommonStyles();
const { projectId, featureId } = useParams<IFeatureViewParams>();
const { feature } = useFeature(projectId, featureId);
const [variants, setVariants] = useState<IFeatureVariant[]>([]);
const clear = () => { const clear = () => {
if (editVariant) { if (editVariant) {
@ -55,6 +63,8 @@ const AddVariant = ({
}); });
if (editVariant.payload) { if (editVariant.payload) {
setPayload(editVariant.payload); setPayload(editVariant.payload);
} else {
setPayload(EMPTY_PAYLOAD);
} }
if (editVariant.overrides) { if (editVariant.overrides) {
setOverrides(editVariant.overrides); setOverrides(editVariant.overrides);
@ -69,6 +79,16 @@ const AddVariant = ({
setError({}); setError({});
}; };
const setClonedVariants = clonedVariants =>
setVariants(cloneDeep(clonedVariants));
useEffect(() => {
if (feature) {
setClonedVariants(feature.variants);
}
/* eslint-disable-next-line react-hooks/exhaustive-deps */
}, [feature.variants]);
useEffect(() => { useEffect(() => {
clear(); clear();
// eslint-disable-next-line react-hooks/exhaustive-deps // eslint-disable-next-line react-hooks/exhaustive-deps
@ -119,10 +139,15 @@ const AddVariant = ({
clear(); clear();
closeDialog(); closeDialog();
} catch (error) { } catch (error) {
if (error.message.includes('duplicate value')) { if (error?.body?.details[0]?.message?.includes('duplicate value')) {
setError({ name: 'A variant with that name already exists.' }); setError({ name: 'A variant with that name already exists.' });
} else if (
error?.body?.details[0]?.message?.includes('must be a number')
) {
setError({ weight: 'Weight must be a number' });
} else { } else {
const msg = error.message || 'Could not add variant'; const msg =
error?.body?.details[0]?.message || 'Could not add variant';
setError({ general: msg }); setError({ general: msg });
} }
} }
@ -215,6 +240,37 @@ const AddVariant = ({
/> />
<br /> <br />
<Grid container> <Grid container>
<ConditionallyRender
condition={variants.length > 0}
show={
<Grid
item
md={12}
style={{ marginBottom: '0.5rem' }}
>
<FormControl>
<FormControlLabel
control={
<PermissionSwitch
permission={UPDATE_FEATURE}
name="weightType"
checked={isFixWeight}
data-test={
'VARIANT_WEIGHT_TYPE'
}
onChange={setVariantWeightType}
/>
}
label="Custom percentage"
/>
</FormControl>
</Grid>
}
/>
<ConditionallyRender
condition={data.weightType === weightTypes.FIX}
show={
<Grid item md={4}> <Grid item md={4}>
<TextField <TextField
id="weight" id="weight"
@ -232,29 +288,18 @@ const AddVariant = ({
), ),
}} }}
style={{ marginRight: '0.8rem' }} style={{ marginRight: '0.8rem' }}
value={data.weight || ''} value={data.weight}
error={Boolean(error.weight)} error={Boolean(error.weight)}
helperText={error.weight}
type="number" type="number"
disabled={!isFixWeight} disabled={!isFixWeight}
onChange={setVariantValue} onChange={e => {
setVariantValue(e);
}}
/> />
</Grid> </Grid>
<Grid item md={6}>
<FormControl>
<FormControlLabel
control={
<PermissionSwitch
permission={UPDATE_FEATURE}
name="weightType"
checked={isFixWeight}
data-test={'VARIANT_WEIGHT_TYPE'}
onChange={setVariantWeightType}
/>
} }
label="Custom percentage"
/> />
</FormControl>
</Grid>
</Grid> </Grid>
<p style={{ marginBottom: '1rem' }}> <p style={{ marginBottom: '1rem' }}>
<strong>Payload </strong> <strong>Payload </strong>
@ -339,6 +384,7 @@ AddVariant.propTypes = {
closeDialog: PropTypes.func.isRequired, closeDialog: PropTypes.func.isRequired,
save: PropTypes.func.isRequired, save: PropTypes.func.isRequired,
validateName: PropTypes.func.isRequired, validateName: PropTypes.func.isRequired,
validateWeight: PropTypes.func.isRequired,
editVariant: PropTypes.object, editVariant: PropTypes.object,
title: PropTypes.string, title: PropTypes.string,
uiConfig: PropTypes.object, uiConfig: PropTypes.object,

View File

@ -162,7 +162,6 @@ const FeatureOverviewVariants = () => {
}; };
const removeVariant = async (name: string) => { const removeVariant = async (name: string) => {
console.log(`Removing variant ${name}`);
let updatedVariants = variants.filter(v => v.name !== name); let updatedVariants = variants.filter(v => v.name !== name);
try { try {
await updateVariants( await updateVariants(
@ -204,17 +203,21 @@ const FeatureOverviewVariants = () => {
successText: string successText: string
) => { ) => {
const newVariants = updateWeight(variants, 1000); const newVariants = updateWeight(variants, 1000);
const patch = createPatch(newVariants); const patch = createPatch(newVariants);
if (patch.length === 0) return; if (patch.length === 0) return;
await patchFeatureToggle(projectId, featureId, patch); await patchFeatureToggle(projectId, featureId, patch)
.then(() => {
refetch(); refetch();
setToastData({ setToastData({
show: true, show: true,
type: 'success', type: 'success',
text: successText, text: successText,
}); });
})
.catch(e => {
throw e;
});
}; };
const validateName = (name: string) => { const validateName = (name: string) => {
@ -222,6 +225,14 @@ const FeatureOverviewVariants = () => {
return { name: 'Name is required' }; return { name: 'Name is required' };
} }
}; };
const validateWeight = (weight: number) => {
const weightValue = parseInt(weight);
if (weightValue > 100 || weightValue < 0) {
return { weight: 'weight must be between 0 and 100' };
}
};
const delDialogueMarkup = useDeleteVariantMarkup({ const delDialogueMarkup = useDeleteVariantMarkup({
show: delDialog.show, show: delDialog.show,
onClick: e => { onClick: e => {
@ -280,7 +291,11 @@ const FeatureOverviewVariants = () => {
<PermissionButton <PermissionButton
onClick={() => { onClick={() => {
setEditing(false); setEditing(false);
setEditVariant({}); if (variants.length === 0) {
setEditVariant({ weight: 1000 });
} else {
setEditVariant({ weightType: 'variable' });
}
setShowAddVariant(true); setShowAddVariant(true);
}} }}
className={styles.addVariantButton} className={styles.addVariantButton}
@ -305,6 +320,7 @@ const FeatureOverviewVariants = () => {
}} }}
editing={editing} editing={editing}
validateName={validateName} validateName={validateName}
validateWeight={validateWeight}
editVariant={editVariant} editVariant={editVariant}
title={editing ? 'Edit variant' : 'Add variant'} title={editing ? 'Edit variant' : 'Add variant'}
/> />

View File

@ -6,11 +6,13 @@ interface IRedirectFeatureViewProps {
featureToggle: any; featureToggle: any;
features: any; features: any;
fetchFeatureToggles: () => void; fetchFeatureToggles: () => void;
newPath: boolean;
} }
const RedirectFeatureView = ({ const RedirectFeatureView = ({
featureToggle, featureToggle,
fetchFeatureToggles, fetchFeatureToggles,
newPath = false,
}: IRedirectFeatureViewProps) => { }: IRedirectFeatureViewProps) => {
useEffect(() => { useEffect(() => {
if (!featureToggle) { if (!featureToggle) {
@ -22,7 +24,7 @@ const RedirectFeatureView = ({
if (!featureToggle) return null; if (!featureToggle) return null;
return ( return (
<Redirect <Redirect
to={getTogglePath(featureToggle?.project, featureToggle?.name)} to={getTogglePath(featureToggle?.project, featureToggle?.name, newPath)}
/> />
); );
}; };

View File

@ -4,8 +4,11 @@ import { fetchFeatureToggles } from '../../../store/feature-toggle/actions';
import RedirectFeatureView from './RedirectFeatureView'; import RedirectFeatureView from './RedirectFeatureView';
import { E } from '../../common/flags';
export default connect( export default connect(
(state, props) => ({ (state, props) => ({
newPath: !!state.uiConfig.toJS().flags[E],
featureToggle: state.features featureToggle: state.features
.toJS() .toJS()
.find(toggle => toggle.name === props.featureToggleName), .find(toggle => toggle.name === props.featureToggleName),

View File

@ -75,6 +75,15 @@ Array [
"title": "Create", "title": "Create",
"type": "protected", "type": "protected",
}, },
Object {
"component": [Function],
"layout": "main",
"menu": Object {},
"parent": "/features",
"path": "/projects/:projectId/features/:name",
"title": ":name",
"type": "protected",
},
Object { Object {
"component": [Function], "component": [Function],
"flag": "P", "flag": "P",

View File

@ -117,6 +117,15 @@ export const routes = [
layout: 'main', layout: 'main',
menu: {}, menu: {},
}, },
{
path: '/projects/:projectId/features/:name',
parent: '/features',
title: ':name',
component: RedirectFeatureViewPage,
type: 'protected',
layout: 'main',
menu: {},
},
{ {
path: '/projects/:id/:activeTab', path: '/projects/:id/:activeTab',
parent: '/projects', parent: '/projects',

View File

@ -8,6 +8,7 @@ import {
} from '../../../../constants/statusCodes'; } from '../../../../constants/statusCodes';
import { import {
AuthenticationError, AuthenticationError,
BadRequestError,
ForbiddenError, ForbiddenError,
headers, headers,
NotFoundError, NotFoundError,
@ -107,7 +108,8 @@ const useAPI = ({
} }
if (propagateErrors) { if (propagateErrors) {
throw new Error(); const response = await res.json();
throw new BadRequestError(res.status, response);
} }
} }

View File

@ -28,13 +28,28 @@ export class AuthenticationError extends Error {
export class ForbiddenError extends Error { export class ForbiddenError extends Error {
constructor(statusCode, body = {}) { constructor(statusCode, body = {}) {
super(body.details?.length > 0 ? body.details[0].message : 'You cannot perform this action'); super(
body.details?.length > 0
? body.details[0].message
: 'You cannot perform this action'
);
this.name = 'ForbiddenError'; this.name = 'ForbiddenError';
this.statusCode = statusCode; this.statusCode = statusCode;
this.body = body; this.body = body;
} }
} }
export class BadRequestError extends Error {
constructor(statusCode, body = {}) {
super(
body.details?.length > 0 ? body.details[0].message : 'Bad request'
);
this.name = 'BadRequestError';
this.statusCode = statusCode;
this.body = body;
}
}
export class NotFoundError extends Error { export class NotFoundError extends Error {
constructor(statusCode) { constructor(statusCode) {
super( super(