From 2f1848f6fd7ce2c56c54f516af245d089266075b Mon Sep 17 00:00:00 2001 From: Fredrik Strand Oseberg Date: Mon, 10 May 2021 13:22:22 +0200 Subject: [PATCH] Fix/feedback on create (#292) * fix: copy feature toggle instead of setting newVariants on the reference * fix: remove console log * fix: update messages * fix: give feedback on strategy actions * fix: do not allow feature toggle to be created with empty name * fix: disable delete if only one strategy is applied * fix: archive view * fix: set name field on add variant required * fix: set required on feature toggle name --- frontend/README.md | 1 - .../context/ContextList/ContextList.jsx | 2 - .../FeatureToggleList/FeatureToggleList.jsx | 99 +++++++++++-------- .../feature/FeatureToggleList/index.jsx | 47 ++++++--- .../create/CreateFeature/CreateFeature.jsx | 1 + .../feature/create/CreateFeature/index.jsx | 19 ++-- .../feature/create/copy-feature-component.jsx | 23 +++-- .../strategy/StrategyCard/StrategyCard.jsx | 7 +- .../StrategyCardHeader/StrategyCardHeader.jsx | 43 ++++++-- .../strategy/strategies-list-component.jsx | 3 + .../feature/variant/AddVariant/AddVariant.jsx | 16 ++- .../variant/update-variant-component.jsx | 10 +- .../variant/update-variant-container.jsx | 20 +++- .../CreateStrategy/CreateStrategy.jsx | 21 +++- .../StrategiesList/StrategiesList.jsx | 48 ++++++++- .../strategies/StrategiesList/index.jsx | 20 ++-- .../user/authentication-custom-component.jsx | 4 +- frontend/src/store/api-helper.js | 38 +++++-- frontend/src/store/error/index.js | 5 + frontend/src/store/feature-toggle/actions.js | 19 ++-- frontend/src/store/feature-toggle/api.js | 8 +- frontend/src/store/strategy/actions.js | 25 +++++ 22 files changed, 347 insertions(+), 132 deletions(-) diff --git a/frontend/README.md b/frontend/README.md index 9819ee560b..6f20b8c6a7 100644 --- a/frontend/README.md +++ b/frontend/README.md @@ -23,7 +23,6 @@ Now you should be able to review rendering information in the console. If you do You need to first start the unleash-api on port 4242 before you can start working on unleash-frontend. - Start webpack-dev-server with hot-reload: ```bash diff --git a/frontend/src/component/context/ContextList/ContextList.jsx b/frontend/src/component/context/ContextList/ContextList.jsx index 6042c1a07a..0ba5874cd8 100644 --- a/frontend/src/component/context/ContextList/ContextList.jsx +++ b/frontend/src/component/context/ContextList/ContextList.jsx @@ -26,8 +26,6 @@ const ContextList = ({ removeContextField, history, contextFields }) => { const [showDelDialogue, setShowDelDialogue] = useState(false); const [name, setName] = useState(); - console.log(contextFields); - const styles = useStyles(); const contextList = () => contextFields.map(field => ( diff --git a/frontend/src/component/feature/FeatureToggleList/FeatureToggleList.jsx b/frontend/src/component/feature/FeatureToggleList/FeatureToggleList.jsx index 3acf5bca66..ed2f4c118a 100644 --- a/frontend/src/component/feature/FeatureToggleList/FeatureToggleList.jsx +++ b/frontend/src/component/feature/FeatureToggleList/FeatureToggleList.jsx @@ -36,6 +36,7 @@ const FeatureToggleList = ({ updateSetting, featureMetrics, toggleFeature, + archive, loading, }) => { const { hasAccess } = useContext(AccessContext); @@ -93,11 +94,23 @@ const FeatureToggleList = ({ /> ))} elseShow={ - - No features available. Get started by adding a new - feature toggle. - Add your first toggle - + + No archived features. + + } + elseShow={ + + No features available. Get started by adding a + new feature toggle. + + Add your first toggle + + + } + /> } /> ); @@ -134,45 +147,49 @@ const FeatureToggleList = ({ /> } /> - - - add - - - } - elseShow={ - + elseShow={ + + } + /> } /> diff --git a/frontend/src/component/feature/FeatureToggleList/index.jsx b/frontend/src/component/feature/FeatureToggleList/index.jsx index 6013c6957f..28b4de94ba 100644 --- a/frontend/src/component/feature/FeatureToggleList/index.jsx +++ b/frontend/src/component/feature/FeatureToggleList/index.jsx @@ -1,5 +1,8 @@ import { connect } from 'react-redux'; -import { toggleFeature, fetchFeatureToggles } from '../../../store/feature-toggle/actions'; +import { + toggleFeature, + fetchFeatureToggles, +} from '../../../store/feature-toggle/actions'; import { updateSettingForGroup } from '../../../store/settings/actions'; import FeatureToggleList from './FeatureToggleList'; @@ -11,18 +14,23 @@ function checkConstraints(strategy, regex) { } function resolveCurrentProjectId(settings) { - if(!settings.currentProjectId || settings.currentProjectId === '*') { + if (!settings.currentProjectId || settings.currentProjectId === '*') { return 'default'; - } return settings.currentProjectId; + } + return settings.currentProjectId; } export const mapStateToPropsConfigurable = isFeature => state => { const featureMetrics = state.featureMetrics.toJS(); const settings = state.settings.toJS().feature || {}; - let features = isFeature ? state.features.toJS() : state.archive.get('list').toArray(); + let features = isFeature + ? state.features.toJS() + : state.archive.get('list').toArray(); if (settings.currentProjectId && settings.currentProjectId !== '*') { - features = features.filter(f => f.project === settings.currentProjectId); + features = features.filter( + f => f.project === settings.currentProjectId + ); } if (settings.filter) { @@ -33,8 +41,11 @@ export const mapStateToPropsConfigurable = isFeature => state => { feature.strategies.some(s => checkConstraints(s, regex)) || regex.test(feature.name) || regex.test(feature.description) || - feature.strategies.some(s => s && s.name && regex.test(s.name)) || - (settings.filter.length > 1 && regex.test(JSON.stringify(feature))) + feature.strategies.some( + s => s && s.name && regex.test(s.name) + ) || + (settings.filter.length > 1 && + regex.test(JSON.stringify(feature))) ); } catch (e) { // Invalid filter regex @@ -56,9 +67,13 @@ export const mapStateToPropsConfigurable = isFeature => state => { a.stale === b.stale ? 0 : a.stale ? -1 : 1 ); } else if (settings.sort === 'created') { - features = features.sort((a, b) => (new Date(a.createdAt) > new Date(b.createdAt) ? -1 : 1)); + features = features.sort((a, b) => + new Date(a.createdAt) > new Date(b.createdAt) ? -1 : 1 + ); } else if (settings.sort === 'Last seen') { - features = features.sort((a, b) => (new Date(a.lastSeenAt) > new Date(b.lastSeenAt) ? -1 : 1)); + features = features.sort((a, b) => + new Date(a.lastSeenAt) > new Date(b.lastSeenAt) ? -1 : 1 + ); } else if (settings.sort === 'name') { features = features.sort((a, b) => { if (a.name < b.name) { @@ -70,7 +85,9 @@ export const mapStateToPropsConfigurable = isFeature => state => { return 0; }); } else if (settings.sort === 'strategies') { - features = features.sort((a, b) => (a.strategies.length > b.strategies.length ? -1 : 1)); + features = features.sort((a, b) => + a.strategies.length > b.strategies.length ? -1 : 1 + ); } else if (settings.sort === 'type') { features = features.sort((a, b) => { if (a.type < b.type) { @@ -82,7 +99,9 @@ export const mapStateToPropsConfigurable = isFeature => state => { return 0; }); } else if (settings.sort === 'metrics') { - const target = settings.showLastHour ? featureMetrics.lastHour : featureMetrics.lastMinute; + const target = settings.showLastHour + ? featureMetrics.lastHour + : featureMetrics.lastMinute; features = features.sort((a, b) => { if (!target[a.name]) { @@ -102,6 +121,7 @@ export const mapStateToPropsConfigurable = isFeature => state => { features, currentProjectId: resolveCurrentProjectId(settings), featureMetrics, + archive: !isFeature, settings, loading: state.apiCalls.fetchTogglesState.loading, }; @@ -113,6 +133,9 @@ const mapDispatchToProps = { updateSetting: updateSettingForGroup('feature'), }; -const FeatureToggleListContainer = connect(mapStateToProps, mapDispatchToProps)(FeatureToggleList); +const FeatureToggleListContainer = connect( + mapStateToProps, + mapDispatchToProps +)(FeatureToggleList); export default FeatureToggleListContainer; diff --git a/frontend/src/component/feature/create/CreateFeature/CreateFeature.jsx b/frontend/src/component/feature/create/CreateFeature/CreateFeature.jsx index df299a7b24..c424e219be 100644 --- a/frontend/src/component/feature/create/CreateFeature/CreateFeature.jsx +++ b/frontend/src/component/feature/create/CreateFeature/CreateFeature.jsx @@ -45,6 +45,7 @@ const CreateFeature = ({ size="small" variant="outlined" label="Name" + required placeholder="Unique-name" className={styles.nameInput} name="name" diff --git a/frontend/src/component/feature/create/CreateFeature/index.jsx b/frontend/src/component/feature/create/CreateFeature/index.jsx index fb2fbc7ffa..a1f451d236 100644 --- a/frontend/src/component/feature/create/CreateFeature/index.jsx +++ b/frontend/src/component/feature/create/CreateFeature/index.jsx @@ -50,7 +50,7 @@ class WrapperComponent extends Component { }; validateName = async featureToggleName => { - const { errors } = {...this.state}; + const { errors } = { ...this.state }; try { await validateName(featureToggleName); errors.name = undefined; @@ -61,7 +61,7 @@ class WrapperComponent extends Component { this.setState({ errors }); }; - onSubmit = evt => { + onSubmit = async evt => { evt.preventDefault(); const { createFeatureToggles, history } = this.props; const { featureToggle } = this.state; @@ -76,10 +76,17 @@ class WrapperComponent extends Component { featureToggle.strategies = [defaultStrategy]; } - - createFeatureToggles(featureToggle).then(() => - history.push(`/features/strategies/${featureToggle.name}`) - ); + try { + await createFeatureToggles(featureToggle).then(() => + history.push(`/features/strategies/${featureToggle.name}`) + ); + } catch (e) { + if (e.toString().includes('not allowed to be empty')) { + this.setState({ + errors: { name: 'Name is not allowed to be empty' }, + }); + } + } }; onCancel = evt => { diff --git a/frontend/src/component/feature/create/copy-feature-component.jsx b/frontend/src/component/feature/create/copy-feature-component.jsx index a9966d6d26..69789c8811 100644 --- a/frontend/src/component/feature/create/copy-feature-component.jsx +++ b/frontend/src/component/feature/create/copy-feature-component.jsx @@ -16,6 +16,8 @@ import { styles as commonStyles } from '../../common'; import styles from './copy-feature-component.module.scss'; import { trim } from '../../common/util'; +import ConditionallyRender from '../../common/ConditionallyRender'; +import { Alert } from '@material-ui/lab'; class CopyFeatureComponent extends Component { // static displayName = `AddFeatureComponent-${getDisplayName(Component)}`; @@ -62,7 +64,7 @@ class CopyFeatureComponent extends Component { } }; - onSubmit = evt => { + onSubmit = async evt => { evt.preventDefault(); const { nameError, newToggleName, replaceGroupId } = this.state; @@ -82,11 +84,15 @@ class CopyFeatureComponent extends Component { }); } - this.props - .createFeatureToggle(copyToggle) - .then(() => - history.push(`/features/strategies/${copyToggle.name}`) - ); + try { + this.props + .createFeatureToggle(copyToggle) + .then(() => + history.push(`/features/strategies/${copyToggle.name}`) + ); + } catch (e) { + this.setState({ apiError: e }); + } }; render() { @@ -104,7 +110,10 @@ class CopyFeatureComponent extends Component {

Copy {copyToggle.name}

- + {this.state.apiError}} + />

