Merge branch '339495-fix-design-management-error-messages-eslint' into 'master'

Address require-string-literal-i18n-helpers offenses in app/assets/javascripts/design_management/utils/error_messages.js

See merge request gitlab-org/gitlab!69729

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

@@ -273,7 +273,7 @@ export default {
       this.onError(UPDATE_IMAGE_DIFF_NOTE_ERROR, e);
     },
     onDesignDeleteError(e) {
-      this.onError(designDeletionError({ singular: true }), e);
+      this.onError(designDeletionError(), e);
     },
     onResolveDiscussionError(e) {
       this.onError(UPDATE_IMAGE_DIFF_NOTE_ERROR, e);

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

@@ -255,7 +255,7 @@ export default {
       if (this.$route.query.version) this.$router.push({ name: DESIGNS_ROUTE_NAME });
     },
     onDesignDeleteError() {
-      const errorMessage = designDeletionError({ singular: this.selectedDesigns.length === 1 });
+      const errorMessage = designDeletionError(this.selectedDesigns.length);
       createFlash({ message: errorMessage });
     },
     onDesignDropzoneError() {

app/assets/javascripts/design_management/utils/cache_update.js

@@ -250,7 +250,7 @@ export const hasErrors = ({ errors = [] }) => errors?.length;
  */
 export const updateStoreAfterDesignsDelete = (store, data, query, designs) => {
   if (hasErrors(data)) {
-    onError(data, designDeletionError({ singular: designs.length === 1 }));
+    onError(data, designDeletionError(designs.length));
   } else {
     deleteDesignsFromStore(store, query, designs);
     addNewVersionToStore(store, query, data.version);

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

-/* eslint-disable @gitlab/require-string-literal-i18n-helpers */
 import { __, s__, n__, sprintf } from '~/locale';
 
 export const ADD_DISCUSSION_COMMENT_ERROR = s__(
@@ -27,12 +26,6 @@ export const DESIGN_NOT_FOUND_ERROR = __('Could not find design.');
 
 export const DESIGN_VERSION_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.',
-)}`;
-
 export const EXISTING_DESIGN_DROP_MANY_FILES_MESSAGE = __(
   'You can only upload one design when dropping onto an existing design.',
 );
@@ -53,12 +46,9 @@ export const DELETE_DESIGN_TODO_ERROR = __('Failed to remove a to-do item for th
 
 export const TOGGLE_TODO_ERROR = __('Failed to toggle the to-do status for the design.');
 
-const MAX_SKIPPED_FILES_LISTINGS = 5;
+const DESIGN_UPLOAD_SKIPPED_MESSAGE = s__('DesignManagement|Upload skipped. %{reason}');
 
-const oneDesignSkippedMessage = (filename) =>
-  `${DESIGN_UPLOAD_SKIPPED_MESSAGE} ${sprintf(s__('DesignManagement|%{filename} did not change.'), {
-    filename,
-  })}`;
+const MAX_SKIPPED_FILES_LISTINGS = 5;
 
 /**
  * Return warning message indicating that some (but not all) uploaded
@@ -66,25 +56,40 @@ const oneDesignSkippedMessage = (filename) =>
  * @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
+  const skippedFilesList = skippedFiles
     .slice(0, MAX_SKIPPED_FILES_LISTINGS)
     .map(({ filename }) => filename)
-    .join(', ')}${skippedFiles.length > MAX_SKIPPED_FILES_LISTINGS ? `, ${moreText}` : '.'}`;
+    .join(', ');
+
+  const uploadSkippedReason =
+    skippedFiles.length > MAX_SKIPPED_FILES_LISTINGS
+      ? sprintf(
+          s__(
+            'DesignManagement|Some of the designs you tried uploading did not change: %{skippedFiles} and %{moreCount} more.',
+          ),
+          {
+            skippedFiles: skippedFilesList,
+            moreCount: skippedFiles.length - MAX_SKIPPED_FILES_LISTINGS,
+          },
+        )
+      : sprintf(
+          s__(
+            'DesignManagement|Some of the designs you tried uploading did not change: %{skippedFiles}.',
+          ),
+          { skippedFiles: skippedFilesList },
+        );
+
+  return sprintf(DESIGN_UPLOAD_SKIPPED_MESSAGE, {
+    reason: uploadSkippedReason,
+  });
 };
 
-export const designDeletionError = ({ singular = true } = {}) => {
-  const design = singular ? __('a design') : __('designs');
-  return sprintf(s__('Could not archive %{design}. Please try again.'), {
-    design,
-  });
+export const designDeletionError = (designsCount = 1) => {
+  return n__(
+    'Failed to archive a design. Please try again.',
+    'Failed to archive designs. Please try again.',
+    designsCount,
+  );
 };
 
 /**
@@ -101,7 +106,18 @@ export const designUploadSkippedWarning = (uploadedDesigns, skippedFiles) => {
   if (skippedFiles.length === uploadedDesigns.length) {
     const { filename } = skippedFiles[0];
 
-    return n__(oneDesignSkippedMessage(filename), ALL_DESIGNS_SKIPPED_MESSAGE, skippedFiles.length);
+    const uploadSkippedReason = sprintf(
+      n__(
+        'DesignManagement|%{filename} did not change.',
+        'DesignManagement|The designs you tried uploading did not change.',
+        skippedFiles.length,
+      ),
+      { filename },
+    );
+
+    return sprintf(DESIGN_UPLOAD_SKIPPED_MESSAGE, {
+      reason: uploadSkippedReason,
+    });
   }
 
   return someDesignsSkippedMessage(skippedFiles);

locale/gitlab.pot

@@ -9414,9 +9414,6 @@ msgstr ""
 msgid "Could not apply %{name} command."
 msgstr ""
 
-msgid "Could not archive %{design}. Please try again."
-msgstr ""
-
 msgid "Could not authorize chat nickname. Try again!"
 msgstr ""
 
@@ -11520,7 +11517,9 @@ msgid "DesignManagement|%{current_design} of %{designs_count}"
 msgstr ""
 
 msgid "DesignManagement|%{filename} did not change."
-msgstr ""
+msgid_plural "DesignManagement|The designs you tried uploading did not change."
+msgstr[0] ""
+msgstr[1] ""
 
 msgid "DesignManagement|Adding a design with the same filename replaces the file in a new version."
 msgstr ""
@@ -11624,6 +11623,12 @@ msgstr ""
 msgid "DesignManagement|Select all"
 msgstr ""
 
+msgid "DesignManagement|Some of the designs you tried uploading did not change: %{skippedFiles} and %{moreCount} more."
+msgstr ""
+
+msgid "DesignManagement|Some of the designs you tried uploading did not change: %{skippedFiles}."
+msgstr ""
+
 msgid "DesignManagement|The maximum number of designs allowed to be uploaded is %{upload_limit}. Please try again."
 msgstr ""
 
@@ -11639,15 +11644,12 @@ msgstr ""
 msgid "DesignManagement|Upload designs"
 msgstr ""
 
-msgid "DesignManagement|Upload skipped."
+msgid "DesignManagement|Upload skipped. %{reason}"
 msgstr ""
 
 msgid "DesignManagement|Your designs are being copied and are on their way… Please refresh to update."
 msgstr ""
 
-msgid "DesignManagement|and %{moreCount} more."
-msgstr ""
-
 msgid "Designs"
 msgstr ""
 
@@ -13977,6 +13979,11 @@ msgstr ""
 msgid "Failed to apply commands."
 msgstr ""
 
+msgid "Failed to archive a design. Please try again."
+msgid_plural "Failed to archive designs. Please try again."
+msgstr[0] ""
+msgstr[1] ""
+
 msgid "Failed to assign a reviewer because no user was found."
 msgstr ""
 
@@ -31736,9 +31743,6 @@ msgstr ""
 msgid "Some common domains are not allowed. %{learn_more_link}."
 msgstr ""
 
-msgid "Some of the designs you tried uploading did not change:"
-msgstr ""
-
 msgid "Someone edited the issue at the same time you did. Please check out %{linkStart}the issue%{linkEnd} and make sure your changes will not unintentionally remove theirs."
 msgstr ""
 
@@ -33855,9 +33859,6 @@ 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 ""
 
@@ -39633,9 +39634,6 @@ msgstr ""
 msgid "a deleted user"
 msgstr ""
 
-msgid "a design"
-msgstr ""
-
 msgid "about 1 hour"
 msgid_plural "about %d hours"
 msgstr[0] ""
@@ -40173,9 +40171,6 @@ msgstr ""
 msgid "design"
 msgstr ""
 
-msgid "designs"
-msgstr ""
-
 msgid "detached"
 msgstr ""
 

spec/frontend/design_management/utils/cache_update_spec.js

@@ -27,7 +27,7 @@ describe('Design Management cache update', () => {
   describe('error handling', () => {
     it.each`
       fnName                                   | subject                                    | errorMessage                    | extraArgs
-      ${'updateStoreAfterDesignsDelete'}       | ${updateStoreAfterDesignsDelete}           | ${designDeletionError({ singular: true })} | ${[[design]]}
+      ${'updateStoreAfterDesignsDelete'}       | ${updateStoreAfterDesignsDelete}           | ${designDeletionError()}        | ${[[design]]}
       ${'updateStoreAfterAddImageDiffNote'}    | ${updateStoreAfterAddImageDiffNote}        | ${ADD_IMAGE_DIFF_NOTE_ERROR}    | ${[]}
       ${'updateStoreAfterUploadDesign'}        | ${updateStoreAfterUploadDesign}            | ${mockErrors[0]}                | ${[]}
       ${'updateStoreAfterUpdateImageDiffNote'} | ${updateStoreAfterRepositionImageDiffNote} | ${UPDATE_IMAGE_DIFF_NOTE_ERROR} | ${[]}

spec/frontend/design_management/utils/error_messages_spec.js

@@ -10,20 +10,21 @@ const mockFilenames = (n) =>
 
 describe('Error message', () => {
   describe('designDeletionError', () => {
-    const singularMsg = 'Could not archive a design. Please try again.';
-    const pluralMsg = 'Could not archive designs. Please try again.';
+    const singularMsg = 'Failed to archive a design. Please try again.';
+    const pluralMsg = 'Failed to archive 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);
-      });
-    });
+    it.each`
+      designsLength | expectedText
+      ${undefined}  | ${singularMsg}
+      ${0}          | ${pluralMsg}
+      ${1}          | ${singularMsg}
+      ${2}          | ${pluralMsg}
+    `(
+      'returns "$expectedText" when designsLength is $designsLength',
+      ({ designsLength, expectedText }) => {
+        expect(designDeletionError(designsLength)).toBe(expectedText);
+      },
+    );
   });
 
   describe.each([
@@ -47,12 +48,12 @@ describe('Error message', () => {
     [
       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.',
+      '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.',
+      '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) => {
     it('returns expected warning message', () => {