Commit cc2875d7 authored by Natalia Tepluhina's avatar Natalia Tepluhina Committed by Mark Florian

Revert "Split design query onto 2 separate queries"

This reverts commit 624e4433e423a661fc778a8c61261e71ab419cdd.
parent aef34407
<script> <script>
import { ApolloMutation } from 'vue-apollo'; 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 destroyDesignMutation from '../graphql/mutations/destroyDesign.mutation.graphql';
import { updateStoreAfterDesignsDelete } from '../utils/cache_update'; import { updateStoreAfterDesignsDelete } from '../utils/cache_update';
...@@ -25,7 +25,7 @@ export default { ...@@ -25,7 +25,7 @@ export default {
computed: { computed: {
projectQueryBody() { projectQueryBody() {
return { return {
query: projectQuery, query: getDesignListQuery,
variables: { fullPath: this.projectPath, iid: this.iid, atVersion: null }, variables: { fullPath: this.projectPath, iid: this.iid, atVersion: null },
}; };
}, },
......
...@@ -9,16 +9,12 @@ fragment DesignItem on Design { ...@@ -9,16 +9,12 @@ fragment DesignItem on Design {
...DesignDiffRefs ...DesignDiffRefs
} }
discussions { discussions {
edges { nodes {
node { id
id replyId
replyId notes {
notes { nodes {
edges { ...DesignNote
node {
...DesignNote
}
}
} }
} }
} }
......
fragment DesignListItem on Design { fragment DesignListItem on Design {
id id
image
event event
filename filename
notesCount notesCount
image
imageV432x230 imageV432x230
} }
#import "../fragments/designList.fragment.graphql" #import "../fragments/designList.fragment.graphql"
#import "../fragments/version.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) { project(fullPath: $fullPath) {
id id
issue(iid: $iid) { issue(iid: $iid) {
......
...@@ -3,7 +3,7 @@ import Vue from 'vue'; ...@@ -3,7 +3,7 @@ import Vue from 'vue';
import createRouter from './router'; import createRouter from './router';
import App from './components/app.vue'; import App from './components/app.vue';
import apolloProvider from './graphql'; 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'; import { DESIGNS_ROUTE_NAME, ROOT_ROUTE_NAME } from './router/constants';
export default () => { export default () => {
...@@ -29,7 +29,7 @@ export default () => { ...@@ -29,7 +29,7 @@ export default () => {
apolloProvider.clients.defaultClient apolloProvider.clients.defaultClient
.watchQuery({ .watchQuery({
query: projectQuery, query: getDesignListQuery,
variables: { variables: {
fullPath: projectPath, fullPath: projectPath,
iid: issueIid, iid: issueIid,
......
import { propertyOf } from 'lodash'; import { propertyOf } from 'lodash';
import createFlash from '~/flash'; import createFlash from '~/flash';
import { s__ } from '~/locale'; 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 { extractNodes } from '../utils/design_management_utils';
import allVersionsMixin from './all_versions'; import allVersionsMixin from './all_versions';
import { DESIGNS_ROUTE_NAME } from '../router/constants'; import { DESIGNS_ROUTE_NAME } from '../router/constants';
...@@ -10,7 +10,7 @@ export default { ...@@ -10,7 +10,7 @@ export default {
mixins: [allVersionsMixin], mixins: [allVersionsMixin],
apollo: { apollo: {
designs: { designs: {
query: projectQuery, query: getDesignListQuery,
variables() { variables() {
return { return {
fullPath: this.projectPath, fullPath: this.projectPath,
......
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 appDataQuery from '../graphql/queries/appData.query.graphql';
import { findVersionId } from '../utils/design_management_utils'; import { findVersionId } from '../utils/design_management_utils';
...@@ -13,7 +13,7 @@ export default { ...@@ -13,7 +13,7 @@ export default {
}, },
}, },
allVersions: { allVersions: {
query: projectQuery, query: getDesignListQuery,
variables() { variables() {
return { return {
fullPath: this.projectPath, fullPath: this.projectPath,
......
...@@ -78,12 +78,18 @@ export default { ...@@ -78,12 +78,18 @@ export default {
}, },
design: { design: {
query: getDesignQuery, 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() { variables() {
return this.designVariables; return this.designVariables;
}, },
update: data => extractDesign(data), 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) { if (!data) {
this.onQueryError(DESIGN_NOT_FOUND_ERROR); this.onQueryError(DESIGN_NOT_FOUND_ERROR);
} }
...@@ -97,8 +103,10 @@ export default { ...@@ -97,8 +103,10 @@ export default {
}, },
}, },
computed: { computed: {
isLoading() { isFirstLoading() {
return this.$apollo.queries.design.loading; // 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() { discussions() {
return extractDiscussions(this.design.discussions); return extractDiscussions(this.design.discussions);
...@@ -256,7 +264,7 @@ export default { ...@@ -256,7 +264,7 @@ export default {
<div <div
class="design-detail js-design-detail fixed-top w-100 position-bottom-0 d-flex justify-content-center flex-column flex-lg-row" 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> <template v-else>
<div class="d-flex overflow-hidden flex-grow-1 flex-column position-relative"> <div class="d-flex overflow-hidden flex-grow-1 flex-column position-relative">
<design-destroyer <design-destroyer
......
...@@ -10,7 +10,7 @@ import DesignVersionDropdown from '../components/upload/design_version_dropdown. ...@@ -10,7 +10,7 @@ import DesignVersionDropdown from '../components/upload/design_version_dropdown.
import DesignDropzone from '../components/upload/design_dropzone.vue'; import DesignDropzone from '../components/upload/design_dropzone.vue';
import uploadDesignMutation from '../graphql/mutations/uploadDesign.mutation.graphql'; import uploadDesignMutation from '../graphql/mutations/uploadDesign.mutation.graphql';
import permissionsQuery from '../graphql/queries/permissions.query.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 allDesignsMixin from '../mixins/all_designs';
import { import {
UPLOAD_DESIGN_ERROR, UPLOAD_DESIGN_ERROR,
...@@ -87,7 +87,7 @@ export default { ...@@ -87,7 +87,7 @@ export default {
}, },
projectQueryBody() { projectQueryBody() {
return { return {
query: projectQuery, query: getDesignListQuery,
variables: { fullPath: this.projectPath, iid: this.issueIid, atVersion: null }, variables: { fullPath: this.projectPath, iid: this.issueIid, atVersion: null },
}; };
}, },
......
/* eslint-disable @gitlab/require-i18n-strings */
import createFlash from '~/flash'; import createFlash from '~/flash';
import { extractCurrentDiscussion, extractDesign } from './design_management_utils'; import { extractCurrentDiscussion, extractDesign } from './design_management_utils';
import { import {
...@@ -53,13 +55,7 @@ const addDiscussionCommentToStore = (store, createNote, query, queryVariables, d ...@@ -53,13 +55,7 @@ const addDiscussionCommentToStore = (store, createNote, query, queryVariables, d
const design = extractDesign(data); const design = extractDesign(data);
const currentDiscussion = extractCurrentDiscussion(design.discussions, discussionId); const currentDiscussion = extractCurrentDiscussion(design.discussions, discussionId);
currentDiscussion.node.notes.edges = [ currentDiscussion.notes.nodes = [...currentDiscussion.notes.nodes, createNote.note];
...currentDiscussion.node.notes.edges,
{
__typename: 'NoteEdge',
node: createNote.note,
},
];
design.notesCount += 1; design.notesCount += 1;
if ( if (
...@@ -72,7 +68,6 @@ const addDiscussionCommentToStore = (store, createNote, query, queryVariables, d ...@@ -72,7 +68,6 @@ const addDiscussionCommentToStore = (store, createNote, query, queryVariables, d
{ {
__typename: 'UserEdge', __typename: 'UserEdge',
node: { node: {
// eslint-disable-next-line @gitlab/require-i18n-strings
__typename: 'User', __typename: 'User',
...createNote.note.author, ...createNote.note.author,
}, },
...@@ -97,27 +92,17 @@ const addImageDiffNoteToStore = (store, createImageDiffNote, query, variables) = ...@@ -97,27 +92,17 @@ const addImageDiffNoteToStore = (store, createImageDiffNote, query, variables) =
variables, variables,
}); });
const newDiscussion = { const newDiscussion = {
__typename: 'DiscussionEdge', __typename: 'Discussion',
node: { id: createImageDiffNote.note.discussion.id,
// False positive i18n lint: https://gitlab.com/gitlab-org/frontend/eslint-plugin-i18n/issues/26 replyId: createImageDiffNote.note.discussion.replyId,
// eslint-disable-next-line @gitlab/require-i18n-strings notes: {
__typename: 'Discussion', __typename: 'NoteConnection',
id: createImageDiffNote.note.discussion.id, nodes: [createImageDiffNote.note],
replyId: createImageDiffNote.note.discussion.replyId,
notes: {
__typename: 'NoteConnection',
edges: [
{
__typename: 'NoteEdge',
node: createImageDiffNote.note,
},
],
},
}, },
}; };
const design = extractDesign(data); const design = extractDesign(data);
const notesCount = design.notesCount + 1; const notesCount = design.notesCount + 1;
design.discussions.edges = [...design.discussions.edges, newDiscussion]; design.discussions.nodes = [...design.discussions.nodes, newDiscussion];
if ( if (
!design.issue.participants.edges.some( !design.issue.participants.edges.some(
participant => participant.node.username === createImageDiffNote.note.author.username, participant => participant.node.username === createImageDiffNote.note.author.username,
...@@ -128,7 +113,6 @@ const addImageDiffNoteToStore = (store, createImageDiffNote, query, variables) = ...@@ -128,7 +113,6 @@ const addImageDiffNoteToStore = (store, createImageDiffNote, query, variables) =
{ {
__typename: 'UserEdge', __typename: 'UserEdge',
node: { node: {
// eslint-disable-next-line @gitlab/require-i18n-strings
__typename: 'User', __typename: 'User',
...createImageDiffNote.note.author, ...createImageDiffNote.note.author,
}, },
...@@ -160,19 +144,9 @@ const updateImageDiffNoteInStore = (store, updateImageDiffNote, query, variables ...@@ -160,19 +144,9 @@ const updateImageDiffNoteInStore = (store, updateImageDiffNote, query, variables
updateImageDiffNote.note.discussion.id, updateImageDiffNote.note.discussion.id,
); );
discussion.node = { discussion.notes = {
...discussion.node, ...discussion.notes,
notes: { nodes: [updateImageDiffNote.note, ...discussion.notes.nodes.slice(1)],
...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),
],
},
}; };
store.writeQuery({ store.writeQuery({
......
...@@ -21,11 +21,10 @@ export const extractNodes = elements => elements.edges.map(({ node }) => node); ...@@ -21,11 +21,10 @@ export const extractNodes = elements => elements.edges.map(({ node }) => node);
*/ */
export const extractDiscussions = discussions => export const extractDiscussions = discussions =>
discussions.edges.map(discussion => { discussions.nodes.map(discussion => ({
const discussionNode = { ...discussion.node }; ...discussion,
discussionNode.notes = extractNodes(discussionNode.notes); notes: discussion.notes.nodes,
return discussionNode; }));
});
/** /**
* Returns a discussion with the given id from discussions array * Returns a discussion with the given id from discussions array
...@@ -34,7 +33,7 @@ export const extractDiscussions = discussions => ...@@ -34,7 +33,7 @@ export const extractDiscussions = discussions =>
*/ */
export const extractCurrentDiscussion = (discussions, id) => 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]; export const findVersionId = id => (id.match('::Version/(.+$)') || [])[1];
...@@ -66,7 +65,7 @@ export const designUploadOptimisticResponse = files => { ...@@ -66,7 +65,7 @@ export const designUploadOptimisticResponse = files => {
}, },
discussions: { discussions: {
__typename: 'DesignDiscussion', __typename: 'DesignDiscussion',
edges: [], nodes: [],
}, },
versions: { versions: {
__typename: 'DesignVersionConnection', __typename: 'DesignVersionConnection',
......
---
title: Improve design management image loads by avoiding refetching image urls for
designs
merge_request: 30280
author:
type: changed
...@@ -25,27 +25,25 @@ export default { ...@@ -25,27 +25,25 @@ export default {
}, },
}, },
discussions: { discussions: {
edges: [ nodes: [
{ {
node: { id: 'discussion-id',
id: 'discussion-id', replyId: 'discussion-reply-id',
replyId: 'discussion-reply-id', notes: {
notes: { edges: [
edges: [ {
{ node: {
node: { id: 'note-id',
id: 'note-id', body: '123',
body: '123', author: {
author: { name: 'Administrator',
name: 'Administrator', username: 'root',
username: 'root', webUrl: 'link-to-author',
webUrl: 'link-to-author', avatarUrl: 'link-to-avatar',
avatarUrl: 'link-to-avatar',
},
}, },
}, },
], },
}, ],
}, },
}, },
], ],
......
...@@ -124,7 +124,7 @@ describe('Design management design index page', () => { ...@@ -124,7 +124,7 @@ describe('Design management design index page', () => {
design: { design: {
...design, ...design,
discussions: { discussions: {
edges: [], nodes: [],
}, },
}, },
}); });
...@@ -160,7 +160,7 @@ describe('Design management design index page', () => { ...@@ -160,7 +160,7 @@ describe('Design management design index page', () => {
design: { design: {
...design, ...design,
discussions: { discussions: {
edges: [], nodes: [],
}, },
}, },
}); });
...@@ -179,7 +179,7 @@ describe('Design management design index page', () => { ...@@ -179,7 +179,7 @@ describe('Design management design index page', () => {
design: { design: {
...design, ...design,
discussions: { discussions: {
edges: [], nodes: [],
}, },
}, },
annotationCoordinates, annotationCoordinates,
...@@ -206,7 +206,7 @@ describe('Design management design index page', () => { ...@@ -206,7 +206,7 @@ describe('Design management design index page', () => {
design: { design: {
...design, ...design,
discussions: { discussions: {
edges: [], nodes: [],
}, },
}, },
annotationCoordinates, annotationCoordinates,
...@@ -234,7 +234,7 @@ describe('Design management design index page', () => { ...@@ -234,7 +234,7 @@ describe('Design management design index page', () => {
design: { design: {
...design, ...design,
discussions: { discussions: {
edges: [], nodes: [],
}, },
}, },
errorMessage: 'woops', errorMessage: 'woops',
......
...@@ -210,7 +210,7 @@ describe('Design management index page', () => { ...@@ -210,7 +210,7 @@ describe('Design management index page', () => {
}, },
discussions: { discussions: {
__typename: 'DesignDiscussion', __typename: 'DesignDiscussion',
edges: [], nodes: [],
}, },
versions: { versions: {
__typename: 'DesignVersionConnection', __typename: 'DesignVersionConnection',
......
...@@ -14,20 +14,18 @@ describe('extractCurrentDiscussion', () => { ...@@ -14,20 +14,18 @@ describe('extractCurrentDiscussion', () => {
beforeEach(() => { beforeEach(() => {
discussions = { discussions = {
edges: [ nodes: [
{ node: { id: 101, payload: 'w' } }, { id: 101, payload: 'w' },
{ node: { id: 102, payload: 'x' } }, { id: 102, payload: 'x' },
{ node: { id: 103, payload: 'y' } }, { id: 103, payload: 'y' },
{ node: { id: 104, payload: 'z' } }, { id: 104, payload: 'z' },
], ],
}; };
}); });
it('finds the relevant discussion if it exists', () => { it('finds the relevant discussion if it exists', () => {
const id = 103; const id = 103;
expect(extractCurrentDiscussion(discussions, id)).toEqual({ expect(extractCurrentDiscussion(discussions, id)).toEqual({ id, payload: 'y' });
node: { id, payload: 'y' },
});
}); });
it('returns null if the relevant discussion does not exist', () => { it('returns null if the relevant discussion does not exist', () => {
...@@ -40,11 +38,11 @@ describe('extractDiscussions', () => { ...@@ -40,11 +38,11 @@ describe('extractDiscussions', () => {
beforeEach(() => { beforeEach(() => {
discussions = { discussions = {
edges: [ nodes: [
{ node: { id: 1, notes: { edges: [{ node: 'a' }] } } }, { id: 1, notes: { nodes: ['a'] } },
{ node: { id: 2, notes: { edges: [{ node: 'b' }] } } }, { id: 2, notes: { nodes: ['b'] } },
{ node: { id: 3, notes: { edges: [{ node: 'c' }] } } }, { id: 3, notes: { nodes: ['c'] } },
{ node: { id: 4, notes: { edges: [{ node: 'd' }] } } }, { id: 4, notes: { nodes: ['d'] } },
], ],
}; };
}); });
...@@ -91,7 +89,7 @@ describe('optimistic responses', () => { ...@@ -91,7 +89,7 @@ describe('optimistic responses', () => {
notesCount: 0, notesCount: 0,
event: 'NONE', event: 'NONE',
diffRefs: { __typename: 'DiffRefs', baseSha: '', startSha: '', headSha: '' }, diffRefs: { __typename: 'DiffRefs', baseSha: '', startSha: '', headSha: '' },
discussions: { __typename: 'DesignDiscussion', edges: [] }, discussions: { __typename: 'DesignDiscussion', nodes: [] },
versions: { versions: {
__typename: 'DesignVersionConnection', __typename: 'DesignVersionConnection',
edges: { edges: {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment