From 974534337dcdd4d6d7b33b21f41c04f38df8c775 Mon Sep 17 00:00:00 2001 From: guillaume-peoch-sonarsource Date: Wed, 17 Jan 2024 18:11:29 +0100 Subject: [PATCH] SONAR-21413 move groups field in Gitlab common config --- .../GitLabAuthenticationTab.tsx | 36 +----- .../GitLabConfigurationForm.tsx | 27 +++- .../__tests__/Authentication-Gitlab-it.tsx | 118 +++++------------- .../main/js/helpers/mocks/alm-integrations.ts | 2 +- .../src/main/js/types/provisioning.ts | 1 + 5 files changed, 60 insertions(+), 124 deletions(-) diff --git a/server/sonar-web/src/main/js/apps/settings/components/authentication/GitLabAuthenticationTab.tsx b/server/sonar-web/src/main/js/apps/settings/components/authentication/GitLabAuthenticationTab.tsx index 1a970bcdf8b..4068bc40dda 100644 --- a/server/sonar-web/src/main/js/apps/settings/components/authentication/GitLabAuthenticationTab.tsx +++ b/server/sonar-web/src/main/js/apps/settings/components/authentication/GitLabAuthenticationTab.tsx @@ -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 { isEqual, omitBy } from 'lodash'; +import { omitBy } from 'lodash'; import React, { FormEvent, useContext } from 'react'; import { FormattedMessage } from 'react-intl'; import GitLabSynchronisationWarning from '../../../../app/components/GitLabSynchronisationWarning'; @@ -53,17 +53,9 @@ interface ChangesForm { provisioningType?: GitLabConfigurationUpdateBody['provisioningType']; allowUsersToSignUp?: GitLabConfigurationUpdateBody['allowUsersToSignUp']; provisioningToken?: GitLabConfigurationUpdateBody['provisioningToken']; - provisioningGroups?: GitLabConfigurationUpdateBody['provisioningGroups']; } const definitions: Record, DefinitionV2> = { - provisioningGroups: { - name: translate('settings.authentication.gitlab.form.provisioningGroups.name'), - key: 'provisioningGroups', - description: translate('settings.authentication.gitlab.form.provisioningGroups.description'), - secured: false, - multiValues: true, - }, allowUsersToSignUp: { name: translate('settings.authentication.gitlab.form.allowUsersToSignUp.name'), secured: false, @@ -146,7 +138,6 @@ export default function GitLabAuthenticationTab() { setChangesWithCheck({ provisioningType: ProvisioningType.jit, provisioningToken: undefined, - provisioningGroups: undefined, }); const setAuto = () => @@ -159,12 +150,10 @@ export default function GitLabAuthenticationTab() { identityProvider?.provider !== undefined && identityProvider.provider !== Provider.Gitlab; const allowUsersToSignUpDefinition = definitions.allowUsersToSignUp; const provisioningTokenDefinition = definitions.provisioningToken; - const provisioningGroupDefinition = definitions.provisioningGroups; const provisioningType = changes?.provisioningType ?? configuration?.provisioningType; const allowUsersToSignUp = changes?.allowUsersToSignUp ?? configuration?.allowUsersToSignUp; const provisioningToken = changes?.provisioningToken; - const groups = changes?.provisioningGroups ?? configuration?.provisioningGroups; const canSave = () => { if (!configuration || changes === undefined || isUpdating) { @@ -174,13 +163,10 @@ export default function GitLabAuthenticationTab() { if (type === ProvisioningType.auto) { const hasConfigGroups = configuration.provisioningGroups && configuration.provisioningGroups.length > 0; - const hasGroups = changes.provisioningGroups - ? changes.provisioningGroups.length > 0 - : hasConfigGroups; const hasToken = hasConfigGroups ? changes.provisioningToken !== '' : !!changes.provisioningToken; - return hasGroups && hasToken; + return hasToken; } return true; }; @@ -196,9 +182,6 @@ export default function GitLabAuthenticationTab() { ? undefined : newChanges.allowUsersToSignUp, provisioningToken: newChanges.provisioningToken, - provisioningGroups: isEqual(configuration?.provisioningGroups, newChanges.provisioningGroups) - ? undefined - : newChanges.provisioningGroups, }; if (Object.values(newValue).some((v) => v !== undefined)) { setChanges(newValue); @@ -388,21 +371,6 @@ export default function GitLabAuthenticationTab() { provisioningToken: value as string, }) } - isNotSet={ - configuration.provisioningType !== ProvisioningType.auto && - configuration.provisioningGroups?.length === 0 - } - /> - - setChangesWithCheck({ - ...changes, - provisioningGroups: values as string[], - }) - } isNotSet={configuration.provisioningType !== ProvisioningType.auto} /> diff --git a/server/sonar-web/src/main/js/apps/settings/components/authentication/GitLabConfigurationForm.tsx b/server/sonar-web/src/main/js/apps/settings/components/authentication/GitLabConfigurationForm.tsx index 010692097de..ad31e99b2e6 100644 --- a/server/sonar-web/src/main/js/apps/settings/components/authentication/GitLabConfigurationForm.tsx +++ b/server/sonar-web/src/main/js/apps/settings/components/authentication/GitLabConfigurationForm.tsx @@ -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 { keyBy } from 'lodash'; +import { isArray, keyBy } from 'lodash'; import * as React from 'react'; import { FormattedMessage } from 'react-intl'; import DocLink from '../../../../components/common/DocLink'; @@ -46,7 +46,7 @@ interface ErrorValue { } interface FormData { - value: string | boolean; + value: string | boolean | string[]; required: boolean; definition: DefinitionV2; } @@ -104,6 +104,19 @@ export default function GitLabConfigurationForm(props: Readonly) { type: SettingType.BOOLEAN, }, }, + provisioningGroups: { + value: data?.provisioningGroups ?? [], + required: true, + definition: { + name: translate('settings.authentication.gitlab.form.provisioningGroups.name'), + secured: false, + key: 'provisioningGroups', + description: translate( + 'settings.authentication.gitlab.form.provisioningGroups.description', + ), + multiValues: true, + }, + }, }); const headerLabel = translate( @@ -111,9 +124,13 @@ export default function GitLabConfigurationForm(props: Readonly) { isCreate ? 'create' : 'edit', ); - const canBeSaved = Object.values(formData).every( - (v) => (!isCreate && v.definition.secured) || !v.required || v.value !== '', - ); + const canBeSaved = Object.values(formData).every(({ definition, required, value }) => { + return ( + (!isCreate && definition.secured) || + !required || + (isArray(value) ? value.some((val) => val !== '') : value !== '') + ); + }); const handleSubmit = (event: React.SyntheticEvent) => { event.preventDefault(); diff --git a/server/sonar-web/src/main/js/apps/settings/components/authentication/__tests__/Authentication-Gitlab-it.tsx b/server/sonar-web/src/main/js/apps/settings/components/authentication/__tests__/Authentication-Gitlab-it.tsx index 4321afdaec9..bde886d2b13 100644 --- a/server/sonar-web/src/main/js/apps/settings/components/authentication/__tests__/Authentication-Gitlab-it.tsx +++ b/server/sonar-web/src/main/js/apps/settings/components/authentication/__tests__/Authentication-Gitlab-it.tsx @@ -102,9 +102,10 @@ const ui = { autoProvisioningUpdateTokenButton: byRole('button', { name: 'settings.almintegration.form.secret.update_field', }), - autoProvisioningGroupsInput: byRole('textbox', { + groups: byRole('textbox', { name: 'property.provisioningGroups.name', }), + deleteGroupButton: byRole('button', { name: /delete_value/ }), removeProvisioniongGroup: byRole('button', { name: /settings.definition.delete_value.property.provisioningGroups.name./, }), @@ -131,7 +132,7 @@ const ui = { }), }; -it('should create a Gitlab configuration and disable it', async () => { +it('should create a Gitlab configuration and disable it with proper validation', async () => { handler.setGitlabConfigurations([]); renderAuthentication(); const user = userEvent.setup(); @@ -142,8 +143,12 @@ it('should create a Gitlab configuration and disable it', async () => { await user.click(ui.createConfigButton.get()); expect(await ui.createDialog.find()).toBeInTheDocument(); await user.type(ui.applicationId.get(), '123'); + expect(ui.saveConfigButton.get()).toBeDisabled(); await user.type(ui.url.get(), 'https://company.ui.com'); await user.type(ui.secret.get(), '123'); + expect(ui.saveConfigButton.get()).toBeDisabled(); + await user.type(ui.groups.get(), 'NWA'); + expect(ui.saveConfigButton.get()).toBeEnabled(); await user.click(ui.synchronizeGroups.get()); await user.click(ui.saveConfigButton.get()); @@ -157,7 +162,7 @@ it('should create a Gitlab configuration and disable it', async () => { expect(ui.disableConfigButton.query()).not.toBeInTheDocument(); }); -it('should edit/delete configuration', async () => { +it('should edit a configuration with proper validation and delete it', async () => { const user = userEvent.setup(); renderAuthentication(); @@ -172,13 +177,33 @@ it('should edit/delete configuration', async () => { expect(ui.url.get()).toHaveValue('URL'); expect(ui.applicationId.get()).toBeInTheDocument(); expect(ui.secret.query()).not.toBeInTheDocument(); + expect(ui.groups.get()).toHaveValue('Cypress Hill'); expect(ui.synchronizeGroups.get()).toBeChecked(); + + expect(ui.applicationId.get()).toBeInTheDocument(); + await user.clear(ui.applicationId.get()); + expect(ui.saveConfigButton.get()).toBeDisabled(); + await user.type(ui.applicationId.get(), '456'); + expect(ui.saveConfigButton.get()).toBeEnabled(); + + expect(ui.url.get()).toBeInTheDocument(); await user.clear(ui.url.get()); - await user.type(ui.url.get(), 'https://company.ui.com'); + expect(ui.saveConfigButton.get()).toBeDisabled(); + await user.type(ui.url.get(), 'www.internet.com'); + expect(ui.saveConfigButton.get()).toBeEnabled(); + + expect(ui.groups.get()).toHaveValue('Cypress Hill'); + await user.click(ui.groups.get()); + await user.click(ui.deleteGroupButton.get()); + expect(ui.groups.get()).not.toHaveValue('Cypress Hill'); + expect(ui.saveConfigButton.get()).toBeDisabled(); + await user.click(ui.groups.get()); + await user.type(ui.groups.get(), 'Run DMC'); + expect(ui.saveConfigButton.get()).toBeEnabled(); await user.click(ui.saveConfigButton.get()); expect(glContainer.get()).not.toHaveTextContent('URL'); - expect(glContainer.get()).toHaveTextContent('https://company.ui.com'); + expect(glContainer.get()).toHaveTextContent('www.internet.com'); expect(ui.disableConfigButton.get()).toBeInTheDocument(); await user.click(ui.disableConfigButton.get()); @@ -189,43 +214,6 @@ it('should edit/delete configuration', async () => { expect(ui.editConfigButton.query()).not.toBeInTheDocument(); }); -it('should change from just-in-time to Auto Provisioning with proper validation', async () => { - const user = userEvent.setup(); - renderAuthentication([Feature.GitlabProvisioning]); - - expect(await ui.editConfigButton.find()).toBeInTheDocument(); - expect(ui.jitProvisioningRadioButton.get()).toBeChecked(); - - user.click(ui.autoProvisioningRadioButton.get()); - expect(await ui.autoProvisioningRadioButton.find()).toBeEnabled(); - expect(ui.saveProvisioning.get()).toBeDisabled(); - - await user.type(ui.autoProvisioningToken.get(), 'JRR Tolkien'); - expect(await ui.saveProvisioning.find()).toBeDisabled(); - - await user.type(ui.autoProvisioningGroupsInput.get(), 'NWA'); - user.click(ui.autoProvisioningRadioButton.get()); - expect(await ui.saveProvisioning.find()).toBeEnabled(); - - await user.click(ui.removeProvisioniongGroup.get()); - expect(await ui.saveProvisioning.find()).toBeDisabled(); - await user.type(ui.autoProvisioningGroupsInput.get(), 'Wu-Tang Clan'); - expect(await ui.saveProvisioning.find()).toBeEnabled(); - - await user.clear(ui.autoProvisioningToken.get()); - expect(await ui.saveProvisioning.find()).toBeDisabled(); - await user.type(ui.autoProvisioningToken.get(), 'tiktoken'); - expect(await ui.saveProvisioning.find()).toBeEnabled(); - - await user.click(ui.saveProvisioning.get()); - expect(ui.confirmAutoProvisioningDialog.get()).toBeInTheDocument(); - await user.click(ui.confirmProvisioningChange.get()); - expect(ui.confirmAutoProvisioningDialog.query()).not.toBeInTheDocument(); - - expect(ui.autoProvisioningRadioButton.get()).toBeChecked(); - expect(await ui.saveProvisioning.find()).toBeDisabled(); -}); - it('should change from auto provisioning to JIT with proper validation', async () => { handler.setGitlabConfigurations([ mockGitlabConfiguration({ @@ -242,7 +230,6 @@ it('should change from auto provisioning to JIT with proper validation', async ( expect(ui.jitProvisioningRadioButton.get()).not.toBeChecked(); expect(ui.autoProvisioningRadioButton.get()).toBeChecked(); - expect(ui.autoProvisioningGroupsInput.get()).toHaveValue('D12'); expect(ui.autoProvisioningToken.query()).not.toBeInTheDocument(); expect(ui.autoProvisioningUpdateTokenButton.get()).toBeInTheDocument(); @@ -295,7 +282,7 @@ it('should be able to allow user to sign up for JIT with proper validation', asy expect(await ui.saveProvisioning.find()).toBeDisabled(); }); -it('should be able to edit groups and token for Auto provisioning with proper validation', async () => { +it('should be able to edit token for Auto provisioning with proper validation', async () => { handler.setGitlabConfigurations([ mockGitlabConfiguration({ allowUsersToSignUp: false, @@ -309,39 +296,13 @@ it('should be able to edit groups and token for Auto provisioning with proper va expect(await ui.autoProvisioningRadioButton.find()).toBeChecked(); expect(ui.autoProvisioningUpdateTokenButton.get()).toBeInTheDocument(); - expect(ui.autoProvisioningGroupsInput.get()).toHaveValue('Cypress Hill'); expect(ui.saveProvisioning.get()).toBeDisabled(); // Changing the Provisioning token should enable save await user.click(ui.autoProvisioningUpdateTokenButton.get()); - await user.type(ui.autoProvisioningGroupsInput.get(), 'Tok Token!'); - expect(ui.saveProvisioning.get()).toBeEnabled(); - await user.click(ui.cancelProvisioningChanges.get()); - expect(ui.saveProvisioning.get()).toBeDisabled(); - - // Adding a group should enable save - await user.click(ui.autoProvisioningGroupsInput.get()); - await user.tab(); - await user.tab(); - await user.tab(); - await user.tab(); - await user.keyboard('Run DMC'); - expect(ui.saveProvisioning.get()).toBeEnabled(); - await user.tab(); - await user.keyboard('{Enter}'); expect(ui.saveProvisioning.get()).toBeDisabled(); - - // Removing a group should enable save - await user.click(ui.autoProvisioningGroupsInput.get()); - await user.tab(); - await user.keyboard('{Enter}'); - expect(ui.saveProvisioning.get()).toBeEnabled(); - - // Removing all groups should disable save - await user.click(ui.autoProvisioningGroupsInput.get()); - await user.tab(); - await user.keyboard('{Enter}'); + await user.click(ui.cancelProvisioningChanges.get()); expect(ui.saveProvisioning.get()).toBeDisabled(); }); @@ -359,22 +320,11 @@ it('should be able to reset Auto Provisioning changes', async () => { expect(await ui.autoProvisioningRadioButton.find()).toBeChecked(); - // Cancel doesn't fully work yet as the AuthenticationFormField needs to be worked on - await user.click(ui.autoProvisioningGroupsInput.get()); - await user.tab(); - await user.tab(); - await user.tab(); - await user.tab(); - await user.keyboard('A Tribe Called Quest'); - await user.click(ui.autoProvisioningGroupsInput.get()); - await user.tab(); - await user.keyboard('{Enter}'); await user.click(ui.autoProvisioningUpdateTokenButton.get()); - await user.type(ui.autoProvisioningGroupsInput.get(), 'ToToken!'); + await user.type(ui.autoProvisioningToken.get(), 'ToToken!'); expect(ui.saveProvisioning.get()).toBeEnabled(); await user.click(ui.cancelProvisioningChanges.get()); - // expect(ui.autoProvisioningUpdateTokenButton.get()).toBeInTheDocument(); - expect(ui.autoProvisioningGroupsInput.get()).toHaveValue('Cypress Hill'); + expect(ui.saveProvisioning.get()).toBeDisabled(); }); describe('Gitlab Provisioning', () => { diff --git a/server/sonar-web/src/main/js/helpers/mocks/alm-integrations.ts b/server/sonar-web/src/main/js/helpers/mocks/alm-integrations.ts index 9fa50a414b3..fb7cec13aac 100644 --- a/server/sonar-web/src/main/js/helpers/mocks/alm-integrations.ts +++ b/server/sonar-web/src/main/js/helpers/mocks/alm-integrations.ts @@ -111,7 +111,7 @@ export function mockGitlabConfiguration( allowUsersToSignUp: false, synchronizeGroups: true, provisioningType: ProvisioningType.jit, - provisioningGroups: [], + provisioningGroups: ['Cypress Hill'], ...overrides, }; } diff --git a/server/sonar-web/src/main/js/types/provisioning.ts b/server/sonar-web/src/main/js/types/provisioning.ts index 1eb20ec4e9a..72b1fe11a6f 100644 --- a/server/sonar-web/src/main/js/types/provisioning.ts +++ b/server/sonar-web/src/main/js/types/provisioning.ts @@ -97,6 +97,7 @@ export interface GitLabConfigurationCreateBody { url: string; secret: string; synchronizeGroups: boolean; + provisioningGroups: string[]; } export type GitLabConfigurationUpdateBody = { -- 2.39.5