Merge branch 'fix-the-mysterious-case-of-the-disappearing-form' into 'master'

Fixed form disappearing when name is empty and general code improvements

See merge request gitlab-org/gitlab!53371

ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue

@@ -2,7 +2,8 @@
 import { visitUrl } from '~/lib/utils/url_utility';
 import * as Sentry from '~/sentry/wrapper';
 import createComplianceFrameworkMutation from '../graphql/queries/create_compliance_framework.mutation.graphql';
-import { initialiseFormData, SAVE_ERROR } from '../constants';
+import { SAVE_ERROR } from '../constants';
+import { initialiseFormData } from '../utils';
 import SharedForm from './shared_form.vue';
 import FormStatus from './form_status.vue';
 

ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/edit_form.vue

@@ -5,7 +5,8 @@ import { convertToGraphQLId } from '~/graphql_shared/utils';
 
 import getComplianceFrameworkQuery from '../graphql/queries/get_compliance_framework.query.graphql';
 import updateComplianceFrameworkMutation from '../graphql/queries/update_compliance_framework.mutation.graphql';
-import { initialiseFormData, FETCH_ERROR, SAVE_ERROR } from '../constants';
+import { FETCH_ERROR, SAVE_ERROR } from '../constants';
+import { initialiseFormData } from '../utils';
 import SharedForm from './shared_form.vue';
 import FormStatus from './form_status.vue';
 
@@ -35,7 +36,8 @@ export default {
   },
   data() {
     return {
-      errorMessage: '',
+      initErrorMessage: '',
+      saveErrorMessage: '',
       formData: initialiseFormData(),
       saving: false,
     };
@@ -53,7 +55,7 @@ export default {
         this.formData = this.extractComplianceFramework(data);
       },
       error(error) {
-        this.setError(error, FETCH_ERROR);
+        this.setInitError(error, FETCH_ERROR);
       },
     },
   },
