Merge branch 'reorder-issues-epic' into 'master'

Add ability to reorder issues in epic

Closes #3694

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

app/assets/javascripts/boards/mixins/sortable_default_options.js

 /* eslint-disable no-unused-vars, no-mixed-operators, comma-dangle */
 /* global DocumentTouch */
 
+import sortableConfig from '../../sortable/sortable_config';
+
 window.gl = window.gl || {};
 window.gl.issueBoards = window.gl.issueBoards || {};
 
@@ -18,19 +20,14 @@ gl.issueBoards.onEnd = () => {
 gl.issueBoards.touchEnabled = ('ontouchstart' in window) || window.DocumentTouch && document instanceof DocumentTouch;
 
 gl.issueBoards.getBoardSortableDefaultOptions = (obj) => {
-  const defaultSortOptions = {
-    animation: 200,
-    forceFallback: true,
-    fallbackClass: 'is-dragging',
-    fallbackOnBody: true,
-    ghostClass: 'is-ghost',
+  const defaultSortOptions = Object.assign({}, sortableConfig, {
     filter: '.board-delete, .btn',
     delay: gl.issueBoards.touchEnabled ? 100 : 0,
     scrollSensitivity: gl.issueBoards.touchEnabled ? 60 : 100,
     scrollSpeed: 20,
     onStart: gl.issueBoards.onStart,
-    onEnd: gl.issueBoards.onEnd
-  };
+    onEnd: gl.issueBoards.onEnd,
+  });
 
   Object.keys(obj).forEach((key) => { defaultSortOptions[key] = obj[key]; });
   return defaultSortOptions;

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

+<script>
+import { __ } from '~/locale';
+import relatedIssueMixin from '../mixins/related_issues_mixin';
+
+export default {
+  name: 'IssueItem',
+  mixins: [relatedIssueMixin],
+  props: {
+    canReorder: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+  },
+  computed: {
+    stateTitle() {
+      return this.isOpen ? __('Open') : __('Closed');
+    },
+  },
+};
+</script>
+
+<template>
+  <div
+    class="flex"
+    :class="{ 'issue-info-container': !canReorder }"
+  >
+    <div class="block-truncated append-right-10">
+      <a
+        class="issue-token-title-text sortable-link"
+        :href="computedPath"
+      >
+        {{ title }}
+      </a>
+      <div class="block text-secondary">
+        <icon
+          v-if="hasState"
+          v-tooltip
+          :css-classes="iconClass"
+          :name="iconName"
+          :size="12"
+          :title="stateTitle"
+          :aria-label="state"
+        />
+        {{ displayReference }}
+      </div>
+    </div>
+    <button
+      v-if="canRemove"
+      v-tooltip
+      ref="removeButton"
+      type="button"
+      class="btn btn-default js-issue-item-remove-button flex-align-self-center flex-right"
+      title="Remove"
+      aria-label="Remove"
+      :disabled="removeDisabled"
+      @click="onRemoveRequest"
+    >
+      <i
+        class="fa fa-times"
+        aria-hidden="true"
+      >
+      </i>
+    </button>
+  </div>
+</template>

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

 <script>
-import eventHub from '../event_hub';
-import tooltip from '../../../vue_shared/directives/tooltip';
+import { __ } from '~/locale';
+import relatedIssueMixin from '../mixins/related_issues_mixin';
 
 export default {
   name: 'IssueToken',
-  data() {
-    return {
-      removeDisabled: false,
-    };
-  },
+  mixins: [relatedIssueMixin],
   props: {
-    idKey: {
-      type: Number,
-      required: true,
-    },
-    displayReference: {
-      type: String,
-      required: true,
-    },
-    eventNamespace: {
-      type: String,
-      required: false,
-      default: '',
-    },
-    title: {
-      type: String,
-      required: false,
-      default: '',
-    },
-    path: {
-      type: String,
-      required: false,
-      default: '',
-    },
-    state: {
-      type: String,
-      required: false,
-      default: '',
-    },
-    canRemove: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
     isCondensed: {
       type: Boolean,
       required: false,
       default: false,
     },
   },
-
-  directives: {
-    tooltip,
-  },
-
   computed: {
     removeButtonLabel() {
       return `Remove ${this.displayReference}`;
     },
-    hasState() {
-      return this.state && this.state.length > 0;
-    },
     stateTitle() {
       if (this.isCondensed) return '';
 
-      return this.isOpen ? 'Open' : 'Closed';
-    },
-    isOpen() {
-      return this.state === 'opened';
-    },
-    isClosed() {
-      return this.state === 'closed';
-    },
-    hasTitle() {
-      return this.title.length > 0;
-    },
-    computedLinkElementType() {
-      return this.path.length > 0 ? 'a' : 'span';
-    },
-    computedPath() {
-      return this.path.length ? this.path : null;
+      return this.isOpen ? __('Open') : __('Closed');
     },
     innerComponentType() {
       return this.isCondensed ? 'span' : 'div';
@@ -88,19 +28,6 @@ export default {
       return this.isCondensed ? this.title : '';
     },
   },
-
-  methods: {
-    onRemoveRequest() {
-      let namespacePrefix = '';
-      if (this.eventNamespace && this.eventNamespace.length > 0) {
-        namespacePrefix = `${this.eventNamespace}-`;
-      }
-
-      eventHub.$emit(`${namespacePrefix}removeRequest`, this.idKey);
-
-      this.removeDisabled = true;
-    },
-  },
 };
 </script>
 
@@ -142,18 +69,15 @@ export default {
           'issue-token-reference': isCondensed,
           'issuable-info': !isCondensed,
         }">
-        <i
-          ref="stateIcon"
+        <icon
           v-if="hasState"
           v-tooltip
-          class="fa"
-          :class="{
-            'issue-token-state-icon-open fa-circle-o': isOpen,
-            'issue-token-state-icon-closed fa-minus': isClosed,
-          }"
+          :css-classes="iconClass"
+          :name="iconName"
+          :size="12"
           :title="stateTitle"
           :aria-label="state"
-        >
+        />
         </i>{{ displayReference }}
       </component>
     </component>

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

 <script>
+import Sortable from 'vendor/Sortable';
 import loadingIcon from '~/vue_shared/components/loading_icon.vue';
 import tooltip from '~/vue_shared/directives/tooltip';
+import sortableConfig from '~/sortable/sortable_config';
 import eventHub from '../event_hub';
-import issueToken from './issue_token.vue';
+import issueItem from './issue_item.vue';
 import addIssuableForm from './add_issuable_form.vue';
 
 export default {
   name: 'RelatedIssuesBlock',
-
   props: {
     isFetching: {
       type: Boolean,
@@ -29,6 +30,11 @@ export default {
       required: false,
       default: false,
     },
+    canReorder: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
     isFormVisible: {
       type: Boolean,
       required: false,
@@ -60,17 +66,14 @@ export default {
       default: 'Related issues',
     },
   },
-
   directives: {
     tooltip,
   },
-
   components: {
     loadingIcon,
     addIssuableForm,
-    issueToken,
+    issueItem,
   },
-
   computed: {
     hasRelatedIssues() {
       return this.relatedIssues.length > 0;
@@ -88,11 +91,33 @@ export default {
       return this.helpPath.length > 0;
     },
   },
-
   methods: {
     toggleAddRelatedIssuesForm() {
       eventHub.$emit('toggleAddRelatedIssuesForm');
     },
+    reordered(event) {
+      this.removeDraggingCursor();
+
+      this.$emit('saveReorder', {
+        issueId: parseInt(event.item.dataset.key, 10),
+        beforeId: this.relatedIssues[event.newIndex - 1].epic_issue_id,
+        afterId: this.relatedIssues[event.newIndex].epic_issue_id,
+      });
+    },
+    addDraggingCursor() {
+      document.body.classList.add('is-dragging');
+    },
+    removeDraggingCursor() {
+      document.body.classList.remove('is-dragging');
+    },
+  },
+  mounted() {
+    if (this.canReorder) {
+      this.sortable = Sortable.create(this.$refs.list, Object.assign({}, sortableConfig, {
+        onStart: this.addDraggingCursor,
+        onEnd: this.reordered,
+      }));
+    }
   },
 };
 </script>
@@ -151,7 +176,8 @@ export default {
       <div
         class="related-issues-token-body panel-body"
         :class="{
-            'collapsed': !shouldShowTokenBody
+          'collapsed': !shouldShowTokenBody,
+          'sortable-container': canReorder
         }">
         <div
           v-if="isFetching"
@@ -161,12 +187,22 @@ export default {
             label="Fetching related issues" />
         </div>
         <ul
-          class="flex-list content-list issuable-list">
+          ref="list"
+          class="flex-list issuable-list"
+          :class="{ 'content-list' : !canReorder }"
+        >
           <li
             :key="issue.id"
             v-for="issue in relatedIssues"
-            class="js-related-issues-token-list-item">
-            <issue-token
+            class="js-related-issues-token-list-item"
+            :class="{
+              'user-can-drag': canReorder,
+              'sortable-row': canReorder,
+              card: canReorder
+            }"
+            :data-key="issue.id"
+          >
+            <issue-item
               event-namespace="relatedIssue"
               :id-key="issue.id"
               :display-reference="issue.reference"
@@ -174,6 +210,7 @@ export default {
               :path="issue.path"
               :state="issue.state"
               :can-remove="canAdmin"
+              :can-reorder="canReorder"
             />
           </li>
         </ul>

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

@@ -34,7 +34,6 @@ const SPACE_FACTOR = 1;
 
 export default {
   name: 'RelatedIssuesRoot',
-
   props: {
     endpoint: {
       type: String,
@@ -45,6 +44,11 @@ export default {
       required: false,
       default: false,
     },
+    canReorder: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
     helpPath: {
       type: String,
       required: false,
@@ -61,7 +65,6 @@ export default {
       default: true,
     },
   },
-
   data() {
     this.store = new RelatedIssuesStore();
 
@@ -73,24 +76,21 @@ export default {
       inputValue: '',
     };
   },
-
   components: {
     relatedIssuesBlock: RelatedIssuesBlock,
   },
-
   computed: {
     autoCompleteSources() {
       if (!this.allowAutoComplete) return {};
       return gl.GfmAutoComplete && gl.GfmAutoComplete.dataSources;
     },
   },
-
   methods: {
     onRelatedIssueRemoveRequest(idToRemove) {
       const issueToRemove = _.find(this.state.relatedIssues, issue => issue.id === idToRemove);
 
       if (issueToRemove) {
-        this.service.removeRelatedIssue(issueToRemove.destroy_relation_path)
+        RelatedIssuesService.remove(issueToRemove.relation_path)
           .then(res => res.json())
           .then((data) => {
             this.store.setRelatedIssues(data.issues);
@@ -155,7 +155,19 @@ export default {
           Flash('An error occurred while fetching issues.');
         });
     },
-
+    saveIssueOrder({ issueId, beforeId, afterId }) {
+      const issueToReorder = _.find(this.state.relatedIssues, issue => issue.id === issueId);
+
+      if (issueToReorder) {
+        RelatedIssuesService.saveOrder({
+          endpoint: issueToReorder.relation_path,
+          move_before_id: beforeId,
+          move_after_id: afterId,
+        }).catch(() => {
+          Flash('An error occurred while reordering issues.');
+        });
+      }
+    },
     onInput(newValue, caretPos) {
       const rawReferences = newValue
         .split(/\s/);
@@ -195,7 +207,6 @@ export default {
       this.inputValue = '';
     },
   },
-
   created() {
     eventHub.$on('relatedIssue-removeRequest', this.onRelatedIssueRemoveRequest);
     eventHub.$on('toggleAddRelatedIssuesForm', this.onToggleAddRelatedIssuesForm);
@@ -208,7 +219,6 @@ export default {
     this.service = new RelatedIssuesService(this.endpoint);
     this.fetchRelatedIssues();
   },
-
   beforeDestroy() {
     eventHub.$off('relatedIssue-removeRequest', this.onRelatedIssueRemoveRequest);
     eventHub.$off('toggleAddRelatedIssuesForm', this.onToggleAddRelatedIssuesForm);
@@ -228,10 +238,12 @@ export default {
     :is-submitting="isSubmitting"
     :related-issues="state.relatedIssues"
     :can-admin="canAdmin"
+    :can-reorder="canReorder"
     :pending-references="state.pendingReferences"
     :is-form-visible="isFormVisible"
     :input-value="inputValue"
     :auto-complete-sources="autoCompleteSources"
     :title="title"
+    @saveReorder="saveIssueOrder"
   />
 </template>

app/assets/javascripts/issuable/related_issues/mixins/related_issues_mixin.js

+import tooltip from '../../../vue_shared/directives/tooltip';
+import icon from '../../../vue_shared/components/icon.vue';
+import eventHub from '../event_hub';
+
+const mixins = {
+  data() {
+    return {
+      removeDisabled: false,
+    };
+  },
+  props: {
+    idKey: {
+      type: Number,
+      required: true,
+    },
+    displayReference: {
+      type: String,
+      required: true,
+    },
+    eventNamespace: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    title: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    path: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    state: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    canRemove: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+  },
+  components: {
+    icon,
+  },
+  directives: {
+    tooltip,
+  },
+  computed: {
+    hasState() {
+      return this.state && this.state.length > 0;
+    },
+    isOpen() {
+      return this.state === 'opened';
+    },
+    isClosed() {
+      return this.state === 'closed';
+    },
+    hasTitle() {
+      return this.title.length > 0;
+    },
+    iconName() {
+      return this.isOpen ? 'issue-open-m' : 'cut';
+    },
+    iconClass() {
+      return this.isOpen ? 'issue-token-state-icon-open' : 'issue-token-state-icon-closed';
+    },
+    computedLinkElementType() {
+      return this.path.length > 0 ? 'a' : 'span';
+    },
+    computedPath() {
+      return this.path.length ? this.path : null;
+    },
+  },
+  methods: {
+    onRemoveRequest() {
+      let namespacePrefix = '';
+      if (this.eventNamespace && this.eventNamespace.length > 0) {
+        namespacePrefix = `${this.eventNamespace}-`;
+      }
+
+      eventHub.$emit(`${namespacePrefix}removeRequest`, this.idKey);
+
+      this.removeDisabled = true;
+    },
+  },
+};
+
+export default mixins;

app/assets/javascripts/issuable/related_issues/services/related_issues_service.js

@@ -18,8 +18,16 @@ class RelatedIssuesService {
     });
   }
 
-  // eslint-disable-next-line class-methods-use-this
-  removeRelatedIssue(endpoint) {
+  static saveOrder({ endpoint, move_before_id, move_after_id }) {
+    return Vue.http.put(endpoint, {
+      epic: {
+        move_before_id,
+        move_after_id,
+      },
+    });
+  }
+
+  static remove(endpoint) {
     return Vue.http.delete(endpoint);
   }
 }

app/assets/javascripts/sortable/sortable_config.js

+export default {
+  animation: 200,
+  forceFallback: true,
+  fallbackClass: 'is-dragging',
+  fallbackOnBody: true,
+  ghostClass: 'is-ghost',
+};

app/assets/stylesheets/framework.scss

@@ -59,3 +59,4 @@
 @import "framework/memory_graph";
 @import "framework/responsive_tables";
 @import "framework/stacked-progress-bar";
+@import "framework/sortable";

app/assets/stylesheets/framework/common.scss

@@ -101,7 +101,7 @@ hr {
   text-overflow: ellipsis;
   white-space: nowrap;
 
-  > div,
+  > div:not(.block),
   .str-truncated {
     display: inline;
   }
@@ -475,5 +475,8 @@ img.emoji {
 .append-bottom-20 { margin-bottom: 20px; }
 .append-bottom-default { margin-bottom: $gl-padding; }
 .inline { display: inline-block; }
+.block { display: block; }
+.flex { display: flex; }
 .center { text-align: center; }
 .vertical-align-middle { vertical-align: middle; }
+.flex-align-self-center { align-self: center; }

app/assets/stylesheets/framework/sortable.scss

+.sortable-container {
+  background-color: $gray-light;
+
+  .flex-list {
+    padding: 5px;
+    margin-bottom: 0;
+  }
+}
+
+.sortable-row {
+  .flex-row {
+    display: flex;
+
+    &.issue-info-container {
+      padding-right: 0;
+    }
+  }
+
+  .sortable-link {
+    color: $black;
+  }
+}

app/assets/stylesheets/pages/boards.scss

@@ -26,6 +26,7 @@
 
 .is-ghost {
   opacity: 0.3;
+  pointer-events: none;
 }
 
 .dropdown-menu-issues-board-new {

app/assets/stylesheets/pages/issues.scss

@@ -374,18 +374,11 @@ ul.related-merge-requests > li {
   }
 }
 
-@mixin issue-token-state-icon {
-  margin-right: 0.35em;
-  font-size: 0.9em;
-}
-
 .issue-token-state-icon-open {
-  @include issue-token-state-icon;
   color: $green-600;
 }
 
 .issue-token-state-icon-closed {
-  @include issue-token-state-icon;
   color: $red-600;
 }
 

app/models/concerns/relative_positioning.rb

@@ -10,12 +10,12 @@ module RelativePositioning
     after_save :save_positionable_neighbours
   end
 
-  def project_ids
-    [project.id]
+  def min_relative_position
+    self.class.in_parents(parent_ids).minimum(:relative_position)
   end
 
   def max_relative_position
-    self.class.in_projects(project_ids).maximum(:relative_position)
+    self.class.in_parents(parent_ids).maximum(:relative_position)
   end
 
   def prev_relative_position
@@ -23,7 +23,7 @@ module RelativePositioning
 
     if self.relative_position
       prev_pos = self.class
-        .in_projects(project_ids)
+        .in_parents(parent_ids)
         .where('relative_position < ?', self.relative_position)
         .maximum(:relative_position)
     end
@@ -36,7 +36,7 @@ module RelativePositioning
 
     if self.relative_position
       next_pos = self.class
-        .in_projects(project_ids)
+        .in_parents(parent_ids)
         .where('relative_position > ?', self.relative_position)
         .minimum(:relative_position)
     end
@@ -63,7 +63,7 @@ module RelativePositioning
     pos_after = before.next_relative_position
 
     if before.shift_after?
-      issue_to_move = self.class.in_projects(project_ids).find_by!(relative_position: pos_after)
+      issue_to_move = self.class.in_parents(parent_ids).find_by!(relative_position: pos_after)
       issue_to_move.move_after
       @positionable_neighbours = [issue_to_move] # rubocop:disable Gitlab/ModuleWithInstanceVariables
 
@@ -78,7 +78,7 @@ module RelativePositioning
     pos_before = after.prev_relative_position
 
     if after.shift_before?
-      issue_to_move = self.class.in_projects(project_ids).find_by!(relative_position: pos_before)
+      issue_to_move = self.class.in_parents(parent_ids).find_by!(relative_position: pos_before)
       issue_to_move.move_before
       @positionable_neighbours = [issue_to_move] # rubocop:disable Gitlab/ModuleWithInstanceVariables
 
@@ -92,6 +92,10 @@ module RelativePositioning
     self.relative_position = position_between(max_relative_position || START_POSITION, MAX_POSITION)
   end
 
+  def move_to_start
+    self.relative_position = position_between(min_relative_position || START_POSITION, MIN_POSITION)
+  end
+
   # Indicates if there is an issue that should be shifted to free the place
   def shift_after?
     next_pos = next_relative_position

app/models/issue.rb

@@ -47,6 +47,8 @@ class Issue < ActiveRecord::Base
 
   validates :project, presence: true
 
+  alias_attribute :parent_ids, :project_id
+
   scope :in_projects, ->(project_ids) { where(project_id: project_ids) }
 
   scope :assigned, -> { where('EXISTS (SELECT TRUE FROM issue_assignees WHERE issue_id = issues.id)') }
@@ -93,6 +95,10 @@ class Issue < ActiveRecord::Base
 
   acts_as_paranoid
 
+  class << self
+    alias_method :in_parents, :in_projects
+  end
+
   def self.reference_prefix
     '#'
   end

changelogs/unreleased-ee/reorder-issues-epic.yml

+---
+title: Add support for reordering issues in epics
+merge_request:
+author:
+type: added

config/routes/group.rb

@@ -85,7 +85,7 @@ constraints(GroupUrlConstrainer.new) do
         get :realtime_changes
       end
 
-      resources :epic_issues, only: [:index, :create, :destroy], as: 'issues', path: 'issues'
+      resources :epic_issues, only: [:index, :create, :destroy, :update], as: 'issues', path: 'issues'
     end
 
     legacy_ee_group_boards_redirect = redirect do |params, request|

db/migrate/20171214115254_add_position_to_epic_issues.rb

+class AddPositionToEpicIssues < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    default_position = Gitlab::Database::MAX_INT_VALUE / 2
+    add_column_with_default(:epic_issues, :relative_position, :integer, default: default_position, allow_null: false)
+  end
+
+  def down
+    remove_column(:epic_issues, :relative_position)
+  end
+end

db/schema.rb

@@ -774,6 +774,7 @@ ActiveRecord::Schema.define(version: 20171229225929) do
   create_table "epic_issues", force: :cascade do |t|
     t.integer "epic_id", null: false
     t.integer "issue_id", null: false
+    t.integer "relative_position", default: 1073741823, null: false
   end
 
   add_index "epic_issues", ["epic_id"], name: "index_epic_issues_on_epic_id", using: :btree

doc/api/epic_issues.md

@@ -312,3 +312,100 @@ Example response:
 ```
 
 **Note**: `assignee` column is deprecated, now we show it as a single-sized array `assignees` to conform to the GitLab EE API.
+
+## Update epic - issue association
+
+Updates an epic - issue association.
+
+```
+PUT /groups/:id/-/epics/:epic_iid/issues/:epic_issue_id
+```
+
+| Attribute           | Type             | Required   | Description                                                                                          |
+| ------------------- | ---------------- | ---------- | -----------------------------------------------------------------------------------------------------|
+| `id`                | integer/string   | yes        | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) owned by the authenticated user                |
+| `epic_iid`          | integer/string   | yes        | The internal ID  of the epic.                |
+| `epic_issue_id`     | integer/string   | yes        | The ID of the issue - epic association.     |
+| `move_before_id`    | integer/string   | no         | The ID of the issue - epic association that should be placed before the link in the question.     |
+| `move_after_id`     | integer/string   | no         | The ID of the issue - epic association that should be placed after the link in the question.     |
+
+```bash
+curl --header PUT "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/groups/1/-/epics/5/issues/11?move_before_id=20
+```
+
+Example response:
+
+```json
+[
+  {
+    "id": 30,
+    "iid": 6,
+    "project_id": 8,
+    "title" : "Consequatur vero maxime deserunt laboriosam est voluptas dolorem.",
+    "description" : "Ratione dolores corrupti mollitia soluta quia.",
+    "state": "opened",
+    "created_at": "2017-11-15T13:39:24.670Z",
+    "updated_at": "2018-01-04T10:49:19.506Z",
+    "closed_at": null,
+    "labels": [],
+    "milestone": {
+      "id": 38,
+      "iid": 3,
+      "project_id": 8,
+      "title": "v2.0",
+      "description": "In tempore culpa inventore quo accusantium.",
+      "state": "closed",
+      "created_at": "2017-11-15T13:39:13.825Z",
+      "updated_at": "2017-11-15T13:39:13.825Z",
+      "due_date": null,
+      "start_date": null
+    },
+    "assignees": [{
+      "id": 7,
+      "name": "Pamella Huel",
+      "username": "arnita",
+      "state": "active",
+      "avatar_url": "http://www.gravatar.com/avatar/a2f5c6fcef64c9c69cb8779cb292be1b?s=80&d=identicon",
+      "web_url": "http://localhost:3001/arnita"
+    }],
+    "assignee": {
+      "id": 7,
+      "name": "Pamella Huel",
+      "username": "arnita",
+      "state": "active",
+      "avatar_url": "http://www.gravatar.com/avatar/a2f5c6fcef64c9c69cb8779cb292be1b?s=80&d=identicon",
+      "web_url": "http://localhost:3001/arnita"
+    },
+    "author": {
+      "id": 13,
+      "name": "Michell Johns",
+      "username": "chris_hahn",
+      "state": "active",
+      "avatar_url": "http://www.gravatar.com/avatar/30e3b2122ccd6b8e45e8e14a3ffb58fc?s=80&d=identicon",
+      "web_url": "http://localhost:3001/chris_hahn"
+    },
+    "user_notes_count": 8,
+    "upvotes": 0,
+    "downvotes": 0,
+    "due_date": null,
+    "confidential": false,
+    "weight": null,
+    "discussion_locked": null,
+    "web_url": "http://localhost:3001/h5bp/html5-boilerplate/issues/6",
+    "time_stats": {
+      "time_estimate": 0,
+      "total_time_spent": 0,
+      "human_time_estimate": null,
+      "human_total_time_spent": null
+    },
+    "_links":{
+      "self": "http://localhost:3001/api/v4/projects/8/issues/6",
+      "notes": "http://localhost:3001/api/v4/projects/8/issues/6/notes",
+      "award_emoji": "http://localhost:3001/api/v4/projects/8/issues/6/award_emoji",
+      "project": "http://localhost:3001/api/v4/projects/8"
+    },
+    "subscribed": true,
+    "epic_issue_id": 11,
+    "relative_position": 55
+  }
+]

doc/user/group/epics/index.md

@@ -34,6 +34,10 @@ When you add an issue to an epic that's already associated with another epic,
 the issue is automatically removed from the previous epic. In other words, an
 issue can be associated with at most one epic.
 
+## Reordering issues in an epic
+
+Drag and drop to reorder issues in an epic. New issues added to an epic appear at the top of the list.
+
 ## Deleting an epic
 
 NOTE: **Note:**

ee/app/assets/javascripts/epics/epic_show/components/epic_show_app.vue

@@ -140,6 +140,7 @@
       <related-issues-root
         :endpoint="issueLinksEndpoint"
         :can-admin="canAdmin"
+        :can-reorder="canAdmin"
         :allow-auto-complete="false"
         title="Issues"
       />

ee/app/controllers/groups/epic_issues_controller.rb

@@ -3,9 +3,16 @@ class Groups::EpicIssuesController < Groups::EpicsController
 
   skip_before_action :authorize_destroy_issuable!
   skip_before_action :authorize_create_epic!
+  skip_before_action :authorize_update_issuable!
 
-  before_action :authorize_admin_epic!, only: [:create, :destroy]
-  before_action :authorize_issue_link_association!, only: :destroy
+  before_action :authorize_admin_epic!, only: [:create, :destroy, :update]
+  before_action :authorize_issue_link_association!, only: [:destroy, :update]
+
+  def update
+    result = EpicIssues::UpdateService.new(link, current_user, params[:epic]).execute
+
+    render json: { message: result[:message] }, status: result[:http_status]
+  end
 
   private
 

ee/app/models/concerns/ee/relative_positioning.rb

@@ -11,7 +11,7 @@ module EE
       board_group && board_group.boards.any?
     end
 
-    def project_ids
+    def parent_ids
       return super unless has_group_boards?
 
       board_group.projects.select(:id)

ee/app/models/ee/epic.rb

@@ -85,9 +85,10 @@ module EE
     end
 
     def issues(current_user)
-      related_issues = ::Issue.select('issues.*, epic_issues.id as epic_issue_id')
+      related_issues = ::Issue.select('issues.*, epic_issues.id as epic_issue_id, epic_issues.relative_position')
         .joins(:epic_issue)
         .where("epic_issues.epic_id = #{id}")
+        .order('epic_issues.relative_position, epic_issues.id')
 
       Ability.issues_readable_by_user(related_issues, current_user)
     end

ee/app/models/epic_issue.rb

 class EpicIssue < ActiveRecord::Base
+  include RelativePositioning
+
   validates :epic, :issue, presence: true
   validates :issue, uniqueness: true
 
   belongs_to :epic
   belongs_to :issue
+
+  alias_attribute :parent_ids, :epic_id
+
+  scope :in_epic, ->(epic_id) { where(epic_id: epic_id) }
+
+  class << self
+    alias_method :in_parents, :in_epic
+  end
 end

ee/app/services/epic_issues/create_service.rb

@@ -5,7 +5,10 @@ module EpicIssues
     def relate_issues(referenced_issue)
       link = EpicIssue.find_or_initialize_by(issue: referenced_issue)
       link.epic = issuable
+      link.move_to_start
       link.save!
+
+      link
     end
 
     def create_notes(referenced_issue)

ee/app/services/epic_issues/list_service.rb

@@ -8,18 +8,22 @@ module EpicIssues
       issuable.issues(current_user)
     end
 
-    def destroy_relation_path(issue)
-      if can_destroy_issue_link?(issue)
+    def relation_path(issue)
+      if can_admin_issue_link?(issue)
         group_epic_issue_path(issuable.group, issuable.iid, issue.epic_issue_id)
       end
     end
 
-    def can_destroy_issue_link?(issue)
+    def can_admin_issue_link?(issue)
       Ability.allowed?(current_user, :admin_epic_issue, issue) && Ability.allowed?(current_user, :admin_epic, issuable)
     end
 
     def reference(issue)
       issue.to_reference(full: true)
     end
+
+    def to_hash(issue)
+      super.merge(epic_issue_id: issue.epic_issue_id)
+    end
   end
 end

ee/app/services/epic_issues/update_service.rb

+module EpicIssues
+  class UpdateService < BaseService
+    attr_reader :epic_issue, :current_user, :params
+
+    def initialize(epic_issue, user, params)
+      @epic_issue = epic_issue
+      @current_user = user
+      @params = params
+    end
+
+    def execute
+      move_issue if params[:move_after_id] || params[:move_before_id]
+      epic_issue.save!
+      success
+    rescue ActiveRecord::RecordNotFound
+      return error('Epic issue not found for given params', 404)
+    end
+
+    private
+
+    def move_issue
+      before_epic_issue = epic.epic_issues.find(params[:move_before_id]) if params[:move_before_id]
+      after_epic_issue = epic.epic_issues.find(params[:move_after_id]) if params[:move_after_id]
+
+      epic_issue.move_between(before_epic_issue, after_epic_issue)
+    end
+
+    def epic
+      epic_issue.epic
+    end
+  end
+end

ee/app/services/issuable_links/create_service.rb

@@ -19,7 +19,11 @@ module IssuableLinks
 
     def create_issue_links
       referenced_issues.each do |referenced_issue|
-        create_notes(referenced_issue) if relate_issues(referenced_issue)
+        link = relate_issues(referenced_issue)
+
+        next unless link.persisted?
+
+        create_notes(referenced_issue)
       end
     end
 

ee/app/services/issuable_links/list_service.rb

@@ -10,25 +10,29 @@ module IssuableLinks
 
     def execute
       issues.map do |referenced_issue|
-        {
-          id: referenced_issue.id,
-          title: referenced_issue.title,
-          state: referenced_issue.state,
-          reference: reference(referenced_issue),
-          path: project_issue_path(referenced_issue.project, referenced_issue.iid),
-          destroy_relation_path: destroy_relation_path(referenced_issue)
-        }
+        to_hash(referenced_issue)
       end
     end
 
     private
 
-    def destroy_relation_path(issue)
+    def relation_path(issue)
       raise NotImplementedError
     end
 
     def reference(issue)
       issue.to_reference(issuable.project)
     end
+
+    def to_hash(issue)
+      {
+        id: issue.id,
+        title: issue.title,
+        state: issue.state,
+        reference: reference(issue),
+        path: project_issue_path(issue.project, issue.iid),
+        relation_path: relation_path(issue)
+      }
+    end
   end
 end

ee/app/services/issue_links/create_service.rb

 module IssueLinks
   class CreateService < IssuableLinks::CreateService
     def relate_issues(referenced_issue)
-      IssueLink.new(source: issuable, target: referenced_issue).save
+      IssueLink.create(source: issuable, target: referenced_issue)
     end
 
     def linkable_issues(issues)

ee/app/services/issue_links/list_service.rb

 module IssueLinks
   class ListService < IssuableLinks::ListService
+    include Gitlab::Utils::StrongMemoize
+
     private
 
     def issues
       issuable.related_issues(current_user, preload: { project: :namespace })
     end
 
-    def destroy_relation_path(issue)
-      current_project = issuable.project
-
+    def relation_path(issue)
       # Make sure the user can admin both the current issue AND the
       # referenced issue projects in order to return the removal link.
-      if can_destroy_issue_link_on_current_project?(current_project) && can_destroy_issue_link?(issue.project)
+      if can_admin_issue_link_on_current_project? && can_admin_issue_link?(issue.project)
         project_issue_link_path(current_project, issuable.iid, issue.issue_link_id)
       end
     end
 
-    def can_destroy_issue_link_on_current_project?(current_project)
-      return @can_destroy_on_current_project if defined?(@can_destroy_on_current_project)
-
-      @can_destroy_on_current_project = can_destroy_issue_link?(current_project)
+    def can_admin_issue_link_on_current_project?
+      strong_memoize(:can_admin_on_current_project) do
+        can_admin_issue_link?(current_project)
+      end
     end
 
-    def can_destroy_issue_link?(project)
+    def can_admin_issue_link?(project)
       Ability.allowed?(current_user, :admin_issue_link, project)
     end
+
+    def current_project
+      issuable.project
+    end
   end
 end

ee/lib/api/epic_issues.rb

@@ -32,6 +32,35 @@ module API
     end
 
     resource :groups, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do
+      desc 'Update epic issue association' do
+      end
+      params do
+        requires :epic_iid, type: Integer, desc: 'The iid of the epic'
+        requires :epic_issue_id, type: Integer, desc: 'The id of the epic issue association to update'
+        optional :move_before_id, type: Integer, desc: 'The id of the epic issue association that should be positioned before the actual issue'
+        optional :move_after_id, type: Integer, desc: 'The id of the epic issue association that should be positioned after the actual issue'
+      end
+      put ':id/-/epics/:epic_iid/issues/:epic_issue_id' do
+        authorize_can_admin!
+
+        update_params = {
+          move_before_id: params[:move_before_id],
+          move_after_id: params[:move_after_id]
+        }
+
+        result = ::EpicIssues::UpdateService.new(link, current_user, update_params).execute
+
+        # For now we return empty body
+        # The issues list in the correct order in body will be returned as part of #4250
+        if result
+          present epic.issues(current_user),
+            with: Entities::EpicIssue,
+            current_user: current_user
+        else
+          render_api_error!({ error: "Issue could not be moved!" }, 400)
+        end
+      end
+
       desc 'Get issues assigned to the epic' do
         success Entities::EpicIssue
       end

lib/api/entities.rb

@@ -503,10 +503,12 @@ module API
 
     class EpicIssue < Issue
       expose :epic_issue_id
+      expose :relative_position
     end
 
     class EpicIssueLink < Grape::Entity
       expose :id
+      expose :relative_position
       expose :epic, using: Entities::Epic
       expose :issue, using: Entities::IssueBasic
     end

spec/ee/fixtures/api/schemas/public_api/v4/epic_issue_link.json

@@ -2,6 +2,7 @@
   "type": "object",
   "properties" : {
     "id": { "type": "integer" },
+    "relative_position": { "type": "integer" },
     "issue": { "type": "object" },
     "epic": {
       "type": "object",
@@ -32,6 +33,6 @@
         ]
      }
   },
-  "required" : [ "id", "epic", "issue" ],
+  "required" : [ "id", "epic", "issue", "relative_position" ],
   "additionalProperties": false
 }

spec/ee/fixtures/api/schemas/public_api/v4/epic_issues.json

@@ -3,9 +3,10 @@
     { "$ref": "./issues.json" },
     {
       "properties": {
-        "issue_link_id": { "type": ["integer", "null"] },
-        "position": { "type": ["integer", "null"] }
+        "epic_issue_id": { "type": ["integer"] },
+        "relative_position": { "type": ["integer"] }
       }
     }
-  ]
+  ],
+  "additionalProperties": false
 }

spec/ee/spec/controllers/groups/epic_issues_controller_spec.rb

@@ -13,31 +13,51 @@ describe Groups::EpicIssuesController do
     sign_in(user)
   end
 
-  describe 'GET #index' do
-    let!(:epic_issues) { create(:epic_issue, epic: epic, issue: issue) }
-
+  shared_examples 'unlicensed epics action' do
     before do
+      stub_licensed_features(epics: false)
       group.add_developer(user)
 
-      get :index, group_id: group, epic_id: epic.to_param
+      subject
     end
 
-    it 'returns status 200' do
-      expect(response.status).to eq(200)
+    it 'returns 400 status' do
+      expect(response).to have_gitlab_http_status(404)
     end
+  end
+
+  describe 'GET #index' do
+    let!(:epic_issue) { create(:epic_issue, epic: epic, issue: issue) }
+
+    subject { get :index, group_id: group, epic_id: epic.to_param }
+
+    it_behaves_like 'unlicensed epics action'
+
+    context 'when epics feature is enabled' do
+      before do
+        group.add_developer(user)
+
+        subject
+      end
+
+      it 'returns status 200' do
+        expect(response.status).to eq(200)
+      end
 
-    it 'returns the correct json' do
-      expected_result = [
-        {
-          'id' => issue.id,
-          'title' => issue.title,
-          'state' => issue.state,
-          'reference' => "#{project.full_path}##{issue.iid}",
-          'path' => "/#{project.full_path}/issues/#{issue.iid}",
-          'destroy_relation_path' => "/groups/#{group.full_path}/-/epics/#{epic.iid}/issues/#{epic_issues.id}"
-         }
-      ]
-      expect(JSON.parse(response.body)).to eq(expected_result)
+      it 'returns the correct json' do
+        expected_result = [
+          {
+            'id' => issue.id,
+            'title' => issue.title,
+            'state' => issue.state,
+            'reference' => "#{project.full_path}##{issue.iid}",
+            'path' => "/#{project.full_path}/issues/#{issue.iid}",
+            'relation_path' => "/groups/#{group.full_path}/-/epics/#{epic.iid}/issues/#{epic_issue.id}",
+            'epic_issue_id' => epic_issue.id
+           }
+        ]
+        expect(JSON.parse(response.body)).to eq(expected_result)
+      end
     end
   end
 
@@ -48,33 +68,37 @@ describe Groups::EpicIssuesController do
       post :create, group_id: group, epic_id: epic.to_param, issue_references: reference
     end
 
-    context 'when user has permissions to create requested association' do
-      before do
-        group.add_developer(user)
-      end
+    it_behaves_like 'unlicensed epics action'
 
-      it 'returns correct response for the correct issue reference' do
-        subject
-        list_service_response = EpicIssues::ListService.new(epic, user).execute
+    context 'when epics feature is enabled' do
+      context 'when user has permissions to create requested association' do
+        before do
+          group.add_developer(user)
+        end
 
-        expect(response).to have_gitlab_http_status(200)
-        expect(json_response).to eq('message' => nil, 'issues' => list_service_response.as_json)
-      end
+        it 'returns correct response for the correct issue reference' do
+          subject
+          list_service_response = EpicIssues::ListService.new(epic, user).execute
 
-      it 'creates a new EpicIssue record' do
-        expect { subject }.to change { EpicIssue.count }.from(0).to(1)
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response).to eq('message' => nil, 'issues' => list_service_response.as_json)
+        end
+
+        it 'creates a new EpicIssue record' do
+          expect { subject }.to change { EpicIssue.count }.from(0).to(1)
+        end
       end
-    end
 
-    context 'when user does not have permissions to create requested association' do
-      it 'returns correct response for the correct issue reference' do
-        subject
+      context 'when user does not have permissions to create requested association' do
+        it 'returns correct response for the correct issue reference' do
+          subject
 
-        expect(response).to have_gitlab_http_status(403)
-      end
+          expect(response).to have_gitlab_http_status(403)
+        end
 
-      it 'does not create a new EpicIssue record' do
-        expect { subject }.not_to change { EpicIssue.count }.from(0)
+        it 'does not create a new EpicIssue record' do
+          expect { subject }.not_to change { EpicIssue.count }.from(0)
+        end
       end
     end
   end
@@ -82,65 +106,131 @@ describe Groups::EpicIssuesController do
   describe 'DELETE #destroy' do
     let!(:epic_issue) { create(:epic_issue, epic: epic, issue: issue) }
 
-    subject do
-      delete :destroy, group_id: group, epic_id: epic.to_param, id: epic_issue.id
-    end
+    subject { delete :destroy, group_id: group, epic_id: epic.to_param, id: epic_issue.id }
 
-    context 'when user has permissions to detele the link' do
-      before do
-        group.add_developer(user)
-      end
+    it_behaves_like 'unlicensed epics action'
 
-      it 'returns status 200' do
-        subject
+    context 'when epics feature is enabled' do
+      context 'when user has permissions to delete the link' do
+        before do
+          group.add_developer(user)
+        end
 
-        expect(response.status).to eq(200)
+        it 'returns status 200' do
+          subject
+
+          expect(response.status).to eq(200)
+        end
+
+        it 'destroys the link' do
+          expect { subject }.to change { EpicIssue.count }.from(1).to(0)
+        end
       end
 
-      it 'destroys the link' do
-        expect { subject }.to change { EpicIssue.count }.from(1).to(0)
+      context 'when user does not have permissions to delete the link' do
+        it 'returns status 404' do
+          subject
+
+          expect(response.status).to eq(403)
+        end
+
+        it 'does not destroy the link' do
+          expect { subject }.not_to change { EpicIssue.count }.from(1)
+        end
       end
-    end
 
-    context 'when user does not have permissions to delete the link' do
-      it 'returns status 404' do
-        subject
+      context 'when the epic from the association does not equal epic from the path' do
+        subject do
+          delete :destroy, group_id: group, epic_id: another_epic.to_param, id: epic_issue.id
+        end
+
+        let(:another_epic) { create(:epic, group: group) }
+
+        before do
+          group.add_developer(user)
+        end
 
-        expect(response.status).to eq(403)
+        it 'returns status 404' do
+          subject
+
+          expect(response.status).to eq(404)
+        end
+
+        it 'does not destroy the link' do
+          expect { subject }.not_to change { EpicIssue.count }.from(1)
+        end
       end
 
-      it 'does not destroy the link' do
-        expect { subject }.not_to change { EpicIssue.count }.from(1)
+      context 'when the epic_issue record does not exists' do
+        it 'returns status 404' do
+          delete :destroy, group_id: group, epic_id: epic.to_param, id: 9999
+
+          expect(response.status).to eq(403)
+        end
       end
     end
+  end
 
-    context 'when the epic from the association does not equal epic from the path' do
-      subject do
-        delete :destroy, group_id: group, epic_id: another_epic.to_param, id: epic_issue.id
-      end
+  describe 'PUT #update' do
+    let(:issue2) { create(:issue, project: project) }
+    let!(:epic_issue1) { create(:epic_issue, epic: epic, issue: issue, relative_position: 1) }
+    let!(:epic_issue2) { create(:epic_issue, epic: epic, issue: issue2, relative_position: 2) }
 
-      let(:another_epic) { create(:epic, group: group) }
+    subject do
+      put :update, group_id: group, epic_id: epic.to_param, id: epic_issue1.id, epic: { move_before_id: epic_issue2.id }
+    end
 
-      before do
-        group.add_developer(user)
+    it_behaves_like 'unlicensed epics action'
+
+    context 'when epics feature is enabled' do
+      context 'when user has permissions to admin the epic' do
+        before do
+          group.add_developer(user)
+        end
+
+        it 'returns status 200' do
+          subject
+
+          expect(response.status).to eq(200)
+        end
+
+        it 'updates the issue position value' do
+          expect { subject }.to change { epic_issue1.reload.relative_position }
+        end
       end
 
-      it 'returns status 404' do
-        subject
+      context 'when user does not have permissions to admin the epic' do
+        it 'returns status 404' do
+          subject
 
-        expect(response.status).to eq(404)
+          expect(response.status).to eq(403)
+        end
       end
 
-      it 'does not destroy the link' do
-        expect { subject }.not_to change { EpicIssue.count }.from(1)
+      context 'when the epic from the association does not equal epic from the path' do
+        subject do
+          put :update, group_id: group, epic_id: another_epic.to_param, id: epic_issue1.id, epic: { after_move_id: epic_issue1.id }
+        end
+
+        let(:another_epic) { create(:epic, group: group) }
+
+        before do
+          group.add_developer(user)
+        end
+
+        it 'returns status 404' do
+          subject
+
+          expect(response.status).to eq(404)
+        end
       end
-    end
 
-    context 'when the epic_issue record does not exists' do
-      it 'returns status 404' do
-        delete :destroy, group_id: group, epic_id: epic.to_param, id: 9999
+      context 'when the epic_issue record does not exists' do
+        it 'returns status 404' do
+          delete :destroy, group_id: group, epic_id: epic.to_param, id: 9999
 
-        expect(response.status).to eq(403)
+          expect(response.status).to eq(403)
+        end
       end
     end
   end

spec/ee/spec/features/epics/epic_issues_spec.rb

 require 'spec_helper'
 
 describe 'Epic Issues', :js do
+  include DragTo
+
   let(:user) { create(:user) }
   let(:group) { create(:group, :public) }
   let(:epic) { create(:epic, group: group) }
@@ -11,8 +13,8 @@ describe 'Epic Issues', :js do
 
   let!(:epic_issues) do
     [
-      create(:epic_issue, epic: epic, issue: public_issue),
-      create(:epic_issue, epic: epic, issue: private_issue)
+      create(:epic_issue, epic: epic, issue: public_issue, relative_position: 1),
+      create(:epic_issue, epic: epic, issue: private_issue, relative_position: 2)
     ]
   end
 
@@ -33,13 +35,17 @@ describe 'Epic Issues', :js do
       within('.related-issues-block ul.issuable-list') do
         expect(page).to have_selector('li', count: 1)
         expect(page).to have_content(public_issue.title)
-        expect(page).not_to have_selector('button.js-issue-token-remove-button')
+        expect(page).not_to have_selector('button.js-issue-item-remove-button')
       end
     end
 
     it 'user cannot add new issues to the epic' do
       expect(page).not_to have_selector('.related-issues-block h3.panel-title button')
     end
+
+    it 'user cannot reorder issues in epic' do
+      expect(page).not_to have_selector('.js-related-issues-token-list-item.user-can-drag')
+    end
   end
 
   context 'when user is a group member' do
@@ -65,7 +71,7 @@ describe 'Epic Issues', :js do
         expect(page).to have_content(public_issue.title)
         expect(page).to have_content(private_issue.title)
 
-        first('li button.js-issue-token-remove-button').click
+        first('li button.js-issue-item-remove-button').click
       end
 
       wait_for_requests
@@ -95,5 +101,15 @@ describe 'Epic Issues', :js do
         expect(page).to have_content(issue_to_add.title)
       end
     end
+
+    it 'user can reorder issues in epic' do
+      expect(first('.js-related-issues-token-list-item')).to have_content(public_issue.title)
+      expect(page.all('.js-related-issues-token-list-item').last).to have_content(private_issue.title)
+
+      drag_to(selector: '.issuable-list', to_index: 1)
+
+      expect(first('.js-related-issues-token-list-item')).to have_content(private_issue.title)
+      expect(page.all('.js-related-issues-token-list-item').last).to have_content(public_issue.title)
+    end
   end
 end

spec/ee/spec/requests/api/epic_issues_spec.rb

@@ -228,4 +228,78 @@ describe API::EpicIssues do
       end
     end
   end
+
+  describe 'PUT /groups/:id/-/epics/:epic_iid/issues/:epic_issue_id' do
+    let(:issues) { create_list(:issue, 2, project: project) }
+    let!(:epic_issue1) { create(:epic_issue, epic: epic, issue: issues[0], relative_position: 1) }
+    let!(:epic_issue2) { create(:epic_issue, epic: epic, issue: issues[1], relative_position: 2) }
+
+    let(:url) { "/groups/#{group.path}/-/epics/#{epic.iid}/issues/#{epic_issue1.id}?move_after_id=#{epic_issue2.id}" }
+
+    context 'when epics feature is disabled' do
+      it 'returns 403 forbidden error' do
+        group.add_developer(user)
+        put api(url, user)
+
+        expect(response).to have_gitlab_http_status(403)
+      end
+    end
+
+    context 'when epics feature is enabled' do
+      before do
+        stub_licensed_features(epics: true)
+      end
+
+      context 'when an error occurs' do
+        it 'returns 401 unauthorized error for non authenticated user' do
+          put api(url)
+
+          expect(response).to have_gitlab_http_status(401)
+        end
+
+        it 'returns 404 not found error for a user without permissions to see the group' do
+          project.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+          group.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+          put api(url, user)
+
+          expect(response).to have_gitlab_http_status(404)
+        end
+
+        it 'returns 403 forbidden error for a user who can not move the issue' do
+          put api(url, user)
+
+          expect(response).to have_gitlab_http_status(403)
+        end
+
+        it 'returns 404 not found error for the link of another epic' do
+          group.add_developer(user)
+          another_epic = create(:epic, group: group)
+          url = "/groups/#{group.path}/-/epics/#{another_epic.iid}/issues/#{epic_issue1.id}?move_after_id=#{epic_issue2.id}"
+
+          put api(url, user)
+
+          expect(response).to have_gitlab_http_status(404)
+        end
+      end
+
+      context 'when the request is correct' do
+        before do
+          group.add_developer(user)
+          put api(url, user)
+        end
+
+        it 'returns 200 status' do
+          expect(response).to have_gitlab_http_status(200)
+        end
+
+        it 'updates the positions values' do
+          expect(epic_issue1.reload.relative_position).to be < epic_issue2.relative_position
+        end
+
+        it 'matches the response schema' do
+          expect(response).to match_response_schema('public_api/v4/epic_issues', dir: 'ee')
+        end
+      end
+    end
+  end
 end

spec/ee/spec/services/epic_issues/create_service_spec.rb

@@ -2,13 +2,17 @@ require 'spec_helper'
 
 describe EpicIssues::CreateService do
   describe '#execute' do
-    let(:group) { create :group }
-    let(:epic) { create :epic, group: group }
+    let(:group) { create(:group) }
+    let(:epic) { create(:epic, group: group) }
     let(:project) { create(:project, group: group) }
-    let(:issue) { create :issue, project: project }
-    let(:user) { create :user }
+    let(:issue) { create(:issue, project: project) }
+    let(:issue2) { create(:issue, project: project) }
+    let(:issue3) { create(:issue, project: project) }
+    let(:user) { create(:user) }
     let(:valid_reference) { issue.to_reference(full: true) }
 
+    let!(:existing_link) { create(:epic_issue, epic: epic, issue: issue3) }
+
     def assign_issue(references)
       params = { issue_references: references }
 
@@ -16,10 +20,18 @@ describe EpicIssues::CreateService do
     end
 
     shared_examples 'returns success' do
-      it 'creates relationships' do
-        expect { subject }.to change(EpicIssue, :count).from(0).to(1)
+      let(:created_link) { EpicIssue.find_by!(issue_id: issue.id) }
+
+      it 'creates a new relationship' do
+        expect { subject }.to change(EpicIssue, :count).from(1).to(2)
+
+        expect(created_link).to have_attributes(epic: epic)
+      end
+
+      it 'orders the epic issue to the first place and moves the existing ones down' do
+        subject
 
-        expect(EpicIssue.find_by!(issue_id: issue.id)).to have_attributes(epic: epic)
+        expect(created_link.relative_position).to be < existing_link.reload.relative_position
       end
 
       it 'returns success status' do
@@ -105,6 +117,12 @@ describe EpicIssues::CreateService do
               allow(SystemNoteService).to receive(:epic_issue)
               allow(SystemNoteService).to receive(:issue_on_epic)
 
+              # Extractor makes a permission check for each issue which messes up the query count check
+              extractor = double
+              allow(Gitlab::ReferenceExtractor).to receive(:new).and_return(extractor)
+              allow(extractor).to receive(:analyze)
+              allow(extractor).to receive(:issues).and_return([issue])
+
               params = { issue_references: [valid_reference] }
               control_count = ActiveRecord::QueryRecorder.new { described_class.new(epic, user, params).execute }.count
 
@@ -115,9 +133,15 @@ describe EpicIssues::CreateService do
               epic = create(:epic, group: group)
               group.add_developer(user)
 
+              allow(extractor).to receive(:issues).and_return(issues)
               params = { issue_references: issues.map { |i| i.to_reference(full: true) } }
 
-              expect { described_class.new(epic, user, params).execute }.not_to exceed_query_limit(control_count)
+              # threshold 24 because 6 queries are generated for each insert
+              # (savepoint, find, exists, relative_position get, insert, release savepoint)
+              # and we insert 5 issues instead of 1 which we do for control count
+              expect { described_class.new(epic, user, params).execute }
+                .not_to exceed_query_limit(control_count)
+                .with_threshold(24)
             end
           end
 
@@ -136,6 +160,35 @@ describe EpicIssues::CreateService do
 
             include_examples 'returns success'
           end
+
+          context 'when multiple valid issues are given' do
+            subject { assign_issue([issue.to_reference(full: true), issue2.to_reference(full: true)]) }
+
+            let(:created_link1) { EpicIssue.find_by!(issue_id: issue.id) }
+            let(:created_link2) { EpicIssue.find_by!(issue_id: issue2.id) }
+
+            it 'creates new relationships' do
+              expect { subject }.to change(EpicIssue, :count).from(1).to(3)
+
+              expect(created_link1).to have_attributes(epic: epic)
+              expect(created_link2).to have_attributes(epic: epic)
+            end
+
+            it 'orders the epic issues to the first place and moves the existing ones down' do
+              subject
+
+              expect(created_link2.relative_position).to be < created_link1.relative_position
+              expect(created_link1.relative_position).to be < existing_link.reload.relative_position
+            end
+
+            it 'returns success status' do
+              expect(subject).to eq(status: :success)
+            end
+
+            it 'creates 2 system notes for each issue' do
+              expect { subject }.to change { Note.count }.from(0).to(4)
+            end
+          end
         end
       end
 
@@ -160,7 +213,7 @@ describe EpicIssues::CreateService do
         end
 
         it 'does not create a new association' do
-          expect { subject }.not_to change(EpicIssue, :count).from(1)
+          expect { subject }.not_to change(EpicIssue, :count).from(2)
         end
 
         it 'updates the existing association' do

spec/ee/spec/services/epic_issues/list_service_spec.rb

@@ -11,9 +11,9 @@ describe EpicIssues::ListService do
   let(:issue2) { create :issue, project: project }
   let(:issue3) { create :issue, project: other_project }
 
-  let!(:epic_issue1) { create(:epic_issue, issue: issue1, epic: epic) }
-  let!(:epic_issue2) { create(:epic_issue, issue: issue2, epic: epic) }
-  let!(:epic_issue3) { create(:epic_issue, issue: issue3, epic: epic) }
+  let!(:epic_issue1) { create(:epic_issue, issue: issue1, epic: epic, relative_position: 2) }
+  let!(:epic_issue2) { create(:epic_issue, issue: issue2, epic: epic, relative_position: 1) }
+  let!(:epic_issue3) { create(:epic_issue, issue: issue3, epic: epic, relative_position: 3) }
 
   describe '#execute' do
     subject { described_class.new(epic, user).execute }
@@ -38,21 +38,23 @@ describe EpicIssues::ListService do
 
         it 'returns related issues JSON' do
           expected_result = [
-            {
-              id: issue1.id,
-              title: issue1.title,
-              state: issue1.state,
-              reference: issue1.to_reference(full: true),
-              path: "/#{project.full_path}/issues/#{issue1.iid}",
-              destroy_relation_path: "/groups/#{group.full_path}/-/epics/#{epic.iid}/issues/#{epic_issue1.id}"
-            },
             {
               id: issue2.id,
               title: issue2.title,
               state: issue2.state,
               reference: issue2.to_reference(full: true),
               path: "/#{project.full_path}/issues/#{issue2.iid}",
-              destroy_relation_path: "/groups/#{group.full_path}/-/epics/#{epic.iid}/issues/#{epic_issue2.id}"
+              relation_path: "/groups/#{group.full_path}/-/epics/#{epic.iid}/issues/#{epic_issue2.id}",
+              epic_issue_id: epic_issue2.id
+            },
+            {
+              id: issue1.id,
+              title: issue1.title,
+              state: issue1.state,
+              reference: issue1.to_reference(full: true),
+              path: "/#{project.full_path}/issues/#{issue1.iid}",
+              relation_path: "/groups/#{group.full_path}/-/epics/#{epic.iid}/issues/#{epic_issue1.id}",
+              epic_issue_id: epic_issue1.id
             },
             {
               id: issue3.id,
@@ -60,10 +62,11 @@ describe EpicIssues::ListService do
               state: issue3.state,
               reference: issue3.to_reference(full: true),
               path: "/#{other_project.full_path}/issues/#{issue3.iid}",
-              destroy_relation_path: "/groups/#{group.full_path}/-/epics/#{epic.iid}/issues/#{epic_issue3.id}"
+              relation_path: "/groups/#{group.full_path}/-/epics/#{epic.iid}/issues/#{epic_issue3.id}",
+              epic_issue_id: epic_issue3.id
             }
           ]
-          expect(subject).to match_array(expected_result)
+          expect(subject).to eq(expected_result)
         end
       end
 
@@ -74,25 +77,27 @@ describe EpicIssues::ListService do
 
         it 'returns related issues JSON' do
           expected_result = [
-            {
-              id: issue1.id,
-              title: issue1.title,
-              state: issue1.state,
-              reference: issue1.to_reference(full: true),
-              path: "/#{project.full_path}/issues/#{issue1.iid}",
-              destroy_relation_path: nil
-            },
             {
               id: issue2.id,
               title: issue2.title,
               state: issue2.state,
               reference: issue2.to_reference(full: true),
               path: "/#{project.full_path}/issues/#{issue2.iid}",
-              destroy_relation_path: nil
+              relation_path: nil,
+              epic_issue_id: epic_issue2.id
+            },
+            {
+              id: issue1.id,
+              title: issue1.title,
+              state: issue1.state,
+              reference: issue1.to_reference(full: true),
+              path: "/#{project.full_path}/issues/#{issue1.iid}",
+              relation_path: nil,
+              epic_issue_id: epic_issue1.id
             }
           ]
 
-          expect(subject).to match_array(expected_result)
+          expect(subject).to eq(expected_result)
         end
       end
     end

spec/ee/spec/services/epic_issues/update_service_spec.rb

+require 'spec_helper'
+
+describe EpicIssues::UpdateService do
+  describe '#execute' do
+    let(:user) { create(:user) }
+    let(:group) { create(:group) }
+    let(:epic) { create(:epic, group: group) }
+    let(:issues) { create_list(:issue, 4) }
+    let!(:epic_issue1) { create(:epic_issue, epic: epic, issue: issues[0], relative_position: 3) }
+    let!(:epic_issue2) { create(:epic_issue, epic: epic, issue: issues[1], relative_position: 600) }
+    let!(:epic_issue3) { create(:epic_issue, epic: epic, issue: issues[2], relative_position: 1200) }
+    let!(:epic_issue4) { create(:epic_issue, epic: epic, issue: issues[3], relative_position: 2000) }
+    let(:default_position_value) { Gitlab::Database::MAX_INT_VALUE / 2 }
+
+    before do
+      group.add_developer(user)
+    end
+
+    def order_issue(issue, params)
+      described_class.new(issue, user, params ).execute
+    end
+
+    def ordered_epics
+      EpicIssue.all.order('relative_position, id')
+    end
+
+    context 'when moving issues between different epics' do
+      before do
+        epic_issue3.update_attribute(:epic, create(:epic, group: group))
+      end
+
+      let(:params) { { move_before_id: epic_issue3.id, move_after_id: epic_issue4.id } }
+
+      subject { order_issue(epic_issue1, params) }
+
+      it 'returns an error' do
+        is_expected.to eq(message: 'Epic issue not found for given params', status: :error, http_status: 404)
+      end
+
+      it 'does not change the relative_position values' do
+        subject
+
+        expect(epic_issue1.relative_position).to eq(3)
+        expect(epic_issue2.relative_position).to eq(600)
+        expect(epic_issue3.relative_position).to eq(1200)
+        expect(epic_issue4.relative_position).to eq(2000)
+      end
+    end
+
+    context 'moving issue to the first position' do
+      let(:params) { { move_after_id: epic_issue1.id } }
+
+      context 'when some positions are close to each other' do
+        before do
+          epic_issue2.update_attribute(:relative_position, 4)
+
+          order_issue(epic_issue3, params)
+        end
+
+        it 'orders issues correctly' do
+          expect(ordered_epics).to eq([epic_issue3, epic_issue1, epic_issue2, epic_issue4])
+        end
+      end
+
+      context 'when there is enough place between positions' do
+        before do
+          order_issue(epic_issue3, params)
+        end
+
+        it 'orders issues correctly' do
+          expect(ordered_epics).to eq([epic_issue3, epic_issue1, epic_issue2, epic_issue4])
+        end
+      end
+    end
+
+    context 'moving issue to the third position' do
+      let(:params) { { move_before_id: epic_issue3.id, move_after_id: epic_issue4.id } }
+
+      context 'when some positions are close to each other' do
+        before do
+          epic_issue2.update_attribute(:relative_position, 1998)
+          epic_issue3.update_attribute(:relative_position, 1999)
+
+          order_issue(epic_issue1, params)
+        end
+
+        it 'orders issues correctly' do
+          expect(ordered_epics).to eq([epic_issue2, epic_issue3, epic_issue1, epic_issue4])
+        end
+      end
+
+      context 'when all positions are same' do
+        before do
+          epic_issue1.update_attribute(:relative_position, 10)
+          epic_issue2.update_attribute(:relative_position, 10)
+          epic_issue3.update_attribute(:relative_position, 10)
+          epic_issue4.update_attribute(:relative_position, 10)
+
+          order_issue(epic_issue1, params)
+        end
+
+        it 'orders affected 2 issues correctly' do
+          expect(epic_issue1.reload.relative_position)
+            .to be_between(epic_issue3.reload.relative_position, epic_issue4.reload.relative_position)
+        end
+      end
+
+      context 'when there is enough place between positions' do
+        before do
+          order_issue(epic_issue1, params)
+        end
+
+        it 'orders issues correctly' do
+          expect(ordered_epics).to eq([epic_issue2, epic_issue3, epic_issue1, epic_issue4])
+        end
+      end
+    end
+
+    context 'moving issues to the last position' do
+      context 'when index of the last possition is correct' do
+        before do
+          order_issue(epic_issue1, move_before_id: epic_issue4.id)
+        end
+
+        it 'orders issues correctly' do
+          expect(ordered_epics).to eq([epic_issue2, epic_issue3, epic_issue4, epic_issue1])
+        end
+      end
+    end
+  end
+end

spec/ee/spec/services/issue_links/list_service_spec.rb

@@ -57,21 +57,21 @@ describe IssueLinks::ListService do
                                            state: issue_b.state,
                                            reference: issue_b.to_reference(project),
                                            path: "/#{project.full_path}/issues/#{issue_b.iid}",
-                                           destroy_relation_path: "/#{project.full_path}/issues/#{issue.iid}/links/#{issue_link_a.id}"))
+                                           relation_path: "/#{project.full_path}/issues/#{issue.iid}/links/#{issue_link_a.id}"))
 
         expect(subject).to include(include(id: issue_c.id,
                                            title: issue_c.title,
                                            state: issue_c.state,
                                            reference: issue_c.to_reference(project),
                                            path: "/#{project.full_path}/issues/#{issue_c.iid}",
-                                           destroy_relation_path: "/#{project.full_path}/issues/#{issue.iid}/links/#{issue_link_b.id}"))
+                                           relation_path: "/#{project.full_path}/issues/#{issue.iid}/links/#{issue_link_b.id}"))
 
         expect(subject).to include(include(id: issue_d.id,
                                            title: issue_d.title,
                                            state: issue_d.state,
                                            reference: issue_d.to_reference(project),
                                            path: "/#{project.full_path}/issues/#{issue_d.iid}",
-                                           destroy_relation_path: "/#{project.full_path}/issues/#{issue.iid}/links/#{issue_link_c.id}"))
+                                           relation_path: "/#{project.full_path}/issues/#{issue.iid}/links/#{issue_link_c.id}"))
       end
     end
 
@@ -164,7 +164,7 @@ describe IssueLinks::ListService do
         it 'returns no destroy relation path' do
           target_project.add_developer(user)
 
-          expect(subject.first[:destroy_relation_path]).to be_nil
+          expect(subject.first[:relation_path]).to be_nil
         end
       end
 
@@ -176,7 +176,7 @@ describe IssueLinks::ListService do
         it 'returns no destroy relation path' do
           target_project.add_guest(user)
 
-          expect(subject.first[:destroy_relation_path]).to be_nil
+          expect(subject.first[:relation_path]).to be_nil
         end
       end
 
@@ -184,7 +184,7 @@ describe IssueLinks::ListService do
         let(:referenced_issue) { create :issue, project: project }
 
         it 'returns related issue destroy relation path' do
-          expect(subject.first[:destroy_relation_path])
+          expect(subject.first[:relation_path])
             .to eq("/#{project.full_path}/issues/#{issue.iid}/links/#{issue_link.id}")
         end
       end

spec/factories/epic_issues.rb

@@ -2,5 +2,6 @@ FactoryBot.define do
   factory :epic_issue do
     epic
     issue
+    relative_position { Gitlab::Database::MAX_INT_VALUE / 2 }
   end
 end

spec/features/issuables/related_issues_spec.rb

@@ -258,7 +258,7 @@ describe 'Related issues', :js do
 
           wait_for_requests
 
-          items = all('.js-related-issues-token-list-item .js-issue-token-title')
+          items = all('.js-related-issues-token-list-item .issue-token-title-text')
 
           # Form gets hidden after submission
           expect(page).not_to have_selector('.js-add-related-issues-form-area')
@@ -275,7 +275,7 @@ describe 'Related issues', :js do
 
           wait_for_requests
 
-          items = all('.js-related-issues-token-list-item .js-issue-token-title')
+          items = all('.js-related-issues-token-list-item .issue-token-title-text')
 
           expect(items.count).to eq(1)
           expect(items[0].text).to eq(issue_project_b_a.title)
@@ -289,7 +289,7 @@ describe 'Related issues', :js do
 
           wait_for_requests
 
-          items = all('.js-related-issues-token-list-item .js-issue-token-title')
+          items = all('.js-related-issues-token-list-item .issue-token-title-text')
 
           expect(items.count).to eq(1)
           expect(items[0].text).to eq(issue_project_b_a.title)
@@ -311,7 +311,7 @@ describe 'Related issues', :js do
         end
 
         it 'shows related issues' do
-          items = all('.js-related-issues-token-list-item .js-issue-token-title')
+          items = all('.js-related-issues-token-list-item .issue-token-title-text')
 
           expect(items.count).to eq(2)
           expect(items[0].text).to eq(issue_b.title)
@@ -319,15 +319,15 @@ describe 'Related issues', :js do
         end
 
         it 'allows us to remove a related issues' do
-          items_before = all('.js-related-issues-token-list-item .js-issue-token-title')
+          items_before = all('.js-related-issues-token-list-item .issue-token-title-text')
 
           expect(items_before.count).to eq(2)
 
-          first('.js-issue-token-remove-button').click
+          first('.js-issue-item-remove-button').click
 
           wait_for_requests
 
-          items_after = all('.js-related-issues-token-list-item .js-issue-token-title')
+          items_after = all('.js-related-issues-token-list-item .issue-token-title-text')
 
           expect(items_after.count).to eq(1)
         end
@@ -339,7 +339,7 @@ describe 'Related issues', :js do
 
           wait_for_requests
 
-          items = all('.js-related-issues-token-list-item .js-issue-token-title')
+          items = all('.js-related-issues-token-list-item .issue-token-title-text')
 
           expect(items.count).to eq(3)
           expect(items[0].text).to eq(issue_b.title)
@@ -355,7 +355,7 @@ describe 'Related issues', :js do
 
           wait_for_requests
 
-          items = all('.js-related-issues-token-list-item .js-issue-token-title')
+          items = all('.js-related-issues-token-list-item .issue-token-title-text')
 
           expect(items.count).to eq(2)
           expect(items[0].text).to eq(issue_b.title)
@@ -370,7 +370,7 @@ describe 'Related issues', :js do
 
           wait_for_requests
 
-          items = all('.js-related-issues-token-list-item .js-issue-token-title')
+          items = all('.js-related-issues-token-list-item .issue-token-title-text')
 
           expect(items.count).to eq(2)
           expect(items[0].text).to eq(issue_b.title)

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

+import Vue from 'vue';
+import issueItem from '~/issuable/related_issues/components/issue_item.vue';
+import eventHub from '~/issuable/related_issues/event_hub';
+import mountComponent from '../../../helpers/vue_mount_component_helper';
+
+describe('issueItem', () => {
+  let vm;
+  const props = {
+    idKey: 1,
+    displayReference: '#1',
+    path: `${gl.TEST_HOST}/path`,
+    title: 'title',
+  };
+
+  beforeEach(() => {
+    const IssueItem = Vue.extend(issueItem);
+    vm = mountComponent(IssueItem, props);
+  });
+
+  it('contains issue-info-container class when canReorder is false', () => {
+    expect(vm.canReorder).toEqual(false);
+    expect(vm.$el.querySelector('.issue-info-container')).toBeNull();
+  });
+
+  it('renders displayReference', () => {
+    expect(vm.$el.querySelector('.text-secondary').innerText.trim()).toEqual(props.displayReference);
+  });
+
+  it('does not render token state', () => {
+    expect(vm.$el.querySelector('.text-secondary svg')).toBeNull();
+  });
+
+  it('does not render remove button', () => {
+    expect(vm.$refs.removeButton).toBeUndefined();
+  });
+
+  describe('token title', () => {
+    it('links to computedPath', () => {
+      expect(vm.$el.querySelector('a').href).toEqual(props.path);
+    });
+
+    it('renders title', () => {
+      expect(vm.$el.querySelector('a').innerText.trim()).toEqual(props.title);
+    });
+  });
+
+  describe('token state', () => {
+    let tokenState;
+
+    beforeEach((done) => {
+      vm.state = 'opened';
+      Vue.nextTick(() => {
+        tokenState = vm.$el.querySelector('.text-secondary svg');
+        done();
+      });
+    });
+
+    it('renders if hasState', () => {
+      expect(tokenState).toBeDefined();
+    });
+
+    it('renders state title', () => {
+      expect(tokenState.getAttribute('data-original-title')).toEqual('Open');
+    });
+
+    it('renders aria label', () => {
+      expect(tokenState.getAttribute('aria-label')).toEqual('opened');
+    });
+
+    it('renders open icon when open state', () => {
+      expect(tokenState.classList.contains('issue-token-state-icon-open')).toEqual(true);
+    });
+
+    it('renders close icon when close state', (done) => {
+      vm.state = 'closed';
+
+      Vue.nextTick(() => {
+        expect(tokenState.classList.contains('issue-token-state-icon-closed')).toEqual(true);
+        done();
+      });
+    });
+  });
+
+  describe('remove button', () => {
+    let removeBtn;
+
+    beforeEach((done) => {
+      vm.canRemove = true;
+      Vue.nextTick(() => {
+        removeBtn = vm.$refs.removeButton;
+        done();
+      });
+    });
+
+    it('renders if canRemove', () => {
+      expect(removeBtn).toBeDefined();
+    });
+
+    it('renders disabled button when removeDisabled', (done) => {
+      vm.removeDisabled = true;
+      Vue.nextTick(() => {
+        expect(removeBtn.hasAttribute('disabled')).toEqual(true);
+        done();
+      });
+    });
+
+    it('triggers onRemoveRequest when clicked', () => {
+      const spy = jasmine.createSpy('spy');
+      eventHub.$on('removeRequest', spy);
+      removeBtn.click();
+
+      expect(spy).toHaveBeenCalled();
+    });
+  });
+});

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

@@ -138,7 +138,8 @@ describe('IssueToken', () => {
     });
 
     it('shows reference, title, and state', () => {
-      expect(vm.$refs.stateIcon.getAttribute('aria-label')).toEqual(state);
+      const stateIcon = vm.$refs.reference.querySelector('svg');
+      expect(stateIcon.getAttribute('aria-label')).toEqual(state);
       expect(vm.$refs.reference.textContent.trim()).toEqual(displayReference);
       expect(vm.$refs.title.textContent.trim()).toEqual(title);
     });

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

@@ -14,7 +14,7 @@ const issuable1 = {
   title: 'issue1',
   path: '/foo/bar/issues/123',
   state: 'opened',
-  destroy_relation_path: '/foo/bar/issues/123/related_issues/1',
+  relation_path: '/foo/bar/issues/123/related_issues/1',
 };
 
 const issuable2 = {
@@ -23,7 +23,7 @@ const issuable2 = {
   title: 'issue1',
   path: '/foo/bar/issues/124',
   state: 'opened',
-  destroy_relation_path: '/foo/bar/issues/124/related_issues/1',
+  relation_path: '/foo/bar/issues/124/related_issues/1',
 };
 
 describe('RelatedIssuesRoot', () => {

spec/javascripts/issuable/related_issues/stores/related_issues_store_spec.js

@@ -6,7 +6,7 @@ const issuable1 = {
   title: 'issue1',
   path: '/foo/bar/issues/123',
   state: 'opened',
-  destroy_relation_path: '/foo/bar/issues/123/related_issues/1',
+  relation_path: '/foo/bar/issues/123/related_issues/1',
 };
 
 const issuable2 = {
@@ -15,7 +15,7 @@ const issuable2 = {
   title: 'issue1',
   path: '/foo/bar/issues/124',
   state: 'opened',
-  destroy_relation_path: '/foo/bar/issues/124/related_issues/1',
+  relation_path: '/foo/bar/issues/124/related_issues/1',
 };
 
 describe('RelatedIssuesStore', () => {

spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb

@@ -2,21 +2,10 @@ require 'spec_helper'
 
 describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do
   include TrackUntrackedUploadsHelpers
+  include MigrationsHelpers
 
   let!(:untracked_files_for_uploads) { described_class::UntrackedFile }
 
-  matcher :be_scheduled_migration do |*expected|
-    match do |migration|
-      BackgroundMigrationWorker.jobs.any? do |job|
-        job['args'] == [migration, expected]
-      end
-    end
-
-    failure_message do |migration|
-      "Migration `#{migration}` with args `#{expected.inspect}` not scheduled!"
-    end
-  end
-
   before do
     DatabaseCleaner.clean
 

spec/migrations/migrate_stage_id_reference_in_background_spec.rb

@@ -35,9 +35,9 @@ describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do
       Timecop.freeze do
         migrate!
 
-        expect(described_class::MIGRATION).to be_scheduled_migration(2.minutes, 1, 2)
-        expect(described_class::MIGRATION).to be_scheduled_migration(2.minutes, 3, 3)
-        expect(described_class::MIGRATION).to be_scheduled_migration(4.minutes, 4, 5)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(2.minutes, 1, 2)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(2.minutes, 3, 3)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(4.minutes, 4, 5)
         expect(BackgroundMigrationWorker.jobs.size).to eq 3
       end
     end

spec/migrations/migrate_stages_statuses_spec.rb

@@ -50,9 +50,9 @@ describe MigrateStagesStatuses, :migration do
       Timecop.freeze do
         migrate!
 
-        expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, 1, 1)
-        expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, 2, 2)
-        expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, 3, 3)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(5.minutes, 1, 1)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(5.minutes, 2, 2)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(10.minutes, 3, 3)
         expect(BackgroundMigrationWorker.jobs.size).to eq 3
       end
     end

spec/migrations/schedule_create_gpg_key_subkeys_from_gpg_keys_spec.rb

@@ -2,18 +2,6 @@ require 'spec_helper'
 require Rails.root.join('db', 'post_migrate', '20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys')
 
 describe ScheduleCreateGpgKeySubkeysFromGpgKeys, :migration, :sidekiq do
-  matcher :be_scheduled_migration do |*expected|
-    match do |migration|
-      BackgroundMigrationWorker.jobs.any? do |job|
-        job['args'] == [migration, expected]
-      end
-    end
-
-    failure_message do |migration|
-      "Migration `#{migration}` with args `#{expected.inspect}` not scheduled!"
-    end
-  end
-
   before do
     create(:gpg_key, id: 1, key: GpgHelpers::User1.public_key)
     create(:gpg_key, id: 2, key: GpgHelpers::User3.public_key)

spec/migrations/schedule_merge_request_diff_migrations_spec.rb

@@ -24,9 +24,9 @@ describe ScheduleMergeRequestDiffMigrations, :migration, :sidekiq do
       Timecop.freeze do
         migrate!
 
-        expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, 1, 1)
-        expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, 2, 2)
-        expect(described_class::MIGRATION).to be_scheduled_migration(15.minutes, 4, 4)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(5.minutes, 1, 1)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(10.minutes, 2, 2)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(15.minutes, 4, 4)
         expect(BackgroundMigrationWorker.jobs.size).to eq 3
       end
     end

spec/migrations/schedule_merge_request_diff_migrations_take_two_spec.rb

@@ -24,9 +24,9 @@ describe ScheduleMergeRequestDiffMigrationsTakeTwo, :migration, :sidekiq do
       Timecop.freeze do
         migrate!
 
-        expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, 1, 1)
-        expect(described_class::MIGRATION).to be_scheduled_migration(20.minutes, 2, 2)
-        expect(described_class::MIGRATION).to be_scheduled_migration(30.minutes, 4, 4)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(10.minutes, 1, 1)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(20.minutes, 2, 2)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(30.minutes, 4, 4)
         expect(BackgroundMigrationWorker.jobs.size).to eq 3
       end
     end

spec/migrations/schedule_merge_request_latest_merge_request_diff_id_migrations_spec.rb

@@ -44,9 +44,9 @@ describe ScheduleMergeRequestLatestMergeRequestDiffIdMigrations, :migration, :si
       Timecop.freeze do
         migrate!
 
-        expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, merge_request_1.id, merge_request_1.id)
-        expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, merge_request_2.id, merge_request_2.id)
-        expect(described_class::MIGRATION).to be_scheduled_migration(15.minutes, merge_request_4.id, merge_request_4.id)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(5.minutes, merge_request_1.id, merge_request_1.id)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(10.minutes, merge_request_2.id, merge_request_2.id)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(15.minutes, merge_request_4.id, merge_request_4.id)
         expect(BackgroundMigrationWorker.jobs.size).to eq 3
       end
     end

