Merge branch 'psi-remove-board-list-vuex' into 'master'

Remove board list to vuex & graphql

See merge request gitlab-org/gitlab!44385

app/assets/javascripts/boards/components/board_settings_sidebar.vue

@@ -69,14 +69,18 @@ export default {
     eventHub.$off('sidebar.closeAll', this.unsetActiveId);
   },
   methods: {
-    ...mapActions(['unsetActiveId']),
+    ...mapActions(['unsetActiveId', 'removeList']),
     showScopedLabels(label) {
       return boardsStore.scopedLabels.enabled && isScopedLabel(label);
     },
     deleteBoard() {
       // eslint-disable-next-line no-alert
-      if (window.confirm(__('Are you sure you want to delete this list?'))) {
-        this.activeList.destroy();
+      if (window.confirm(__('Are you sure you want to remove this list?'))) {
+        if (this.shouldUseGraphQL) {
+          this.removeList(this.activeId);
+        } else {
+          this.activeList.destroy();
+        }
         this.unsetActiveId();
       }
     },

app/assets/javascripts/boards/components/new_list_dropdown.js

@@ -7,6 +7,7 @@ import { deprecatedCreateFlash as flash } from '~/flash';
 import CreateLabelDropdown from '../../create_label';
 import boardsStore from '../stores/boards_store';
 import { fullLabelId } from '../boards_util';
+import { getIdFromGraphQLId } from '~/graphql_shared/utils';
 import store from '~/boards/stores';
 import initDeprecatedJQueryDropdown from '~/deprecated_jquery_dropdown';
 
@@ -61,7 +62,7 @@ export default function initNewListDropdown() {
         const active = boardsStore.findListByLabelId(label.id);
         const $li = $('<li />');
         const $a = $('<a />', {
-          class: active ? `is-active js-board-list-${active.id}` : '',
+          class: active ? `is-active js-board-list-${getIdFromGraphQLId(active.id)}` : '',
           text: label.title,
           href: '#',
         });

app/assets/javascripts/boards/queries/board_list_destroy.mutation.graphql

+mutation DestroyBoardList($listId: ID!) {
+  destroyBoardList(input: { listId: $listId }) {
+    errors
+  }
+}

app/assets/javascripts/boards/stores/actions.js

@@ -18,6 +18,7 @@ import boardLabelsQuery from '../queries/board_labels.query.graphql';
 import createBoardListMutation from '../queries/board_list_create.mutation.graphql';
 import updateBoardListMutation from '../queries/board_list_update.mutation.graphql';
 import issueMoveListMutation from '../queries/issue_move_list.mutation.graphql';
+import destroyBoardListMutation from '../queries/board_list_destroy.mutation.graphql';
 import updateAssignees from '~/vue_shared/components/sidebar/queries/updateAssignees.mutation.graphql';
 import issueSetLabels from '../queries/issue_set_labels.mutation.graphql';
 import issueSetDueDate from '../queries/issue_set_due_date.mutation.graphql';
@@ -212,8 +213,26 @@ export default {
       });
   },
 
-  deleteList: () => {
-    notImplemented();
+  removeList: ({ state, commit }, listId) => {
+    const listsBackup = { ...state.boardLists };
+
+    commit(types.REMOVE_LIST, listId);
+
+    return gqlClient
+      .mutate({
+        mutation: destroyBoardListMutation,
+        variables: {
+          listId,
+        },
+      })
+      .then(({ data: { destroyBoardList: { errors } } }) => {
+        if (errors.length > 0) {
+          commit(types.REMOVE_LIST_FAILURE, listsBackup);
+        }
+      })
+      .catch(() => {
+        commit(types.REMOVE_LIST_FAILURE, listsBackup);
+      });
   },
 
   fetchIssuesForList: ({ state, commit }, { listId, fetchNext = false }) => {

app/assets/javascripts/boards/stores/boards_store.js

 /* eslint-disable no-shadow, no-param-reassign,consistent-return */
 /* global List */
 /* global ListIssue */
-import $ from 'jquery';
 import { sortBy, pick } from 'lodash';
 import Vue from 'vue';
 import Cookies from 'js-cookie';
@@ -119,8 +118,12 @@ const boardsStore = {
         // https://gitlab.com/gitlab-org/gitlab-foss/issues/30821
       });
   },
+
   updateNewListDropdown(listId) {
-    $(`.js-board-list-${listId}`).removeClass('is-active');
+    // eslint-disable-next-line no-unused-expressions
+    document
+      .querySelector(`.js-board-list-${getIdFromGraphQLId(listId)}`)
+      ?.classList.remove('is-active');
   },
   shouldAddBlankState() {
     // Decide whether to add the blank state

app/assets/javascripts/boards/stores/mutation_types.js

@@ -12,9 +12,8 @@ export const RECEIVE_ADD_LIST_SUCCESS = 'RECEIVE_ADD_LIST_SUCCESS';
 export const RECEIVE_ADD_LIST_ERROR = 'RECEIVE_ADD_LIST_ERROR';
 export const MOVE_LIST = 'MOVE_LIST';
 export const UPDATE_LIST_FAILURE = 'UPDATE_LIST_FAILURE';
-export const REQUEST_REMOVE_LIST = 'REQUEST_REMOVE_LIST';
-export const RECEIVE_REMOVE_LIST_SUCCESS = 'RECEIVE_REMOVE_LIST_SUCCESS';
-export const RECEIVE_REMOVE_LIST_ERROR = 'RECEIVE_REMOVE_LIST_ERROR';
+export const REMOVE_LIST = 'REMOVE_LIST';
+export const REMOVE_LIST_FAILURE = 'REMOVE_LIST_FAILURE';
 export const REQUEST_ISSUES_FOR_LIST = 'REQUEST_ISSUES_FOR_LIST';
 export const RECEIVE_ISSUES_FOR_LIST_FAILURE = 'RECEIVE_ISSUES_FOR_LIST_FAILURE';
 export const RECEIVE_ISSUES_FOR_LIST_SUCCESS = 'RECEIVE_ISSUES_FOR_LIST_SUCCESS';

app/assets/javascripts/boards/stores/mutations.js

@@ -93,16 +93,13 @@ export default {
     Vue.set(state, 'boardLists', backupList);
   },
 
-  [mutationTypes.REQUEST_REMOVE_LIST]: () => {
-    notImplemented();
+  [mutationTypes.REMOVE_LIST]: (state, listId) => {
+    Vue.delete(state.boardLists, listId);
   },
 
-  [mutationTypes.RECEIVE_REMOVE_LIST_SUCCESS]: () => {
-    notImplemented();
-  },
-
-  [mutationTypes.RECEIVE_REMOVE_LIST_ERROR]: () => {
-    notImplemented();
+  [mutationTypes.REMOVE_LIST_FAILURE](state, listsBackup) {
+    state.error = s__('Boards|An error occurred while removing the list. Please try again.');
+    state.boardLists = listsBackup;
   },
 
   [mutationTypes.REQUEST_ISSUES_FOR_LIST]: (state, { listId, fetchNext }) => {

app/assets/javascripts/graphql_shared/utils.js

@@ -7,7 +7,7 @@
  * @returns {Number}
  */
 export const getIdFromGraphQLId = (gid = '') =>
-  parseInt((gid || '').replace(/gid:\/\/gitlab\/.*\//g, ''), 10) || null;
+  parseInt(`${gid}`.replace(/gid:\/\/gitlab\/.*\//g, ''), 10) || null;
 
 export const MutationOperationMode = {
   Append: 'APPEND',

locale/gitlab.pot

@@ -3588,9 +3588,6 @@ msgstr ""
 msgid "Are you sure you want to delete this device? This action cannot be undone."
 msgstr ""
 
-msgid "Are you sure you want to delete this list?"
-msgstr ""
-
 msgid "Are you sure you want to delete this pipeline schedule?"
 msgstr ""
 
@@ -3641,6 +3638,9 @@ msgstr ""
 msgid "Are you sure you want to remove this identity?"
 msgstr ""
 
+msgid "Are you sure you want to remove this list?"
+msgstr ""
+
 msgid "Are you sure you want to reset registration token?"
 msgstr ""
 
@@ -4409,6 +4409,9 @@ msgstr ""
 msgid "Boards|An error occurred while moving the issue. Please try again."
 msgstr ""
 
+msgid "Boards|An error occurred while removing the list. Please try again."
+msgstr ""
+
 msgid "Boards|An error occurred while updating the list. Please try again."
 msgstr ""
 

spec/frontend/boards/mock_data.js

@@ -2,6 +2,7 @@
 /* global List */
 
 import Vue from 'vue';
+import { keyBy } from 'lodash';
 import '~/boards/models/list';
 import '~/boards/models/issue';
 import boardsStore from '~/boards/stores/boards_store';
@@ -310,6 +311,8 @@ export const mockLists = [
   },
 ];
 
+export const mockListsById = keyBy(mockLists, 'id');
+
 export const mockListsWithModel = mockLists.map(listMock =>
   Vue.observable(new List({ ...listMock, doNotFetchIssues: true })),
 );

spec/frontend/boards/stores/actions_spec.js

@@ -2,6 +2,7 @@ import testAction from 'helpers/vuex_action_helper';
 import {
   mockListsWithModel,
   mockLists,
+  mockListsById,
   mockIssue,
   mockIssueWithModel,
   mockIssue2WithModel,
@@ -13,6 +14,7 @@ import actions, { gqlClient } from '~/boards/stores/actions';
 import * as types from '~/boards/stores/mutation_types';
 import { inactiveId } from '~/boards/constants';
 import issueMoveListMutation from '~/boards/queries/issue_move_list.mutation.graphql';
+import destroyBoardListMutation from '~/boards/queries/board_list_destroy.mutation.graphql';
 import updateAssignees from '~/vue_shared/components/sidebar/queries/updateAssignees.mutation.graphql';
 import { fullBoardId, formatListIssues, formatBoardLists } from '~/boards/boards_util';
 
@@ -318,8 +320,82 @@ describe('updateList', () => {
   });
 });
 
-describe('deleteList', () => {
-  expectNotImplemented(actions.deleteList);
+describe('removeList', () => {
+  let state;
+  const list = mockLists[0];
+  const listId = list.id;
+  const mutationVariables = {
+    mutation: destroyBoardListMutation,
+    variables: {
+      listId,
+    },
+  };
+
+  beforeEach(() => {
+    state = {
+      boardLists: mockListsById,
+    };
+  });
+
+  afterEach(() => {
+    state = null;
+  });
+
+  it('optimistically deletes the list', () => {
+    const commit = jest.fn();
+
+    actions.removeList({ commit, state }, listId);
+
+    expect(commit.mock.calls).toEqual([[types.REMOVE_LIST, listId]]);
+  });
+
+  it('keeps the updated list if remove succeeds', async () => {
+    const commit = jest.fn();
+    jest.spyOn(gqlClient, 'mutate').mockResolvedValue({
+      data: {
+        destroyBoardList: {
+          errors: [],
+        },
+      },
+    });
+
+    await actions.removeList({ commit, state }, listId);
+
+    expect(gqlClient.mutate).toHaveBeenCalledWith(mutationVariables);
+    expect(commit.mock.calls).toEqual([[types.REMOVE_LIST, listId]]);
+  });
+
+  it('restores the list if update fails', async () => {
+    const commit = jest.fn();
+    jest.spyOn(gqlClient, 'mutate').mockResolvedValue(Promise.reject());
+
+    await actions.removeList({ commit, state }, listId);
+
+    expect(gqlClient.mutate).toHaveBeenCalledWith(mutationVariables);
+    expect(commit.mock.calls).toEqual([
+      [types.REMOVE_LIST, listId],
+      [types.REMOVE_LIST_FAILURE, mockListsById],
+    ]);
+  });
+
+  it('restores the list if update response has errors', async () => {
+    const commit = jest.fn();
+    jest.spyOn(gqlClient, 'mutate').mockResolvedValue({
+      data: {
+        destroyBoardList: {
+          errors: ['update failed, ID invalid'],
+        },
+      },
+    });
+
+    await actions.removeList({ commit, state }, listId);
+
+    expect(gqlClient.mutate).toHaveBeenCalledWith(mutationVariables);
+    expect(commit.mock.calls).toEqual([
+      [types.REMOVE_LIST, listId],
+      [types.REMOVE_LIST_FAILURE, mockListsById],
+    ]);
+  });
 });
 
 describe('fetchIssuesForList', () => {

spec/frontend/boards/stores/mutations_spec.js

@@ -184,16 +184,43 @@ describe('Board Store Mutations', () => {
     });
   });
 
-  describe('REQUEST_REMOVE_LIST', () => {
-    expectNotImplemented(mutations.REQUEST_REMOVE_LIST);
-  });
+  describe('REMOVE_LIST', () => {
+    it('removes list from boardLists', () => {
+      const [list, secondList] = mockListsWithModel;
+      const expected = {
+        [secondList.id]: secondList,
+      };
+      state = {
+        ...state,
+        boardLists: { ...initialBoardListsState },
+      };
 
-  describe('RECEIVE_REMOVE_LIST_SUCCESS', () => {
-    expectNotImplemented(mutations.RECEIVE_REMOVE_LIST_SUCCESS);
+      mutations[types.REMOVE_LIST](state, list.id);
+
+      expect(state.boardLists).toEqual(expected);
+    });
   });
 
-  describe('RECEIVE_REMOVE_LIST_ERROR', () => {
-    expectNotImplemented(mutations.RECEIVE_REMOVE_LIST_ERROR);
+  describe('REMOVE_LIST_FAILURE', () => {
+    it('restores lists from backup', () => {
+      const backupLists = { ...initialBoardListsState };
+
+      mutations[types.REMOVE_LIST_FAILURE](state, backupLists);
+
+      expect(state.boardLists).toEqual(backupLists);
+    });
+
+    it('sets error state', () => {
+      const backupLists = { ...initialBoardListsState };
+      state = {
+        ...state,
+        error: undefined,
+      };
+
+      mutations[types.REMOVE_LIST_FAILURE](state, backupLists);
+
+      expect(state.error).toEqual('An error occurred while removing the list. Please try again.');
+    });
   });
 
   describe('RESET_ISSUES', () => {

spec/frontend/graphql_shared/utils_spec.js

@@ -10,6 +10,10 @@ describe('getIdFromGraphQLId', () => {
       input: null,
       output: null,
     },
+    {
+      input: 2,
+      output: 2,
+    },
     {
       input: 'gid://',
       output: null,