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

…2601) (#3071) Issue: AAH-1845 (cherry picked from commit 77ecd51) (manual resolution of group field differences) Co-authored-by: MilanPospisil <arkanus@seznam.cz>
ansible · Jan 2, 2023 · 5646664 · 5646664
1 parent 34060e4
commit 5646664
Show file tree

Hide file tree

Showing 6 changed files with 240 additions and 176 deletions.
diff --git a/CHANGES/1845.bug 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
@@ -15,12 +15,6 @@ import {
 } from '@patternfly/react-core';
 import { TagIcon } from '@patternfly/react-icons';
 import { isEqual, isEmpty, xorWith, cloneDeep } from 'lodash';
-import {
-  AlertType,
-  APISearchTypeAhead,
-  ObjectPermissionField,
-  HelperText,
-} from 'src/components';
 import {
   ContainerDistributionAPI,
   GroupObjectPermissionType,
@@ -29,7 +23,22 @@ import {
   ExecutionEnvironmentRemoteAPI,
 } from 'src/api';
 import { Constants } from 'src/constants';
-import { errorMessage } from 'src/utilities';
+import {
+  APISearchTypeAhead,
+  AlertList,
+  AlertType,
+  HelperText,
+  ObjectPermissionField,
+  closeAlertMixin,
+} from 'src/components';
+import {
+  ErrorMessagesType,
+  alertErrorsWithoutFields,
+  errorMessage,
+  isFieldValid,
+  isFormValid,
+  mapErrorMessages,
+} from 'src/utilities';
 
 interface IProps {
   name: string;
@@ -49,26 +58,23 @@ interface IProps {
   upstreamName?: string;
   remotePulpId?: string;
   addAlert?: (variant, title, description?) => void;
-  formError: { title: string; detail: string }[];
 }
 
 interface IState {
   name: string;
   description: string;
   selectedGroups: GroupObjectPermissionType[];
   originalSelectedGroups: GroupObjectPermissionType[];
-
+  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;
-    groups?: AlertType;
-  };
+  formErrors: ErrorMessagesType;
+  groupError?: AlertType;
 }
 
 export class RepositoryForm extends React.Component<IProps, IState> {
@@ -87,10 +93,9 @@ export class RepositoryForm extends React.Component<IProps, IState> {
       registries: null,
       registrySelection: [],
       upstreamName: this.props.upstreamName || '',
-      formErrors: {
-        registries: null,
-        groups: null,
-      },
+      formErrors: {},
+      alerts: [],
+      groupError: null,
     };
   }
 
@@ -109,15 +114,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 },
           });
         });
     }
@@ -128,8 +132,7 @@ export class RepositoryForm extends React.Component<IProps, IState> {
   }
 
   render() {
-    const { onSave, onCancel, namespace, isNew, isRemote, formError } =
-      this.props;
+    const { onSave, onCancel, namespace, isNew, isRemote } = this.props;
     const {
       name,
       description,
@@ -141,6 +144,7 @@ export class RepositoryForm extends React.Component<IProps, IState> {
       addTagsInclude,
       addTagsExclude,
       formErrors,
+      groupError,
     } = this.state;
 
     return (
@@ -165,6 +169,10 @@ export class RepositoryForm extends React.Component<IProps, IState> {
           </Button>,
         ]}
       >
+        <AlertList
+          alerts={this.state.alerts}
+          closeAlert={(i) => this.closeAlert(i)}
+        />
         <Form>
           {!isRemote ? (
             <>
@@ -197,15 +205,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>
 
@@ -233,16 +244,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
@@ -253,6 +264,7 @@ export class RepositoryForm extends React.Component<IProps, IState> {
                             registrySelection: registries.filter(
                               ({ name }) => name === value,
                             ),
+                            formErrors: { ...formErrors, registry: null },
                           })
                         }
                         placeholderText={t`Select a registry`}
@@ -381,9 +393,9 @@ export class RepositoryForm extends React.Component<IProps, IState> {
               autoResize={true}
             />
           </FormGroup>
-          {formErrors?.groups ? (
-            <Alert title={formErrors.groups.title} variant='danger' isInline>
-              {formErrors.groups.description}
+          {groupError ? (
+            <Alert title={groupError.title} variant='danger' isInline>
+              {groupError.description}
             </Alert>
           ) : (
             <FormGroup
@@ -409,29 +421,14 @@ export class RepositoryForm extends React.Component<IProps, IState> {
                 onError={(err) => {
                   const { status, statusText } = err.response;
                   this.setState({
-                    formErrors: {
-                      ...this.state.formErrors,
-                      groups: {
-                        title: t`Groups list could not be displayed.`,
-                        description: errorMessage(status, statusText),
-                        variant: 'danger',
-                      },
+                    groupError: {
+                      title: t`Groups list could not be displayed.`,
+                      description: errorMessage(status, statusText),
+                      variant: 'danger',
                     },
                   });
                 }}
               ></ObjectPermissionField>
-              {!!formError &&
-                formError.length > 0 &&
-                formError.map((error) => (
-                  <Alert
-                    title={error.title}
-                    variant='danger'
-                    isInline
-                    key={error.title}
-                  >
-                    {error.detail}
-                  </Alert>
-                ))}
             </FormGroup>
           )}
         </Form>
@@ -442,9 +439,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 } });
     }
   }
 
@@ -454,8 +453,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?) {
@@ -480,13 +484,10 @@ export class RepositoryForm extends React.Component<IProps, IState> {
       .catch((e) => {
         const { status, statusText } = e.response;
         this.setState({
-          formErrors: {
-            ...this.state.formErrors,
-            groups: {
-              title: t`Groups list could not be displayed.`,
-              description: errorMessage(status, statusText),
-              variant: 'danger',
-            },
+          groupError: {
+            title: t`Groups list could not be displayed.`,
+            description: errorMessage(status, statusText),
+            variant: 'danger',
           },
         });
       });
@@ -534,39 +535,62 @@ 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 }),
+        // remote edit or local edit - groups, if changed
+        !this.compareGroupsAndPerms(
+          selectedGroups.sort(),
+          originalSelectedGroups.sort(),
+        ) &&
+          ExecutionEnvironmentNamespaceAPI.update(namespace, {
+            groups: selectedGroups,
+          }),
+      ]);
     }
 
-    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 }),
-      // remote edit or local edit - groups, if changed
-      !this.compareGroupsAndPerms(
-        selectedGroups.sort(),
-        originalSelectedGroups.sort(),
-      ) &&
-        ExecutionEnvironmentNamespaceAPI.update(namespace, {
-          groups: selectedGroups,
-        }),
-    ]);
+    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');
   }
 
   //Compare groups and compare their permissions