Merge branch 'skipped-designs-warning' into 'master'

Display warning flash if design upload is skipped

See merge request gitlab-org/gitlab!21615

doc/user/project/issues/design_management.md

@@ -75,6 +75,12 @@ of the design, and will replace the previous version.
 Designs cannot be added if the issue has been moved, or its
 [discussion is locked](../../discussions/#lock-discussions).
 
+### Skipped designs
+
+Designs with the same filename as an existing uploaded design _and_ whose content has not changed will be skipped.
+This means that no new version of the design will be created. When designs are skipped, you will be made aware via a warning
+message on the Issue.
+
 ## Viewing designs
 
 Images on the Design Management page can be enlarged by clicking on them.

ee/app/assets/javascripts/design_management/graphql/mutations/uploadDesign.mutation.graphql

@@ -11,7 +11,10 @@ mutation uploadDesign($files: [Upload!]!, $projectPath: ID!, $iid: ID!) {
             sha
           }
         }
-      }
+      },
+    }
+    skippedDesigns {
+      filename
     }
   }
 }

ee/app/assets/javascripts/design_management/pages/index.vue

@@ -11,7 +11,11 @@ import uploadDesignMutation from '../graphql/mutations/uploadDesign.mutation.gra
 import permissionsQuery from '../graphql/queries/permissions.query.graphql';
 import projectQuery from '../graphql/queries/project.query.graphql';
 import allDesignsMixin from '../mixins/all_designs';
-import { UPLOAD_DESIGN_ERROR, designDeletionError } from '../utils/error_messages';
+import {
+  UPLOAD_DESIGN_ERROR,
+  designUploadSkippedWarning,
+  designDeletionError,
+} from '../utils/error_messages';
 import { updateStoreAfterUploadDesign } from '../utils/cache_update';
 import { designUploadOptimisticResponse } from '../utils/design_management_utils';
 import { DESIGNS_ROUTE_NAME } from '../router/constants';