@@ -64,8 +66,13 @@ export default {
     isLoading() {
       return this.$apollo.loading || this.saving;
     },
-    hasFormData() {
-      return Boolean(this.formData?.name);
+    showForm() {
+      return (
+        Object.values(this.formData).filter((d) => d !== null).length > 0 && !this.initErrorMessage
+      );
+    },
+    errorMessage() {
+      return this.initErrorMessage || this.saveErrorMessage;
     },
   },
   methods: {
@@ -73,7 +80,7 @@ export default {
       const complianceFrameworks = data.namespace?.complianceFrameworks?.nodes || [];
 
       if (!complianceFrameworks.length) {
-        this.setError(new Error(FETCH_ERROR), FETCH_ERROR);
+        this.setInitError(new Error(FETCH_ERROR), FETCH_ERROR);
 
         return initialiseFormData();
       }
@@ -86,13 +93,17 @@ export default {
         color,
       };
     },
-    setError(error, userFriendlyText) {
-      this.errorMessage = userFriendlyText;
+    setInitError(error, userFriendlyText) {
+      this.initErrorMessage = userFriendlyText;
+      Sentry.captureException(error);
+    },
+    setSavingError(error, userFriendlyText) {
+      this.saveErrorMessage = userFriendlyText;
       Sentry.captureException(error);
     },
     async onSubmit() {
       this.saving = true;
-      this.errorMessage = '';
+      this.saveErrorMessage = '';
 
       try {
         const { name, description, color } = this.formData;
@@ -113,13 +124,13 @@ export default {
         const [error] = data?.updateComplianceFramework?.errors || [];
 
         if (error) {
-          this.setError(new Error(error), error);
+          this.setSavingError(new Error(error), error);
         } else {
           this.saving = false;
           visitUrl(this.groupEditPath);
         }
       } catch (e) {
-        this.setError(e, SAVE_ERROR);
+        this.setSavingError(e, SAVE_ERROR);
       }
 
       this.saving = false;
@@ -130,7 +141,7 @@ export default {
 <template>
   <form-status :loading="isLoading" :error="errorMessage">
     <shared-form
-      v-if="hasFormData"
+      v-if="showForm"
       :group-edit-path="groupEditPath"
       :name.sync="formData.name"
       :description.sync="formData.description"

ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/shared_form.vue

@@ -103,7 +103,12 @@ export default {
         </gl-sprintf>
       </template>
 
-      <gl-form-input :value="name" data-testid="name-input" @input="$emit('update:name', $event)" />
+      <gl-form-input
+        :value="name"
+        :state="isValidName"
+        data-testid="name-input"
+        @input="$emit('update:name', $event)"
+      />
     </gl-form-group>
 
     <gl-form-group
@@ -114,6 +119,7 @@ export default {
     >
       <gl-form-input
         :value="description"
+        :state="isValidDescription"
         data-testid="description-input"
         @input="$emit('update:description', $event)"
       />

ee/app/assets/javascripts/groups/settings/compliance_frameworks/constants.js

 import { s__ } from '~/locale';
 
-export const initialiseFormData = () => ({
-  name: null,
-  description: null,
-  color: null,
-});
-
 export const FETCH_ERROR = s__(
   'ComplianceFrameworks|Error fetching compliance frameworks data. Please refresh the page',
 );

ee/app/assets/javascripts/groups/settings/compliance_frameworks/utils.js

+export const initialiseFormData = () => ({
+  name: null,
+  description: null,
+  pipelineConfigurationFullPath: null,
+  color: null,
+});

ee/spec/frontend/groups/settings/compliance_frameworks/components/create_form_spec.js

@@ -21,14 +21,15 @@ jest.mock('~/lib/utils/url_utility');
 
 describe('CreateForm', () => {
   let wrapper;
-  const sentryError = new Error('Network error');
-  const sentrySaveError = new Error('Invalid values given');
+
   const propsData = {
     groupPath: 'group-1',
     groupEditPath: 'group-1/edit',
-    scopedLabelsHelpPath: 'help/scoped-labels',
   };
 
+  const sentryError = new Error('Network error');
+  const sentrySaveError = new Error('Invalid values given');
+
   const create = jest.fn().mockResolvedValue(validCreateResponse);
   const createWithNetworkErrors = jest.fn().mockRejectedValue(sentryError);
   const createWithErrors = jest.fn().mockResolvedValue(errorCreateResponse);

ee/spec/frontend/groups/settings/compliance_frameworks/components/edit_form_spec.js

@@ -28,16 +28,16 @@ jest.mock('~/lib/utils/url_utility');
 
 describe('EditForm', () => {
   let wrapper;
-  const sentryError = new Error('Network error');
-  const sentrySaveError = new Error('Invalid values given');
   const propsData = {
     graphqlFieldName: 'ComplianceManagement::Framework',
-    groupPath: 'group-1',
     groupEditPath: 'group-1/edit',
+    groupPath: 'group-1',
     id: '1',
-    scopedLabelsHelpPath: 'help/scoped-labels',
   };
 
+  const sentryError = new Error('Network error');
+  const sentrySaveError = new Error('Invalid values given');
+
   const fetchOne = jest.fn().mockResolvedValue(validFetchOneResponse);
   const fetchEmpty = jest.fn().mockResolvedValue(emptyFetchResponse);
   const fetchLoading = jest.fn().mockResolvedValue(new Promise(() => {}));
@@ -96,7 +96,7 @@ describe('EditForm', () => {
       await waitForPromises();
 
       expect(fetchOne).toHaveBeenCalledTimes(1);
-      expect(findForm().props()).toMatchObject({
+      expect(findForm().props()).toStrictEqual({
         name: frameworkFoundResponse.name,
         description: frameworkFoundResponse.description,
         color: frameworkFoundResponse.color,

ee/spec/frontend/groups/settings/compliance_frameworks/components/shared_form_spec.js

@@ -28,6 +28,15 @@ describe('SharedForm', () => {
       },
       stubs: {
         GlFormGroup,
+        GlFormInput: {
+          name: 'gl-form-input-stub',
+          props: ['state'],
+          template: `
+            <div>
+              <slot></slot>
+            </div>
+          `,
+        },
         GlSprintf,
       },
     });
@@ -67,10 +76,11 @@ describe('SharedForm', () => {
       ${null}     | ${null}
       ${''}       | ${false}
       ${'foobar'} | ${true}
-    `('sets the correct state to the name input group', ({ name, validity }) => {
+    `('sets the correct state to the name input and group', ({ name, validity }) => {
       wrapper = createComponent({ name });
 
       expect(findNameGroup().props('state')).toBe(validity);
+      expect(findNameInput().props('state')).toBe(validity);
     });
 
     it.each`
@@ -78,10 +88,11 @@ describe('SharedForm', () => {
       ${null}     | ${null}
       ${''}       | ${false}
       ${'foobar'} | ${true}
-    `('sets the correct state to the description input group', ({ description, validity }) => {
+    `('sets the correct state to the description input and group', ({ description, validity }) => {
       wrapper = createComponent({ description });
 
       expect(findDescriptionGroup().props('state')).toBe(validity);
+      expect(findDescriptionInput().props('state')).toBe(validity);
     });
 
     it.each`

ee/spec/frontend/groups/settings/compliance_frameworks/init_form_spec.js

@@ -48,7 +48,7 @@ describe('createComplianceFrameworksFormApp', () => {
     });
 
     it('parses and passes props', () => {
-      expect(findFormApp(CreateForm).props()).toMatchObject({
+      expect(findFormApp(CreateForm).props()).toStrictEqual({
         groupEditPath,
         groupPath,
       });
@@ -61,7 +61,8 @@ describe('createComplianceFrameworksFormApp', () => {
     });
 
     it('parses and passes props', () => {
-      expect(findFormApp(EditForm).props()).toMatchObject({
+      expect(findFormApp(EditForm).props()).toStrictEqual({
+        graphqlFieldName,
         groupEditPath,
         groupPath,
         id: testId,

ee/spec/frontend/groups/settings/compliance_frameworks/utils_spec.js

+import * as Utils from 'ee/groups/settings/compliance_frameworks/utils';
+
+describe('Utils', () => {
+  describe('initialiseFormData', () => {
+    it('returns the initial form data object', () => {
+      expect(Utils.initialiseFormData()).toStrictEqual({
+        name: null,
+        description: null,
+        pipelineConfigurationFullPath: null,
+        color: null,
+      });
+    });
+  });
+});