Merge branch...

Merge branch '35073-refactor-design_management-pages-index-vue-to-use-apollomutation-component' into 'master'

Refactor design_management/pages/index.vue to use ApolloMutation component

See merge request gitlab-org/gitlab!21022

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

 <script>
 import { GlLoadingIcon, GlEmptyState, GlButton } from '@gitlab/ui';
-import _ from 'underscore';
 import createFlash from '~/flash';
 import { s__, sprintf } from '~/locale';
 import UploadButton from '../components/upload/button.vue';
@@ -14,6 +13,7 @@ import projectQuery from '../graphql/queries/project.query.graphql';
 import allDesignsMixin from '../mixins/all_designs';
 import { UPLOAD_DESIGN_ERROR } from '../utils/error_messages';
 import { updateStoreAfterUploadDesign } from '../utils/cache_update';
+import { designUploadOptimisticResponse } from '../utils/design_management_utils';
 
 const MAXIMUM_FILE_UPLOAD_LIMIT = 10;
 
@@ -85,8 +85,15 @@ export default {
     },
   },
   methods: {
-    onUploadDesign(files) {
-      if (!this.canCreateDesign) return null;
+    resetFilesToBeSaved() {
+      this.filesToBeSaved = [];
+    },
+    /**
+     * Determine if a design upload is valid, given [files]
+     * @param {Array<File>} files
+     */
+    isValidDesignUpload(files) {
+      if (!this.canCreateDesign) return false;
 
       if (files.length > MAXIMUM_FILE_UPLOAD_LIMIT) {
         createFlash(
@@ -100,77 +107,49 @@ export default {
           ),
         );
 
-        return null;
+        return false;
       }
-
+      return true;
+    },
+    onUploadDesign(files) {
+      // convert to Array so that we have Array methods (.map, .some, etc.)
       this.filesToBeSaved = Array.from(files);
-      const optimisticResponse = this.filesToBeSaved.map(file => ({
-        // False positive i18n lint: https://gitlab.com/gitlab-org/frontend/eslint-plugin-i18n/issues/26
-        // eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings
-        __typename: 'Design',
-        id: -_.uniqueId(),
-        image: '',
-        filename: file.name,
-        fullPath: '',
-        notesCount: 0,
-        event: 'NONE',
-        diffRefs: {
-          __typename: 'DiffRefs',
-          baseSha: '',
-          startSha: '',
-          headSha: '',
+      if (!this.isValidDesignUpload(this.filesToBeSaved)) return null;
+
+      const mutationPayload = {
+        optimisticResponse: designUploadOptimisticResponse(this.filesToBeSaved),
+        variables: {
+          files: this.filesToBeSaved,
+          projectPath: this.projectPath,
+          iid: this.issueIid,
         },
-        discussions: {
-          __typename: 'DesignDiscussion',
-          edges: [],
+        context: {
+          hasUpload: true,
         },
-        versions: {
-          __typename: 'DesignVersionConnection',
-          edges: {
-            __typename: 'DesignVersionEdge',
-            node: {
-              __typename: 'DesignVersion',
-              id: -_.uniqueId(),
-              sha: -_.uniqueId(),
-            },
-          },
-        },
-      }));
+        mutation: uploadDesignMutation,
+        update: this.afterUploadDesign,
+      };
 
       return this.$apollo
-        .mutate({
-          mutation: uploadDesignMutation,
-          variables: {
-            files,
-            projectPath: this.projectPath,
-            iid: this.issueIid,
-          },
-          context: {
-            hasUpload: true,
-          },
-          update: (store, { data: { designManagementUpload } }) => {
-            updateStoreAfterUploadDesign(store, designManagementUpload, this.projectQueryBody);
-          },
-          optimisticResponse: {
-            // False positive i18n lint: https://gitlab.com/gitlab-org/frontend/eslint-plugin-i18n/issues/26
-            // eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings
-            __typename: 'Mutation',
-            designManagementUpload: {
-              __typename: 'DesignManagementUploadPayload',
-              designs: optimisticResponse,
-            },
-          },
-        })
-        .then(() => {
-          this.$router.push({ name: 'designs' });
-        })
-        .catch(e => {
-          createFlash(UPLOAD_DESIGN_ERROR);
-          throw e;
-        })
-        .finally(() => {
-          this.filesToBeSaved = [];
-        });
+        .mutate(mutationPayload)
+        .then(() => this.onUploadDesignDone())
+        .catch(() => this.onUploadDesignError());
+    },
+    afterUploadDesign(
+      store,
+      {
+        data: { designManagementUpload },
+      },
+    ) {
+      updateStoreAfterUploadDesign(store, designManagementUpload, this.projectQueryBody);
+    },
+    onUploadDesignDone() {
+      this.resetFilesToBeSaved();
+      this.$router.push({ name: 'designs' });
+    },
+    onUploadDesignError() {
+      this.resetFilesToBeSaved();
+      createFlash(UPLOAD_DESIGN_ERROR);
     },
     changeSelectedDesigns(filename) {
       if (this.isDesignSelected(filename)) {

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

+import { uniqueId } from 'underscore';
+
 /**
  * Returns formatted array that doesn't contain
  * `edges`->`node` nesting
@@ -35,3 +37,52 @@ export const findVersionId = id => (id.match('::Version/(.+$)') || [])[1];
 export const findNoteId = id => (id.match('DiffNote/(.+$)') || [])[1];
 
 export const extractDesign = data => data.project.issue.designCollection.designs.edges[0].node;
+
+/**
+ * Generates optimistic response for a design upload mutation
+ * @param {Array<File>} files
+ */
+export const designUploadOptimisticResponse = files => {
+  const designs = files.map(file => ({
+    // False positive i18n lint: https://gitlab.com/gitlab-org/frontend/eslint-plugin-i18n/issues/26
+    // eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings
+    __typename: 'Design',
+    id: -uniqueId(),
+    image: '',
+    filename: file.name,
+    fullPath: '',
+    notesCount: 0,
+    event: 'NONE',
+    diffRefs: {
+      __typename: 'DiffRefs',
+      baseSha: '',
+      startSha: '',
+      headSha: '',
+    },
+    discussions: {
+      __typename: 'DesignDiscussion',
+      edges: [],
+    },
+    versions: {
+      __typename: 'DesignVersionConnection',
+      edges: {
+        __typename: 'DesignVersionEdge',
+        node: {
+          __typename: 'DesignVersion',
+          id: -uniqueId(),
+          sha: -uniqueId(),
+        },
+      },
+    },
+  }));
+
+  return {
+    // False positive i18n lint: https://gitlab.com/gitlab-org/frontend/eslint-plugin-i18n/issues/26
+    // eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings
+    __typename: 'Mutation',
+    designManagementUpload: {
+      __typename: 'DesignManagementUploadPayload',
+      designs,
+    },
+  };
+};

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

 import { createLocalVue, shallowMount } from '@vue/test-utils';
+import { ApolloMutation } from 'vue-apollo';
 import VueRouter from 'vue-router';
+import { GlEmptyState } from '@gitlab/ui';
+
 import Index from 'ee/design_management/pages/index.vue';
 import uploadDesignQuery from 'ee/design_management/graphql/mutations/uploadDesign.mutation.graphql';
 import DesignDestroyer from 'ee/design_management/components/design_destroyer.vue';
+import UploadButton from 'ee/design_management/components/upload/button.vue';
+import DeleteButton from 'ee/design_management/components/delete_button.vue';
 import createFlash from '~/flash';
 
 const localVue = createLocalVue();
@@ -55,14 +60,16 @@ describe('Design management index page', () => {
 
   const findDesignCheckboxes = () => wrapper.findAll('.design-checkbox');
   const findSelectAllButton = () => wrapper.find('.js-select-all');
-  const findDeleteButton = () => wrapper.find('deletebutton-stub');
   const findToolbar = () => wrapper.find('.qa-selector-toolbar');
+  const findDeleteButton = () => wrapper.find(DeleteButton);
+  const findUploadButton = () => wrapper.find(UploadButton);
 
   function createComponent({
     loading = false,
     designs = [],
     allVersions = [],
     createDesign = true,
+    stubs = {},
   } = {}) {
     mutate = jest.fn(() => Promise.resolve());
     const $apollo = {
@@ -82,7 +89,7 @@ describe('Design management index page', () => {
       mocks: { $apollo },
       localVue,
       router,
-      stubs: { DesignDestroyer },
+      stubs: { DesignDestroyer, ApolloMutation, ...stubs },
     });
 
     wrapper.setData({
@@ -155,76 +162,67 @@ describe('Design management index page', () => {
     });
   });
 
-  describe('onUploadDesign', () => {
-    it('calls apollo mutate', () => {
-      createComponent();
+  describe('uploading designs', () => {
+    it('calls mutation on upload', () => {
+      createComponent({ stubs: { GlEmptyState } });
 
-      return wrapper.vm
-        .onUploadDesign([
-          {
-            name: 'test',
-          },
-        ])
-        .then(() => {
-          expect(mutate).toHaveBeenCalledWith({
-            context: {
-              hasUpload: true,
-            },
-            mutation: uploadDesignQuery,
-            variables: {
-              files: [{ name: 'test' }],
-              projectPath: '',
-              iid: '1',
-            },
-            update: expect.anything(),
-            optimisticResponse: {
-              __typename: 'Mutation',
-              designManagementUpload: {
-                __typename: 'DesignManagementUploadPayload',
-                designs: [
-                  {
-                    __typename: 'Design',
-                    id: expect.anything(),
-                    image: '',
-                    filename: 'test',
-                    fullPath: '',
-                    event: 'NONE',
-                    notesCount: 0,
-                    diffRefs: {
-                      __typename: 'DiffRefs',
-                      baseSha: '',
-                      startSha: '',
-                      headSha: '',
-                    },
-                    discussions: {
-                      __typename: 'DesignDiscussion',
-                      edges: [],
-                    },
-                    versions: {
-                      __typename: 'DesignVersionConnection',
-                      edges: {
-                        __typename: 'DesignVersionEdge',
-                        node: {
-                          __typename: 'DesignVersion',
-                          id: expect.anything(),
-                          sha: expect.anything(),
-                        },
-                      },
+      const mutationVariables = {
+        update: expect.anything(),
+        context: {
+          hasUpload: true,
+        },
+        mutation: uploadDesignQuery,
+        variables: {
+          files: [{ name: 'test' }],
+          projectPath: '',
+          iid: '1',
+        },
+        optimisticResponse: {
+          __typename: 'Mutation',
+          designManagementUpload: {
+            __typename: 'DesignManagementUploadPayload',
+            designs: [
+              {
+                __typename: 'Design',
+                id: expect.anything(),
+                image: '',
+                filename: 'test',
+                fullPath: '',
+                event: 'NONE',
+                notesCount: 0,
+                diffRefs: {
+                  __typename: 'DiffRefs',
+                  baseSha: '',
+                  startSha: '',
+                  headSha: '',
+                },
+                discussions: {
+                  __typename: 'DesignDiscussion',
+                  edges: [],
+                },
+                versions: {
+                  __typename: 'DesignVersionConnection',
+                  edges: {
+                    __typename: 'DesignVersionEdge',
+                    node: {
+                      __typename: 'DesignVersion',
+                      id: expect.anything(),
+                      sha: expect.anything(),
                     },
                   },
-                ],
+                },
               },
-            },
-          });
-        });
-    });
-
-    it('does not call apollo mutate if createDesign is false', () => {
-      createComponent({ createDesign: false });
-
-      wrapper.vm.onUploadDesign([]);
+            ],
+          },
+        },
+      };
 
-      expect(mutate).not.toHaveBeenCalled();
+      return wrapper.vm.$nextTick().then(() => {
+        findUploadButton().vm.$emit('upload', [{ name: 'test' }]);
+        expect(mutate).toHaveBeenCalledWith(mutationVariables);
+        expect(wrapper.vm.filesToBeSaved).toEqual([{ name: 'test' }]);
+        expect(wrapper.vm.isSaving).toBeTruthy();
+      });
     });
 
     it('sets isSaving', () => {
@@ -243,6 +241,40 @@ describe('Design management index page', () => {
       });
     });
 
+    it('updates state appropriately after upload complete', () => {
+      createComponent({ stubs: { GlEmptyState } });
+      wrapper.setData({ filesToBeSaved: [{ name: 'test' }] });
+
+      wrapper.vm.onUploadDesignDone();
+      return wrapper.vm.$nextTick().then(() => {
+        expect(wrapper.vm.filesToBeSaved).toEqual([]);
+        expect(wrapper.vm.isSaving).toBeFalsy();
+        expect(wrapper.vm.$router.currentRoute.path).toEqual('/designs');
+      });
+    });
+
+    it('updates state appropriately after upload error', () => {
+      createComponent({ stubs: { GlEmptyState } });
+      wrapper.setData({ filesToBeSaved: [{ name: 'test' }] });
+
+      wrapper.vm.onUploadDesignError();
+      return wrapper.vm.$nextTick().then(() => {
+        expect(wrapper.vm.filesToBeSaved).toEqual([]);
+        expect(wrapper.vm.isSaving).toBeFalsy();
+        expect(createFlash).toHaveBeenCalled();
+
+        createFlash.mockReset();
+      });
+    });
+
+    it('does not call mutation if createDesign is false', () => {
+      createComponent({ createDesign: false });
+
+      wrapper.vm.onUploadDesign([]);
+
+      expect(mutate).not.toHaveBeenCalled();
+    });
+
     describe('upload count limit', () => {
       const MAXIMUM_FILE_UPLOAD_LIMIT = 10;
 

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

+import underscore from 'underscore';
 import {
   extractCurrentDiscussion,
   extractDiscussions,
   findVersionId,
+  designUploadOptimisticResponse,
 } from 'ee/design_management/utils/design_management_utils';
 
 describe('extractCurrentDiscussion', () => {
@@ -68,3 +70,36 @@ describe('version parser', () => {
     expect(findVersionId(testInvalidVersionString)).toBeUndefined();
   });
 });
+
+describe('optimistic responses', () => {
+  it('correctly generated for design upload', () => {
+    jest.spyOn(underscore, 'uniqueId').mockImplementation(() => 1);
+    const expectedResponse = {
+      __typename: 'Mutation',
+      designManagementUpload: {
+        __typename: 'DesignManagementUploadPayload',
+        designs: [
+          {
+            __typename: 'Design',
+            id: -1,
+            image: '',
+            filename: 'test',
+            fullPath: '',
+            notesCount: 0,
+            event: 'NONE',
+            diffRefs: { __typename: 'DiffRefs', baseSha: '', startSha: '', headSha: '' },
+            discussions: { __typename: 'DesignDiscussion', edges: [] },
+            versions: {
+              __typename: 'DesignVersionConnection',
+              edges: {
+                __typename: 'DesignVersionEdge',
+                node: { __typename: 'DesignVersion', id: -1, sha: -1 },
+              },
+            },
+          },
+        ],
+      },
+    };
+    expect(designUploadOptimisticResponse([{ name: 'test' }])).toEqual(expectedResponse);
+  });
+});