From f8e34d53ffe0b7540c3584a0bd3aa000e2a72e95 Mon Sep 17 00:00:00 2001 From: Fredrik Strand Oseberg Date: Wed, 28 Apr 2021 14:27:25 +0200 Subject: [PATCH] Fix/bugfixes (#279) * fix: add try catch to copy * fix: show constraints on default strategy * fix: require name to submit context field * fix: require name and project id to be set in order to create a project * fix: change documentation icon * fix: only validate unique names on create * Update src/component/context/form-context-component.jsx Co-authored-by: Christopher Kolstad Co-authored-by: Christopher Kolstad --- .../component/context/ContextList/styles.js | 4 + .../context/form-context-component.jsx | 145 +++++++++++++++--- .../StrategyCardContent.jsx | 33 +++- .../StrategyCardContentDefault.jsx | 23 ++- frontend/src/component/menu/Header/Header.jsx | 5 +- .../component/project/ProjectList/styles.js | 4 + .../project/form-project-component.jsx | 71 +++++++-- .../UserInviteLink/UserInviteLink.tsx | 37 +++-- frontend/src/store/context/actions.js | 5 +- 9 files changed, 262 insertions(+), 65 deletions(-) diff --git a/frontend/src/component/context/ContextList/styles.js b/frontend/src/component/context/ContextList/styles.js index 1fd39fd466..fb054b3439 100644 --- a/frontend/src/component/context/ContextList/styles.js +++ b/frontend/src/component/context/ContextList/styles.js @@ -3,5 +3,9 @@ import { makeStyles } from '@material-ui/styles'; export const useStyles = makeStyles({ listItem: { padding: 0, + ['& a']: { + textDecoration: 'none', + color: 'inherit', + }, }, }); diff --git a/frontend/src/component/context/form-context-component.jsx b/frontend/src/component/context/form-context-component.jsx index 4134c2d4c2..2b4514ebfe 100644 --- a/frontend/src/component/context/form-context-component.jsx +++ b/frontend/src/component/context/form-context-component.jsx @@ -1,11 +1,20 @@ -import React, { Component } from 'react'; +import { Component } from 'react'; import PropTypes from 'prop-types'; -import { Button, Chip, TextField, Switch, Icon, Typography } from '@material-ui/core'; +import { + Button, + Chip, + TextField, + Switch, + Icon, + Typography, +} from '@material-ui/core'; import styles from './Context.module.scss'; import classnames from 'classnames'; import { FormButtons, styles as commonStyles } from '../common'; import { trim } from '../common/util'; import PageContent from '../common/PageContent/PageContent'; +import ConditionallyRender from '../common/ConditionallyRender'; +import { Alert } from '@material-ui/lab'; const sortIgnoreCase = (a, b) => { a = a.toLowerCase(); @@ -23,9 +32,26 @@ class AddContextComponent extends Component { errors: {}, currentLegalValue: '', dirty: false, + focusedLegalValue: false, }; } + handleKeydown = e => { + if (e.key === 'Enter' && this.state.focusedLegalValue) { + this.addLegalValue(e); + } else if (e.key === 'Enter') { + this.onSubmit(e); + } + }; + + componentDidMount() { + window.addEventListener('keydown', this.handleKeydown); + } + + componentWillUnmount() { + window.removeEventListener('keydown', this.handleKeydown); + } + static getDerivedStateFromProps(props, state) { if (state.contextField.initial && !props.contextField.initial) { return { contextField: props.contextField }; @@ -42,15 +68,19 @@ class AddContextComponent extends Component { validateContextName = async name => { const { errors } = this.state; - const { validateName } = this.props; + const { validateName, editMode } = this.props; + + if (editMode) return true; + try { await validateName(name); errors.name = undefined; } catch (err) { errors.name = err.message; } - this.setState({ errors }); + if (errors.name) return false; + return true; }; onCancel = evt => { @@ -58,10 +88,23 @@ class AddContextComponent extends Component { this.props.history.push('/context'); }; - onSubmit = evt => { + onSubmit = async evt => { evt.preventDefault(); const { contextField } = this.state; - this.props.submit(contextField).then(() => this.props.history.push('/context')); + + const valid = await this.validateContextName(contextField.name); + + if (valid) { + this.props + .submit(contextField) + .then(() => this.props.history.push('/context')) + .catch(e => + this.setState(prev => ({ + ...prev, + errors: { api: e.toString() }, + })) + ); + } }; updateCurrentLegalValue = evt => { @@ -82,7 +125,9 @@ class AddContextComponent extends Component { return; } - const legalValues = contextField.legalValues.concat(trim(currentLegalValue)); + const legalValues = contextField.legalValues.concat( + trim(currentLegalValue) + ); contextField.legalValues = legalValues.sort(sortIgnoreCase); this.setState({ contextField, @@ -93,7 +138,9 @@ class AddContextComponent extends Component { removeLegalValue = index => { const { contextField } = this.state; - const legalValues = contextField.legalValues.filter((_, i) => i !== index); + const legalValues = contextField.legalValues.filter( + (_, i) => i !== index + ); contextField.legalValues = legalValues; this.setState({ contextField }); }; @@ -115,11 +162,20 @@ class AddContextComponent extends Component { return (
- Context fields are a basic building block used in Unleash to control roll-out. They can be used - together with strategy constraints as part of the activation strategy evaluation. + Context fields are a basic building block used in Unleash to + control roll-out. They can be used together with strategy + constraints as part of the activation strategy evaluation.
+ + {this.state.errors.api} + + } + /> this.validateContextName(v.target.value)} - onChange={v => this.setValue('name', trim(v.target.value))} + onBlur={v => + this.validateContextName(v.target.value) + } + onChange={v => + this.setValue('name', trim(v.target.value)) + } /> this.setValue('description', v.target.value)} + onChange={v => + this.setValue('description', v.target.value) + } />

