Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Error messages aren't displaying on the remote container create form #2601

Merged
merged 2 commits into from
Dec 14, 2022
Merged

Error messages aren't displaying on the remote container create form #2601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
010ddfc
WIP
MilanPospisil Sep 20, 2022
c3b3e00
Repair forms validation in EE form.
MilanPospisil Sep 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES/1845.bug
View file
Open in desktop
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
View file
Open in desktop
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;
remoteId?: 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);
himdel marked this conversation as resolved.
Show resolved Hide resolved
}}
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(remoteId, {
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(remoteId, {
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
View file
Open in desktop
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.id}
Expand Down
Loading