From: Wouter Admiraal Date: Tue, 27 Aug 2019 10:11:41 +0000 (+0200) Subject: SONAR-12360 Improve project creation validation X-Git-Tag: 8.0~133 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=2fba950fd31175b42a3b097ff58ddda9c76fddfa;p=sonarqube.git SONAR-12360 Improve project creation validation --- diff --git a/server/sonar-web/src/main/js/apps/create/components/ProjectKeyInput.tsx b/server/sonar-web/src/main/js/apps/create/components/ProjectKeyInput.tsx deleted file mode 100644 index 0fd857b8b2d..00000000000 --- a/server/sonar-web/src/main/js/apps/create/components/ProjectKeyInput.tsx +++ /dev/null @@ -1,129 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2019 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -import * as classNames from 'classnames'; -import { debounce } from 'lodash'; -import * as React from 'react'; -import ValidationInput from 'sonar-ui-common/components/controls/ValidationInput'; -import { translate } from 'sonar-ui-common/helpers/l10n'; -import { doesComponentExists } from '../../../api/components'; - -interface Props { - className?: string; - value: string; - onChange: (value: string | undefined) => void; -} - -interface State { - error?: string; - touched: boolean; - validating: boolean; -} - -export default class ProjectKeyInput extends React.PureComponent { - mounted = false; - constructor(props: Props) { - super(props); - this.state = { error: undefined, touched: false, validating: false }; - this.checkFreeKey = debounce(this.checkFreeKey, 250); - } - - componentDidMount() { - this.mounted = true; - if (this.props.value) { - this.validateKey(this.props.value); - } - } - - componentWillUnmount() { - this.mounted = false; - } - - checkFreeKey = (key: string) => { - this.setState({ validating: true }); - return doesComponentExists({ component: key }) - .then(alreadyExist => { - if (this.mounted && key === this.props.value) { - if (!alreadyExist) { - this.setState({ error: undefined, validating: false }); - } else { - this.setState({ - error: translate('onboarding.create_project.project_key.taken'), - touched: true, - validating: false - }); - } - } - }) - .catch(() => { - if (this.mounted && key === this.props.value) { - this.setState({ error: undefined, validating: false }); - } - }); - }; - - handleChange = (event: React.ChangeEvent) => { - const { value } = event.currentTarget; - this.setState({ touched: true }); - this.validateKey(value); - this.props.onChange(value); - }; - - validateKey(key: string) { - if (key.length > 400 || !/^[\w-.:]*[a-zA-Z]+[\w-.:]*$/.test(key)) { - this.setState({ - error: translate('onboarding.create_project.project_key.error'), - touched: true - }); - } else { - this.checkFreeKey(key); - } - } - - render() { - const isInvalid = this.state.touched && this.state.error !== undefined; - const isValid = this.state.touched && !this.state.validating && this.state.error === undefined; - return ( - - - - ); - } -} diff --git a/server/sonar-web/src/main/js/apps/create/components/__tests__/ProjectKeyInput-test.tsx b/server/sonar-web/src/main/js/apps/create/components/__tests__/ProjectKeyInput-test.tsx deleted file mode 100644 index 4f0214d7e9a..00000000000 --- a/server/sonar-web/src/main/js/apps/create/components/__tests__/ProjectKeyInput-test.tsx +++ /dev/null @@ -1,87 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2019 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -import { shallow } from 'enzyme'; -import * as React from 'react'; -import { waitAndUpdate } from 'sonar-ui-common/helpers/testUtils'; -import ProjectKeyInput from '../ProjectKeyInput'; - -jest.useFakeTimers(); - -jest.mock('../../../../api/components', () => ({ - doesComponentExists: jest - .fn() - .mockImplementation(({ component }) => Promise.resolve(component === 'exists')) -})); - -it('should render correctly', async () => { - const wrapper = shallow(); - expect(wrapper).toMatchSnapshot(); - wrapper.setState({ touched: true }); - await waitAndUpdate(wrapper); - expect(wrapper.find('ValidationInput').prop('isValid')).toBe(true); -}); - -it('should not display any status when the key is not defined', async () => { - const wrapper = shallow(); - await waitAndUpdate(wrapper); - expect(wrapper.find('ValidationInput').prop('isInvalid')).toBe(false); - expect(wrapper.find('ValidationInput').prop('isValid')).toBe(false); -}); - -it('should have an error when the key is invalid', async () => { - const wrapper = shallow(); - await waitAndUpdate(wrapper); - expect(wrapper.find('ValidationInput').prop('isInvalid')).toBe(true); -}); - -it('should have an error when the key already exists', async () => { - const wrapper = shallow(); - await waitAndUpdate(wrapper); - - jest.runAllTimers(); - await new Promise(setImmediate); - expect(wrapper.find('ValidationInput').prop('isInvalid')).toBe(true); -}); - -it('should handle Change', async () => { - const onChange = jest.fn(); - const wrapper = shallow(); - await waitAndUpdate(wrapper); - - wrapper.find('input').simulate('change', { currentTarget: { value: 'key' } }); - - expect(wrapper.state('touched')).toBe(true); - expect(onChange).toBeCalledWith('key'); -}); - -it('should ignore promise return if value has been changed in the meantime', async () => { - const onChange = (value: string) => wrapper.setProps({ value }); - const wrapper = shallow(); - await waitAndUpdate(wrapper); - - wrapper.find('input').simulate('change', { currentTarget: { value: 'exists' } }); - wrapper.find('input').simulate('change', { currentTarget: { value: 'exists%' } }); - - jest.runAllTimers(); - await new Promise(setImmediate); - - expect(wrapper.state('touched')).toBe(true); - expect(wrapper.state('error')).toBe('onboarding.create_project.project_key.error'); -}); diff --git a/server/sonar-web/src/main/js/apps/create/components/__tests__/__snapshots__/ProjectKeyInput-test.tsx.snap b/server/sonar-web/src/main/js/apps/create/components/__tests__/__snapshots__/ProjectKeyInput-test.tsx.snap deleted file mode 100644 index 58f34b9f7d4..00000000000 --- a/server/sonar-web/src/main/js/apps/create/components/__tests__/__snapshots__/ProjectKeyInput-test.tsx.snap +++ /dev/null @@ -1,24 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`should render correctly 1`] = ` - - - -`; diff --git a/server/sonar-web/src/main/js/apps/create/project/ManualProjectCreate.css b/server/sonar-web/src/main/js/apps/create/project/ManualProjectCreate.css index 54c67f4766d..5897ef1a9fa 100644 --- a/server/sonar-web/src/main/js/apps/create/project/ManualProjectCreate.css +++ b/server/sonar-web/src/main/js/apps/create/project/ManualProjectCreate.css @@ -18,7 +18,7 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ .manual-project-create { - max-width: 650px; + max-width: 700px; } .manual-project-create .visibility-select-option { diff --git a/server/sonar-web/src/main/js/apps/create/project/ManualProjectCreate.tsx b/server/sonar-web/src/main/js/apps/create/project/ManualProjectCreate.tsx index 40513c90d50..485da00e2f8 100644 --- a/server/sonar-web/src/main/js/apps/create/project/ManualProjectCreate.tsx +++ b/server/sonar-web/src/main/js/apps/create/project/ManualProjectCreate.tsx @@ -18,15 +18,15 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ import * as classNames from 'classnames'; +import { debounce } from 'lodash'; import * as React from 'react'; import { SubmitButton } from 'sonar-ui-common/components/controls/buttons'; -import HelpTooltip from 'sonar-ui-common/components/controls/HelpTooltip'; +import ValidationInput from 'sonar-ui-common/components/controls/ValidationInput'; import DeferredSpinner from 'sonar-ui-common/components/ui/DeferredSpinner'; import { translate } from 'sonar-ui-common/helpers/l10n'; -import { createProject } from '../../../api/components'; +import { createProject, doesComponentExists } from '../../../api/components'; import VisibilitySelector from '../../../components/common/VisibilitySelector'; import { isSonarCloud } from '../../../helpers/system'; -import ProjectKeyInput from '../components/ProjectKeyInput'; import UpgradeOrganizationBox from '../components/UpgradeOrganizationBox'; import './ManualProjectCreate.css'; import OrganizationInput from './OrganizationInput'; @@ -42,10 +42,14 @@ interface Props { interface State { projectName: string; projectNameChanged: boolean; + projectNameError?: string; projectKey: string; + projectKeyError?: string; selectedOrganization?: T.Organization; selectedVisibility?: T.Visibility; submitting: boolean; + touched: boolean; + validating: boolean; } type ValidState = State & Required>; @@ -60,8 +64,11 @@ export default class ManualProjectCreate extends React.PureComponent { + return doesComponentExists({ component: key }) + .then(alreadyExist => { + if (this.mounted && key === this.state.projectKey) { + if (!alreadyExist) { + this.setState({ projectKeyError: undefined, validating: false }); + } else { + this.setState({ + projectKeyError: translate('onboarding.create_project.project_key.taken'), + touched: true, + validating: false + }); + } + } + }) + .catch(() => { + if (this.mounted && key === this.state.projectKey) { + this.setState({ projectKeyError: undefined, validating: false }); + } + }); + }; + canChoosePrivate = (selectedOrganization: T.Organization | undefined) => { return Boolean(selectedOrganization && selectedOrganization.subscription === 'PAID'); }; canSubmit(state: State): state is ValidState { + const { + projectKey, + projectKeyError, + projectName, + projectNameError, + selectedOrganization + } = state; return Boolean( - state.projectKey && state.projectName && (!isSonarCloud() || state.selectedOrganization) + projectKeyError === undefined && + projectNameError === undefined && + projectKey.length > 0 && + projectName.length > 0 && + (!isSonarCloud() || selectedOrganization) ); } @@ -151,24 +191,67 @@ export default class ManualProjectCreate extends React.PureComponent) => { - const projectName = event.currentTarget.value; - this.setState({ projectName, projectNameChanged: true }); + handleProjectKeyChange = (event: React.ChangeEvent) => { + const projectKey = event.currentTarget.value || ''; + const projectKeyError = this.validateKey(projectKey); + + this.setState(prevState => { + const projectName = prevState.projectNameChanged ? prevState.projectName : projectKey; + return { + projectKey, + projectKeyError, + projectName, + projectNameError: this.validateName(projectName), + touched: true, + validating: projectKeyError === undefined + }; + }); + + if (projectKeyError === undefined) { + this.checkFreeKey(projectKey); + } }; - handleProjectKeyChange = (projectKey: string) => { - this.setState(state => ({ - projectKey, - projectName: state.projectNameChanged ? state.projectName : projectKey || '' - })); + handleProjectNameChange = (event: React.ChangeEvent) => { + const projectName = event.currentTarget.value; + this.setState({ + projectName, + projectNameChanged: true, + projectNameError: this.validateName(projectName) + }); }; handleVisibilityChange = (selectedVisibility: T.Visibility) => { this.setState({ selectedVisibility }); }; + validateKey = (projectKey: string) => { + return projectKey.length > 400 || !/^[\w-.:]*[a-zA-Z]+[\w-.:]*$/.test(projectKey) + ? translate('onboarding.create_project.project_key.error') + : undefined; + }; + + validateName = (projectName: string) => { + return projectName.length === 0 || projectName.length > 255 + ? translate('onboarding.create_project.display_name.error') + : undefined; + }; + render() { - const { selectedOrganization, submitting } = this.state; + const { + projectKey, + projectKeyError, + projectName, + projectNameError, + selectedOrganization, + submitting, + touched, + validating + } = this.state; + const projectKeyIsInvalid = touched && projectKeyError !== undefined; + const projectKeyIsValid = touched && !validating && projectKeyError === undefined; + const projectNameIsInvalid = touched && projectNameError !== undefined; + const projectNameIsValid = touched && projectNameError === undefined; const canChoosePrivate = this.canChoosePrivate(selectedOrganization); return ( @@ -182,37 +265,56 @@ export default class ManualProjectCreate extends React.PureComponent )} - -
- -
- -
-
- {translate('onboarding.create_project.display_name.description')} -
-
+ description={translate('onboarding.create_project.project_key.description')} + error={projectKeyError} + help={translate('onboarding.create_project.project_key.help')} + id="project-key" + isInvalid={projectKeyIsInvalid} + isValid={projectKeyIsValid} + label={translate('onboarding.create_project.project_key')} + required={true}> + + + + + + + {isSonarCloud() && selectedOrganization && (
)} + {translate('set_up')} diff --git a/server/sonar-web/src/main/js/apps/create/project/__tests__/ManualProjectCreate-test.tsx b/server/sonar-web/src/main/js/apps/create/project/__tests__/ManualProjectCreate-test.tsx index d0815b405c7..93ec3cfb70f 100644 --- a/server/sonar-web/src/main/js/apps/create/project/__tests__/ManualProjectCreate-test.tsx +++ b/server/sonar-web/src/main/js/apps/create/project/__tests__/ManualProjectCreate-test.tsx @@ -17,6 +17,7 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +/* eslint-disable sonarjs/no-duplicate-string */ import { shallow } from 'enzyme'; import * as React from 'react'; import { change, submit, waitAndUpdate } from 'sonar-ui-common/helpers/testUtils'; @@ -24,7 +25,10 @@ import { createProject } from '../../../../api/components'; import ManualProjectCreate from '../ManualProjectCreate'; jest.mock('../../../../api/components', () => ({ - createProject: jest.fn().mockResolvedValue({ project: { key: 'bar', name: 'Bar' } }) + createProject: jest.fn().mockResolvedValue({ project: { key: 'bar', name: 'Bar' } }), + doesComponentExists: jest + .fn() + .mockImplementation(({ component }) => Promise.resolve(component === 'exists')) })); jest.mock('../../../../helpers/system', () => ({ @@ -32,19 +36,19 @@ jest.mock('../../../../helpers/system', () => ({ })); beforeEach(() => { - (createProject as jest.Mock).mockClear(); + jest.clearAllMocks(); }); it('should render correctly', () => { - expect(getWrapper()).toMatchSnapshot(); + expect(shallowRender()).toMatchSnapshot(); }); it('should correctly create a public project', async () => { const onProjectCreate = jest.fn(); - const wrapper = getWrapper({ onProjectCreate }); + const wrapper = shallowRender({ onProjectCreate }); wrapper.find('withRouter(OrganizationInput)').prop('onChange')({ key: 'foo' }); - change(wrapper.find('ProjectKeyInput'), 'bar'); + change(wrapper.find('input#project-key'), 'bar'); change(wrapper.find('input#project-name'), 'Bar'); expect(wrapper.find('SubmitButton').prop('disabled')).toBe(false); @@ -62,10 +66,10 @@ it('should correctly create a public project', async () => { it('should correctly create a private project', async () => { const onProjectCreate = jest.fn(); - const wrapper = getWrapper({ onProjectCreate }); + const wrapper = shallowRender({ onProjectCreate }); wrapper.find('withRouter(OrganizationInput)').prop('onChange')({ key: 'bar' }); - change(wrapper.find('ProjectKeyInput'), 'bar'); + change(wrapper.find('input#project-key'), 'bar'); change(wrapper.find('input#project-name'), 'Bar'); submit(wrapper.find('form')); @@ -80,8 +84,77 @@ it('should correctly create a private project', async () => { expect(onProjectCreate).toBeCalledWith(['bar']); }); -function getWrapper(props = {}) { - return shallow( +it('should not display any status when the key is not defined', () => { + const wrapper = shallowRender(); + const projectKeyInput = wrapper.find('ValidationInput').first(); + expect(projectKeyInput.prop('isInvalid')).toBe(false); + expect(projectKeyInput.prop('isValid')).toBe(false); +}); + +it('should not display any status when the name is not defined', () => { + const wrapper = shallowRender(); + const projectKeyInput = wrapper.find('ValidationInput').last(); + expect(projectKeyInput.prop('isInvalid')).toBe(false); + expect(projectKeyInput.prop('isValid')).toBe(false); +}); + +it('should have an error when the key is invalid', () => { + const wrapper = shallowRender(); + change(wrapper.find('input#project-key'), 'KEy-with#speci@l_char'); + expect( + wrapper + .find('ValidationInput') + .first() + .prop('isInvalid') + ).toBe(true); +}); + +it('should have an error when the key already exists', async () => { + const wrapper = shallowRender(); + change(wrapper.find('input#project-key'), 'exists'); + + await waitAndUpdate(wrapper); + expect( + wrapper + .find('ValidationInput') + .first() + .prop('isInvalid') + ).toBe(true); +}); + +it('should ignore promise return if value has been changed in the meantime', async () => { + const wrapper = shallowRender(); + + change(wrapper.find('input#project-key'), 'exists'); + change(wrapper.find('input#project-key'), 'exists%'); + + await waitAndUpdate(wrapper); + + expect(wrapper.state('touched')).toBe(true); + expect(wrapper.state('projectKeyError')).toBe('onboarding.create_project.project_key.error'); +}); + +it('should autofill the name based on the key', () => { + const wrapper = shallowRender(); + change(wrapper.find('input#project-key'), 'bar'); + expect(wrapper.find('input#project-name').prop('value')).toBe('bar'); +}); + +it('should have an error when the name is empty', () => { + const wrapper = shallowRender(); + change(wrapper.find('input#project-key'), 'bar'); + change(wrapper.find('input#project-name'), ''); + expect( + wrapper + .find('ValidationInput') + .last() + .prop('isInvalid') + ).toBe(true); + expect(wrapper.state('projectNameError')).toBe('onboarding.create_project.display_name.error'); +}); + +function shallowRender(props: Partial = {}) { + return shallow( - -
+ + + - -
- -
-
- onboarding.create_project.display_name.description -
-
+ + diff --git a/sonar-core/src/main/resources/org/sonar/l10n/core.properties b/sonar-core/src/main/resources/org/sonar/l10n/core.properties index d7c8865ddcc..9bb347d45f2 100644 --- a/sonar-core/src/main/resources/org/sonar/l10n/core.properties +++ b/sonar-core/src/main/resources/org/sonar/l10n/core.properties @@ -2857,7 +2857,7 @@ onboarding.create_project.project_key.error=The provided value doesn't match the onboarding.create_project.project_key.help=Your project key is a unique identifier for your project. If you are using Maven, make sure the key matches the "groupId:artifactId" format. onboarding.create_project.project_key.taken=This project key is already taken. onboarding.create_project.display_name=Display name -onboarding.create_project.display_name.error=The provided value doesn't match the expected format. +onboarding.create_project.display_name.error=The display name is required. onboarding.create_project.display_name.description=Up to 255 characters onboarding.create_project.display_name.help=Some scanners might override the value you provide. onboarding.create_project.repository_imported=Already imported: {link}