From 17453c73bc0eeab032ca66619347dee91bfec4bb Mon Sep 17 00:00:00 2001 From: Wouter Admiraal Date: Mon, 19 Apr 2021 14:30:09 +0200 Subject: [PATCH] SONAR-13737 Show descriptive error messages for GitLab PAT validation --- .../src/main/js/api/alm-integrations.ts | 16 ++- .../create/project/AzureProjectCreate.tsx | 2 +- .../create/project/BitbucketProjectCreate.tsx | 2 +- .../create/project/GitlabProjectCreate.tsx | 38 +++--- .../project/GitlabProjectCreateRenderer.tsx | 7 +- .../project/PersonalAccessTokenForm.tsx | 8 +- .../__tests__/AzureProjectCreate-test.tsx | 8 +- .../__tests__/BitbucketProjectCreate-test.tsx | 8 +- .../__tests__/GitlabProjectCreate-test.tsx | 21 +-- .../GitlabProjectCreateRenderer-test.tsx | 4 +- .../PersonalAccessTokenForm-test.tsx | 3 + .../GitlabProjectCreate-test.tsx.snap | 1 - .../GitlabProjectCreateRenderer-test.tsx.snap | 32 +++++ .../PersonalAccessTokenForm-test.tsx.snap | 121 ++++++++++++++++++ 14 files changed, 220 insertions(+), 51 deletions(-) diff --git a/server/sonar-web/src/main/js/api/alm-integrations.ts b/server/sonar-web/src/main/js/api/alm-integrations.ts index cc287712341..7bdd9e14622 100644 --- a/server/sonar-web/src/main/js/api/alm-integrations.ts +++ b/server/sonar-web/src/main/js/api/alm-integrations.ts @@ -17,7 +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. */ -import { get, getJSON, post, postJSON } from 'sonar-ui-common/helpers/request'; +import { get, getJSON, parseError, post, postJSON } from 'sonar-ui-common/helpers/request'; import throwGlobalError from '../app/utils/throwGlobalError'; import { AzureProject, @@ -34,15 +34,17 @@ export function setAlmPersonalAccessToken(almSetting: string, pat: string): Prom return post('/api/alm_integrations/set_pat', { almSetting, pat }).catch(throwGlobalError); } -export function checkPersonalAccessTokenIsValid(almSetting: string): Promise { +export function checkPersonalAccessTokenIsValid( + almSetting: string +): Promise<{ status: boolean; error?: string }> { return get('/api/alm_integrations/check_pat', { almSetting }) - .then(() => true) - .catch((response: Response) => { + .then(() => ({ status: true })) + .catch(async (response: Response) => { if (response.status === 400) { - return false; - } else { - return throwGlobalError(response); + const error = await parseError(response); + return { status: false, error }; } + return throwGlobalError(response); }); } diff --git a/server/sonar-web/src/main/js/apps/create/project/AzureProjectCreate.tsx b/server/sonar-web/src/main/js/apps/create/project/AzureProjectCreate.tsx index 04dba03bc88..0bb78cd2957 100644 --- a/server/sonar-web/src/main/js/apps/create/project/AzureProjectCreate.tsx +++ b/server/sonar-web/src/main/js/apps/create/project/AzureProjectCreate.tsx @@ -240,7 +240,7 @@ export default class AzureProjectCreate extends React.PureComponent status); }; handlePersonalAccessTokenCreate = async (token: string) => { diff --git a/server/sonar-web/src/main/js/apps/create/project/BitbucketProjectCreate.tsx b/server/sonar-web/src/main/js/apps/create/project/BitbucketProjectCreate.tsx index 7c6f4fe80e0..c3d57d0cfbc 100644 --- a/server/sonar-web/src/main/js/apps/create/project/BitbucketProjectCreate.tsx +++ b/server/sonar-web/src/main/js/apps/create/project/BitbucketProjectCreate.tsx @@ -122,7 +122,7 @@ export default class BitbucketProjectCreate extends React.PureComponent status); }; fetchBitbucketProjects = (): Promise => { diff --git a/server/sonar-web/src/main/js/apps/create/project/GitlabProjectCreate.tsx b/server/sonar-web/src/main/js/apps/create/project/GitlabProjectCreate.tsx index 10560b235eb..39b8537581f 100644 --- a/server/sonar-web/src/main/js/apps/create/project/GitlabProjectCreate.tsx +++ b/server/sonar-web/src/main/js/apps/create/project/GitlabProjectCreate.tsx @@ -19,6 +19,7 @@ */ import * as React from 'react'; import { WithRouterProps } from 'react-router'; +import { translate } from 'sonar-ui-common/helpers/l10n'; import { checkPersonalAccessTokenIsValid, getGitlabProjects, @@ -26,7 +27,7 @@ import { setAlmPersonalAccessToken } from '../../../api/alm-integrations'; import { GitlabProject } from '../../../types/alm-integration'; -import { AlmSettingsInstance } from '../../../types/alm-settings'; +import { AlmKeys, AlmSettingsInstance } from '../../../types/alm-settings'; import GitlabProjectCreateRenderer from './GitlabProjectCreateRenderer'; interface Props extends Pick { @@ -44,7 +45,7 @@ interface State { projectsPaging: T.Paging; submittingToken: boolean; tokenIsValid: boolean; - tokenValidationFailed: boolean; + tokenValidationErrorMessage?: string; searching: boolean; searchQuery: string; settings?: AlmSettingsInstance; @@ -66,8 +67,7 @@ export default class GitlabProjectCreate extends React.PureComponent { this.setState({ loading: true }); - const tokenIsValid = await this.checkPersonalAccessToken(); + const { status, error } = await this.checkPersonalAccessToken(); let result; - if (tokenIsValid) { + if (status) { result = await this.fetchProjects(); } @@ -104,14 +104,15 @@ export default class GitlabProjectCreate extends React.PureComponent false); + return checkPersonalAccessTokenIsValid(settings.key); }; handleError = () => { @@ -231,21 +235,21 @@ export default class GitlabProjectCreate extends React.PureComponent ); } diff --git a/server/sonar-web/src/main/js/apps/create/project/GitlabProjectCreateRenderer.tsx b/server/sonar-web/src/main/js/apps/create/project/GitlabProjectCreateRenderer.tsx index d5d46201059..e75f5809845 100644 --- a/server/sonar-web/src/main/js/apps/create/project/GitlabProjectCreateRenderer.tsx +++ b/server/sonar-web/src/main/js/apps/create/project/GitlabProjectCreateRenderer.tsx @@ -43,7 +43,7 @@ export interface GitlabProjectCreateRendererProps { settings?: AlmSettingsInstance; showPersonalAccessTokenForm?: boolean; submittingToken?: boolean; - tokenValidationFailed: boolean; + tokenValidationErrorMessage?: string; } export default function GitlabProjectCreateRenderer(props: GitlabProjectCreateRendererProps) { @@ -59,7 +59,7 @@ export default function GitlabProjectCreateRenderer(props: GitlabProjectCreateRe settings, showPersonalAccessTokenForm, submittingToken, - tokenValidationFailed + tokenValidationErrorMessage } = props; return ( @@ -91,7 +91,8 @@ export default function GitlabProjectCreateRenderer(props: GitlabProjectCreateRe almSetting={settings} onPersonalAccessTokenCreate={props.onPersonalAccessTokenCreate} submitting={submittingToken} - validationFailed={tokenValidationFailed} + validationFailed={Boolean(tokenValidationErrorMessage)} + validationErrorMessage={tokenValidationErrorMessage} /> ) : ( void; submitting?: boolean; validationFailed: boolean; + validationErrorMessage?: string; } function getPatUrl(alm: AlmKeys, url: string) { @@ -50,7 +51,8 @@ export default function PersonalAccessTokenForm(props: PersonalAccessTokenFormPr const { almSetting: { alm, url }, submitting = false, - validationFailed + validationFailed, + validationErrorMessage } = props; const [touched, setTouched] = React.useState(false); @@ -59,6 +61,8 @@ export default function PersonalAccessTokenForm(props: PersonalAccessTokenFormPr }, [submitting]); const isInvalid = validationFailed && !touched; + const errorMessage = + validationErrorMessage ?? translate('onboarding.create_project.pat_incorrect', alm); return (
@@ -75,7 +79,7 @@ export default function PersonalAccessTokenForm(props: PersonalAccessTokenFormPr

{ return { - checkPersonalAccessTokenIsValid: jest.fn().mockResolvedValue(true), + checkPersonalAccessTokenIsValid: jest.fn().mockResolvedValue({ status: true }), setAlmPersonalAccessToken: jest.fn().mockResolvedValue(null), getAzureProjects: jest.fn().mockResolvedValue({ projects: [] }), getAzureRepositories: jest.fn().mockResolvedValue({ repositories: [] }), @@ -59,7 +59,7 @@ it('should correctly fetch binding info on mount', async () => { }); it('should correctly handle a valid PAT', async () => { - (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce(true); + (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce({ status: true }); const wrapper = shallowRender(); await waitAndUpdate(wrapper); expect(checkPersonalAccessTokenIsValid).toBeCalled(); @@ -67,7 +67,7 @@ it('should correctly handle a valid PAT', async () => { }); it('should correctly handle an invalid PAT', async () => { - (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce(false); + (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce({ status: false }); const wrapper = shallowRender(); await waitAndUpdate(wrapper); expect(checkPersonalAccessTokenIsValid).toBeCalled(); @@ -81,7 +81,7 @@ it('should correctly handle setting a new PAT', async () => { expect(setAlmPersonalAccessToken).toBeCalledWith('foo', 'token'); expect(wrapper.state().submittingToken).toBe(true); - (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce(false); + (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce({ status: false }); await waitAndUpdate(wrapper); expect(checkPersonalAccessTokenIsValid).toBeCalled(); expect(wrapper.state().submittingToken).toBe(false); diff --git a/server/sonar-web/src/main/js/apps/create/project/__tests__/BitbucketProjectCreate-test.tsx b/server/sonar-web/src/main/js/apps/create/project/__tests__/BitbucketProjectCreate-test.tsx index e1317528df6..bb3fd5ae7df 100644 --- a/server/sonar-web/src/main/js/apps/create/project/__tests__/BitbucketProjectCreate-test.tsx +++ b/server/sonar-web/src/main/js/apps/create/project/__tests__/BitbucketProjectCreate-test.tsx @@ -39,7 +39,7 @@ jest.mock('../../../../api/alm-integrations', () => { '../../../../helpers/mocks/alm-integrations' ); return { - checkPersonalAccessTokenIsValid: jest.fn().mockResolvedValue(true), + checkPersonalAccessTokenIsValid: jest.fn().mockResolvedValue({ status: true }), getBitbucketServerProjects: jest.fn().mockResolvedValue({ projects: [ mockBitbucketProject({ key: 'project1', name: 'Project 1' }), @@ -76,7 +76,7 @@ it('should correctly fetch binding info on mount', async () => { }); it('should correctly handle a valid PAT', async () => { - (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce(true); + (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce({ status: true }); const wrapper = shallowRender(); await waitAndUpdate(wrapper); expect(checkPersonalAccessTokenIsValid).toBeCalled(); @@ -84,7 +84,7 @@ it('should correctly handle a valid PAT', async () => { }); it('should correctly handle an invalid PAT', async () => { - (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce(false); + (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce({ status: false }); const wrapper = shallowRender(); await waitAndUpdate(wrapper); expect(checkPersonalAccessTokenIsValid).toBeCalled(); @@ -97,7 +97,7 @@ it('should correctly handle setting a new PAT', async () => { expect(setAlmPersonalAccessToken).toBeCalledWith('foo', 'token'); expect(wrapper.state().submittingToken).toBe(true); - (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce(false); + (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce({ status: false }); await waitAndUpdate(wrapper); expect(checkPersonalAccessTokenIsValid).toBeCalled(); expect(wrapper.state().submittingToken).toBe(false); diff --git a/server/sonar-web/src/main/js/apps/create/project/__tests__/GitlabProjectCreate-test.tsx b/server/sonar-web/src/main/js/apps/create/project/__tests__/GitlabProjectCreate-test.tsx index 890ccadc4f9..e5663ff7734 100644 --- a/server/sonar-web/src/main/js/apps/create/project/__tests__/GitlabProjectCreate-test.tsx +++ b/server/sonar-web/src/main/js/apps/create/project/__tests__/GitlabProjectCreate-test.tsx @@ -33,7 +33,7 @@ import { AlmKeys } from '../../../../types/alm-settings'; import GitlabProjectCreate from '../GitlabProjectCreate'; jest.mock('../../../../api/alm-integrations', () => ({ - checkPersonalAccessTokenIsValid: jest.fn().mockResolvedValue(true), + checkPersonalAccessTokenIsValid: jest.fn().mockResolvedValue({ status: true }), setAlmPersonalAccessToken: jest.fn().mockResolvedValue(null), getGitlabProjects: jest.fn().mockRejectedValue('error'), importGitlabProject: jest.fn().mockRejectedValue('error') @@ -65,7 +65,7 @@ it('should correctly check PAT when settings are added after mount', async () => }); it('should correctly handle a valid PAT', async () => { - (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce(true); + (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce({ status: true }); (getGitlabProjects as jest.Mock).mockResolvedValueOnce({ projects: [mockGitlabProject()], projectsPaging: { @@ -80,7 +80,7 @@ it('should correctly handle a valid PAT', async () => { }); it('should correctly handle an invalid PAT', async () => { - (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce(false); + (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce({ status: false }); const wrapper = shallowRender(); await waitAndUpdate(wrapper); expect(wrapper.state().tokenIsValid).toBe(false); @@ -95,7 +95,8 @@ describe('setting a new PAT', () => { }); it('should correctly handle it if invalid', async () => { - (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce(false); + const error = 'error message'; + (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce({ status: false, error }); wrapper.instance().handlePersonalAccessTokenCreate('invalidtoken'); expect(setAlmPersonalAccessToken).toBeCalledWith(almSettingKey, 'invalidtoken'); @@ -103,11 +104,11 @@ describe('setting a new PAT', () => { await waitAndUpdate(wrapper); expect(checkPersonalAccessTokenIsValid).toBeCalled(); expect(wrapper.state().submittingToken).toBe(false); - expect(wrapper.state().tokenValidationFailed).toBe(true); + expect(wrapper.state().tokenValidationErrorMessage).toBe(error); }); it('should correctly handle it if valid', async () => { - (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce(true); + (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce({ status: true }); wrapper.instance().handlePersonalAccessTokenCreate('validtoken'); expect(setAlmPersonalAccessToken).toBeCalledWith(almSettingKey, 'validtoken'); @@ -115,14 +116,14 @@ describe('setting a new PAT', () => { await waitAndUpdate(wrapper); expect(checkPersonalAccessTokenIsValid).toBeCalled(); expect(wrapper.state().submittingToken).toBe(false); - expect(wrapper.state().tokenValidationFailed).toBe(false); + expect(wrapper.state().tokenValidationErrorMessage).toBeUndefined(); expect(routerReplace).toBeCalled(); }); }); it('should fetch more projects and preserve search', async () => { - (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce(true); + (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce({ status: true }); const projects = [ mockGitlabProject({ id: '1' }), @@ -166,7 +167,7 @@ it('should fetch more projects and preserve search', async () => { }); it('should search for projects', async () => { - (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce(true); + (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce({ status: true }); const projects = [ mockGitlabProject({ id: '1' }), @@ -210,7 +211,7 @@ it('should search for projects', async () => { }); it('should import', async () => { - (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce(true); + (checkPersonalAccessTokenIsValid as jest.Mock).mockResolvedValueOnce({ status: true }); const projects = [mockGitlabProject({ id: '1' }), mockGitlabProject({ id: '2' })]; (getGitlabProjects as jest.Mock).mockResolvedValueOnce({ diff --git a/server/sonar-web/src/main/js/apps/create/project/__tests__/GitlabProjectCreateRenderer-test.tsx b/server/sonar-web/src/main/js/apps/create/project/__tests__/GitlabProjectCreateRenderer-test.tsx index 76ac851e494..3a41f9c6fde 100644 --- a/server/sonar-web/src/main/js/apps/create/project/__tests__/GitlabProjectCreateRenderer-test.tsx +++ b/server/sonar-web/src/main/js/apps/create/project/__tests__/GitlabProjectCreateRenderer-test.tsx @@ -35,6 +35,9 @@ it('should render correctly', () => { expect(shallowRender({ showPersonalAccessTokenForm: false })).toMatchSnapshot( 'project selection form' ); + expect(shallowRender({ tokenValidationErrorMessage: 'error' })).toMatchSnapshot( + 'pat validation error' + ); }); function shallowRender(props: Partial = {}) { @@ -53,7 +56,6 @@ function shallowRender(props: Partial = {}) { searchQuery="" showPersonalAccessTokenForm={true} submittingToken={false} - tokenValidationFailed={false} settings={mockAlmSettingsInstance({ alm: AlmKeys.GitLab })} {...props} /> diff --git a/server/sonar-web/src/main/js/apps/create/project/__tests__/PersonalAccessTokenForm-test.tsx b/server/sonar-web/src/main/js/apps/create/project/__tests__/PersonalAccessTokenForm-test.tsx index 5e7571a211b..23fc3ad3138 100644 --- a/server/sonar-web/src/main/js/apps/create/project/__tests__/PersonalAccessTokenForm-test.tsx +++ b/server/sonar-web/src/main/js/apps/create/project/__tests__/PersonalAccessTokenForm-test.tsx @@ -29,6 +29,9 @@ it('should render correctly', () => { expect(shallowRender()).toMatchSnapshot('bitbucket'); expect(shallowRender({ submitting: true })).toMatchSnapshot('submitting'); expect(shallowRender({ validationFailed: true })).toMatchSnapshot('validation failed'); + expect( + shallowRender({ validationFailed: true, validationErrorMessage: 'error' }) + ).toMatchSnapshot('validation failed, custom error message'); expect( shallowRender({ almSetting: mockAlmSettingsInstance({ alm: AlmKeys.GitLab, url: 'https://gitlab.com/api/v4' }) diff --git a/server/sonar-web/src/main/js/apps/create/project/__tests__/__snapshots__/GitlabProjectCreate-test.tsx.snap b/server/sonar-web/src/main/js/apps/create/project/__tests__/__snapshots__/GitlabProjectCreate-test.tsx.snap index 7d9b8e454d4..8b9520b04ec 100644 --- a/server/sonar-web/src/main/js/apps/create/project/__tests__/__snapshots__/GitlabProjectCreate-test.tsx.snap +++ b/server/sonar-web/src/main/js/apps/create/project/__tests__/__snapshots__/GitlabProjectCreate-test.tsx.snap @@ -26,6 +26,5 @@ exports[`should render correctly 1`] = ` } showPersonalAccessTokenForm={true} submittingToken={false} - tokenValidationFailed={false} /> `; diff --git a/server/sonar-web/src/main/js/apps/create/project/__tests__/__snapshots__/GitlabProjectCreateRenderer-test.tsx.snap b/server/sonar-web/src/main/js/apps/create/project/__tests__/__snapshots__/GitlabProjectCreateRenderer-test.tsx.snap index 7a78bb9b202..72c564b381a 100644 --- a/server/sonar-web/src/main/js/apps/create/project/__tests__/__snapshots__/GitlabProjectCreateRenderer-test.tsx.snap +++ b/server/sonar-web/src/main/js/apps/create/project/__tests__/__snapshots__/GitlabProjectCreateRenderer-test.tsx.snap @@ -102,6 +102,38 @@ exports[`should render correctly: pat form 1`] = ` `; +exports[`should render correctly: pat validation error 1`] = ` + + + + onboarding.create_project.gitlab.title + + } + /> + + +`; + exports[`should render correctly: project selection form 1`] = `
`; + +exports[`should render correctly: validation failed, custom error message 1`] = ` +
+
+

+ onboarding.create_project.pat_form.title.bitbucket +

+

+ onboarding.create_project.pat_form.help.bitbucket +

+ + + + + save + + + + +

+ onboarding.create_project.pat_help.title +

+

+ +

+ +

+ onboarding.create_project.pat_help.instructions2.bitbucket +

+
    +
  • + + onboarding.create_project.pat_help.read_permission + , + } + } + /> +
    • +
  • + + onboarding.create_project.pat_help.read_permission + , + } + } + /> +
    • +
+ +
+`; -- 2.39.5