Merge branch '4627-fix-epic-issue-reordering' into 'master'

Fix Epic issue item reordering to handle different scenarios

Closes #4627

See merge request gitlab-org/gitlab-ee!4142

app/assets/javascripts/issuable/related_issues/components/related_issues_block.vue

@@ -103,33 +103,18 @@ export default {
     toggleAddRelatedIssuesForm() {
       eventHub.$emit('toggleAddRelatedIssuesForm');
     },
-    getBeforeAfterId(newIndex, lastIndex) {
-      let beforeId = null;
-      let afterId = null;
-
-      if (newIndex === 0) {
-        // newIndex is 0, item was moved to top => send only afterId
-        afterId = this.relatedIssues[newIndex].epic_issue_id;
-      } else if (newIndex === lastIndex) {
-        // newIndex is lastIndex, item was moved to bottom => send only beforeId
-        beforeId = this.relatedIssues[newIndex].epic_issue_id;
-      } else {
-        // leave default
-        beforeId = this.relatedIssues[newIndex - 1].epic_issue_id;
-        afterId = this.relatedIssues[newIndex].epic_issue_id;
-      }
+    getBeforeAfterId(itemEl) {
+      const prevItemEl = itemEl.previousElementSibling;
+      const nextItemEl = itemEl.nextElementSibling;
 
       return {
-        beforeId,
-        afterId,
+        beforeId: prevItemEl && parseInt(prevItemEl.dataset.epicIssueId, 0),
+        afterId: nextItemEl && parseInt(nextItemEl.dataset.epicIssueId, 0),
       };
     },
     reordered(event) {
       this.removeDraggingCursor();
-      const {
-        beforeId,
-        afterId,
-      } = this.getBeforeAfterId(event.newIndex, this.relatedIssues.length - 1);
+      const { beforeId, afterId } = this.getBeforeAfterId(event.item);
 
       this.$emit('saveReorder', {
         issueId: parseInt(event.item.dataset.key, 10),
@@ -240,6 +225,7 @@ issue-count-badge-add-button btn btn-sm btn-default"
               card: canReorder
             }"
             :data-key="issue.id"
+            :data-epic-issue-id="issue.epic_issue_id"
           >
             <issue-item
               event-namespace="relatedIssue"

changelogs/unreleased-ee/4627-fix-epic-issue-reordering.yml

+---
+title: Fix Epic issue item reordering to handle different scenarios
+merge_request: 4142
+author:
+type: fixed

spec/javascripts/issuable/related_issues/components/related_issues_block_spec.js

@@ -42,6 +42,16 @@ const issuable4 = {
   state: 'opened',
 };
 
+const issuable5 = {
+  id: 204,
+  epic_issue_id: 5,
+  reference: 'foo/bar#127',
+  displayReference: '#127',
+  title: 'some other other other thing',
+  path: '/foo/bar/issues/127',
+  state: 'opened',
+};
+
 describe('RelatedIssuesBlock', () => {
   let RelatedIssuesBlock;
   let vm;
@@ -148,6 +158,7 @@ describe('RelatedIssuesBlock', () => {
             issuable2,
             issuable3,
             issuable4,
+            issuable5,
           ],
         },
       }).$mount();
@@ -160,21 +171,27 @@ describe('RelatedIssuesBlock', () => {
     });
 
     it('reorder item correctly when an item is moved to the top', () => {
-      const beforeAfterIds = vm.getBeforeAfterId(0, 3);
+      const beforeAfterIds = vm.getBeforeAfterId(vm.$el.querySelector('ul li:first-child'));
       expect(beforeAfterIds.beforeId).toBeNull();
-      expect(beforeAfterIds.afterId).toBe(1);
+      expect(beforeAfterIds.afterId).toBe(2);
     });
 
     it('reorder item correctly when an item is moved to the bottom', () => {
-      const beforeAfterIds = vm.getBeforeAfterId(3, 3);
+      const beforeAfterIds = vm.getBeforeAfterId(vm.$el.querySelector('ul li:last-child'));
       expect(beforeAfterIds.beforeId).toBe(4);
       expect(beforeAfterIds.afterId).toBeNull();
     });
 
-    it('reorder item correctly when an item is moved somewhere in the middle', () => {
-      const beforeAfterIds = vm.getBeforeAfterId(2, 3);
+    it('reorder item correctly when an item is swapped with adjecent item', () => {
+      const beforeAfterIds = vm.getBeforeAfterId(vm.$el.querySelector('ul li:nth-child(3)'));
       expect(beforeAfterIds.beforeId).toBe(2);
-      expect(beforeAfterIds.afterId).toBe(3);
+      expect(beforeAfterIds.afterId).toBe(4);
+    });
+
+    it('reorder item correctly when an item is moved somewhere in the middle', () => {
+      const beforeAfterIds = vm.getBeforeAfterId(vm.$el.querySelector('ul li:nth-child(4)'));
+      expect(beforeAfterIds.beforeId).toBe(3);
+      expect(beforeAfterIds.afterId).toBe(5);
     });
 
     it('when expanding add related issue form', () => {