@@ -150,8 +212,10 @@ class AddContextComponent extends Component {
Legal values

- By defining the legal values the Unleash Admin UI will validate the user input. A concrete - example would be that we know all values for our “environment” (local, development, stage, + By defining the legal values the Unleash Admin UI + will validate the user input. A concrete example + would be that we know all values for our + “environment” (local, development, stage, production).

@@ -159,6 +223,18 @@ class AddContextComponent extends Component { label="Value" name="value" className={styles.valueField} + onFocus={() => + this.setState(prev => ({ + ...prev, + focusedLegalValue: true, + })) + } + onBlur={() => + this.setState(prev => ({ + ...prev, + focusedLegalValue: false, + })) + } value={this.state.currentLegalValue} error={!!errors.currentLegalValue} helperText={errors.currentLegalValue} @@ -176,15 +252,28 @@ class AddContextComponent extends Component { Add
-
{contextField.legalValues.map(this.renderLegalValue)}
+
+ {contextField.legalValues.map( + this.renderLegalValue + )} +

- Custom stickiness (beta) -

- By enabling stickiness on this context field you can use it together with the - flexible-rollout strategy. This will guarantee a consistent behavior for specific values of - this context field. PS! Not all client SDK's support this feature yet!{' '} + + Custom stickiness (beta) + +

+ By enabling stickiness on this context field you can + use it together with the flexible-rollout strategy. + This will guarantee a consistent behavior for + specific values of this context field. PS! Not all + client SDK's support this feature yet!{' '}

+ {console.log(contextField.stickiness)} this.setValue('stickiness', !contextField.stickiness)} + onChange={() => + this.setValue( + 'stickiness', + !contextField.stickiness + ) + } />
- +
diff --git a/frontend/src/component/feature/strategy/StrategyCard/StrategyCardContent/StrategyCardContent.jsx b/frontend/src/component/feature/strategy/StrategyCard/StrategyCardContent/StrategyCardContent.jsx index a7017b6324..aab597ec71 100644 --- a/frontend/src/component/feature/strategy/StrategyCard/StrategyCardContent/StrategyCardContent.jsx +++ b/frontend/src/component/feature/strategy/StrategyCard/StrategyCardContent/StrategyCardContent.jsx @@ -12,22 +12,45 @@ const StrategyCardContent = ({ strategy, strategyDefinition }) => { const resolveContent = () => { switch (strategy.name) { case 'default': - return ; + return ; case 'flexibleRollout': return ; case 'userWithId': - return ; + return ( + + ); case 'gradualRolloutRandom': return ; case 'remoteAddress': - return ; + return ( + + ); case 'applicationHostname': - return ; + return ( + + ); case 'gradualRolloutUserId': case 'gradualRolloutSessionId': return ; default: - return ; + return ( + + ); } }; diff --git a/frontend/src/component/feature/strategy/StrategyCard/StrategyCardContent/StrategyCardContentDefault/StrategyCardContentDefault.jsx b/frontend/src/component/feature/strategy/StrategyCard/StrategyCardContent/StrategyCardContentDefault/StrategyCardContentDefault.jsx index 177b827416..dd87c6b95c 100644 --- a/frontend/src/component/feature/strategy/StrategyCard/StrategyCardContent/StrategyCardContentDefault/StrategyCardContentDefault.jsx +++ b/frontend/src/component/feature/strategy/StrategyCard/StrategyCardContent/StrategyCardContentDefault/StrategyCardContentDefault.jsx @@ -3,14 +3,29 @@ import React from 'react'; import { Typography } from '@material-ui/core'; import { useCommonStyles } from '../../../../../../common.styles'; +import ConditionallyRender from '../../../../../common/ConditionallyRender'; +import StrategyCardConstraints from '../common/StrategyCardConstraints/StrategyCardConstraints'; -const StrategyCardContentDefault = () => { +const StrategyCardContentDefault = ({ strategy }) => { const commonStyles = useCommonStyles(); + const { constraints } = strategy; + return ( - - The default strategy is either on or off for all users. - + <> + + The default strategy is either on or off for all users. + + 0} + show={ + <> +
+ + + } + /> + ); }; diff --git a/frontend/src/component/menu/Header/Header.jsx b/frontend/src/component/menu/Header/Header.jsx index 2ce39fd127..0955a35cbd 100644 --- a/frontend/src/component/menu/Header/Header.jsx +++ b/frontend/src/component/menu/Header/Header.jsx @@ -15,8 +15,7 @@ import MenuIcon from '@material-ui/icons/Menu'; import Breadcrumb from '../breadcrumb'; import UserProfile from '../../user/UserProfile'; import ConditionallyRender from '../../common/ConditionallyRender/ConditionallyRender'; -import HelpIcon from '@material-ui/icons/Help'; - +import MenuBookIcon from '@material-ui/icons/MenuBook'; import { useStyles } from './styles'; const Header = ({ uiConfig, init }) => { @@ -64,7 +63,7 @@ const Header = ({ uiConfig, init }) => { rel="noopener noreferrer" className={styles.docsLink} > - + diff --git a/frontend/src/component/project/ProjectList/styles.js b/frontend/src/component/project/ProjectList/styles.js index 1fd39fd466..fb054b3439 100644 --- a/frontend/src/component/project/ProjectList/styles.js +++ b/frontend/src/component/project/ProjectList/styles.js @@ -3,5 +3,9 @@ import { makeStyles } from '@material-ui/styles'; export const useStyles = makeStyles({ listItem: { padding: 0, + ['& a']: { + textDecoration: 'none', + color: 'inherit', + }, }, }); diff --git a/frontend/src/component/project/form-project-component.jsx b/frontend/src/component/project/form-project-component.jsx index 03e8ae8248..ec3870cbbb 100644 --- a/frontend/src/component/project/form-project-component.jsx +++ b/frontend/src/component/project/form-project-component.jsx @@ -40,7 +40,9 @@ class AddContextComponent extends Component { validateId = async id => { const { errors } = this.state; - const { validateId } = this.props; + const { validateId, editMode } = this.props; + if (editMode) return true; + try { await validateId(id); errors.id = undefined; @@ -49,6 +51,26 @@ class AddContextComponent extends Component { } this.setState({ errors }); + if (errors.id) return false; + return true; + }; + + validateName = () => { + const { project } = this.state; + if (project.name.length === 0) { + this.setState(prev => ({ + errors: { ...prev.errors, name: 'Name can not be empty.' }, + })); + return false; + } + return true; + }; + + validate = async id => { + const validId = await this.validateId(id); + const validName = this.validateName(); + + return validId && validName; }; onCancel = evt => { @@ -59,8 +81,13 @@ class AddContextComponent extends Component { onSubmit = async evt => { evt.preventDefault(); const { project } = this.state; - await this.props.submit(project); - this.props.history.push('/projects'); + + const valid = await this.validate(project.id); + + if (valid) { + await this.props.submit(project); + this.props.history.push('/projects'); + } }; render() { @@ -71,12 +98,19 @@ class AddContextComponent extends Component { return ( - - Projects allows you to group feature toggles together in the management UI. + + Projects allows you to group feature toggles together in the + management UI.
this.validateId(v.target.value)} - onChange={v => this.setValue('id', trim(v.target.value))} + onChange={v => + this.setValue('id', trim(v.target.value)) + } />
this.setValue('description', v.target.value)} + onChange={v => + this.setValue('description', v.target.value) + } + /> + + +
+ } /> - - - - } /> - ); diff --git a/frontend/src/page/admin/users/ConfirmUserAdded/ConfirmUserLink/UserInviteLink/UserInviteLink.tsx b/frontend/src/page/admin/users/ConfirmUserAdded/ConfirmUserLink/UserInviteLink/UserInviteLink.tsx index 2999d6f068..22b21e8d56 100644 --- a/frontend/src/page/admin/users/ConfirmUserAdded/ConfirmUserLink/UserInviteLink/UserInviteLink.tsx +++ b/frontend/src/page/admin/users/ConfirmUserAdded/ConfirmUserLink/UserInviteLink/UserInviteLink.tsx @@ -22,24 +22,31 @@ const UserInviteLink = ({ inviteLink }: IInviteLinkProps) => { }); const handleCopy = () => { - return navigator.clipboard - .writeText(inviteLink) - .then(() => { - setSnackbar({ - show: true, - type: 'success', - text: 'Successfully copied invite link.', + try { + return navigator.clipboard + .writeText(inviteLink) + .then(() => { + setSnackbar({ + show: true, + type: 'success', + text: 'Successfully copied invite link.', + }); + }) + .catch(() => { + setError(); }); - }) - .catch(() => { - setSnackbar({ - show: true, - type: 'error', - text: 'Could not copy invite link.', - }); - }); + } catch (e) { + setError(); + } }; + const setError = () => + setSnackbar({ + show: true, + type: 'error', + text: 'Could not copy invite link.', + }); + return (
dispatch(addContextField(context))) - .catch(dispatchError(dispatch, ERROR_ADD_CONTEXT_FIELD)); + .catch(e => { + dispatchError(dispatch, ERROR_ADD_CONTEXT_FIELD); + throw e; + }); } export function updateContextField(context) {