From 2bce93a51bb1bd1ca9f9ceb96678942b6641584e Mon Sep 17 00:00:00 2001 From: Youssef Khedher Date: Wed, 20 Oct 2021 12:12:48 +0100 Subject: [PATCH] Fix: improve create new feature v2 (#441) --- .../feature/FeatureCreate/FeatureCreate.tsx | 98 +++++++++++-------- .../FeatureToggleList/FeatureToggleList.jsx | 2 +- .../list-component-test.jsx.snap | 4 +- .../feature/FeatureView2/FeatureView2.tsx | 12 ++- .../create/CopyFeature/CopyFeature.jsx | 7 +- .../create/CreateFeature/CreateFeature.jsx | 4 +- .../feature/create/CreateFeature/index.jsx | 18 ++-- .../ProjectFeatureToggles.tsx | 7 +- frontend/src/utils/route-path-helpers.ts | 22 ++++- 9 files changed, 108 insertions(+), 66 deletions(-) diff --git a/frontend/src/component/feature/FeatureCreate/FeatureCreate.tsx b/frontend/src/component/feature/FeatureCreate/FeatureCreate.tsx index ffc487b739..127be7e938 100644 --- a/frontend/src/component/feature/FeatureCreate/FeatureCreate.tsx +++ b/frontend/src/component/feature/FeatureCreate/FeatureCreate.tsx @@ -1,10 +1,10 @@ -import { useEffect, useState } from 'react'; +import { useEffect, useState, FormEvent } from 'react'; import { useHistory, useParams } from 'react-router'; import { useStyles } from './FeatureCreate.styles'; import { IFeatureViewParams } from '../../../interfaces/params'; import PageContent from '../../common/PageContent'; import useFeatureApi from '../../../hooks/api/actions/useFeatureApi/useFeatureApi'; -import { CardActions, TextField } from '@material-ui/core'; +import { CardActions } from '@material-ui/core'; import FeatureTypeSelect from '../FeatureView2/FeatureSettings/FeatureSettingsMetadata/FeatureTypeSelect/FeatureTypeSelect'; import { CF_CREATE_BTN_ID, @@ -12,26 +12,28 @@ import { CF_NAME_ID, CF_TYPE_ID, } from '../../../testIds'; -import { loadNameFromUrl, trim } from '../../common/util'; import { getTogglePath } from '../../../utils/route-path-helpers'; import { IFeatureToggleDTO } from '../../../interfaces/featureToggle'; -import { FormEventHandler } from 'react-router/node_modules/@types/react'; import { useCommonStyles } from '../../../common.styles'; import { FormButtons } from '../../common'; - -interface Errors { - name?: string; - description?: string; -} +import useQueryParams from '../../../hooks/useQueryParams'; +import useUiConfig from '../../../hooks/api/getters/useUiConfig/useUiConfig'; +import Input from '../../common/Input/Input'; +import ProjectSelect from '../project-select-container'; +import { projectFilterGenerator } from '../../../utils/project-filter-generator'; +import useUser from '../../../hooks/api/getters/useUser/useUser'; +import { trim } from '../../common/util'; +import { CREATE_FEATURE } from '../../providers/AccessProvider/permissions'; const FeatureCreate = () => { const styles = useStyles(); const commonStyles = useCommonStyles(); const { projectId } = useParams(); + const params = useQueryParams(); const { createFeatureToggle, validateFeatureToggleName } = useFeatureApi(); const history = useHistory(); const [toggle, setToggle] = useState({ - name: loadNameFromUrl(), + name: params.get('name') || '', description: '', type: 'release', stale: false, @@ -39,7 +41,10 @@ const FeatureCreate = () => { project: projectId, archived: false, }); - const [errors, setErrors] = useState({}); + + const [nameError, setNameError] = useState(''); + const { uiConfig } = useUiConfig(); + const { permissions } = useUser(); useEffect(() => { window.onbeforeunload = () => @@ -54,34 +59,40 @@ const FeatureCreate = () => { const onCancel = () => history.push(`/projects/${projectId}`); const validateName = async (featureToggleName: string) => { - const e = { ...errors }; - try { - await validateFeatureToggleName(featureToggleName); - e.name = undefined; - } catch (err: any) { - e.name = err && err.message ? err.message : 'Could not check name'; + if (featureToggleName.length > 0) { + try { + await validateFeatureToggleName(featureToggleName); + } catch (err: any) { + setNameError( + err && err.message ? err.message : 'Could not check name' + ); + } } - - setErrors(e); }; - const onSubmit = async (evt: FormEventHandler) => { + const onSubmit = async (evt: FormEvent) => { evt.preventDefault(); - const errorList = Object.values(errors).filter(i => i); + await validateName(toggle.name); - if (errorList.length > 0) { + if(!toggle.name) { + setNameError('Name is not allowed to be empty'); + return; + } + + if (nameError) { return; } try { - await createFeatureToggle(projectId, toggle).then(() => - history.push(getTogglePath(toggle.project, toggle.name, true)) - ); + await createFeatureToggle(toggle.project, toggle) + history.push(getTogglePath(toggle.project, toggle.name, uiConfig.flags.E)); // Trigger - } catch (e: any) { - if (e.toString().includes('not allowed to be empty')) { - setErrors({ name: 'Name is not allowed to be empty' }); + } catch (err) { + if(err instanceof Error) { + if (err.toString().includes('not allowed to be empty')) { + setNameError('Name is not allowed to be empty'); + } } } }; @@ -98,11 +109,8 @@ const FeatureCreate = () => {
- { 'data-test': CF_NAME_ID, }} value={toggle.name} - error={errors.name !== undefined} - helperText={errors.name} + error={Boolean(nameError)} + helperText={nameError} onBlur={v => validateName(v.target.value)} - onChange={v => setValue('name', trim(v.target.value))} + onChange={v => { + setValue('name', trim(v.target.value)); + setNameError(''); + }} />
-
+
setValue('type', v.target.value)} @@ -127,18 +138,21 @@ const FeatureCreate = () => { 'data-test': CF_TYPE_ID, }} /> -
+
- setValue('project', v.target.value)} + filter={projectFilterGenerator({ permissions }, CREATE_FEATURE)} + /> +
+
+ { features.forEach(e => { diff --git a/frontend/src/component/feature/FeatureToggleList/__tests__/__snapshots__/list-component-test.jsx.snap b/frontend/src/component/feature/FeatureToggleList/__tests__/__snapshots__/list-component-test.jsx.snap index 0a8318a4f3..26b5cd0c9c 100644 --- a/frontend/src/component/feature/FeatureToggleList/__tests__/__snapshots__/list-component-test.jsx.snap +++ b/frontend/src/component/feature/FeatureToggleList/__tests__/__snapshots__/list-component-test.jsx.snap @@ -144,7 +144,7 @@ exports[`renders correctly with one feature 1`] = ` aria-disabled={true} className="MuiButtonBase-root MuiButton-root MuiButton-contained MuiButton-containedPrimary Mui-disabled Mui-disabled" data-test="NAVIGATE_TO_CREATE_FEATURE" - href="/projects/default/create-toggle?project=default" + href="/projects/default/create-toggle" onBlur={[Function]} onClick={[Function]} onDragLeave={[Function]} @@ -344,7 +344,7 @@ exports[`renders correctly with one feature without permissions 1`] = ` aria-disabled={true} className="MuiButtonBase-root MuiButton-root MuiButton-contained MuiButton-containedPrimary Mui-disabled Mui-disabled" data-test="NAVIGATE_TO_CREATE_FEATURE" - href="/projects/default/create-toggle?project=default" + href="/projects/default/create-toggle" onBlur={[Function]} onClick={[Function]} onDragLeave={[Function]} diff --git a/frontend/src/component/feature/FeatureView2/FeatureView2.tsx b/frontend/src/component/feature/FeatureView2/FeatureView2.tsx index dac3985cb3..a548ee93b7 100644 --- a/frontend/src/component/feature/FeatureView2/FeatureView2.tsx +++ b/frontend/src/component/feature/FeatureView2/FeatureView2.tsx @@ -21,6 +21,7 @@ import FeatureSettings from './FeatureSettings/FeatureSettings'; import useLoading from '../../../hooks/useLoading'; import ConditionallyRender from '../../common/ConditionallyRender'; import { getCreateTogglePath } from '../../../utils/route-path-helpers'; +import useUiConfig from '../../../hooks/api/getters/useUiConfig/useUiConfig'; const FeatureView2 = () => { const { projectId, featureId } = useParams(); @@ -33,6 +34,7 @@ const FeatureView2 = () => { const styles = useStyles(); const history = useHistory(); const ref = useLoading(loading); + const { uiConfig } = useUiConfig(); const basePath = `/projects/${projectId}/features2/${featureId}`; @@ -108,7 +110,15 @@ const FeatureView2 = () => {

The feature {featureId.substring(0, 30)}{' '} does not exist. Do you want to   - create it + + create it +  ?

diff --git a/frontend/src/component/feature/create/CopyFeature/CopyFeature.jsx b/frontend/src/component/feature/create/CopyFeature/CopyFeature.jsx index 347dfae21a..84b7401ea5 100644 --- a/frontend/src/component/feature/create/CopyFeature/CopyFeature.jsx +++ b/frontend/src/component/feature/create/CopyFeature/CopyFeature.jsx @@ -21,6 +21,7 @@ import { Alert } from '@material-ui/lab'; import { getTogglePath } from '../../../../utils/route-path-helpers'; import useFeatureApi from '../../../../hooks/api/actions/useFeatureApi/useFeatureApi'; import useFeature from '../../../../hooks/api/getters/useFeature/useFeature'; +import useUiConfig from '../../../../hooks/api/getters/useUiConfig/useUiConfig'; const CopyFeature = props => { // static displayName = `AddFeatureComponent-${getDisplayName(Component)}`; @@ -32,6 +33,7 @@ const CopyFeature = props => { const inputRef = useRef(); const { name: copyToggleName, id: projectId } = useParams(); const { feature } = useFeature(projectId, copyToggleName); + const { uiConfig } = useUiConfig(); useEffect(() => { inputRef.current?.focus(); @@ -70,7 +72,7 @@ const CopyFeature = props => { { name: newToggleName, replaceGroupId } ); props.history.push( - getTogglePath(projectId, newToggleName) + getTogglePath(projectId, newToggleName, uiConfig.flags.E) ) } catch (e) { setApiError(e); @@ -96,7 +98,7 @@ const CopyFeature = props => { You are about to create a new feature toggle by cloning the configuration of feature toggle  {copyToggleName} @@ -115,6 +117,7 @@ const CopyFeature = props => { variant="outlined" size="small" inputRef={inputRef} + required /> { - setValue('project', v.target.value); - }} + onChange={v => setValue('project', v.target.value)} filter={projectFilterGenerator(user, CREATE_FEATURE)} />
diff --git a/frontend/src/component/feature/create/CreateFeature/index.jsx b/frontend/src/component/feature/create/CreateFeature/index.jsx index 6367076169..ad6de0bd91 100644 --- a/frontend/src/component/feature/create/CreateFeature/index.jsx +++ b/frontend/src/component/feature/create/CreateFeature/index.jsx @@ -52,15 +52,17 @@ class WrapperComponent extends Component { }; validateName = async featureToggleName => { - const { errors } = { ...this.state }; - try { - await validateName(featureToggleName); - errors.name = undefined; - } catch (err) { - errors.name = err.message; - } + if (featureToggleName.length > 0) { + const { errors } = { ...this.state }; + try { + await validateName(featureToggleName); + errors.name = undefined; + } catch (err) { + errors.name = err.message; + } - this.setState({ errors }); + this.setState({ errors }); + } }; onSubmit = async evt => { diff --git a/frontend/src/component/project/Project/ProjectFeatureToggles/ProjectFeatureToggles.tsx b/frontend/src/component/project/Project/ProjectFeatureToggles/ProjectFeatureToggles.tsx index 12ae44c2a9..bd4354f0fe 100644 --- a/frontend/src/component/project/Project/ProjectFeatureToggles/ProjectFeatureToggles.tsx +++ b/frontend/src/component/project/Project/ProjectFeatureToggles/ProjectFeatureToggles.tsx @@ -55,14 +55,11 @@ const ProjectFeatureToggles = ({ } /> - + history.push( - getCreateTogglePath( - id, - uiConfig.flags.E - ) + getCreateTogglePath(id, uiConfig.flags.E) ) } maxWidth="700px" diff --git a/frontend/src/utils/route-path-helpers.ts b/frontend/src/utils/route-path-helpers.ts index 0f5a35ecf1..3707618813 100644 --- a/frontend/src/utils/route-path-helpers.ts +++ b/frontend/src/utils/route-path-helpers.ts @@ -9,8 +9,26 @@ export const getToggleCopyPath = ( return `/projects/${projectId}/features/${featureToggleName}/strategies/copy`; }; -export const getCreateTogglePath = (projectId: string, newpath: boolean = false) => { - return newpath ? `/projects/${projectId}/create-toggle2` : `/projects/${projectId}/create-toggle?project=${projectId}`; +export const getCreateTogglePath = ( + projectId: string, + newPath: boolean = false, + query?: Object +) => { + const path = newPath + ? `/projects/${projectId}/create-toggle2` + : `/projects/${projectId}/create-toggle`; + + let queryString; + if (query) { + queryString = Object.keys(query).reduce((acc, curr) => { + acc += `${curr}=${query[curr]}`; + return acc; + }, ''); + } + if (queryString) { + return `${path}?${queryString}`; + } + return path; }; export const getProjectEditPath = (projectId: string) => {