From d811642a8b2381efe43fbc15da10e7774f5c2786 Mon Sep 17 00:00:00 2001 From: Wouter Admiraal Date: Mon, 11 May 2020 15:14:53 +0200 Subject: [PATCH] SONAR-12884 Improve the validation messages when creating a project --- .../create/project/ManualProjectCreate.tsx | 51 +++---- .../__tests__/ManualProjectCreate-test.tsx | 106 +++++++------- .../ManualProjectCreate-test.tsx.snap | 25 +--- .../main/js/apps/create/project/constants.ts | 21 +++ .../js/components/common/ProjectKeyInput.tsx | 71 ++++++++++ .../common/__tests__/ProjectKeyInput-test.tsx | 48 +++++++ .../ProjectKeyInput-test.tsx.snap | 132 ++++++++++++++++++ .../js/helpers/__tests__/projects-test.ts | 42 ++++++ .../src/main/js/helpers/constants.ts | 2 + .../sonar-web/src/main/js/helpers/projects.ts | 39 ++++++ .../sonar-web/src/main/js/types/component.ts | 17 ++- .../resources/org/sonar/l10n/core.properties | 10 +- 12 files changed, 460 insertions(+), 104 deletions(-) create mode 100644 server/sonar-web/src/main/js/apps/create/project/constants.ts create mode 100644 server/sonar-web/src/main/js/components/common/ProjectKeyInput.tsx create mode 100644 server/sonar-web/src/main/js/components/common/__tests__/ProjectKeyInput-test.tsx create mode 100644 server/sonar-web/src/main/js/components/common/__tests__/__snapshots__/ProjectKeyInput-test.tsx.snap create mode 100644 server/sonar-web/src/main/js/helpers/__tests__/projects-test.ts create mode 100644 server/sonar-web/src/main/js/helpers/projects.ts 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 1cb5f7f93b4..65212ebb525 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 @@ -25,6 +25,10 @@ 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, doesComponentExists } from '../../../api/components'; +import ProjectKeyInput from '../../../components/common/ProjectKeyInput'; +import { validateProjectKey } from '../../../helpers/projects'; +import { ProjectKeyValidationResult } from '../../../types/component'; +import { PROJECT_NAME_MAX_LEN } from './constants'; import CreateProjectPageHeader from './CreateProjectPageHeader'; import './ManualProjectCreate.css'; @@ -153,15 +157,19 @@ export default class ManualProjectCreate extends React.PureComponent { - return projectKey.length > 400 || !/^[\w-.:]*[a-zA-Z]+[\w-.:]*$/.test(projectKey) - ? translate('onboarding.create_project.project_key.error') - : undefined; + const result = validateProjectKey(projectKey); + return result === ProjectKeyValidationResult.Valid + ? undefined + : translate('onboarding.create_project.project_key.error', result); }; validateName = (projectName: string) => { - return projectName.length === 0 || projectName.length > 255 - ? translate('onboarding.create_project.display_name.error') - : undefined; + if (projectName.length === 0) { + return translate('onboarding.create_project.display_name.error.empty'); + } else if (projectName.length > PROJECT_NAME_MAX_LEN) { + return translate('onboarding.create_project.display_name.error.too_long'); + } + return undefined; }; render() { @@ -175,8 +183,6 @@ export default class ManualProjectCreate extends React.PureComponent
- - - + onProjectKeyChange={this.handleProjectKeyChange} + projectKey={projectKey} + touched={touched} + validating={validating} + /> ({ @@ -31,9 +38,10 @@ jest.mock('../../../../api/components', () => ({ .mockImplementation(({ component }) => Promise.resolve(component === 'exists')) })); -jest.mock('../../../../helpers/system', () => ({ - isSonarCloud: jest.fn().mockReturnValue(true) -})); +jest.mock('../../../../helpers/projects', () => { + const { ProjectKeyValidationResult } = jest.requireActual('../../../../types/component'); + return { validateProjectKey: jest.fn(() => ProjectKeyValidationResult.Valid) }; +}); beforeEach(() => { jest.clearAllMocks(); @@ -47,9 +55,14 @@ it('should correctly create a project', async () => { const onProjectCreate = jest.fn(); const wrapper = shallowRender({ onProjectCreate }); - change(wrapper.find('input#project-key'), 'bar'); + wrapper + .find(ProjectKeyInput) + .props() + .onProjectKeyChange(mockEvent({ currentTarget: { value: 'bar' } })); change(wrapper.find('input#project-name'), 'Bar'); - expect(wrapper.find('SubmitButton').prop('disabled')).toBe(false); + expect(wrapper.find(SubmitButton).props().disabled).toBe(false); + expect(validateProjectKey).toBeCalledWith('bar'); + expect(doesComponentExists).toBeCalledWith({ component: 'bar' }); submit(wrapper.find('form')); expect(createProject).toBeCalledWith({ @@ -61,73 +74,72 @@ it('should correctly create a project', async () => { expect(onProjectCreate).toBeCalledWith(['bar']); }); -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); + const projectNameInput = wrapper.find(ValidationInput); + expect(projectNameInput.props().isInvalid).toBe(false); + expect(projectNameInput.props().isValid).toBe(false); }); it('should have an error when the key is invalid', () => { + (validateProjectKey as jest.Mock).mockReturnValueOnce(ProjectKeyValidationResult.TooLong); const wrapper = shallowRender(); - change(wrapper.find('input#project-key'), 'KEy-with#speci@l_char'); - expect( - wrapper - .find('ValidationInput') - .first() - .prop('isInvalid') - ).toBe(true); + const instance = wrapper.instance(); + instance.handleProjectKeyChange(mockEvent()); + expect(wrapper.find(ProjectKeyInput).props().error).toBe( + `onboarding.create_project.project_key.error.${ProjectKeyValidationResult.TooLong}` + ); }); it('should have an error when the key already exists', async () => { const wrapper = shallowRender(); - change(wrapper.find('input#project-key'), 'exists'); - + wrapper.instance().handleProjectKeyChange(mockEvent({ currentTarget: { value: 'exists' } })); await waitAndUpdate(wrapper); - expect( - wrapper - .find('ValidationInput') - .first() - .prop('isInvalid') - ).toBe(true); + expect(wrapper.state().projectKeyError).toBe('onboarding.create_project.project_key.taken'); }); it('should ignore promise return if value has been changed in the meantime', async () => { + (validateProjectKey as jest.Mock) + .mockReturnValueOnce(ProjectKeyValidationResult.Valid) + .mockReturnValueOnce(ProjectKeyValidationResult.InvalidChar); const wrapper = shallowRender(); + const instance = wrapper.instance(); - change(wrapper.find('input#project-key'), 'exists'); - change(wrapper.find('input#project-key'), 'exists%'); + instance.handleProjectKeyChange(mockEvent({ currentTarget: { value: 'exists' } })); + instance.handleProjectKeyChange(mockEvent({ currentTarget: { value: 'exists%' } })); await waitAndUpdate(wrapper); - expect(wrapper.state('touched')).toBe(true); - expect(wrapper.state('projectKeyError')).toBe('onboarding.create_project.project_key.error'); + expect(wrapper.state().touched).toBe(true); + expect(wrapper.state().projectKeyError).toBe( + `onboarding.create_project.project_key.error.${ProjectKeyValidationResult.InvalidChar}` + ); }); 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'); + wrapper.instance().handleProjectKeyChange(mockEvent({ currentTarget: { value: 'bar' } })); + expect(wrapper.find('input#project-name').props().value).toBe('bar'); }); -it('should have an error when the name is empty', () => { +it('should have an error when the name is incorrect', () => { 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'); + wrapper.setState({ touched: true }); + const instance = wrapper.instance(); + + instance.handleProjectNameChange(mockEvent({ currentTarget: { value: '' } })); + expect(wrapper.find(ValidationInput).props().isInvalid).toBe(true); + expect(wrapper.state().projectNameError).toBe( + 'onboarding.create_project.display_name.error.empty' + ); + + instance.handleProjectNameChange( + mockEvent({ currentTarget: { value: new Array(PROJECT_NAME_MAX_LEN + 1).fill('a').join('') } }) + ); + expect(wrapper.find(ValidationInput).props().isInvalid).toBe(true); + expect(wrapper.state().projectNameError).toBe( + 'onboarding.create_project.display_name.error.too_long' + ); }); function shallowRender(props: Partial = {}) { diff --git a/server/sonar-web/src/main/js/apps/create/project/__tests__/__snapshots__/ManualProjectCreate-test.tsx.snap b/server/sonar-web/src/main/js/apps/create/project/__tests__/__snapshots__/ManualProjectCreate-test.tsx.snap index baaac6282ea..806b2c17e3e 100644 --- a/server/sonar-web/src/main/js/apps/create/project/__tests__/__snapshots__/ManualProjectCreate-test.tsx.snap +++ b/server/sonar-web/src/main/js/apps/create/project/__tests__/__snapshots__/ManualProjectCreate-test.tsx.snap @@ -15,27 +15,14 @@ exports[`should render correctly 1`] = ` className="manual-project-create" onSubmit={[Function]} > - - - + onProjectKeyChange={[Function]} + projectKey="" + touched={false} + validating={false} + /> ) => void; + placeholder?: string; + projectKey?: string; + touched: boolean; + validating?: boolean; +} + +export default function ProjectKeyInput(props: ProjectKeyInputProps) { + const { error, help, label, placeholder, projectKey, touched, validating } = props; + + const isInvalid = touched && error !== undefined; + const isValid = touched && !validating && error === undefined; + + return ( + + + + ); +} diff --git a/server/sonar-web/src/main/js/components/common/__tests__/ProjectKeyInput-test.tsx b/server/sonar-web/src/main/js/components/common/__tests__/ProjectKeyInput-test.tsx new file mode 100644 index 00000000000..f8d51f9001b --- /dev/null +++ b/server/sonar-web/src/main/js/components/common/__tests__/ProjectKeyInput-test.tsx @@ -0,0 +1,48 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 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 ValidationInput from 'sonar-ui-common/components/controls/ValidationInput'; +import ProjectKeyInput, { ProjectKeyInputProps } from '../ProjectKeyInput'; + +it('should render correctly', () => { + expect(shallowRender()).toMatchSnapshot('default'); + expect(shallowRender({ projectKey: 'foo' })).toMatchSnapshot('with value'); + expect( + shallowRender({ help: 'foo.help', label: 'foo.label', placeholder: 'foo.placeholder' }) + ).toMatchSnapshot('with label, help, and placeholder'); + expect(shallowRender({ touched: true })).toMatchSnapshot('valid'); + expect(shallowRender({ touched: true, error: 'bar.baz' })).toMatchSnapshot('invalid'); + expect(shallowRender({ touched: true, validating: true })).toMatchSnapshot('validating'); +}); + +it('should not display any status when the key is not defined', () => { + const wrapper = shallowRender(); + const input = wrapper.find(ValidationInput); + expect(input.props().isInvalid).toBe(false); + expect(input.props().isValid).toBe(false); +}); + +function shallowRender(props: Partial = {}) { + return shallow( + + ); +} diff --git a/server/sonar-web/src/main/js/components/common/__tests__/__snapshots__/ProjectKeyInput-test.tsx.snap b/server/sonar-web/src/main/js/components/common/__tests__/__snapshots__/ProjectKeyInput-test.tsx.snap new file mode 100644 index 00000000000..4265dc8d667 --- /dev/null +++ b/server/sonar-web/src/main/js/components/common/__tests__/__snapshots__/ProjectKeyInput-test.tsx.snap @@ -0,0 +1,132 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`should render correctly: default 1`] = ` + + + +`; + +exports[`should render correctly: invalid 1`] = ` + + + +`; + +exports[`should render correctly: valid 1`] = ` + + + +`; + +exports[`should render correctly: validating 1`] = ` + + + +`; + +exports[`should render correctly: with label, help, and placeholder 1`] = ` + + + +`; + +exports[`should render correctly: with value 1`] = ` + + + +`; diff --git a/server/sonar-web/src/main/js/helpers/__tests__/projects-test.ts b/server/sonar-web/src/main/js/helpers/__tests__/projects-test.ts new file mode 100644 index 00000000000..cfee705d54e --- /dev/null +++ b/server/sonar-web/src/main/js/helpers/__tests__/projects-test.ts @@ -0,0 +1,42 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 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 { ProjectKeyValidationResult } from '../../types/component'; +import { PROJECT_KEY_MAX_LEN } from '../constants'; +import { validateProjectKey } from '../projects'; + +describe('validateProjectKey', () => { + it('should correctly flag an invalid key', () => { + // Cannot have special characters except whitelist. + expect(validateProjectKey('foo/bar')).toBe(ProjectKeyValidationResult.InvalidChar); + // Cannot contain only numbers. + expect(validateProjectKey('123')).toBe(ProjectKeyValidationResult.OnlyDigits); + // Cannot be more than 400 chars long. + expect(validateProjectKey(new Array(PROJECT_KEY_MAX_LEN + 1).fill('a').join(''))).toBe( + ProjectKeyValidationResult.TooLong + ); + // Cannot be empty. + expect(validateProjectKey('')).toBe(ProjectKeyValidationResult.Empty); + }); + + it('should not flag a valid key', () => { + expect(validateProjectKey('foo:bar_baz-12.is')).toBe(ProjectKeyValidationResult.Valid); + expect(validateProjectKey('12:34')).toBe(ProjectKeyValidationResult.Valid); + }); +}); diff --git a/server/sonar-web/src/main/js/helpers/constants.ts b/server/sonar-web/src/main/js/helpers/constants.ts index c82212bee8e..f0d60622137 100644 --- a/server/sonar-web/src/main/js/helpers/constants.ts +++ b/server/sonar-web/src/main/js/helpers/constants.ts @@ -53,3 +53,5 @@ export const RATING_COLORS = [ colors.orange, colors.red ]; + +export const PROJECT_KEY_MAX_LEN = 400; diff --git a/server/sonar-web/src/main/js/helpers/projects.ts b/server/sonar-web/src/main/js/helpers/projects.ts new file mode 100644 index 00000000000..f64688d73e8 --- /dev/null +++ b/server/sonar-web/src/main/js/helpers/projects.ts @@ -0,0 +1,39 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 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 { ProjectKeyValidationResult } from '../types/component'; +import { PROJECT_KEY_MAX_LEN } from './constants'; + +export function validateProjectKey(projectKey: string): ProjectKeyValidationResult { + // This is the regex used on the backend: + // [\p{Alnum}\-_.:]*[\p{Alpha}\-_.:]+[\p{Alnum}\-_.:]* + // See sonar-core/src/main/java/org/sonar/core/component/ComponentKeys.java + const regex = /^[\w\-.:]*[a-z\-_.:]+[\w\-.:]*$/i; + if (projectKey.length === 0) { + return ProjectKeyValidationResult.Empty; + } else if (projectKey.length > PROJECT_KEY_MAX_LEN) { + return ProjectKeyValidationResult.TooLong; + } else if (regex.test(projectKey)) { + return ProjectKeyValidationResult.Valid; + } else { + return /^[0-9]+$/.test(projectKey) + ? ProjectKeyValidationResult.OnlyDigits + : ProjectKeyValidationResult.InvalidChar; + } +} diff --git a/server/sonar-web/src/main/js/types/component.ts b/server/sonar-web/src/main/js/types/component.ts index 682fe0ecc8d..ac2812362af 100644 --- a/server/sonar-web/src/main/js/types/component.ts +++ b/server/sonar-web/src/main/js/types/component.ts @@ -17,6 +17,10 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +export enum Visibility { + Public = 'public', + Private = 'private' +} export enum ComponentQualifier { Application = 'APP', @@ -30,6 +34,14 @@ export enum ComponentQualifier { TestFile = 'UTS' } +export enum ProjectKeyValidationResult { + Valid = 'valid', + Empty = 'empty', + TooLong = 'too_long', + InvalidChar = 'invalid_char', + OnlyDigits = 'only_digits' +} + export function isPortfolioLike(componentQualifier?: string | ComponentQualifier) { return Boolean( componentQualifier && @@ -39,8 +51,3 @@ export function isPortfolioLike(componentQualifier?: string | ComponentQualifier ].includes(componentQualifier) ); } - -export enum Visibility { - Public = 'public', - Private = 'private' -} 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 197dd5b2431..a7acab3b9f6 100644 --- a/sonar-core/src/main/resources/org/sonar/l10n/core.properties +++ b/sonar-core/src/main/resources/org/sonar/l10n/core.properties @@ -3077,12 +3077,16 @@ onboarding.project_analysis.guide_to_integrate_pipelines=follow the guide to int onboarding.create_project.setup_manually=Create manually onboarding.create_project.project_key=Project key -onboarding.create_project.project_key.description=Up to 400 characters. All letters, digits, dash, underscore, period or colon. -onboarding.create_project.project_key.error=The provided value doesn't match the expected format. +onboarding.create_project.project_key.description=Up to 400 characters. Allowed characters are alphanumeric, '-' (dash), '_' (underscore), '.' (period) and ':' (colon), with at least one non-digit. +onboarding.create_project.project_key.error.empty=You must provide at least one character. +onboarding.create_project.project_key.error.too_long=The provided key is too long. +onboarding.create_project.project_key.error.invalid_char=The provided key contains invalid characters. +onboarding.create_project.project_key.error.only_digits=The provided key contains only digits. 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 display name is required. +onboarding.create_project.display_name.error.empty=The display name is required. +onboarding.create_project.display_name.error.too_long=The display name is too long. 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 set up -- 2.39.5