From: Revanshu Paliwal Date: Wed, 14 Sep 2022 14:10:05 +0000 (+0200) Subject: SONAR-17295 Improving user experience for SAML authentication page X-Git-Tag: 9.7.0.61563~234 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=97620ecea145ecd5f0494caa718b759b5537c19a;p=sonarqube.git SONAR-17295 Improving user experience for SAML authentication page --- diff --git a/server/sonar-web/src/main/js/api/mocks/AuthenticationServiceMock.ts b/server/sonar-web/src/main/js/api/mocks/AuthenticationServiceMock.ts index c0b5b8a8bcc..17dda673074 100644 --- a/server/sonar-web/src/main/js/api/mocks/AuthenticationServiceMock.ts +++ b/server/sonar-web/src/main/js/api/mocks/AuthenticationServiceMock.ts @@ -49,6 +49,14 @@ export default class AuthenticationServiceMock { }; setValueHandler = (definition: SettingDefinition, value: string) => { + if (value === 'error') { + const res = new Response('', { + status: 400, + statusText: 'fail' + }); + + return Promise.reject(res); + } const updatedSettingValue = this.settingValues.find(set => set.key === definition.key); if (updatedSettingValue) { updatedSettingValue.value = value; @@ -58,16 +66,14 @@ export default class AuthenticationServiceMock { resetValueHandler = (data: { keys: string; component?: string } & BranchParameters) => { if (data.keys) { - return Promise.resolve( - this.settingValues.map(set => { - if (data.keys.includes(set.key)) { - set.value = ''; - } - return set; - }) - ); + this.settingValues.forEach(set => { + if (data.keys.includes(set.key)) { + set.value = ''; + } + return set; + }); } - return Promise.resolve(this.settingValues); + return Promise.resolve(); }; resetValues = () => { diff --git a/server/sonar-web/src/main/js/apps/settings/components/authentication/SamlAuthentication.tsx b/server/sonar-web/src/main/js/apps/settings/components/authentication/SamlAuthentication.tsx index e92b8333c06..05a8a9b44bf 100644 --- a/server/sonar-web/src/main/js/apps/settings/components/authentication/SamlAuthentication.tsx +++ b/server/sonar-web/src/main/js/apps/settings/components/authentication/SamlAuthentication.tsx @@ -23,10 +23,13 @@ import React from 'react'; import { getValues, resetSettingValue, setSettingValue } from '../../../../api/settings'; import { SubmitButton } from '../../../../components/controls/buttons'; import Tooltip from '../../../../components/controls/Tooltip'; +import { Location, withRouter } from '../../../../components/hoc/withRouter'; +import AlertSuccessIcon from '../../../../components/icons/AlertSuccessIcon'; +import AlertWarnIcon from '../../../../components/icons/AlertWarnIcon'; import DetachIcon from '../../../../components/icons/DetachIcon'; import DeferredSpinner from '../../../../components/ui/DeferredSpinner'; -import { translate } from '../../../../helpers/l10n'; -import { parseError } from '../../../../helpers/request'; +import { translate, translateWithParameters } from '../../../../helpers/l10n'; +import { isSuccessStatus, parseError } from '../../../../helpers/request'; import { getBaseUrl } from '../../../../helpers/system'; import { ExtendedSettingDefinition, SettingType, SettingValue } from '../../../../types/settings'; import SamlFormField from './SamlFormField'; @@ -34,6 +37,7 @@ import SamlToggleField from './SamlToggleField'; interface SamlAuthenticationProps { definitions: ExtendedSettingDefinition[]; + location: Location; } interface SamlAuthenticationState { @@ -42,6 +46,7 @@ interface SamlAuthenticationState { dirtyFields: string[]; securedFieldsSubmitted: string[]; error: { [key: string]: string }; + success?: boolean; } const CONFIG_TEST_PATH = '/api/saml/validation_init'; @@ -60,6 +65,8 @@ class SamlAuthentication extends React.PureComponent< SamlAuthenticationProps, SamlAuthenticationState > { + formFieldRef: React.RefObject = React.createRef(); + constructor(props: SamlAuthenticationProps) { super(props); const settingValue = props.definitions.map(def => { @@ -83,6 +90,17 @@ class SamlAuthentication extends React.PureComponent< this.loadSettingValues(keys); } + componentDidUpdate(prevProps: SamlAuthenticationProps) { + const { location } = this.props; + if (this.formFieldRef.current && prevProps.location.hash !== location.hash) { + this.formFieldRef.current.scrollIntoView({ + behavior: 'smooth', + block: 'center', + inline: 'nearest' + }); + } + } + onFieldChange = (id: string, value: string | boolean) => { const { settingValue, dirtyFields } = this.state; const updatedSettingValue = settingValue?.map(set => { @@ -151,52 +169,50 @@ class SamlAuthentication extends React.PureComponent< return; } - this.setState({ submitting: true, error: {} }); + this.setState({ submitting: true, error: {}, success: false }); const promises: Promise[] = []; - settingValue?.forEach(set => { - const definition = definitions.find(def => def.key === set.key); - if (definition && set.value !== undefined && dirtyFields.includes(set.key)) { + dirtyFields.forEach(field => { + const definition = definitions.find(def => def.key === field); + const value = settingValue.find(def => def.key === field)?.value; + if (definition && value !== undefined) { const apiCall = - set.value.length > 0 - ? setSettingValue(definition, set.value) + value.length > 0 + ? setSettingValue(definition, value) : resetSettingValue({ keys: definition.key }); - const promise = apiCall.catch(async e => { + + promises.push(apiCall); + } + }); + + await Promise.all(promises.map(p => p.catch(e => e))).then(data => { + const dataWithError = data + .map((data, index) => ({ data, index })) + .filter(d => d.data !== undefined && !isSuccessStatus(d.data.status)); + if (dataWithError.length > 0) { + dataWithError.forEach(async d => { + const validationMessage = await parseError(d.data as Response); const { error } = this.state; - const validationMessage = await parseError(e as Response); this.setState({ - submitting: false, - dirtyFields: [], - error: { ...error, ...{ [set.key]: validationMessage } } + error: { ...error, ...{ [dirtyFields[d.index]]: validationMessage } } }); }); - promises.push(promise); } + this.setState({ success: dirtyFields.length !== dataWithError.length }); }); - await Promise.all(promises); await this.loadSettingValues(dirtyFields.join(',')); - this.setState({ submitting: false, dirtyFields: [] }); }; allowEnabling = () => { - const { settingValue, securedFieldsSubmitted } = this.state; + const { settingValue } = this.state; const enabledFlagSettingValue = settingValue.find(set => set.key === SAML_ENABLED_FIELD); + if (enabledFlagSettingValue && enabledFlagSettingValue.value === 'true') { return true; } - for (const setting of settingValue) { - const isMandatory = !OPTIONAL_FIELDS.includes(setting.key); - const isSecured = this.isSecuredField(setting.key); - const isSecuredAndNotSubmitted = isSecured && !securedFieldsSubmitted.includes(setting.key); - const isNotSecuredAndNotSubmitted = - !isSecured && (setting.value === '' || setting.value === undefined); - if (isMandatory && (isSecuredAndNotSubmitted || isNotSecuredAndNotSubmitted)) { - return false; - } - } - return true; + return this.getEmptyRequiredFields().length === 0; }; onEnableFlagChange = (value: boolean) => { @@ -232,13 +248,43 @@ class SamlAuthentication extends React.PureComponent< return null; }; + getEmptyRequiredFields = () => { + const { settingValue, securedFieldsSubmitted } = this.state; + const { definitions } = this.props; + + const updatedRequiredFields: string[] = []; + + for (const setting of settingValue) { + const isMandatory = !OPTIONAL_FIELDS.includes(setting.key); + const isSecured = this.isSecuredField(setting.key); + const isSecuredAndNotSubmitted = isSecured && !securedFieldsSubmitted.includes(setting.key); + const isNotSecuredAndNotSubmitted = + !isSecured && (setting.value === '' || setting.value === undefined); + if (isMandatory && (isSecuredAndNotSubmitted || isNotSecuredAndNotSubmitted)) { + const settingDef = definitions.find(def => def.key === setting.key); + + if (settingDef && settingDef.name) { + updatedRequiredFields.push(settingDef.name); + } + } + } + return updatedRequiredFields; + }; + render() { const { definitions } = this.props; - const { submitting, settingValue, securedFieldsSubmitted, error, dirtyFields } = this.state; + const { + submitting, + settingValue, + securedFieldsSubmitted, + error, + dirtyFields, + success + } = this.state; const enabledFlagDefinition = definitions.find(def => def.key === SAML_ENABLED_FIELD); const formIsIncomplete = !this.allowEnabling(); - const preventTestingConfig = formIsIncomplete || dirtyFields.length > 0; + const preventTestingConfig = this.getEmptyRequiredFields().length > 0 || dirtyFields.length > 0; return (
@@ -247,32 +293,71 @@ class SamlAuthentication extends React.PureComponent< return null; } return ( - set.key === def.key)} - definition={def} - mandatory={!OPTIONAL_FIELDS.includes(def.key)} - onFieldChange={this.onFieldChange} - showSecuredTextArea={ - !securedFieldsSubmitted.includes(def.key) || dirtyFields.includes(def.key) - } - error={error} +
+ ref={this.props.location.hash.substring(1) === def.key ? this.formFieldRef : null}> + set.key === def.key)} + definition={def} + mandatory={!OPTIONAL_FIELDS.includes(def.key)} + onFieldChange={this.onFieldChange} + showSecuredTextArea={ + !securedFieldsSubmitted.includes(def.key) || dirtyFields.includes(def.key) + } + error={error} + /> +
); })} -
+
{enabledFlagDefinition && ( -
- - set.key === enabledFlagDefinition.key)} - toggleDisabled={formIsIncomplete} - onChange={this.onEnableFlagChange} - /> -
+ +
+ + set.key === enabledFlagDefinition.key)} + toggleDisabled={formIsIncomplete} + onChange={this.onEnableFlagChange} + /> +
+ )} -
+
+ {success && ( +
+ 0 + ? translateWithParameters( + 'settings.authentication.saml.form.save_warn', + Object.keys(error).length + ) + : null + }> + {Object.keys(error).length > 0 ? ( + + + {translate('settings.authentication.saml.form.save_partial')} + + ) : ( + + + {translate('settings.authentication.saml.form.save_success')} + + )} + {} + +
+ )} {translate('settings.authentication.saml.form.save')} @@ -298,4 +383,4 @@ class SamlAuthentication extends React.PureComponent< } } -export default SamlAuthentication; +export default withRouter(SamlAuthentication); diff --git a/server/sonar-web/src/main/js/apps/settings/components/authentication/SamlSecuredField.tsx b/server/sonar-web/src/main/js/apps/settings/components/authentication/SamlSecuredField.tsx index 4cc6353b001..b78ca353f74 100644 --- a/server/sonar-web/src/main/js/apps/settings/components/authentication/SamlSecuredField.tsx +++ b/server/sonar-web/src/main/js/apps/settings/components/authentication/SamlSecuredField.tsx @@ -44,7 +44,7 @@ export default function SamlSecuredField(props: SamlToggleFieldProps) {