diff --git a/CHANGES/1845.bug b/CHANGES/1845.bug new file mode 100644 index 0000000000..027c4c816b --- /dev/null +++ b/CHANGES/1845.bug @@ -0,0 +1 @@ +Repair error mesages in EE form. diff --git a/src/components/execution-environment/repository-form.tsx b/src/components/execution-environment/repository-form.tsx index 8a41b057be..4e5e167876 100644 --- a/src/components/execution-environment/repository-form.tsx +++ b/src/components/execution-environment/repository-form.tsx @@ -15,14 +15,27 @@ import { } from '@patternfly/react-core'; import { ExternalLinkAltIcon, TagIcon } from '@patternfly/react-icons'; import { Link } from 'react-router-dom'; -import { AlertType, APISearchTypeAhead, HelperText } from 'src/components'; +import { + AlertType, + APISearchTypeAhead, + HelperText, + AlertList, + closeAlertMixin, +} from 'src/components'; import { ContainerDistributionAPI, ExecutionEnvironmentRegistryAPI, ExecutionEnvironmentRemoteAPI, } from 'src/api'; import { Paths, formatPath } from 'src/paths'; -import { errorMessage } from 'src/utilities'; +import { + errorMessage, + ErrorMessagesType, + isFieldValid, + isFormValid, + mapErrorMessages, + alertErrorsWithoutFields, +} from 'src/utilities'; interface IProps { name: string; @@ -42,13 +55,12 @@ interface IProps { upstreamName?: string; remotePulpId?: string; addAlert?: (variant, title, description?) => void; - formError: { title: string; detail: string }[]; } interface IState { name: string; description: string; - + alerts: AlertType[]; addTagsInclude: string; addTagsExclude: string; excludeTags?: string[]; @@ -56,9 +68,7 @@ interface IState { registries?: { id: string; name: string }[]; registrySelection?: { id: string; name: string }[]; upstreamName: string; - formErrors?: { - registries?: AlertType; - }; + formErrors: ErrorMessagesType; } export class RepositoryForm extends React.Component { @@ -75,9 +85,8 @@ export class RepositoryForm extends React.Component { registries: null, registrySelection: [], upstreamName: this.props.upstreamName || '', - formErrors: { - registries: null, - }, + formErrors: {}, + alerts: [], }; } @@ -96,15 +105,14 @@ export class RepositoryForm extends React.Component { }) .catch((e) => { const { status, statusText } = e.response; + const errorTitle = t`Registries list could not be displayed.`; + this.addAlert({ + variant: 'danger', + title: errorTitle, + description: errorMessage(status, statusText), + }); this.setState({ - formErrors: { - ...this.state.formErrors, - registries: { - title: t`Registries list could not be displayed.`, - description: errorMessage(status, statusText), - variant: 'danger', - }, - }, + formErrors: { ...this.state.formErrors, registries: errorTitle }, }); }); } @@ -147,6 +155,10 @@ export class RepositoryForm extends React.Component { , ]} > + this.closeAlert(i)} + />
{!isRemote ? ( <> @@ -179,15 +191,18 @@ export class RepositoryForm extends React.Component { key='name' fieldId='name' label={t`Name`} - helperTextInvalid={t`Container names can only contain alphanumeric characters, ".", "_", "-" and a up to one "/".`} - validated={this.validateName(name)} + helperTextInvalid={this.state.formErrors['name']} + validated={isFieldValid(this.state.formErrors, 'name')} > this.setState({ name: value })} - validated={this.validateName(name)} + onChange={(value) => { + this.setState({ name: value }); + this.validateName(value); + }} + validated={isFieldValid(this.state.formErrors, 'name')} /> @@ -215,16 +230,16 @@ export class RepositoryForm extends React.Component { label={t`Registry`} className='hub-formgroup-registry' isRequired={true} + helperTextInvalid={ + this.state.formErrors['registries'] || + this.state.formErrors['registry'] + } + validated={isFieldValid(this.state.formErrors, [ + 'registries', + 'registry', + ])} > - {formErrors?.registries ? ( - - {formErrors.registries.description} - - ) : ( + {!formErrors?.registries && ( <> {registries ? ( { registrySelection: registries.filter( ({ name }) => name === value, ), + formErrors: { ...formErrors, registry: null }, }) } placeholderText={t`Select a registry`} @@ -390,9 +406,11 @@ export class RepositoryForm extends React.Component { private validateName(name) { const regex = /^([0-9A-Za-z._-]+\/)?[0-9A-Za-z._-]+$/; if (name === '' || regex.test(name)) { - return 'default'; + this.setState({ formErrors: { ...this.state.formErrors, name: null } }); + return; } else { - return 'error'; + const error = t`Container names can only contain alphanumeric characters, ".", "_", "-" and a up to one "/".`; + this.setState({ formErrors: { ...this.state.formErrors, name: error } }); } } @@ -402,8 +420,13 @@ export class RepositoryForm extends React.Component { // no validation for local return true; } - const nameValid = name && this.validateName(name) === 'default'; - return nameValid && upstreamName && registrySelection.length; + + if (!isFormValid(this.state.formErrors)) { + return false; + } + + // validation for non empty fields + return name && upstreamName && registrySelection.length; } private loadRegistries(name?) { @@ -455,30 +478,53 @@ export class RepositoryForm extends React.Component { upstreamName: upstream_name, } = this.state; + let promise = null; if (isRemote && isNew) { - return ExecutionEnvironmentRemoteAPI.create({ + promise = ExecutionEnvironmentRemoteAPI.create({ name, upstream_name, registry, include_tags, exclude_tags, }); + } else { + promise = Promise.all([ + // remote edit - upstream, tags, registry + isRemote && + !isNew && + ExecutionEnvironmentRemoteAPI.update(remotePulpId, { + name: originalName, // readonly but required + upstream_name, + registry, + include_tags, + exclude_tags, + }), + // remote edit or local edit - description, if changed + description !== originalDescription && + ContainerDistributionAPI.patch(distributionPulpId, { description }), + ]); } - return Promise.all([ - // remote edit - upstream, tags, registry - isRemote && - !isNew && - ExecutionEnvironmentRemoteAPI.update(remotePulpId, { - name: originalName, // readonly but required - upstream_name, - registry, - include_tags, - exclude_tags, - }), - // remote edit or local edit - description, if changed - description !== originalDescription && - ContainerDistributionAPI.patch(distributionPulpId, { description }), - ]); + return promise.catch((e) => { + this.setState({ formErrors: mapErrorMessages(e) }); + alertErrorsWithoutFields( + this.state.formErrors, + ['name', 'registry', 'registries'], + (alert) => this.addAlert(alert), + t`Error when saving registry.`, + (state) => this.setState({ formErrors: state }), + ); + return Promise.reject(new Error(e)); + }); + } + + private addAlert(alert) { + this.setState({ + alerts: [...this.state.alerts, alert], + }); + } + + private get closeAlert() { + return closeAlertMixin('alerts'); } } diff --git a/src/containers/execution-environment-detail/base.tsx b/src/containers/execution-environment-detail/base.tsx index 78c1341052..a5a7f003a2 100644 --- a/src/containers/execution-environment-detail/base.tsx +++ b/src/containers/execution-environment-detail/base.tsx @@ -39,7 +39,6 @@ interface IState { editing: boolean; alerts: AlertType[]; showDeleteModal: boolean; - formError: { title: string; detail: string }[]; } export interface IDetailSharedProps extends RouteComponentProps { @@ -64,7 +63,6 @@ export function withContainerRepo(WrappedComponent) { editing: false, alerts: [], showDeleteModal: false, - formError: [], }; } @@ -223,45 +221,32 @@ export function withContainerRepo(WrappedComponent) { namespace={this.state.repo.namespace.name} description={this.state.repo.description} permissions={permissions} - formError={this.state.formError} onSave={(promise) => { - promise - .then((results) => { - const task = results.find((x) => x.data && x.data.task); - this.setState({ - editing: false, - loading: true, - alerts: alerts.concat({ - variant: 'success', - title: ( - - Saved changes to execution environment " - {this.state.repo.name}". - - ), - }), - }); - if (task) { - waitForTask( - task.data.task.split('tasks/')[1].replace('/', ''), - ).then(() => { - this.loadRepo(); - }); - } else { - this.loadRepo(); - } - }) - .catch((err) => - this.setState({ - formError: err.response.data.errors.map((error) => { - return { - title: error.title, - detail: - error.source.parameter + ': ' + error.detail, - }; - }), + promise.then((results) => { + const task = results.find((x) => x.data && x.data.task); + this.setState({ + editing: false, + loading: true, + alerts: alerts.concat({ + variant: 'success', + title: ( + + Saved changes to execution environment " + {this.state.repo.name}". + + ), }), - ); + }); + if (task) { + waitForTask( + task.data.task.split('tasks/')[1].replace('/', ''), + ).then(() => { + this.loadRepo(); + }); + } else { + this.loadRepo(); + } + }); }} onCancel={() => this.setState({ editing: false })} distributionPulpId={this.state.repo.pulp.distribution.pulp_id} diff --git a/src/containers/execution-environment-list/execution_environment_list.tsx b/src/containers/execution-environment-list/execution_environment_list.tsx index 1216ea0bf9..6b30548ff0 100644 --- a/src/containers/execution-environment-list/execution_environment_list.tsx +++ b/src/containers/execution-environment-list/execution_environment_list.tsx @@ -59,7 +59,6 @@ interface IState { showDeleteModal: boolean; selectedItem: ExecutionEnvironmentType; inputText: string; - formError: { title: string; detail: string }[]; } class ExecutionEnvironmentList extends React.Component< @@ -95,7 +94,6 @@ class ExecutionEnvironmentList extends React.Component< showDeleteModal: false, selectedItem: null, inputText: '', - formError: [], }; } @@ -441,42 +439,30 @@ class ExecutionEnvironmentList extends React.Component< permissions={namespace?.my_permissions || []} remotePulpId={pulp_id} distributionPulpId={distributionPulpId} - formError={this.state.formError} onSave={(promise, form) => { - promise - .then(() => { - this.setState( - { - showRemoteModal: false, - itemToEdit: null, - alerts: alerts.concat({ - variant: 'success', - title: isNew ? ( - - Execution environment "{form.name}" has been - added successfully. - - ) : ( - - Saved changes to execution environment "{form.name} - ". - - ), - }), - }, - () => this.queryEnvironments(), - ); - }) - .catch((err) => { - this.setState({ - formError: err.response.data.errors.map((error) => { - return { - title: error.title, - detail: error.source.parameter + ': ' + error.detail, - }; + promise.then(() => { + this.setState( + { + showRemoteModal: false, + itemToEdit: null, + alerts: alerts.concat({ + variant: 'success', + title: isNew ? ( + + Execution environment "{form.name}" has been + added successfully. + + ) : ( + + Saved changes to execution environment "{form.name} + ". + + ), }), - }); - }); + }, + () => this.queryEnvironments(), + ); + }); }} onCancel={() => this.setState({ @@ -484,9 +470,6 @@ class ExecutionEnvironmentList extends React.Component< itemToEdit: null, }) } - addAlert={(variant, title, description) => - this.addAlert(title, variant, description) - } /> ); } diff --git a/src/utilities/index.ts b/src/utilities/index.ts index b3b1141145..2c3c7263b4 100644 --- a/src/utilities/index.ts +++ b/src/utilities/index.ts @@ -1,7 +1,13 @@ export { convertContentSummaryCounts } from './content-summary'; export { ParamHelper } from './param-helper'; export { sanitizeDocsUrls } from './sanitize-docs-urls'; -export { mapErrorMessages, ErrorMessagesType } from './map-error-messages'; +export { + mapErrorMessages, + ErrorMessagesType, + isFieldValid, + isFormValid, + alertErrorsWithoutFields, +} from './map-error-messages'; export { getRepoUrl, getContainersURL } from './get-repo-url'; export { twoWayMapper } from './two-way-mapper'; export { diff --git a/src/utilities/map-error-messages.ts b/src/utilities/map-error-messages.ts index 9120f0bd3d..f3d15a2330 100644 --- a/src/utilities/map-error-messages.ts +++ b/src/utilities/map-error-messages.ts @@ -26,3 +26,68 @@ export function mapErrorMessages(err): ErrorMessagesType { return messages; } + +export function isFieldValid( + errorMessagesType: ErrorMessagesType, + name, +): 'default' | 'error' { + let names = []; + if (Array.isArray(name)) { + names = name; + } else { + names.push(name); + } + + if (!errorMessagesType) { + return 'default'; + } + + return names.find((n) => errorMessagesType[n]) ? 'error' : 'default'; +} + +export function isFormValid(errorMessages: ErrorMessagesType) { + if (!errorMessages) { + return true; + } + + return !Object.values(errorMessages).find(Boolean); +} + +export function alertErrorsWithoutFields( + errorMessages: ErrorMessagesType, + fields, + addAlert, + title, + setErrorMessages, +) { + if (!errorMessages) { + return; + } + + // select only errors without associated field + const errors = Object.keys(errorMessages) + .filter((field) => !fields.includes(field)) + .map((field) => errorMessages[field]); + + if (errors.length) { + // alert them + addAlert({ + variant: 'danger', + title: title, + description: errors.join('\n'), + }); + + // filter only errors with field, rest will be removed from the state, because they were already alerted + const formErrors = {}; + + Object.keys(errorMessages).forEach((field) => { + if (fields.includes(field)) { + formErrors[field] = errorMessages[field]; + } + }); + + setErrorMessages(formErrors); + } + + return; +}