From f3180f874f095ffa1d26f876a6fd4acfb3cdf6c4 Mon Sep 17 00:00:00 2001 From: Philippe Perrin Date: Thu, 24 Jun 2021 14:15:06 +0200 Subject: [PATCH] SONAR-14932 Trigger alm settings validation before closing the modal --- .../apps/create/project/CreateProjectPage.tsx | 1 + .../__tests__/CreateProjectPage-test.tsx | 76 ++++++++----- .../CreateProjectPage-test.tsx.snap | 101 +++++++++++++++++- .../AlmBindingDefinitionForm.tsx | 76 ++++++++++--- .../AlmBindingDefinitionFormRenderer.tsx | 19 +++- .../AlmBindingDefinitionForm-test.tsx | 31 +++++- .../AlmBindingDefinitionFormRenderer-test.tsx | 3 + ...indingDefinitionFormRenderer-test.tsx.snap | 73 +++++++++++++ .../resources/org/sonar/l10n/core.properties | 1 + 9 files changed, 330 insertions(+), 51 deletions(-) diff --git a/server/sonar-web/src/main/js/apps/create/project/CreateProjectPage.tsx b/server/sonar-web/src/main/js/apps/create/project/CreateProjectPage.tsx index b4c8c35affb..43a9a8fb69c 100644 --- a/server/sonar-web/src/main/js/apps/create/project/CreateProjectPage.tsx +++ b/server/sonar-web/src/main/js/apps/create/project/CreateProjectPage.tsx @@ -257,6 +257,7 @@ export class CreateProjectPage extends React.PureComponent { alreadyHaveInstanceConfigured={false} onCancel={this.handleOnCancelCreation} afterSubmit={this.handleAfterSubmit} + enforceValidation={true} /> )} diff --git a/server/sonar-web/src/main/js/apps/create/project/__tests__/CreateProjectPage-test.tsx b/server/sonar-web/src/main/js/apps/create/project/__tests__/CreateProjectPage-test.tsx index da7ede592d4..2845ff4d050 100644 --- a/server/sonar-web/src/main/js/apps/create/project/__tests__/CreateProjectPage-test.tsx +++ b/server/sonar-web/src/main/js/apps/create/project/__tests__/CreateProjectPage-test.tsx @@ -19,9 +19,12 @@ */ import { shallow } from 'enzyme'; import * as React from 'react'; +import { waitAndUpdate } from 'sonar-ui-common/helpers/testUtils'; import { getAlmSettings } from '../../../../api/alm-settings'; import { mockLocation, mockLoggedInUser, mockRouter } from '../../../../helpers/testMocks'; import { AlmKeys } from '../../../../types/alm-settings'; +import AlmBindingDefinitionForm from '../../../settings/components/almIntegration/AlmBindingDefinitionForm'; +import CreateProjectModeSelection from '../CreateProjectModeSelection'; import { CreateProjectPage } from '../CreateProjectPage'; import { CreateProjectModes } from '../types'; @@ -36,42 +39,65 @@ it('should render correctly', () => { expect(getAlmSettings).toBeCalled(); }); -it('should render correctly if the manual method is selected', () => { +it.each([ + [CreateProjectModes.Manual], + [CreateProjectModes.AzureDevOps], + [CreateProjectModes.BitbucketServer], + [CreateProjectModes.BitbucketCloud], + [CreateProjectModes.GitHub], + [CreateProjectModes.GitLab] +])('should render correctly for %s mode', (mode: CreateProjectModes) => { expect( shallowRender({ - location: mockLocation({ query: { mode: CreateProjectModes.Manual } }) + location: mockLocation({ query: { mode } }) }) ).toMatchSnapshot(); }); -it('should render correctly if the Azure method is selected', () => { - expect( - shallowRender({ - location: mockLocation({ query: { mode: CreateProjectModes.AzureDevOps } }) - }) - ).toMatchSnapshot(); -}); +it('should render alm configuration creation correctly', () => { + const wrapper = shallowRender(); -it('should render correctly if the BBS method is selected', () => { - expect( - shallowRender({ - location: mockLocation({ query: { mode: CreateProjectModes.BitbucketServer } }) - }) - ).toMatchSnapshot(); + wrapper + .find(CreateProjectModeSelection) + .props() + .onConfigMode(AlmKeys.Azure); + expect(wrapper).toMatchSnapshot(); }); -it('should render correctly if the GitHub method is selected', () => { - const wrapper = shallowRender({ - location: mockLocation({ query: { mode: CreateProjectModes.GitHub } }) - }); - expect(wrapper).toMatchSnapshot(); +it('should cancel alm configuration creation properly', () => { + const wrapper = shallowRender(); + + wrapper + .find(CreateProjectModeSelection) + .props() + .onConfigMode(AlmKeys.Azure); + expect(wrapper.state().creatingAlmDefinition).toBe(AlmKeys.Azure); + + wrapper + .find(AlmBindingDefinitionForm) + .props() + .onCancel(); + expect(wrapper.state().creatingAlmDefinition).toBeUndefined(); }); -it('should render correctly if the GitLab method is selected', () => { - const wrapper = shallowRender({ - location: mockLocation({ query: { mode: CreateProjectModes.GitLab } }) - }); - expect(wrapper).toMatchSnapshot(); +it('should submit alm configuration creation properly', async () => { + const push = jest.fn(); + const wrapper = shallowRender({ router: mockRouter({ push }) }); + + wrapper + .find(CreateProjectModeSelection) + .props() + .onConfigMode(AlmKeys.Azure); + expect(wrapper.state().creatingAlmDefinition).toBe(AlmKeys.Azure); + + wrapper + .find(AlmBindingDefinitionForm) + .props() + .afterSubmit({ key: 'test-key' }); + await waitAndUpdate(wrapper); + expect(wrapper.state().creatingAlmDefinition).toBeUndefined(); + expect(getAlmSettings).toHaveBeenCalled(); + expect(push).toHaveBeenCalledWith({ pathname: '/path', query: { mode: AlmKeys.Azure } }); }); it('should submit alm configuration creation properly for BBC', async () => { diff --git a/server/sonar-web/src/main/js/apps/create/project/__tests__/__snapshots__/CreateProjectPage-test.tsx.snap b/server/sonar-web/src/main/js/apps/create/project/__tests__/__snapshots__/CreateProjectPage-test.tsx.snap index 58fa5abb10e..e23064c7a37 100644 --- a/server/sonar-web/src/main/js/apps/create/project/__tests__/__snapshots__/CreateProjectPage-test.tsx.snap +++ b/server/sonar-web/src/main/js/apps/create/project/__tests__/__snapshots__/CreateProjectPage-test.tsx.snap @@ -1,5 +1,45 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`should render alm configuration creation correctly 1`] = ` + + + +
+ + +
+ +`; + exports[`should render correctly 1`] = ` `; -exports[`should render correctly if the Azure method is selected 1`] = ` +exports[`should render correctly for azure mode 1`] = ` `; -exports[`should render correctly if the BBS method is selected 1`] = ` +exports[`should render correctly for bitbucket mode 1`] = ` `; -exports[`should render correctly if the GitHub method is selected 1`] = ` +exports[`should render correctly for bitbucketcloud mode 1`] = ` + + + +
+ +
+ +`; + +exports[`should render correctly for github mode 1`] = ` `; -exports[`should render correctly if the GitLab method is selected 1`] = ` +exports[`should render correctly for gitlab mode 1`] = ` `; -exports[`should render correctly if the manual method is selected 1`] = ` +exports[`should render correctly for manual mode 1`] = ` void; - afterSubmit?: (data: AlmBindingDefinitionBase) => void; + onCancel: () => void; + afterSubmit: (data: AlmBindingDefinitionBase) => void; + enforceValidation?: boolean; } interface State { @@ -56,6 +59,8 @@ interface State { touched: boolean; submitting: boolean; bitbucketVariant?: AlmKeys.BitbucketServer | AlmKeys.BitbucketCloud; + alreadySavedFormData?: AlmBindingDefinition; + validationError?: string; } const BINDING_PER_ALM = { @@ -144,33 +149,71 @@ export default class AlmBindingDefinitionForm extends React.PureComponent { - const { alm } = this.props; - const { formData, bitbucketVariant } = this.state; + const { alm, enforceValidation } = this.props; + const { formData, bitbucketVariant, alreadySavedFormData, validationError } = this.state; const apiAlm = bitbucketVariant ?? alm; - const apiMethod = this.props.bindingDefinition?.key - ? BINDING_PER_ALM[apiAlm].updateApi({ - newKey: formData.key, - ...formData, - key: this.props.bindingDefinition.key - } as any) - : BINDING_PER_ALM[apiAlm].createApi({ ...formData } as any); + let apiMethod; + + if (alreadySavedFormData && validationError) { + apiMethod = BINDING_PER_ALM[apiAlm].updateApi({ + newKey: formData.key, + ...formData, + key: alreadySavedFormData.key + } as any); + } else if (this.props.bindingDefinition?.key) { + apiMethod = BINDING_PER_ALM[apiAlm].updateApi({ + newKey: formData.key, + ...formData, + key: this.props.bindingDefinition.key + } as any); + } else { + apiMethod = BINDING_PER_ALM[apiAlm].createApi({ ...formData } as any); + } this.setState({ submitting: true }); try { await apiMethod; - if (this.props.afterSubmit) { + if (!this.mounted) { + return; + } + + this.setState({ alreadySavedFormData: formData }); + + let error: string | undefined; + + if (enforceValidation) { + error = await validateAlmSettings(formData.key); + } + + if (!this.mounted) { + return; + } + + if (error) { + this.setState({ validationError: error }); + } else { this.props.afterSubmit(formData); } } finally { if (this.mounted) { - this.setState({ submitting: false }); + this.setState({ submitting: false, touched: false }); } } }; + handleOnCancel = async () => { + const { alreadySavedFormData } = this.state; + + if (alreadySavedFormData) { + await deleteConfiguration(alreadySavedFormData.key); + } + + this.props.onCancel(); + }; + handleBitbucketVariantChange = ( bitbucketVariant: AlmKeys.BitbucketServer | AlmKeys.BitbucketCloud ) => { @@ -188,7 +231,7 @@ export default class AlmBindingDefinitionForm extends React.PureComponent this.props.onCancel && this.props.onCancel()} + onCancel={this.handleOnCancel} onSubmit={this.handleFormSubmit} onFieldChange={this.handleFieldChange} formData={formData} submitting={submitting} bitbucketVariant={bitbucketVariant} onBitbucketVariantChange={this.handleBitbucketVariantChange} + validationError={validationError} /> ); } diff --git a/server/sonar-web/src/main/js/apps/settings/components/almIntegration/AlmBindingDefinitionFormRenderer.tsx b/server/sonar-web/src/main/js/apps/settings/components/almIntegration/AlmBindingDefinitionFormRenderer.tsx index c50f5d2ed4e..1aca5499b39 100644 --- a/server/sonar-web/src/main/js/apps/settings/components/almIntegration/AlmBindingDefinitionFormRenderer.tsx +++ b/server/sonar-web/src/main/js/apps/settings/components/almIntegration/AlmBindingDefinitionFormRenderer.tsx @@ -51,6 +51,7 @@ export interface AlmBindingDefinitionFormProps { onBitbucketVariantChange: ( bitbucketVariant: AlmKeys.BitbucketServer | AlmKeys.BitbucketCloud ) => void; + validationError?: string; } export default class AlmBindingDefinitionFormRenderer extends React.PureComponent< @@ -99,7 +100,13 @@ export default class AlmBindingDefinitionFormRenderer extends React.PureComponen }; render() { - const { isUpdate, alreadyHaveInstanceConfigured, canSubmit, submitting } = this.props; + const { + isUpdate, + alreadyHaveInstanceConfigured, + canSubmit, + submitting, + validationError + } = this.props; const header = translate('settings.almintegration.form.header', isUpdate ? 'edit' : 'create'); const handleSubmit = (event: React.SyntheticEvent) => { @@ -125,6 +132,16 @@ export default class AlmBindingDefinitionFormRenderer extends React.PureComponen )} {this.renderForm()} + {validationError && !canSubmit && ( + +

+ {translate('settings.almintegration.configuration_invalid')} +

+
    +
  • {validationError}
    • +
+ + )}
diff --git a/server/sonar-web/src/main/js/apps/settings/components/almIntegration/__tests__/AlmBindingDefinitionForm-test.tsx b/server/sonar-web/src/main/js/apps/settings/components/almIntegration/__tests__/AlmBindingDefinitionForm-test.tsx index 09f5f58c8a3..b4ca5b1e7b4 100644 --- a/server/sonar-web/src/main/js/apps/settings/components/almIntegration/__tests__/AlmBindingDefinitionForm-test.tsx +++ b/server/sonar-web/src/main/js/apps/settings/components/almIntegration/__tests__/AlmBindingDefinitionForm-test.tsx @@ -26,11 +26,13 @@ import { createBitbucketServerConfiguration, createGithubConfiguration, createGitlabConfiguration, + deleteConfiguration, updateAzureConfiguration, updateBitbucketCloudConfiguration, updateBitbucketServerConfiguration, updateGithubConfiguration, - updateGitlabConfiguration + updateGitlabConfiguration, + validateAlmSettings } from '../../../../../api/alm-settings'; import { mockAzureBindingDefinition, @@ -53,7 +55,9 @@ jest.mock('../../../../../api/alm-settings', () => ({ updateBitbucketCloudConfiguration: jest.fn().mockResolvedValue({}), updateBitbucketServerConfiguration: jest.fn().mockResolvedValue({}), updateGithubConfiguration: jest.fn().mockResolvedValue({}), - updateGitlabConfiguration: jest.fn().mockResolvedValue({}) + updateGitlabConfiguration: jest.fn().mockResolvedValue({}), + validateAlmSettings: jest.fn().mockResolvedValue(undefined), + deleteConfiguration: jest.fn().mockResolvedValue(undefined) })); beforeEach(() => { @@ -89,12 +93,31 @@ it('should handle form submit', async () => { const wrapper = shallowRender({ afterSubmit }); wrapper.instance().setState({ formData }); - await waitAndUpdate(wrapper); await wrapper.instance().handleFormSubmit(); - await waitAndUpdate(wrapper); expect(afterSubmit).toHaveBeenCalledWith(formData); }); +it('should handle validation error during submit, and cancellation', async () => { + const afterSubmit = jest.fn(); + const formData = mockGithubBindingDefinition(); + const error = 'This a test error message'; + (validateAlmSettings as jest.Mock).mockResolvedValueOnce(error); + + const wrapper = shallowRender({ afterSubmit, enforceValidation: true }); + + wrapper.instance().setState({ formData }); + await wrapper.instance().handleFormSubmit(); + expect(validateAlmSettings).toHaveBeenCalledWith(formData.key); + expect(wrapper.state().validationError).toBe(error); + expect(afterSubmit).not.toHaveBeenCalledWith(); + + wrapper + .find(AlmBindingDefinitionFormRenderer) + .props() + .onCancel(); + expect(deleteConfiguration).toHaveBeenCalledWith(formData.key); +}); + it.each([ [AlmKeys.Azure, undefined, createAzureConfiguration], [AlmKeys.Azure, mockAzureBindingDefinition(), updateAzureConfiguration], diff --git a/server/sonar-web/src/main/js/apps/settings/components/almIntegration/__tests__/AlmBindingDefinitionFormRenderer-test.tsx b/server/sonar-web/src/main/js/apps/settings/components/almIntegration/__tests__/AlmBindingDefinitionFormRenderer-test.tsx index a28503cd58c..e60c01849c8 100644 --- a/server/sonar-web/src/main/js/apps/settings/components/almIntegration/__tests__/AlmBindingDefinitionFormRenderer-test.tsx +++ b/server/sonar-web/src/main/js/apps/settings/components/almIntegration/__tests__/AlmBindingDefinitionFormRenderer-test.tsx @@ -34,6 +34,9 @@ it('should render correctly', () => { expect(shallowRender({ alreadyHaveInstanceConfigured: true })).toMatchSnapshot('second instance'); expect(shallowRender({ submitting: true })).toMatchSnapshot('submitting'); expect(shallowRender({ isUpdate: true })).toMatchSnapshot('editing'); + expect(shallowRender({ validationError: 'this is a validation error' })).toMatchSnapshot( + 'with validation error' + ); }); it.each([[AlmKeys.Azure], [AlmKeys.GitHub], [AlmKeys.GitLab], [AlmKeys.BitbucketServer]])( diff --git a/server/sonar-web/src/main/js/apps/settings/components/almIntegration/__tests__/__snapshots__/AlmBindingDefinitionFormRenderer-test.tsx.snap b/server/sonar-web/src/main/js/apps/settings/components/almIntegration/__tests__/__snapshots__/AlmBindingDefinitionFormRenderer-test.tsx.snap index ef718f37281..fceec04e547 100644 --- a/server/sonar-web/src/main/js/apps/settings/components/almIntegration/__tests__/__snapshots__/AlmBindingDefinitionFormRenderer-test.tsx.snap +++ b/server/sonar-web/src/main/js/apps/settings/components/almIntegration/__tests__/__snapshots__/AlmBindingDefinitionFormRenderer-test.tsx.snap @@ -464,3 +464,76 @@ exports[`should render correctly: submitting 1`] = ` `; + +exports[`should render correctly: with validation error 1`] = ` + +
+
+

+ settings.almintegration.form.header.create +

+
+
+ + +

+ settings.almintegration.configuration_invalid +

+
    +
  • + this is a validation error +
    • +
+ +
+
+ + settings.almintegration.form.save + + + + cancel + +
+
+ +`; 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 d48356c261b..300ba99d008 100644 --- a/sonar-core/src/main/resources/org/sonar/l10n/core.properties +++ b/sonar-core/src/main/resources/org/sonar/l10n/core.properties @@ -1112,6 +1112,7 @@ settings.almintegration.create.tooltip.link=Enterprise Edition settings.almintegration.check_configuration=Check configuration settings.almintegration.checking_configuration=Checking configuration settings.almintegration.configuration_valid=Configuration valid +settings.almintegration.configuration_invalid=You have the following errors in your configuration: settings.almintegration.could_not_validate=Could not validate this configuration. settings.almintegration.delete.header=Delete configuration settings.almintegration.delete.message=Are you sure you want to delete the {id} configuration? -- 2.39.5