You are about to create a new feature toggle by cloning diff --git a/frontend/src/component/feature/strategy/StrategyCard/StrategyCard.jsx b/frontend/src/component/feature/strategy/StrategyCard/StrategyCard.jsx index 226e038460..2226e8ac03 100644 --- a/frontend/src/component/feature/strategy/StrategyCard/StrategyCard.jsx +++ b/frontend/src/component/feature/strategy/StrategyCard/StrategyCard.jsx @@ -14,6 +14,7 @@ const StrategyCard = ({ connectDragPreview, connectDragSource, removeStrategy, + disableDelete, editStrategy, connectDropTarget, }) => { @@ -28,9 +29,13 @@ const StrategyCard = ({ connectDragSource={connectDragSource} removeStrategy={removeStrategy} editStrategy={editStrategy} + disableDelete={disableDelete} /> - + diff --git a/frontend/src/component/feature/strategy/StrategyCard/StrategyCardHeader/StrategyCardHeader.jsx b/frontend/src/component/feature/strategy/StrategyCard/StrategyCardHeader/StrategyCardHeader.jsx index 8b80695fd1..1cf0624897 100644 --- a/frontend/src/component/feature/strategy/StrategyCard/StrategyCardHeader/StrategyCardHeader.jsx +++ b/frontend/src/component/feature/strategy/StrategyCard/StrategyCardHeader/StrategyCardHeader.jsx @@ -11,12 +11,14 @@ import { import { useStyles } from './StrategyCardHeader.styles.js'; import { ReactComponent as ReorderIcon } from '../../../../../assets/icons/reorder.svg'; +import ConditionallyRender from '../../../../common/ConditionallyRender/ConditionallyRender'; const StrategyCardHeader = ({ name, connectDragSource, removeStrategy, editStrategy, + disableDelete, }) => { const styles = useStyles(); @@ -51,13 +53,40 @@ const StrategyCardHeader = ({ )} - - - - delete - - - + + + + + delete + + + + + } + elseShow={ + + + + delete + + + + } + /> } diff --git a/frontend/src/component/feature/strategy/strategies-list-component.jsx b/frontend/src/component/feature/strategy/strategies-list-component.jsx index 294d1d6588..ae87d0b2cd 100644 --- a/frontend/src/component/feature/strategy/strategies-list-component.jsx +++ b/frontend/src/component/feature/strategy/strategies-list-component.jsx @@ -124,6 +124,8 @@ const StrategiesList = props => { return strategies.find(s => s.name === strategyName); }; + const disableDelete = editableStrategies.length === 1; + const cards = editableStrategies.map((strategy, i) => ( { moveStrategy={moveStrategy} editStrategy={() => setEditStrategyIndex(i)} index={i} + disableDelete={disableDelete} movable /> )); diff --git a/frontend/src/component/feature/variant/AddVariant/AddVariant.jsx b/frontend/src/component/feature/variant/AddVariant/AddVariant.jsx index 5c27ab2fca..785ae3ba45 100644 --- a/frontend/src/component/feature/variant/AddVariant/AddVariant.jsx +++ b/frontend/src/component/feature/variant/AddVariant/AddVariant.jsx @@ -86,10 +86,10 @@ const AddVariant = ({ }; const submit = async e => { + setError({}); e.preventDefault(); const validationError = validateName(data.name); - if (validationError) { setError(validationError); return; @@ -112,8 +112,12 @@ const AddVariant = ({ clear(); closeDialog(); } catch (error) { - const msg = error.message || 'Could not add variant'; - setError({ general: msg }); + if (error.message.includes('duplicate value')) { + setError({ name: 'A variant with that name already exists.' }); + } else { + const msg = error.message || 'Could not add variant'; + setError({ general: msg }); + } } }; @@ -188,9 +192,11 @@ const AddVariant = ({ name="name" placeholder="" className={commonStyles.fullWidth} + helperText={error.name} value={data.name || ''} - error={error.name} + error={Boolean(error.name)} variant="outlined" + required size="small" type="name" onChange={setVariantValue} @@ -214,7 +220,7 @@ const AddVariant = ({ }} style={{ marginRight: '0.8rem' }} value={data.weight || ''} - error={error.weight} + error={Boolean(error.weight)} type="number" disabled={!isFixWeight} onChange={setVariantValue} diff --git a/frontend/src/component/feature/variant/update-variant-component.jsx b/frontend/src/component/feature/variant/update-variant-component.jsx index 982a82fe64..8154add14d 100644 --- a/frontend/src/component/feature/variant/update-variant-component.jsx +++ b/frontend/src/component/feature/variant/update-variant-component.jsx @@ -1,4 +1,4 @@ -import React, { Component } from 'react'; +import { Component } from 'react'; import PropTypes from 'prop-types'; import classnames from 'classnames'; @@ -63,12 +63,16 @@ class UpdateVariantComponent extends Component { onRemoveVariant = (e, index) => { e.preventDefault(); - this.props.removeVariant(index); + try { + this.props.removeVariant(index); + } catch (e) { + console.log('An exception was caught.'); + } }; renderVariant = (variant, index) => ( this.openEditVariant(e, index, variant)} removeVariant={e => this.onRemoveVariant(e, index)} diff --git a/frontend/src/component/feature/variant/update-variant-container.jsx b/frontend/src/component/feature/variant/update-variant-container.jsx index 0344e0b52f..20868a5221 100644 --- a/frontend/src/component/feature/variant/update-variant-container.jsx +++ b/frontend/src/component/feature/variant/update-variant-container.jsx @@ -6,7 +6,10 @@ import { updateWeight } from '../../common/util'; const mapStateToProps = (state, ownProps) => ({ variants: ownProps.featureToggle.variants || [], - stickinessOptions: ['default', ...state.context.filter(c => c.stickiness).map(c => c.name)], + stickinessOptions: [ + 'default', + ...state.context.filter(c => c.stickiness).map(c => c.name), + ], }); const mapDispatchToProps = (dispatch, ownProps) => ({ @@ -15,11 +18,15 @@ const mapDispatchToProps = (dispatch, ownProps) => ({ const currentVariants = featureToggle.variants || []; const variants = [...currentVariants, variant]; updateWeight(variants, 1000); - return requestUpdateFeatureToggleVariants(featureToggle, variants)(dispatch); + return requestUpdateFeatureToggleVariants( + featureToggle, + variants + )(dispatch); }, removeVariant: index => { const { featureToggle } = ownProps; const currentVariants = featureToggle.variants || []; + const variants = currentVariants.filter((v, i) => i !== index); if (variants.length > 0) { updateWeight(variants, 1000); @@ -29,7 +36,9 @@ const mapDispatchToProps = (dispatch, ownProps) => ({ updateVariant: (index, variant) => { const { featureToggle } = ownProps; const currentVariants = featureToggle.variants || []; - const variants = currentVariants.map((v, i) => (i === index ? variant : v)); + const variants = currentVariants.map((v, i) => + i === index ? variant : v + ); updateWeight(variants, 1000); requestUpdateFeatureToggleVariants(featureToggle, variants)(dispatch); }, @@ -41,4 +50,7 @@ const mapDispatchToProps = (dispatch, ownProps) => ({ }, }); -export default connect(mapStateToProps, mapDispatchToProps)(UpdateFeatureToggleComponent); +export default connect( + mapStateToProps, + mapDispatchToProps +)(UpdateFeatureToggleComponent); diff --git a/frontend/src/component/strategies/CreateStrategy/CreateStrategy.jsx b/frontend/src/component/strategies/CreateStrategy/CreateStrategy.jsx index 3fc1e4fd2b..711083281b 100644 --- a/frontend/src/component/strategies/CreateStrategy/CreateStrategy.jsx +++ b/frontend/src/component/strategies/CreateStrategy/CreateStrategy.jsx @@ -88,9 +88,24 @@ const CreateStrategy = ({ Add parameter - + Update + + } + elseShow={ + + } /> diff --git a/frontend/src/component/strategies/StrategiesList/StrategiesList.jsx b/frontend/src/component/strategies/StrategiesList/StrategiesList.jsx index 3df688edd9..6ef53722d3 100644 --- a/frontend/src/component/strategies/StrategiesList/StrategiesList.jsx +++ b/frontend/src/component/strategies/StrategiesList/StrategiesList.jsx @@ -1,4 +1,4 @@ -import React, { useContext, useEffect } from 'react'; +import { useContext, useEffect, useState } from 'react'; import PropTypes from 'prop-types'; import classnames from 'classnames'; import { Link, useHistory } from 'react-router-dom'; @@ -25,6 +25,7 @@ import HeaderTitle from '../../common/HeaderTitle'; import { useStyles } from './styles'; import AccessContext from '../../../contexts/AccessContext'; +import Dialogue from '../../common/Dialogue'; const StrategiesList = ({ strategies, @@ -37,6 +38,7 @@ const StrategiesList = ({ const styles = useStyles(); const smallScreen = useMediaQuery('(max-width:700px)'); const { hasAccess } = useContext(AccessContext); + const [dialogueMetaData, setDialogueMetaData] = useState({ show: false }); useEffect(() => { fetchStrategies(); @@ -86,7 +88,15 @@ const StrategiesList = ({ const reactivateButton = strategy => ( - reactivateStrategy(strategy)}> + + setDialogueMetaData({ + show: true, + title: 'Really reactivate strategy?', + onConfirm: () => reactivateStrategy(strategy), + }) + } + > visibility @@ -107,7 +117,16 @@ const StrategiesList = ({ elseShow={

- deprecateStrategy(strategy)}> + + setDialogueMetaData({ + show: true, + title: 'Really deprecate strategy?', + onConfirm: () => + deprecateStrategy(strategy), + }) + } + > visibility_off
@@ -121,7 +140,15 @@ const StrategiesList = ({ condition={strategy.editable} show={ - removeStrategy(strategy)}> + + setDialogueMetaData({ + show: true, + title: 'Really delete strategy?', + onConfirm: () => removeStrategy(strategy), + }) + } + > delete @@ -167,12 +194,25 @@ const StrategiesList = ({ )); + const onDialogConfirm = () => { + dialogueMetaData?.onConfirm(); + setDialogueMetaData(prev => ({ ...prev, show: false })); + }; + return ( } > + + setDialogueMetaData(prev => ({ ...prev, show: false })) + } + /> 0} diff --git a/frontend/src/component/strategies/StrategiesList/index.jsx b/frontend/src/component/strategies/StrategiesList/index.jsx index 1d1cb12518..9703490dc9 100644 --- a/frontend/src/component/strategies/StrategiesList/index.jsx +++ b/frontend/src/component/strategies/StrategiesList/index.jsx @@ -17,26 +17,20 @@ const mapStateToProps = state => { const mapDispatchToProps = dispatch => ({ removeStrategy: strategy => { - // eslint-disable-next-line no-alert - if (window.confirm('Are you sure you want to remove this strategy?')) { - removeStrategy(strategy)(dispatch); - } + removeStrategy(strategy)(dispatch); }, deprecateStrategy: strategy => { - // eslint-disable-next-line no-alert - if (window.confirm('Are you sure you want to deprecate this strategy?')) { - deprecateStrategy(strategy)(dispatch); - } + deprecateStrategy(strategy)(dispatch); }, reactivateStrategy: strategy => { - // eslint-disable-next-line no-alert - if (window.confirm('Are you sure you want to reactivate this strategy?')) { - reactivateStrategy(strategy)(dispatch); - } + reactivateStrategy(strategy)(dispatch); }, fetchStrategies: () => fetchStrategies()(dispatch), }); -const StrategiesListContainer = connect(mapStateToProps, mapDispatchToProps)(StrategiesList); +const StrategiesListContainer = connect( + mapStateToProps, + mapDispatchToProps +)(StrategiesList); export default StrategiesListContainer; diff --git a/frontend/src/component/user/authentication-custom-component.jsx b/frontend/src/component/user/authentication-custom-component.jsx index 7c0467838f..dee299b2dc 100644 --- a/frontend/src/component/user/authentication-custom-component.jsx +++ b/frontend/src/component/user/authentication-custom-component.jsx @@ -14,9 +14,7 @@ class AuthenticationCustomComponent extends React.Component {

{authDetails.message}

- + diff --git a/frontend/src/store/api-helper.js b/frontend/src/store/api-helper.js index 19b0af36a2..9cf0a1e8ad 100644 --- a/frontend/src/store/api-helper.js +++ b/frontend/src/store/api-helper.js @@ -1,7 +1,9 @@ const defaultErrorMessage = 'Unexpected exception when talking to unleash-api'; function extractJoiMsg(body) { - return body.details.length > 0 ? body.details[0].message : defaultErrorMessage; + return body.details.length > 0 + ? body.details[0].message + : defaultErrorMessage; } function extractLegacyMsg(body) { return body && body.length > 0 ? body[0].msg : defaultErrorMessage; @@ -35,7 +37,9 @@ export class ForbiddenError extends Error { export class NotFoundError extends Error { constructor(statusCode) { - super('The requested resource could not be found but may be available in the future'); + super( + 'The requested resource could not be found but may be available in the future' + ); this.name = 'NotFoundError'; this.statusCode = statusCode; } @@ -45,11 +49,19 @@ export function throwIfNotSuccess(response) { if (!response.ok) { if (response.status === 401) { return new Promise((resolve, reject) => { - response.json().then(body => reject(new AuthenticationError(response.status, body))); + response + .json() + .then(body => + reject(new AuthenticationError(response.status, body)) + ); }); } else if (response.status === 403) { return new Promise((resolve, reject) => { - response.json().then(body => reject(new ForbiddenError(response.status, body))); + response + .json() + .then(body => + reject(new ForbiddenError(response.status, body)) + ); }); } else if (response.status === 404) { return new Promise((resolve, reject) => { @@ -57,12 +69,18 @@ export function throwIfNotSuccess(response) { }); } else if (response.status > 399 && response.status < 501) { return new Promise((resolve, reject) => { - response.json().then(body => { - const errorMsg = body && body.isJoi ? extractJoiMsg(body) : extractLegacyMsg(body); - let error = new Error(errorMsg); - error.statusCode = response.status; - reject(error); - }).catch(() => reject(new Error(defaultErrorMessage))) + response + .json() + .then(body => { + const errorMsg = + body && body.isJoi + ? extractJoiMsg(body) + : extractLegacyMsg(body); + let error = new Error(errorMsg); + error.statusCode = response.status; + reject(error); + }) + .catch(() => reject(new Error(defaultErrorMessage))); }); } else { return Promise.reject(new ServiceError(response.status)); diff --git a/frontend/src/store/error/index.js b/frontend/src/store/error/index.js index 3c19646fbc..4544163bb0 100644 --- a/frontend/src/store/error/index.js +++ b/frontend/src/store/error/index.js @@ -13,6 +13,7 @@ import { ERROR_UPDATING_STRATEGY, ERROR_CREATING_STRATEGY, ERROR_RECEIVE_STRATEGIES, + UPDATE_STRATEGY_SUCCESS, } from '../strategy/actions'; import { @@ -79,9 +80,13 @@ const strategies = (state = getInitState(), action) => { return state.update('list', list => list.remove(list.indexOf(action.error)) ); + // This reducer controls not only errors, but general information and success + // messages. This can be a little misleading, given it's naming. We should + // revise how this works in a future update. case UPDATE_FEATURE_TOGGLE: case UPDATE_FEATURE_TOGGLE_STRATEGIES: case UPDATE_APPLICATION_FIELD: + case UPDATE_STRATEGY_SUCCESS: return addErrorIfNotAlreadyInList(state, action.info); default: return state; diff --git a/frontend/src/store/feature-toggle/actions.js b/frontend/src/store/feature-toggle/actions.js index c263d3a1e4..05f4f82637 100644 --- a/frontend/src/store/feature-toggle/actions.js +++ b/frontend/src/store/feature-toggle/actions.js @@ -110,7 +110,10 @@ export function createFeatureToggles(featureToggle) { featureToggle: createdFeature, }); }) - .catch(dispatchError(dispatch, ERROR_CREATING_FEATURE_TOGGLE)); + .catch(e => { + dispatchError(dispatch, ERROR_CREATING_FEATURE_TOGGLE); + throw e; + }); }; } @@ -189,24 +192,28 @@ export function requestUpdateFeatureToggleStrategies( export function requestUpdateFeatureToggleVariants(featureToggle, newVariants) { return dispatch => { - featureToggle.variants = newVariants; + const newFeature = { ...featureToggle }; + newFeature.variants = newVariants; dispatch({ type: START_UPDATE_FEATURE_TOGGLE }); return api - .update(featureToggle) + .update(newFeature) .then(() => { - const info = `${featureToggle.name} successfully updated!`; + const info = `${newFeature.name} successfully updated!`; setTimeout( () => dispatch({ type: MUTE_ERROR, error: info }), 1000 ); return dispatch({ type: UPDATE_FEATURE_TOGGLE_STRATEGIES, - featureToggle, + featureToggle: newFeature, info, }); }) - .catch(dispatchError(dispatch, ERROR_UPDATE_FEATURE_TOGGLE)); + .catch(e => { + dispatchError(dispatch, ERROR_UPDATE_FEATURE_TOGGLE); + throw e; + }); }; } diff --git a/frontend/src/store/feature-toggle/api.js b/frontend/src/store/feature-toggle/api.js index 6a24fbc183..d354d99702 100644 --- a/frontend/src/store/feature-toggle/api.js +++ b/frontend/src/store/feature-toggle/api.js @@ -50,14 +50,14 @@ function validate(featureToggle) { function update(featureToggle) { return validateToggle(featureToggle) - .then(() => - fetch(`${URI}/${featureToggle.name}`, { + .then(() => { + return fetch(`${URI}/${featureToggle.name}`, { method: 'PUT', headers, credentials: 'include', body: JSON.stringify(featureToggle), - }) - ) + }); + }) .then(throwIfNotSuccess); } diff --git a/frontend/src/store/strategy/actions.js b/frontend/src/store/strategy/actions.js index 6a34b9c567..b56a010294 100644 --- a/frontend/src/store/strategy/actions.js +++ b/frontend/src/store/strategy/actions.js @@ -1,6 +1,7 @@ import api from './api'; import applicationApi from '../application/api'; import { dispatchError } from '../util'; +import { MUTE_ERROR } from '../error/actions'; export const ADD_STRATEGY = 'ADD_STRATEGY'; export const UPDATE_STRATEGY = 'UPDATE_STRATEGY'; @@ -19,6 +20,7 @@ export const ERROR_UPDATING_STRATEGY = 'ERROR_UPDATING_STRATEGY'; export const ERROR_REMOVING_STRATEGY = 'ERROR_REMOVING_STRATEGY'; export const ERROR_DEPRECATING_STRATEGY = 'ERROR_DEPRECATING_STRATEGY'; export const ERROR_REACTIVATING_STRATEGY = 'ERROR_REACTIVATING_STRATEGY'; +export const UPDATE_STRATEGY_SUCCESS = 'UPDATE_STRATEGY_SUCCESS'; export const receiveStrategies = json => ({ type: RECEIVE_STRATEGIES, @@ -46,6 +48,14 @@ const reactivateStrategyEvent = strategy => ({ strategy, }); +const setInfoMessage = (info, dispatch) => { + dispatch({ + type: UPDATE_STRATEGY_SUCCESS, + info: info, + }); + setTimeout(() => dispatch({ type: MUTE_ERROR, error: info }), 1500); +}; + export function fetchStrategies() { return dispatch => { dispatch(startRequest()); @@ -64,6 +74,9 @@ export function createStrategy(strategy) { return api .create(strategy) .then(() => dispatch(addStrategy(strategy))) + .then(() => { + setInfoMessage('Strategy successfully created.', dispatch); + }) .catch(e => { dispatchError(dispatch, ERROR_CREATING_STRATEGY); throw e; @@ -78,6 +91,9 @@ export function updateStrategy(strategy) { return api .update(strategy) .then(() => dispatch(updatedStrategy(strategy))) + .then(() => { + setInfoMessage('Strategy successfully updated.', dispatch); + }) .catch(dispatchError(dispatch, ERROR_UPDATING_STRATEGY)); }; } @@ -87,6 +103,9 @@ export function removeStrategy(strategy) { api .remove(strategy) .then(() => dispatch(createRemoveStrategy(strategy))) + .then(() => { + setInfoMessage('Strategy successfully deleted.', dispatch); + }) .catch(dispatchError(dispatch, ERROR_REMOVING_STRATEGY)); } @@ -99,6 +118,9 @@ export function deprecateStrategy(strategy) { dispatch(startDeprecate()); api.deprecate(strategy) .then(() => dispatch(deprecateStrategyEvent(strategy))) + .then(() => + setInfoMessage('Strategy successfully deprecated', dispatch) + ) .catch(dispatchError(dispatch, ERROR_DEPRECATING_STRATEGY)); }; } @@ -108,6 +130,9 @@ export function reactivateStrategy(strategy) { dispatch(startReactivate()); api.reactivate(strategy) .then(() => dispatch(reactivateStrategyEvent(strategy))) + .then(() => + setInfoMessage('Strategy successfully reactivated', dispatch) + ) .catch(dispatchError(dispatch, ERROR_REACTIVATING_STRATEGY)); }; }