spec/migrations/schedule_populate_merge_request_metrics_with_events_data_spec.rb

@@ -12,10 +12,10 @@ describe SchedulePopulateMergeRequestMetricsWithEventsData, :migration, :sidekiq
         migrate!
 
         expect(described_class::MIGRATION)
-          .to be_scheduled_migration(10.minutes, mrs.first.id, mrs.second.id)
+          .to be_scheduled_delayed_migration(10.minutes, mrs.first.id, mrs.second.id)
 
         expect(described_class::MIGRATION)
-          .to be_scheduled_migration(20.minutes, mrs.third.id, mrs.third.id)
+          .to be_scheduled_delayed_migration(20.minutes, mrs.third.id, mrs.third.id)
 
         expect(BackgroundMigrationWorker.jobs.size).to eq(2)
       end

spec/migrations/track_untracked_uploads_spec.rb

@@ -4,18 +4,6 @@ require Rails.root.join('db', 'post_migrate', '20171103140253_track_untracked_up
 describe TrackUntrackedUploads, :migration, :sidekiq do
   include TrackUntrackedUploadsHelpers
 
-  matcher :be_scheduled_migration do
-    match do |migration|
-      BackgroundMigrationWorker.jobs.any? do |job|
-        job['args'] == [migration]
-      end
-    end
-
-    failure_message do |migration|
-      "Migration `#{migration}` with args `#{expected.inspect}` not scheduled!"
-    end
-  end
-
   it 'correctly schedules the follow-up background migration' do
     Sidekiq::Testing.fake! do
       migrate!

spec/support/background_migrations_matchers.rb

-RSpec::Matchers.define :be_scheduled_migration do |delay, *expected|
+RSpec::Matchers.define :be_scheduled_delayed_migration do |delay, *expected|
   match do |migration|
     BackgroundMigrationWorker.jobs.any? do |job|
       job['args'] == [migration, expected] &&
@@ -11,3 +11,16 @@ RSpec::Matchers.define :be_scheduled_migration do |delay, *expected|
       'not scheduled in expected time!'
   end
 end
+
+RSpec::Matchers.define :be_scheduled_migration do |*expected|
+  match do |migration|
+    BackgroundMigrationWorker.jobs.any? do |job|
+      args = job['args'].size == 1 ? [BackgroundMigrationWorker.jobs[0]['args'][0], []] : job['args']
+      args == [migration, expected]
+    end
+  end
+
+  failure_message do |migration|
+    "Migration `#{migration}` with args `#{expected.inspect}` not scheduled!"
+  end
+end