Boards - Move issue using VueX action

Review feedback

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

@@ -3,6 +3,7 @@ import Draggable from 'vuedraggable';
 import { mapActions, mapState } from 'vuex';
 import { GlLoadingIcon } from '@gitlab/ui';
 import defaultSortableConfig from '~/sortable/sortable_config';
+import { sortableStart, sortableEnd } from '~/boards/mixins/sortable_default_options';
 import BoardNewIssue from './board_new_issue_new.vue';
 import BoardCard from './board_card.vue';
 import eventHub from '../eventhub';
@@ -146,24 +147,26 @@ export default {
       });
     },
     handleDragOnStart() {
-      document.body.classList.add('is-dragging');
+      sortableStart();
     },
     handleDragOnEnd(params) {
-      document.body.classList.remove('is-dragging');
+      sortableEnd();
       const { newIndex, oldIndex, from, to, item } = params;
       const { issueId, issueIid, issuePath } = item.dataset;
       const { children } = to;
       let moveBeforeId;
       let moveAfterId;
 
+      const getIssueId = el => Number(el.dataset.issueId);
+
       // If issue is being moved within the same list
       if (from === to) {
         if (newIndex > oldIndex && children.length > 1) {
           // If issue is being moved down we look for the issue that ends up before
-          moveBeforeId = Number(children[newIndex].dataset.issueId);
+          moveBeforeId = getIssueId(children[newIndex]);
         } else if (newIndex < oldIndex && children.length > 1) {
           // If issue is being moved up we look for the issue that ends up after
-          moveAfterId = Number(children[newIndex].dataset.issueId);
+          moveAfterId = getIssueId(children[newIndex]);
         } else {
           // If issue remains in the same list at the same position we do nothing
           return;
@@ -171,11 +174,11 @@ export default {
       } else {
         // We look for the issue that ends up before the moved issue if it exists
         if (children[newIndex - 1]) {
-          moveBeforeId = Number(children[newIndex - 1].dataset.issueId);
+          moveBeforeId = getIssueId(children[newIndex - 1]);
         }
         // We look for the issue that ends up after the moved issue if it exists
         if (children[newIndex]) {
-          moveAfterId = Number(children[newIndex].dataset.issueId);
+          moveAfterId = getIssueId(children[newIndex]);
         }
       }
 

spec/frontend/boards/board_list_new_spec.js

@@ -88,18 +88,19 @@ const createComponent = ({
 
 describe('Board list component', () => {
   let wrapper;
+  const findByTestId = testId => wrapper.find(`[data-testid="${testId}"]`);
   useFakeRequestAnimationFrame();
 
+  afterEach(() => {
+    wrapper.destroy();
+    wrapper = null;
+  });
+
   describe('When Expanded', () => {
     beforeEach(() => {
       wrapper = createComponent();
     });
 
-    afterEach(() => {
-      wrapper.destroy();
-      wrapper = null;
-    });
-
     it('renders component', () => {
       expect(wrapper.find('.board-list-component').exists()).toBe(true);
     });
@@ -109,7 +110,7 @@ describe('Board list component', () => {
         state: { listsFlags: { 'gid://gitlab/List/1': { isLoading: true } } },
       });
 
-      expect(wrapper.find('[data-testid="board_list_loading"').exists()).toBe(true);
+      expect(findByTestId('board_list_loading').exists()).toBe(true);
     });
 
     it('renders issues', () => {
@@ -173,11 +174,6 @@ describe('Board list component', () => {
       });
     });
 
-    afterEach(() => {
-      wrapper.destroy();
-      wrapper = null;
-    });
-
     it('loads more issues after scrolling', () => {
       wrapper.vm.listRef.dispatchEvent(new Event('scroll'));
 
@@ -207,11 +203,6 @@ describe('Board list component', () => {
       });
     });
 
-    afterEach(() => {
-      wrapper.destroy();
-      wrapper = null;
-    });
-
     describe('when issue count exceeds max issue count', () => {
       it('sets background to bg-danger-100', async () => {
         wrapper.setProps({ list: { issuesSize: 4, maxIssueCount: 3 } });
@@ -243,16 +234,11 @@ describe('Board list component', () => {
       wrapper = createComponent();
     });
 
-    afterEach(() => {
-      wrapper.destroy();
-      wrapper = null;
-    });
-
     describe('handleDragOnStart', () => {
       it('adds a class `is-dragging` to document body', () => {
         expect(document.body.classList.contains('is-dragging')).toBe(false);
 
-        wrapper.find(`[data-testid="tree-root-wrapper"]`).vm.$emit('start');
+        findByTestId('tree-root-wrapper').vm.$emit('start');
 
         expect(document.body.classList.contains('is-dragging')).toBe(true);
       });
@@ -263,7 +249,7 @@ describe('Board list component', () => {
         jest.spyOn(wrapper.vm, 'moveIssue').mockImplementation(() => {});
         document.body.classList.add('is-dragging');
 
-        wrapper.find(`[data-testid="tree-root-wrapper"]`).vm.$emit('end', {
+        findByTestId('tree-root-wrapper').vm.$emit('end', {
           oldIndex: 1,
           newIndex: 0,
           item: {