Merge branch 'design-management-create-todo' into 'master'

Part 1: Add todo-related queries/resolvers to Design Management

See merge request gitlab-org/gitlab!40222

app/assets/javascripts/design_management/components/design_sidebar.vue

@@ -8,7 +8,7 @@ import { extractDiscussions, extractParticipants } from '../utils/design_managem
 import { ACTIVE_DISCUSSION_SOURCE_TYPES } from '../constants';
 import DesignDiscussion from './design_notes/design_discussion.vue';
 import Participants from '~/sidebar/components/participants/participants.vue';
-import TodoButton from '~/vue_shared/components/todo_button.vue';
+import DesignTodoButton from './design_todo_button.vue';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 
 export default {
@@ -18,7 +18,7 @@ export default {
     GlCollapse,
     GlButton,
     GlPopover,
-    TodoButton,
+    DesignTodoButton,
   },
   mixins: [glFeatureFlagsMixin()],
   props: {
@@ -41,6 +41,14 @@ export default {
       discussionWithOpenForm: '',
     };
   },
+  inject: {
+    projectPath: {
+      default: '',
+    },
+    issueIid: {
+      default: '',
+    },
+  },
   computed: {
     discussions() {
       return extractDiscussions(this.design.discussions);
@@ -119,7 +127,7 @@ export default {
       class="gl-py-4 gl-mb-4 gl-display-flex gl-justify-content-space-between gl-align-items-center gl-border-b-1 gl-border-b-solid gl-border-b-gray-100"
     >
       <span>{{ __('To-Do') }}</span>
-      <todo-button issuable-type="design" :issuable-id="design.iid" />
+      <design-todo-button :design="design" @error="$emit('todoError', $event)" />
     </div>
     <h2 class="gl-font-weight-bold gl-mt-0">
       {{ issue.title }}

app/assets/javascripts/design_management/components/design_todo_button.vue

+<script>
+import todoMarkDoneMutation from '~/graphql_shared/mutations/todo_mark_done.mutation.graphql';
+import getDesignQuery from '../graphql/queries/get_design.query.graphql';
+import createDesignTodoMutation from '../graphql/mutations/create_design_todo.mutation.graphql';
+import TodoButton from '~/vue_shared/components/todo_button.vue';
+import allVersionsMixin from '../mixins/all_versions';
+import { updateStoreAfterDeleteDesignTodo } from '../utils/cache_update';
+import { findIssueId } from '../utils/design_management_utils';
+import { CREATE_DESIGN_TODO_ERROR, DELETE_DESIGN_TODO_ERROR } from '../utils/error_messages';
+
+export default {
+  components: {
+    TodoButton,
+  },
+  mixins: [allVersionsMixin],
+  props: {
+    design: {
+      type: Object,
+      required: true,
+    },
+  },
+  inject: {
+    projectPath: {
+      default: '',
+    },
+    issueIid: {
+      default: '',
+    },
+  },
+  data() {
+    return {
+      todoLoading: false,
+    };
+  },
+  computed: {
+    designVariables() {
+      return {
+        fullPath: this.projectPath,
+        iid: this.issueIid,
+        filenames: [this.$route.params.id],
+        atVersion: this.designsVersion,
+      };
+    },
+    designTodoVariables() {
+      return {
+        projectPath: this.projectPath,
+        issueId: findIssueId(this.design.issue.id),
+        issueIid: this.issueIid,
+        filenames: [this.$route.params.id],
+        atVersion: this.designsVersion,
+      };
+    },
+    pendingTodo() {
+      // TODO data structure pending BE MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40555#note_405732940
+      return this.design.currentUserTodos?.nodes[0];
+    },
+    hasPendingTodo() {
+      return Boolean(this.pendingTodo);
+    },
+  },
+  methods: {
+    createTodo() {
+      this.todoLoading = true;
+      return this.$apollo
+        .mutate({
+          mutation: createDesignTodoMutation,
+          variables: this.designTodoVariables,
+          update: (store, { data: { createDesignTodo } }) => {
+            // because this is a @client mutation,
+            // we control what is in errors, and therefore
+            // we are certain that there is at most 1 item in the array
+            const createDesignTodoError = (createDesignTodo.errors || [])[0];
+            if (createDesignTodoError) {
+              this.$emit('error', Error(createDesignTodoError.message));
+            }
+          },
+        })
+        .catch(err => {
+          this.$emit('error', Error(CREATE_DESIGN_TODO_ERROR));
+          throw err;
+        })
+        .finally(() => {
+          this.todoLoading = false;
+        });
+    },
+    deleteTodo() {
+      if (!this.hasPendingTodo) return Promise.reject();
+
+      const { id } = this.pendingTodo;
+      const { designVariables } = this;
+
+      this.todoLoading = true;
+      return this.$apollo
+        .mutate({
+          mutation: todoMarkDoneMutation,
+          variables: {
+            id,
+          },
+          update(
+            store,
+            {
+              data: { todoMarkDone },
+            },
+          ) {
+            const todoMarkDoneFirstError = (todoMarkDone.errors || [])[0];
+            if (todoMarkDoneFirstError) {
+              this.$emit('error', Error(todoMarkDoneFirstError));
+            } else {
+              updateStoreAfterDeleteDesignTodo(
+                store,
+                todoMarkDone,
+                getDesignQuery,
+                designVariables,
+              );
+            }
+          },
+        })
+        .catch(err => {
+          this.$emit('error', Error(DELETE_DESIGN_TODO_ERROR));
+          throw err;
+        })
+        .finally(() => {
+          this.todoLoading = false;
+        });
+    },
+    toggleTodo() {
+      if (this.hasPendingTodo) {
+        return this.deleteTodo();
+      }
+
+      return this.createTodo();
+    },
+  },
+};
+</script>
+
+<template>
+  <todo-button
+    issuable-type="design"
+    :issuable-id="design.iid"
+    :is-todo="hasPendingTodo"
+    :loading="todoLoading"
+    @click.stop.prevent="toggleTodo"
+  />
+</template>

app/assets/javascripts/design_management/graphql.js

@@ -3,9 +3,14 @@ import VueApollo from 'vue-apollo';
 import { uniqueId } from 'lodash';
 import produce from 'immer';
 import { defaultDataIdFromObject } from 'apollo-cache-inmemory';
+import axios from '~/lib/utils/axios_utils';
 import createDefaultClient from '~/lib/graphql';
 import activeDiscussionQuery from './graphql/queries/active_discussion.query.graphql';
+import getDesignQuery from './graphql/queries/get_design.query.graphql';
 import typeDefs from './graphql/typedefs.graphql';
+import { extractTodoIdFromDeletePath, createPendingTodo } from './utils/design_management_utils';
+import { CREATE_DESIGN_TODO_EXISTS_ERROR } from './utils/error_messages';
+import { addPendingTodoToStore } from './utils/cache_update';
 
 Vue.use(VueApollo);
 
@@ -25,6 +30,37 @@ const resolvers = {
 
       cache.writeQuery({ query: activeDiscussionQuery, data });
     },
+    createDesignTodo: (_, { projectPath, issueId, issueIid, filenames, atVersion }, { cache }) => {
+      return axios
+        .post(`/${projectPath}/todos`, {
+          issue_id: issueId,
+          issuable_id: issueIid,
+          issuable_type: 'design',
+        })
+        .then(({ data }) => {
+          const { delete_path } = data;
+          const todoId = extractTodoIdFromDeletePath(delete_path);
+          if (!todoId) {
+            return {
+              errors: [
+                {
+                  message: CREATE_DESIGN_TODO_EXISTS_ERROR,
+                },
+              ],
+            };
+          }
+
+          const pendingTodo = createPendingTodo(todoId);
+          addPendingTodoToStore(cache, pendingTodo, getDesignQuery, {
+            fullPath: projectPath,
+            iid: issueIid,
+            filenames,
+            atVersion,
+          });
+
+          return pendingTodo;
+        });
+    },
   },
 };
 

app/assets/javascripts/design_management/graphql/mutations/create_design_todo.mutation.graphql

+mutation createDesignTodo(
+  $projectPath: String!
+  $issueId: String!
+  $issueIid: String!
+  $filenames: [String]!
+  $atVersion: String
+) {
+  createDesignTodo(
+    projectPath: $projectPath
+    issueId: $issueId
+    issueIid: $issueIid
+    filenames: $filenames
+    atVersion: $atVersion
+  ) @client
+}

app/assets/javascripts/design_management/graphql/queries/get_design.query.graphql

@@ -10,6 +10,7 @@ query getDesign($fullPath: ID!, $iid: String!, $atVersion: ID, $filenames: [Stri
           nodes {
             ...DesignItem
             issue {
+              id
               title
               webPath
               webUrl

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

@@ -33,6 +33,7 @@ import {
   DESIGN_NOT_FOUND_ERROR,
   DESIGN_VERSION_NOT_EXIST_ERROR,
   UPDATE_NOTE_ERROR,
+  TOGGLE_TODO_ERROR,
   designDeletionError,
 } from '../../utils/error_messages';
 import { trackDesignDetailView } from '../../utils/tracking';
@@ -226,7 +227,7 @@ export default {
     },
     onError(message, e) {
       this.errorMessage = message;
-      throw e;
+      if (e) throw e;
     },
     onCreateImageDiffNoteError(e) {
       this.onError(ADD_IMAGE_DIFF_NOTE_ERROR, e);
@@ -246,6 +247,9 @@ export default {
     onResolveDiscussionError(e) {
       this.onError(UPDATE_IMAGE_DIFF_NOTE_ERROR, e);
     },
+    onTodoError(e) {
+      this.onError(e?.message || TOGGLE_TODO_ERROR, e);
+    },
     openCommentForm(annotationCoordinates) {
       this.annotationCoordinates = annotationCoordinates;
       if (this.$refs.newDiscussionForm) {
@@ -349,6 +353,7 @@ export default {
         @updateNoteError="onUpdateNoteError"
         @resolveDiscussionError="onResolveDiscussionError"
         @toggleResolvedComments="toggleResolvedComments"
+        @todoError="onTodoError"
       >
         <template #replyForm>
           <apollo-mutation

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

@@ -7,6 +7,7 @@ import { extractCurrentDiscussion, extractDesign, extractDesigns } from './desig
 import {
   ADD_IMAGE_DIFF_NOTE_ERROR,
   UPDATE_IMAGE_DIFF_NOTE_ERROR,
+  DELETE_DESIGN_TODO_ERROR,
   designDeletionError,
 } from './error_messages';
 
@@ -188,6 +189,30 @@ const moveDesignInStore = (store, designManagementMove, query) => {
   });
 };
 
+export const addPendingTodoToStore = (store, pendingTodo, query, queryVariables) => {
+  const data = store.readQuery({
+    query,
+    variables: queryVariables,
+  });
+
+  // TODO produce new version of data that includes the new pendingTodo.
+  // This is only possible after BE MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40555
+
+  store.writeQuery({ query, variables: queryVariables, data });
+};
+
+export const deletePendingTodoFromStore = (store, pendingTodo, query, queryVariables) => {
+  const data = store.readQuery({
+    query,
+    variables: queryVariables,
+  });
+
+  // TODO produce new version of data without the pendingTodo.
+  // This is only possible after BE MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40555
+
+  store.writeQuery({ query, variables: queryVariables, data });
+};
+
 const onError = (data, message) => {
   createFlash(message);
   throw new Error(data.errors);
@@ -243,3 +268,11 @@ export const updateDesignsOnStoreAfterReorder = (store, data, query) => {
     moveDesignInStore(store, data, query);
   }
 };
+
+export const updateStoreAfterDeleteDesignTodo = (store, data, query, queryVariables) => {
+  if (hasErrors(data)) {
+    onError(data, DELETE_DESIGN_TODO_ERROR);
+  } else {
+    deletePendingTodoFromStore(store, data, query, queryVariables);
+  }
+};

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

@@ -30,6 +30,8 @@ export const findVersionId = id => (id.match('::Version/(.+$)') || [])[1];
 
 export const findNoteId = id => (id.match('DiffNote/(.+$)') || [])[1];
 
+export const findIssueId = id => (id.match('Issue/(.+$)') || [])[1];
+
 export const extractDesigns = data => data.project.issue.designCollection.designs.nodes;
 
 export const extractDesign = data => (extractDesigns(data) || [])[0];
@@ -146,3 +148,22 @@ const normalizeAuthor = author => ({
 export const extractParticipants = users => users.map(node => normalizeAuthor(node));
 
 export const getPageLayoutElement = () => document.querySelector('.layout-page');
+
+/**
+ * Extract the ID of the To-Do for a given 'delete' path
+ * Example of todoDeletePath: /delete/1234
+ * @param {String} todoDeletePath delete_path from REST API response
+ */
+export const extractTodoIdFromDeletePath = todoDeletePath =>
+  (todoDeletePath.match('todos/([0-9]+$)') || [])[1];
+
+const createTodoGid = todoId => {
+  return `gid://gitlab/Todo/${todoId}`;
+};
+
+export const createPendingTodo = todoId => {
+  return {
+    __typename: 'Todo', // eslint-disable-line @gitlab/require-i18n-strings
+    id: createTodoGid(todoId),
+  };
+};

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

@@ -44,6 +44,14 @@ export const MOVE_DESIGN_ERROR = __(
   'Something went wrong when reordering designs. Please try again',
 );
 
+export const CREATE_DESIGN_TODO_ERROR = __('Failed to create To-Do for the design.');
+
+export const CREATE_DESIGN_TODO_EXISTS_ERROR = __('There is already a To-Do for this design.');
+
+export const DELETE_DESIGN_TODO_ERROR = __('Failed to remove To-Do for the design.');
+
+export const TOGGLE_TODO_ERROR = __('Failed to toggle To-Do for the design.');
+
 const MAX_SKIPPED_FILES_LISTINGS = 5;
 
 const oneDesignSkippedMessage = filename =>

locale/gitlab.pot

@@ -10414,6 +10414,9 @@ msgstr ""
 msgid "Failed to create Merge Request. Please try again."
 msgstr ""
 
+msgid "Failed to create To-Do for the design."
+msgstr ""
+
 msgid "Failed to create a branch for this issue. Please try again."
 msgstr ""
 
@@ -10504,6 +10507,9 @@ msgstr ""
 msgid "Failed to publish issue on status page."
 msgstr ""
 
+msgid "Failed to remove To-Do for the design."
+msgstr ""
+
 msgid "Failed to remove a Zoom meeting"
 msgstr ""
 
@@ -10546,6 +10552,9 @@ msgstr ""
 msgid "Failed to signing using smartcard authentication"
 msgstr ""
 
+msgid "Failed to toggle To-Do for the design."
+msgstr ""
+
 msgid "Failed to update branch!"
 msgstr ""
 
@@ -25123,6 +25132,9 @@ msgstr ""
 msgid "There is a limit of %{ci_project_subscriptions_limit} subscriptions from or to a project."
 msgstr ""
 
+msgid "There is already a To-Do for this design."
+msgstr ""
+
 msgid "There is already a repository with that name on disk"
 msgstr ""
 

spec/frontend/design_management/components/design_sidebar_spec.js

@@ -6,7 +6,7 @@ import Participants from '~/sidebar/components/participants/participants.vue';
 import DesignDiscussion from '~/design_management/components/design_notes/design_discussion.vue';
 import design from '../mock_data/design';
 import updateActiveDiscussionMutation from '~/design_management/graphql/mutations/update_active_discussion.mutation.graphql';
-import TodoButton from '~/vue_shared/components/todo_button.vue';
+import DesignTodoButton from '~/design_management/components/design_todo_button.vue';
 
 const scrollIntoViewMock = jest.fn();
 HTMLElement.prototype.scrollIntoView = scrollIntoViewMock;
@@ -248,7 +248,7 @@ describe('Design management design sidebar component', () => {
 
   it('does not render To-Do button by default', () => {
     createComponent();
-    expect(wrapper.find(TodoButton).exists()).toBe(false);
+    expect(wrapper.find(DesignTodoButton).exists()).toBe(false);
   });
 
   describe('when `design_management_todo_button` feature flag is enabled', () => {
@@ -260,8 +260,8 @@ describe('Design management design sidebar component', () => {
       expect(wrapper.classes()).toContain('gl-pt-0');
     });
 
-    it('renders todo_button component', () => {
-      expect(wrapper.find(TodoButton).exists()).toBe(true);
+    it('renders To-Do button', () => {
+      expect(wrapper.find(DesignTodoButton).exists()).toBe(true);
     });
   });
 });

spec/frontend/design_management/components/design_todo_button_spec.js

+import { shallowMount, mount } from '@vue/test-utils';
+import TodoButton from '~/vue_shared/components/todo_button.vue';
+import DesignTodoButton from '~/design_management/components/design_todo_button.vue';
+import createDesignTodoMutation from '~/design_management/graphql/mutations/create_design_todo.mutation.graphql';
+import todoMarkDoneMutation from '~/graphql_shared/mutations/todo_mark_done.mutation.graphql';
+import mockDesign from '../mock_data/design';
+
+const mockDesignWithPendingTodos = {
+  ...mockDesign,
+  currentUserTodos: {
+    nodes: [
+      {
+        id: 'todo-id',
+      },
+    ],
+  },
+};
+
+const mutate = jest.fn().mockResolvedValue();
+
+describe('Design management design todo button', () => {
+  let wrapper;
+
+  function createComponent(props = {}, { mountFn = shallowMount } = {}) {
+    wrapper = mountFn(DesignTodoButton, {
+      propsData: {
+        design: mockDesign,
+        ...props,
+      },
+      provide: {
+        projectPath: 'project-path',
+        issueIid: '10',
+      },
+      mocks: {
+        $route: {
+          params: {
+            id: 'my-design.jpg',
+          },
+          query: {},
+        },
+        $apollo: {
+          mutate,
+        },
+      },
+    });
+  }
+
+  beforeEach(() => {
+    createComponent();
+  });
+
+  afterEach(() => {
+    wrapper.destroy();
+    wrapper = null;
+  });
+
+  it('renders TodoButton component', () => {
+    expect(wrapper.find(TodoButton).exists()).toBe(true);
+  });
+
+  describe('when design has a pending todo', () => {
+    beforeEach(() => {
+      createComponent({ design: mockDesignWithPendingTodos }, { mountFn: mount });
+    });
+
+    it('renders correct button text', () => {
+      expect(wrapper.text()).toBe('Mark as done');
+    });
+
+    describe('when clicked', () => {
+      beforeEach(() => {
+        createComponent({ design: mockDesignWithPendingTodos }, { mountFn: mount });
+        wrapper.trigger('click');
+        return wrapper.vm.$nextTick();
+      });
+
+      it('calls `$apollo.mutate` with the `todoMarkDone` mutation and variables containing `id`', async () => {
+        const todoMarkDoneMutationVariables = {
+          mutation: todoMarkDoneMutation,
+          update: expect.anything(),
+          variables: {
+            id: 'todo-id',
+          },
+        };
+
+        expect(mutate).toHaveBeenCalledTimes(1);
+        expect(mutate).toHaveBeenCalledWith(todoMarkDoneMutationVariables);
+      });
+    });
+  });
+
+  describe('when design has no pending todos', () => {
+    beforeEach(() => {
+      createComponent({}, { mountFn: mount });
+    });
+
+    it('renders correct button text', () => {
+      expect(wrapper.text()).toBe('Add a To-Do');
+    });
+
+    describe('when clicked', () => {
+      beforeEach(() => {
+        createComponent({}, { mountFn: mount });
+        wrapper.trigger('click');
+        return wrapper.vm.$nextTick();
+      });
+
+      it('calls `$apollo.mutate` with the `createDesignTodoMutation` mutation and variables containing `issuable_id`, `issue_id`, & `projectPath`', async () => {
+        const createDesignTodoMutationVariables = {
+          mutation: createDesignTodoMutation,
+          update: expect.anything(),
+          variables: {
+            atVersion: null,
+            filenames: ['my-design.jpg'],
+            issueId: '1',
+            issueIid: '10',
+            projectPath: 'project-path',
+          },
+        };
+
+        expect(mutate).toHaveBeenCalledTimes(1);
+        expect(mutate).toHaveBeenCalledWith(createDesignTodoMutationVariables);
+      });
+    });
+  });
+});

spec/frontend/design_management/mock_data/design.js

 export default {
-  id: 'design-id',
+  id: 'gid::/gitlab/Design/1',
   filename: 'test.jpg',
   fullPath: 'full-design-path',
   image: 'test.jpg',
@@ -8,6 +8,7 @@ export default {
     name: 'test',
   },
   issue: {
+    id: 'gid::/gitlab/Issue/1',
     title: 'My precious issue',
     webPath: 'full-issue-path',
     webUrl: 'full-issue-url',

spec/frontend/design_management/pages/design/__snapshots__/index_spec.js.snap

@@ -59,11 +59,11 @@ exports[`Design management design index page renders design index 1`] = `
      
     <design-discussion-stub
       data-testid="unresolved-discussion"
-      designid="design-id"
+      designid="gid::/gitlab/Design/1"
       discussion="[object Object]"
       discussionwithopenform=""
       markdownpreviewpath="/project-path/preview_markdown?target_type=Issue"
-      noteableid="design-id"
+      noteableid="gid::/gitlab/Design/1"
     />
      
     <gl-button-stub
@@ -107,11 +107,11 @@ exports[`Design management design index page renders design index 1`] = `
     >
       <design-discussion-stub
         data-testid="resolved-discussion"
-        designid="design-id"
+        designid="gid::/gitlab/Design/1"
         discussion="[object Object]"
         discussionwithopenform=""
         markdownpreviewpath="/project-path/preview_markdown?target_type=Issue"
-        noteableid="design-id"
+        noteableid="gid::/gitlab/Design/1"
       />
     </gl-collapse-stub>