Merge branch...

Merge branch '213940-design-management-improve-design-management-image-loads-by-frontend-avoiding-refetching' into 'master'

Avoid refetching image urls for designs

See merge request gitlab-org/gitlab!30280

ee/app/assets/javascripts/design_management/components/design_destroyer.vue

 <script>
 import { ApolloMutation } from 'vue-apollo';
-import projectQuery from '../graphql/queries/project.query.graphql';
+import getDesignListQuery from '../graphql/queries/get_design_list.query.graphql';
 import destroyDesignMutation from '../graphql/mutations/destroyDesign.mutation.graphql';
 import { updateStoreAfterDesignsDelete } from '../utils/cache_update';
 
@@ -25,7 +25,7 @@ export default {
   computed: {
     projectQueryBody() {
       return {
-        query: projectQuery,
+        query: getDesignListQuery,
         variables: { fullPath: this.projectPath, iid: this.iid, atVersion: null },
       };
     },

ee/app/assets/javascripts/design_management/graphql/fragments/design.fragment.graphql

@@ -9,16 +9,12 @@ fragment DesignItem on Design {
     ...DesignDiffRefs
   }
   discussions {
-    edges {
-      node {
-        id
-        replyId
-        notes {
-          edges {
-            node {
-              ...DesignNote
-            }
-          }
+    nodes {
+      id
+      replyId
+      notes {
+        nodes {
+          ...DesignNote
         }
       }
     }

ee/app/assets/javascripts/design_management/graphql/fragments/designList.fragment.graphql

 fragment DesignListItem on Design {
   id
-  image
   event
   filename
   notesCount
+  image
   imageV432x230
 }

ee/app/assets/javascripts/design_management/graphql/queries/project.query.graphql → ee/app/assets/javascripts/design_management/graphql/queries/get_design_list.query.graphql

 #import "../fragments/designList.fragment.graphql"
 #import "../fragments/version.fragment.graphql"
 
-query project($fullPath: ID!, $iid: String!, $atVersion: ID) {
+query getDesignList($fullPath: ID!, $iid: String!, $atVersion: ID) {
   project(fullPath: $fullPath) {
     id
     issue(iid: $iid) {

ee/app/assets/javascripts/design_management/index.js

@@ -3,7 +3,7 @@ import Vue from 'vue';
 import createRouter from './router';
 import App from './components/app.vue';
 import apolloProvider from './graphql';
-import projectQuery from './graphql/queries/project.query.graphql';
+import getDesignListQuery from './graphql/queries/get_design_list.query.graphql';
 import { DESIGNS_ROUTE_NAME, ROOT_ROUTE_NAME } from './router/constants';
 
 export default () => {
@@ -29,7 +29,7 @@ export default () => {
 
   apolloProvider.clients.defaultClient
     .watchQuery({
-      query: projectQuery,
+      query: getDesignListQuery,
       variables: {
         fullPath: projectPath,
         iid: issueIid,

ee/app/assets/javascripts/design_management/mixins/all_designs.js

 import { propertyOf } from 'lodash';
 import createFlash from '~/flash';
 import { s__ } from '~/locale';
-import projectQuery from '../graphql/queries/project.query.graphql';
+import getDesignListQuery from '../graphql/queries/get_design_list.query.graphql';
 import { extractNodes } from '../utils/design_management_utils';
 import allVersionsMixin from './all_versions';
 import { DESIGNS_ROUTE_NAME } from '../router/constants';
@@ -10,7 +10,7 @@ export default {
   mixins: [allVersionsMixin],
   apollo: {
     designs: {
-      query: projectQuery,
+      query: getDesignListQuery,
       variables() {
         return {
           fullPath: this.projectPath,

ee/app/assets/javascripts/design_management/mixins/all_versions.js

-import projectQuery from '../graphql/queries/project.query.graphql';
+import getDesignListQuery from '../graphql/queries/get_design_list.query.graphql';
 import appDataQuery from '../graphql/queries/appData.query.graphql';
 import { findVersionId } from '../utils/design_management_utils';
 
@@ -13,7 +13,7 @@ export default {
       },
     },
     allVersions: {
-      query: projectQuery,
+      query: getDesignListQuery,
       variables() {
         return {
           fullPath: this.projectPath,

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

@@ -78,12 +78,18 @@ export default {
     },
     design: {
       query: getDesignQuery,
-      fetchPolicy: fetchPolicies.NETWORK_ONLY,
+      // We want to see cached design version if we have one, and fetch newer version on the background to update discussions
+      fetchPolicy: fetchPolicies.CACHE_AND_NETWORK,
       variables() {
         return this.designVariables;
       },
       update: data => extractDesign(data),
-      result({ data }) {
+      result({ data, loading }) {
+        // On the initial load with cache-and-network policy data is undefined while loading is true
+        // To prevent throwing an error, we don't perform any logic until loading is false
+        if (loading) {
+          return;
+        }
         if (!data) {
           this.onQueryError(DESIGN_NOT_FOUND_ERROR);
         }
@@ -97,8 +103,10 @@ export default {
     },
   },
   computed: {
-    isLoading() {
-      return this.$apollo.queries.design.loading;
+    isFirstLoading() {
+      // We only want to show spinner on initial design load (when opened from a deep link to design)
+      // If we already have cached a design, loading shouldn't be indicated to user
+      return this.$apollo.queries.design.loading && !this.design.filename;
     },
     discussions() {
       return extractDiscussions(this.design.discussions);
@@ -256,7 +264,7 @@ export default {
   <div
     class="design-detail js-design-detail fixed-top w-100 position-bottom-0 d-flex justify-content-center flex-column flex-lg-row"
   >
-    <gl-loading-icon v-if="isLoading" size="xl" class="align-self-center" />
+    <gl-loading-icon v-if="isFirstLoading" size="xl" class="align-self-center" />
     <template v-else>
       <div class="d-flex overflow-hidden flex-grow-1 flex-column position-relative">
         <design-destroyer

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

@@ -10,7 +10,7 @@ import DesignVersionDropdown from '../components/upload/design_version_dropdown.
 import DesignDropzone from '../components/upload/design_dropzone.vue';
 import uploadDesignMutation from '../graphql/mutations/uploadDesign.mutation.graphql';
 import permissionsQuery from '../graphql/queries/permissions.query.graphql';
-import projectQuery from '../graphql/queries/project.query.graphql';
+import getDesignListQuery from '../graphql/queries/get_design_list.query.graphql';
 import allDesignsMixin from '../mixins/all_designs';
 import {
   UPLOAD_DESIGN_ERROR,
@@ -87,7 +87,7 @@ export default {
     },
     projectQueryBody() {
       return {
-        query: projectQuery,
+        query: getDesignListQuery,
         variables: { fullPath: this.projectPath, iid: this.issueIid, atVersion: null },
       };
     },

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

+/* eslint-disable @gitlab/require-i18n-strings */
+
 import createFlash from '~/flash';
 import { extractCurrentDiscussion, extractDesign } from './design_management_utils';
 import {
@@ -53,13 +55,7 @@ const addDiscussionCommentToStore = (store, createNote, query, queryVariables, d
 
   const design = extractDesign(data);
   const currentDiscussion = extractCurrentDiscussion(design.discussions, discussionId);
-  currentDiscussion.node.notes.edges = [
-    ...currentDiscussion.node.notes.edges,
-    {
-      __typename: 'NoteEdge',
-      node: createNote.note,
-    },
-  ];
+  currentDiscussion.notes.nodes = [...currentDiscussion.notes.nodes, createNote.note];
 
   design.notesCount += 1;
   if (
@@ -72,7 +68,6 @@ const addDiscussionCommentToStore = (store, createNote, query, queryVariables, d
       {
         __typename: 'UserEdge',
         node: {
-          // eslint-disable-next-line @gitlab/require-i18n-strings
           __typename: 'User',
           ...createNote.note.author,
         },
@@ -97,27 +92,17 @@ const addImageDiffNoteToStore = (store, createImageDiffNote, query, variables) =
     variables,
   });
   const newDiscussion = {
-    __typename: 'DiscussionEdge',
-    node: {
-      // False positive i18n lint: https://gitlab.com/gitlab-org/frontend/eslint-plugin-i18n/issues/26
-      // eslint-disable-next-line @gitlab/require-i18n-strings
-      __typename: 'Discussion',
-      id: createImageDiffNote.note.discussion.id,
-      replyId: createImageDiffNote.note.discussion.replyId,
-      notes: {
-        __typename: 'NoteConnection',
-        edges: [
-          {
-            __typename: 'NoteEdge',
-            node: createImageDiffNote.note,
-          },
-        ],
-      },
+    __typename: 'Discussion',
+    id: createImageDiffNote.note.discussion.id,
+    replyId: createImageDiffNote.note.discussion.replyId,
+    notes: {
+      __typename: 'NoteConnection',
+      nodes: [createImageDiffNote.note],
     },
   };
   const design = extractDesign(data);
   const notesCount = design.notesCount + 1;
-  design.discussions.edges = [...design.discussions.edges, newDiscussion];
+  design.discussions.nodes = [...design.discussions.nodes, newDiscussion];
   if (
     !design.issue.participants.edges.some(
       participant => participant.node.username === createImageDiffNote.note.author.username,
@@ -128,7 +113,6 @@ const addImageDiffNoteToStore = (store, createImageDiffNote, query, variables) =
       {
         __typename: 'UserEdge',
         node: {
-          // eslint-disable-next-line @gitlab/require-i18n-strings
           __typename: 'User',
           ...createImageDiffNote.note.author,
         },
@@ -160,19 +144,9 @@ const updateImageDiffNoteInStore = (store, updateImageDiffNote, query, variables
     updateImageDiffNote.note.discussion.id,
   );
 
-  discussion.node = {
-    ...discussion.node,
-    notes: {
-      ...discussion.node.notes,
-      edges: [
-        // the first note is original discussion, and includes the pin `position`
-        {
-          __typename: 'NoteEdge',
-          node: updateImageDiffNote.note,
-        },
-        ...discussion.node.notes.edges.slice(1),
-      ],
-    },
+  discussion.notes = {
+    ...discussion.notes,
+    nodes: [updateImageDiffNote.note, ...discussion.notes.nodes.slice(1)],
   };
 
   store.writeQuery({

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

@@ -21,11 +21,10 @@ export const extractNodes = elements => elements.edges.map(({ node }) => node);
  */
 
 export const extractDiscussions = discussions =>
-  discussions.edges.map(discussion => {
-    const discussionNode = { ...discussion.node };
-    discussionNode.notes = extractNodes(discussionNode.notes);
-    return discussionNode;
-  });
+  discussions.nodes.map(discussion => ({
+    ...discussion,
+    notes: discussion.notes.nodes,
+  }));
 
 /**
  * Returns a discussion with the given id from discussions array
@@ -34,7 +33,7 @@ export const extractDiscussions = discussions =>
  */
 
 export const extractCurrentDiscussion = (discussions, id) =>
-  discussions.edges.find(({ node }) => node.id === id);
+  discussions.nodes.find(discussion => discussion.id === id);
 
 export const findVersionId = id => (id.match('::Version/(.+$)') || [])[1];
 
@@ -66,7 +65,7 @@ export const designUploadOptimisticResponse = files => {
     },
     discussions: {
       __typename: 'DesignDiscussion',
-      edges: [],
+      nodes: [],
     },
     versions: {
       __typename: 'DesignVersionConnection',

ee/changelogs/unreleased/213940-design-management-improve-design-management-image-loads-by-fron.yml

+---
+title: Improve design management image loads by avoiding refetching image urls for
+  designs
+merge_request: 30280
+author:
+type: changed

ee/spec/frontend/design_management/mock_data/design.js

@@ -25,27 +25,25 @@ export default {
     },
   },
   discussions: {
-    edges: [
+    nodes: [
       {
-        node: {
-          id: 'discussion-id',
-          replyId: 'discussion-reply-id',
-          notes: {
-            edges: [
-              {
-                node: {
-                  id: 'note-id',
-                  body: '123',
-                  author: {
-                    name: 'Administrator',
-                    username: 'root',
-                    webUrl: 'link-to-author',
-                    avatarUrl: 'link-to-avatar',
-                  },
+        id: 'discussion-id',
+        replyId: 'discussion-reply-id',
+        notes: {
+          edges: [
+            {
+              node: {
+                id: 'note-id',
+                body: '123',
+                author: {
+                  name: 'Administrator',
+                  username: 'root',
+                  webUrl: 'link-to-author',
+                  avatarUrl: 'link-to-avatar',
                 },
               },
-            ],
-          },
+            },
+          ],
         },
       },
     ],

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

@@ -124,7 +124,7 @@ describe('Design management design index page', () => {
         design: {
           ...design,
           discussions: {
-            edges: [],
+            nodes: [],
           },
         },
       });
@@ -160,7 +160,7 @@ describe('Design management design index page', () => {
       design: {
         ...design,
         discussions: {
-          edges: [],
+          nodes: [],
         },
       },
     });
@@ -179,7 +179,7 @@ describe('Design management design index page', () => {
       design: {
         ...design,
         discussions: {
-          edges: [],
+          nodes: [],
         },
       },
       annotationCoordinates,
@@ -206,7 +206,7 @@ describe('Design management design index page', () => {
       design: {
         ...design,
         discussions: {
-          edges: [],
+          nodes: [],
         },
       },
       annotationCoordinates,
@@ -234,7 +234,7 @@ describe('Design management design index page', () => {
         design: {
           ...design,
           discussions: {
-            edges: [],
+            nodes: [],
           },
         },
         errorMessage: 'woops',

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

@@ -210,7 +210,7 @@ describe('Design management index page', () => {
                 },
                 discussions: {
                   __typename: 'DesignDiscussion',
-                  edges: [],
+                  nodes: [],
                 },
                 versions: {
                   __typename: 'DesignVersionConnection',

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

@@ -14,20 +14,18 @@ describe('extractCurrentDiscussion', () => {
 
   beforeEach(() => {
     discussions = {
-      edges: [
-        { node: { id: 101, payload: 'w' } },
-        { node: { id: 102, payload: 'x' } },
-        { node: { id: 103, payload: 'y' } },
-        { node: { id: 104, payload: 'z' } },
+      nodes: [
+        { id: 101, payload: 'w' },
+        { id: 102, payload: 'x' },
+        { id: 103, payload: 'y' },
+        { id: 104, payload: 'z' },
       ],
     };
   });
 
   it('finds the relevant discussion if it exists', () => {
     const id = 103;
-    expect(extractCurrentDiscussion(discussions, id)).toEqual({
-      node: { id, payload: 'y' },
-    });
+    expect(extractCurrentDiscussion(discussions, id)).toEqual({ id, payload: 'y' });
   });
 
   it('returns null if the relevant discussion does not exist', () => {
@@ -40,11 +38,11 @@ describe('extractDiscussions', () => {
 
   beforeEach(() => {
     discussions = {
-      edges: [
-        { node: { id: 1, notes: { edges: [{ node: 'a' }] } } },
-        { node: { id: 2, notes: { edges: [{ node: 'b' }] } } },
-        { node: { id: 3, notes: { edges: [{ node: 'c' }] } } },
-        { node: { id: 4, notes: { edges: [{ node: 'd' }] } } },
+      nodes: [
+        { id: 1, notes: { nodes: ['a'] } },
+        { id: 2, notes: { nodes: ['b'] } },
+        { id: 3, notes: { nodes: ['c'] } },
+        { id: 4, notes: { nodes: ['d'] } },
       ],
     };
   });
@@ -91,7 +89,7 @@ describe('optimistic responses', () => {
             notesCount: 0,
             event: 'NONE',
             diffRefs: { __typename: 'DiffRefs', baseSha: '', startSha: '', headSha: '' },
-            discussions: { __typename: 'DesignDiscussion', edges: [] },
+            discussions: { __typename: 'DesignDiscussion', nodes: [] },
             versions: {
               __typename: 'DesignVersionConnection',
               edges: {