Skip to content

Commit

Permalink
Error messages aren't displaying on the remote container create form (a…
Browse files Browse the repository at this point in the history
…nsible#2601)

Issue: AAH-1845

(cherry picked from commit 77ecd51)

(manual resolution of remoteId vs remotePulpId)
  • Loading branch information
MilanPospisil authored and himdel committed Dec 23, 2022
1 parent 9d910d0 commit e0c3391
Show file tree
Hide file tree
Showing 6 changed files with 216 additions and 130 deletions.
1 change: 1 addition & 0 deletions CHANGES/1845.bug
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Repair error mesages in EE form.
148 changes: 97 additions & 51 deletions src/components/execution-environment/repository-form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -42,23 +55,20 @@ 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[];
includeTags?: string[];
registries?: { id: string; name: string }[];
registrySelection?: { id: string; name: string }[];
upstreamName: string;
formErrors?: {
registries?: AlertType;
};
formErrors: ErrorMessagesType;
}

export class RepositoryForm extends React.Component<IProps, IState> {
Expand All @@ -75,9 +85,8 @@ export class RepositoryForm extends React.Component<IProps, IState> {
registries: null,
registrySelection: [],
upstreamName: this.props.upstreamName || '',
formErrors: {
registries: null,
},
formErrors: {},
alerts: [],
};
}

Expand All @@ -96,15 +105,14 @@ export class RepositoryForm extends React.Component<IProps, IState> {
})
.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 },
});
});
}
Expand Down Expand Up @@ -147,6 +155,10 @@ export class RepositoryForm extends React.Component<IProps, IState> {
</Button>,
]}
>
<AlertList
alerts={this.state.alerts}
closeAlert={(i) => this.closeAlert(i)}
/>
<Form>
{!isRemote ? (
<>
Expand Down Expand Up @@ -179,15 +191,18 @@ export class RepositoryForm extends React.Component<IProps, IState> {
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')}
>
<TextInput
id='name'
value={name}
isDisabled={!isNew}
onChange={(value) => this.setState({ name: value })}
validated={this.validateName(name)}
onChange={(value) => {
this.setState({ name: value });
this.validateName(value);
}}
validated={isFieldValid(this.state.formErrors, 'name')}
/>
</FormGroup>

Expand Down Expand Up @@ -215,16 +230,16 @@ export class RepositoryForm extends React.Component<IProps, IState> {
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 ? (
<Alert
title={formErrors.registries.title}
variant='danger'
isInline
>
{formErrors.registries.description}
</Alert>
) : (
{!formErrors?.registries && (
<>
{registries ? (
<APISearchTypeAhead
Expand All @@ -235,6 +250,7 @@ export class RepositoryForm extends React.Component<IProps, IState> {
registrySelection: registries.filter(
({ name }) => name === value,
),
formErrors: { ...formErrors, registry: null },
})
}
placeholderText={t`Select a registry`}
Expand Down Expand Up @@ -390,9 +406,11 @@ export class RepositoryForm extends React.Component<IProps, IState> {
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 } });
}
}

Expand All @@ -402,8 +420,13 @@ export class RepositoryForm extends React.Component<IProps, IState> {
// 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?) {
Expand Down Expand Up @@ -455,30 +478,53 @@ export class RepositoryForm extends React.Component<IProps, IState> {
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');
}
}
63 changes: 24 additions & 39 deletions src/containers/execution-environment-detail/base.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ interface IState {
editing: boolean;
alerts: AlertType[];
showDeleteModal: boolean;
formError: { title: string; detail: string }[];
}

export interface IDetailSharedProps extends RouteComponentProps {
Expand All @@ -64,7 +63,6 @@ export function withContainerRepo(WrappedComponent) {
editing: false,
alerts: [],
showDeleteModal: false,
formError: [],
};
}

Expand Down Expand Up @@ -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: (
<Trans>
Saved changes to execution environment &quot;
{this.state.repo.name}&quot;.
</Trans>
),
}),
});
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: (
<Trans>
Saved changes to execution environment &quot;
{this.state.repo.name}&quot;.
</Trans>
),
}),
);
});
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}
Expand Down
Loading

0 comments on commit e0c3391

Please sign in to comment.