Remove placeholder and move error message in the form

Fix cache update during errors
Fix the tests to match new behaviour

Changelog: changed

app/assets/javascripts/packages_and_registries/settings/project/components/settings_form.vue

@@ -13,7 +13,6 @@ import {
   REMOVE_INFO_TEXT,
   EXPIRATION_SCHEDULE_LABEL,
   NAME_REGEX_LABEL,
-  NAME_REGEX_PLACEHOLDER,
   NAME_REGEX_DESCRIPTION,
   CADENCE_LABEL,
   EXPIRATION_POLICY_FOOTER_NOTE,
@@ -68,7 +67,6 @@ export default {
     REMOVE_INFO_TEXT,
     EXPIRATION_SCHEDULE_LABEL,
     NAME_REGEX_LABEL,
-    NAME_REGEX_PLACEHOLDER,
     NAME_REGEX_DESCRIPTION,
     CADENCE_LABEL,
     EXPIRATION_POLICY_FOOTER_NOTE,
@@ -141,6 +139,17 @@ export default {
         [model]: state,
       };
     },
+    encapsulateError(path, message) {
+      return {
+        graphQLErrors: [
+          {
+            extensions: {
+              problems: [{ path: [path], message }],
+            },
+          },
+        ],
+      };
+    },
     submit() {
       this.track('submit_form');
       this.apiErrors = {};
@@ -156,7 +165,8 @@ export default {
         .then(({ data }) => {
           const errorMessage = data?.updateContainerExpirationPolicy?.errors[0];
           if (errorMessage) {
-            this.$toast.show(errorMessage);
+            const customError = this.encapsulateError('nameRegex', errorMessage);
+            throw customError;
           } else {
             this.$toast.show(UPDATE_SETTINGS_SUCCESS_MESSAGE);
           }
@@ -273,7 +283,6 @@ export default {
             :error="apiErrors.nameRegex"
             :disabled="isFieldDisabled"
             :label="$options.i18n.NAME_REGEX_LABEL"
-            :placeholder="$options.i18n.NAME_REGEX_PLACEHOLDER"
             :description="$options.i18n.NAME_REGEX_DESCRIPTION"
             name="remove-regex"
             data-testid="remove-regex-input"

app/assets/javascripts/packages_and_registries/settings/project/constants.js

@@ -32,7 +32,6 @@ export const REMOVE_INFO_TEXT = s__(
 );
 export const EXPIRATION_SCHEDULE_LABEL = s__('ContainerRegistry|Remove tags older than:');
 export const NAME_REGEX_LABEL = s__('ContainerRegistry|Remove tags matching:');
-export const NAME_REGEX_PLACEHOLDER = '.*';
 export const NAME_REGEX_DESCRIPTION = s__(
   'ContainerRegistry|Tags with names that match this regex pattern are removed. %{linkStart}View regex examples.%{linkEnd}',
 );

app/assets/javascripts/packages_and_registries/settings/project/graphql/utils/cache_update.js

@@ -10,6 +10,7 @@ export const updateContainerExpirationPolicy = (projectPath) => (client, { data:
 
   const data = produce(sourceData, (draftState) => {
     draftState.project.containerExpirationPolicy = {
+      ...draftState.project.containerExpirationPolicy,
       ...updatedData.updateContainerExpirationPolicy.containerExpirationPolicy,
     };
   });

spec/frontend/packages_and_registries/settings/project/settings/components/__snapshots__/settings_form_spec.js.snap

@@ -58,7 +58,7 @@ exports[`Settings Form Remove regex matches snapshot 1`] = `
   error=""
   label="Remove tags matching:"
   name="remove-regex"
-  placeholder=".*"
+  placeholder=""
   value="asdasdssssdfdf"
 />
 `;

spec/frontend/packages_and_registries/settings/project/settings/components/settings_form_spec.js

@@ -49,6 +49,11 @@ describe('Settings Form', () => {
   const findOlderThanDropdown = () => wrapper.find('[data-testid="older-than-dropdown"]');
   const findRemoveRegexInput = () => wrapper.find('[data-testid="remove-regex-input"]');
 
+  const submitForm = async () => {
+    findForm().trigger('submit');
+    return waitForPromises();
+  };
+
   const mountComponent = ({
     props = defaultProps,
     data,
@@ -318,27 +323,24 @@ describe('Settings Form', () => {
           mutationResolver: jest.fn().mockResolvedValue(expirationPolicyMutationPayload()),
         });
 
-        findForm().trigger('submit');
-        await waitForPromises();
-        await nextTick();
+        await submitForm();
 
         expect(wrapper.vm.$toast.show).toHaveBeenCalledWith(UPDATE_SETTINGS_SUCCESS_MESSAGE);
       });
 
       describe('when submit fails', () => {
         describe('user recoverable errors', () => {
-          it('when there is an error is shown in a toast', async () => {
+          it('when there is an error is shown in the nameRegex field t', async () => {
             mountComponentWithApollo({
               mutationResolver: jest
                 .fn()
                 .mockResolvedValue(expirationPolicyMutationPayload({ errors: ['foo'] })),
             });
 
-            findForm().trigger('submit');
-            await waitForPromises();
-            await nextTick();
+            await submitForm();
 
-            expect(wrapper.vm.$toast.show).toHaveBeenCalledWith('foo');
+            expect(wrapper.vm.$toast.show).toHaveBeenCalledWith(UPDATE_SETTINGS_ERROR_MESSAGE);
+            expect(findRemoveRegexInput().props('error')).toBe('foo');
           });
         });
 
@@ -348,9 +350,7 @@ describe('Settings Form', () => {
               mutationResolver: jest.fn().mockRejectedValue(expirationPolicyMutationPayload()),
             });
 
-            findForm().trigger('submit');
-            await waitForPromises();
-            await nextTick();
+            await submitForm();
 
             expect(wrapper.vm.$toast.show).toHaveBeenCalledWith(UPDATE_SETTINGS_ERROR_MESSAGE);
           });
@@ -367,9 +367,7 @@ describe('Settings Form', () => {
             });
             mountComponent({ mocks: { $apollo: { mutate } } });
 
-            findForm().trigger('submit');
-            await waitForPromises();
-            await nextTick();
+            await submitForm();
 
             expect(findKeepRegexInput().props('error')).toEqual('baz');
           });

spec/frontend/packages_and_registries/settings/project/settings/graphql/cache_updated_spec.js

@@ -4,15 +4,15 @@ import { updateContainerExpirationPolicy } from '~/packages_and_registries/setti
 describe('Registry settings cache update', () => {
   let client;
 
-  const payload = {
+  const payload = (value) => ({
     data: {
       updateContainerExpirationPolicy: {
         containerExpirationPolicy: {
-          enabled: true,
+          ...value,
         },
       },
     },
-  };
+  });
 
   const cacheMock = {
     project: {
@@ -35,12 +35,12 @@ describe('Registry settings cache update', () => {
   });
   describe('Registry settings cache update', () => {
     it('calls readQuery', () => {
-      updateContainerExpirationPolicy('foo')(client, payload);
+      updateContainerExpirationPolicy('foo')(client, payload({ enabled: true }));
       expect(client.readQuery).toHaveBeenCalledWith(queryAndVariables);
     });
 
     it('writes the correct result in the cache', () => {
-      updateContainerExpirationPolicy('foo')(client, payload);
+      updateContainerExpirationPolicy('foo')(client, payload({ enabled: true }));
       expect(client.writeQuery).toHaveBeenCalledWith({
         ...queryAndVariables,
         data: {
@@ -52,5 +52,20 @@ describe('Registry settings cache update', () => {
         },
       });
     });
+
+    it('with an empty update preserves the state', () => {
+      updateContainerExpirationPolicy('foo')(client, payload());
+
+      expect(client.writeQuery).toHaveBeenCalledWith({
+        ...queryAndVariables,
+        data: {
+          project: {
+            containerExpirationPolicy: {
+              enabled: false,
+            },
+          },
+        },
+      });
+    });
   });
 });