@@ -133,7 +137,7 @@ export default {
 
       return this.$apollo
         .mutate(mutationPayload)
-        .then(() => this.onUploadDesignDone())
+        .then(res => this.onUploadDesignDone(res))
         .catch(() => this.onUploadDesignError());
     },
     afterUploadDesign(
@@ -144,9 +148,18 @@ export default {
     ) {
       updateStoreAfterUploadDesign(store, designManagementUpload, this.projectQueryBody);
     },
-    onUploadDesignDone() {
+    onUploadDesignDone(res) {
+      const skippedFiles = res?.data?.designManagementUpload?.skippedDesigns || [];
+      const skippedWarningMessage = designUploadSkippedWarning(this.filesToBeSaved, skippedFiles);
+      if (skippedWarningMessage) {
+        createFlash(skippedWarningMessage, 'warning');
+      }
+
+      // if this upload resulted in a new version being created, redirect user to the latest version
+      if (!this.isLatestVersion) {
+        this.$router.push({ name: DESIGNS_ROUTE_NAME });
+      }
       this.resetFilesToBeSaved();
-      this.$router.push({ name: DESIGNS_ROUTE_NAME });
     },
     onUploadDesignError() {
       this.resetFilesToBeSaved();

ee/app/assets/javascripts/design_management/utils/error_messages.js

-import { __, s__, sprintf } from '~/locale';
+import { __, s__, n__, sprintf } from '~/locale';
 
-export const designDeletionError = ({ singular = true } = {}) => {
-  const design = singular ? __('a design') : __('designs');
-  return sprintf(s__('Could not delete %{design}. Please try again.'), {
-    design,
-  });
-};
 export const ADD_DISCUSSION_COMMENT_ERROR = s__(
   'DesignManagement|Could not add a new comment. Please try again.',
 );
+
 export const ADD_IMAGE_DIFF_NOTE_ERROR = s__(
   'DesignManagement|Could not create new discussion. Please try again.',
 );
+
 export const UPLOAD_DESIGN_ERROR = s__(
   'DesignManagement|Error uploading a new design. Please try again.',
 );
+
 export const DESIGN_NOT_FOUND_ERROR = __('Could not find design');
+
 export const DESIGN_NOT_EXIST_ERROR = __('Requested design version does not exist');
+
+const DESIGN_UPLOAD_SKIPPED_MESSAGE = s__('DesignManagement|Upload skipped.');
+
+const ALL_DESIGNS_SKIPPED_MESSAGE = `${DESIGN_UPLOAD_SKIPPED_MESSAGE} ${s__(
+  'The designs you tried uploading did not change.',
+)}`;
+
+const MAX_SKIPPED_FILES_LISTINGS = 5;
+
+const oneDesignSkippedMessage = filename =>
+  `${DESIGN_UPLOAD_SKIPPED_MESSAGE} ${sprintf(s__('DesignManagement|%{filename} did not change.'), {
+    filename,
+  })}`;
+
+/**
+ * Return warning message indicating that some (but not all) uploaded
+ * files were skipped.
+ * @param {Array<{ filename }>} skippedFiles
+ */
+const someDesignsSkippedMessage = skippedFiles => {
+  const designsSkippedMessage = `${DESIGN_UPLOAD_SKIPPED_MESSAGE} ${s__(
+    'Some of the designs you tried uploading did not change:',
+  )}`;
+
+  const moreText = sprintf(s__(`DesignManagement|and %{moreCount} more.`), {
+    moreCount: skippedFiles.length - MAX_SKIPPED_FILES_LISTINGS,
+  });
+
+  return `${designsSkippedMessage} ${skippedFiles
+    .slice(0, MAX_SKIPPED_FILES_LISTINGS)
+    .map(({ filename }) => filename)
+    .join(', ')}${skippedFiles.length > MAX_SKIPPED_FILES_LISTINGS ? `, ${moreText}` : '.'}`;
+};
+
+export const designDeletionError = ({ singular = true } = {}) => {
+  const design = singular ? __('a design') : __('designs');
+  return sprintf(s__('Could not delete %{design}. Please try again.'), {
+    design,
+  });
+};
+
+/**
+ * Return warning message, if applicable, that one, some or all uploaded
+ * files were skipped.
+ * @param {Array<{ filename }>} uploadedDesigns
+ * @param {Array<{ filename }>} skippedFiles
+ */
+export const designUploadSkippedWarning = (uploadedDesigns, skippedFiles) => {
+  if (skippedFiles.length === 0) {
+    return null;
+  }
+
+  if (skippedFiles.length === uploadedDesigns.length) {
+    const { filename } = skippedFiles[0];
+
+    return n__(oneDesignSkippedMessage(filename), ALL_DESIGNS_SKIPPED_MESSAGE, skippedFiles.length);
+  }
+
+  return someDesignsSkippedMessage(skippedFiles);
+};

ee/changelogs/unreleased/skipped-designs-warning.yml

+---
+title: Display warning flash if design upload is skipped
+merge_request: 21615
+author:
+type: added

ee/spec/frontend/design_management/pages/index_spec.js

@@ -71,8 +71,9 @@ describe('Design management index page', () => {
     allVersions = [],
     createDesign = true,
     stubs = {},
+    mockMutate = jest.fn().mockResolvedValue(),
   } = {}) {
-    mutate = jest.fn(() => Promise.resolve());
+    mutate = mockMutate;
     const $apollo = {
       queries: {
         designs: {
@@ -253,7 +254,7 @@ describe('Design management index page', () => {
       return wrapper.vm.$nextTick().then(() => {
         expect(wrapper.vm.filesToBeSaved).toEqual([]);
         expect(wrapper.vm.isSaving).toBeFalsy();
-        expect(wrapper.vm.$router.currentRoute.path).toEqual('/designs');
+        expect(wrapper.vm.isLatestVersion).toBe(true);
       });
     });
 
@@ -302,6 +303,29 @@ describe('Design management index page', () => {
         expect(createFlash).toHaveBeenCalled();
       });
     });
+
+    it('flashes warning if designs are skipped', () => {
+      createComponent({
+        mockMutate: () =>
+          Promise.resolve({
+            data: { designManagementUpload: { skippedDesigns: [{ filename: 'test.jpg' }] } },
+          }),
+      });
+
+      const uploadDesign = wrapper.vm.onUploadDesign([
+        {
+          name: 'test',
+        },
+      ]);
+
+      return uploadDesign.then(() => {
+        expect(createFlash).toHaveBeenCalledTimes(1);
+        expect(createFlash).toHaveBeenCalledWith(
+          'Upload skipped. test.jpg did not change.',
+          'warning',
+        );
+      });
+    });
   });
 
   describe('on latest version when has designs', () => {

ee/spec/frontend/design_management/utils/error_messages_spec.js

+import {
+  designDeletionError,
+  designUploadSkippedWarning,
+} from 'ee/design_management/utils/error_messages';
+
+const mockFilenames = n =>
+  Array(n)
+    .fill(0)
+    .map((_, i) => ({ filename: `${i + 1}.jpg` }));
+
+describe('Error message', () => {
+  describe('designDeletionError', () => {
+    const singularMsg = 'Could not delete a design. Please try again.';
+    const pluralMsg = 'Could not delete designs. Please try again.';
+
+    describe('when [singular=true]', () => {
+      it.each([[undefined], [true]])('uses singular grammar', singularOption => {
+        expect(designDeletionError({ singular: singularOption })).toEqual(singularMsg);
+      });
+    });
+
+    describe('when [singular=false]', () => {
+      it('uses plural grammar', () => {
+        expect(designDeletionError({ singular: false })).toEqual(pluralMsg);
+      });
+    });
+  });
+
+  describe.each([
+    [[], [], null],
+    [mockFilenames(1), mockFilenames(1), 'Upload skipped. 1.jpg did not change.'],
+    [
+      mockFilenames(2),
+      mockFilenames(2),
+      'Upload skipped. The designs you tried uploading did not change.',
+    ],
+    [
+      mockFilenames(2),
+      mockFilenames(1),
+      'Upload skipped. Some of the designs you tried uploading did not change: 1.jpg.',
+    ],
+    [
+      mockFilenames(6),
+      mockFilenames(5),
+      'Upload skipped. Some of the designs you tried uploading did not change: 1.jpg, 2.jpg, 3.jpg, 4.jpg, 5.jpg.',
+    ],
+    [
+      mockFilenames(7),
+      mockFilenames(6),
+      'Upload skipped. Some of the designs you tried uploading did not change: 1.jpg, 2.jpg, 3.jpg, 4.jpg, 5.jpg, and 1 more.',
+    ],
+    [
+      mockFilenames(8),
+      mockFilenames(7),
+      'Upload skipped. Some of the designs you tried uploading did not change: 1.jpg, 2.jpg, 3.jpg, 4.jpg, 5.jpg, and 2 more.',
+    ],
+  ])('designUploadSkippedWarning', (uploadedFiles, skippedFiles, expected) => {
+    test('returns expected warning message', () => {
+      expect(designUploadSkippedWarning(uploadedFiles, skippedFiles)).toBe(expected);
+    });
+  });
+});

locale/gitlab.pot

@@ -6453,6 +6453,9 @@ msgstr ""
 msgid "DesignManagement|%{current_design} of %{designs_count}"
 msgstr ""
 
+msgid "DesignManagement|%{filename} did not change."
+msgstr ""
+
 msgid "DesignManagement|Add designs"
 msgstr ""
 
@@ -6519,6 +6522,12 @@ msgstr ""
 msgid "DesignManagement|Upload and view the latest designs for this issue. Consistent and easy to find, so everyone is up to date."
 msgstr ""
 
+msgid "DesignManagement|Upload skipped."
+msgstr ""
+
+msgid "DesignManagement|and %{moreCount} more."
+msgstr ""
+
 msgid "Designs"
 msgstr ""
 
@@ -17467,6 +17476,9 @@ msgstr ""
 msgid "Some email servers do not support overriding the email sender name. Enable this option to include the name of the author of the issue, merge request or comment in the email body instead."
 msgstr ""
 
+msgid "Some of the designs you tried uploading did not change:"
+msgstr ""
+
 msgid "Some of your epics may not be visible. A roadmap is limited to the first 1,000 epics, in your selected sort order."
 msgstr ""
 
@@ -18690,6 +18702,9 @@ msgstr ""
 msgid "The deployment of this job to %{environmentLink} did not succeed."
 msgstr ""
 
+msgid "The designs you tried uploading did not change."
+msgstr ""
+
 msgid "The directory has been successfully created."
 msgstr ""