Add endpoint for ordering issues in epics

config/routes/group.rb

@@ -85,7 +85,11 @@ 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], as: 'issues', path: 'issues' do
+        member do
+          put :order
+        end
+      end
     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
+    add_column_with_default(:epic_issues, :position, :integer, default: Arel.sql('id'), allow_null: false)
+  end
+
+  def down
+    remove_column(:epic_issues, :position)
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20171213160445) do
+ActiveRecord::Schema.define(version: 20171214115254) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -765,6 +765,7 @@ ActiveRecord::Schema.define(version: 20171213160445) do
   create_table "epic_issues", force: :cascade do |t|
     t.integer "epic_id", null: false
     t.integer "issue_id", null: false
+    t.integer "position", default: 0, null: false
   end
 
   add_index "epic_issues", ["epic_id"], name: "index_epic_issues_on_epic_id", using: :btree

ee/app/controllers/groups/epic_issues_controller.rb

@@ -4,8 +4,14 @@ class Groups::EpicIssuesController < Groups::EpicsController
   skip_before_action :authorize_destroy_issuable!
   skip_before_action :authorize_create_epic!
 
-  before_action :authorize_admin_epic!, only: [:create, :destroy]
-  before_action :authorize_issue_link_association!, only: :destroy
+  before_action :authorize_admin_epic!, only: [:create, :destroy, :order]
+  before_action :authorize_issue_link_association!, only: [:destroy, :order]
+
+  def order
+    result = EpicIssues::OrderService.new(link, current_user, params).execute
+
+    render json: { message: result[:message] }, status: result[:http_status]
+  end
 
   private
 

ee/app/services/epic_issues/order_service.rb

+module EpicIssues
+  class OrderService < BaseService
+    attr_reader :epic_issue, :current_user, :new_position, :old_position
+
+    def initialize(epic_issue, user, params)
+      @epic_issue = epic_issue
+      @current_user = user
+      @new_position = params[:position].to_i
+      @old_position = epic_issue.position
+    end
+
+    def execute
+      move_issue
+      success
+    end
+
+    private
+
+    def move_issue
+      epic_issue.update_attribute(:position, new_position)
+      issues_to_move.update_all("position = position #{update_operator} 1")
+    end
+
+    def epic
+      @epic ||= epic_issue.epic
+    end
+
+    def issues_to_move
+      @issues_to_move ||= epic.epic_issues
+        .where('position >= ? AND position <= ? AND id != ?', from, to, epic_issue.id)
+        .order(:position)
+    end
+
+    def from
+      [new_position, old_position].min
+    end
+
+    def to
+      [new_position, old_position].max
+    end
+
+    def update_operator
+      new_position > old_position ? '-' : '+'
+    end
+  end
+end

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

@@ -86,7 +86,7 @@ describe Groups::EpicIssuesController do
       delete :destroy, group_id: group, epic_id: epic.to_param, id: epic_issue.id
     end
 
-    context 'when user has permissions to detele the link' do
+    context 'when user has permissions to delete the link' do
       before do
         group.add_developer(user)
       end
@@ -144,4 +144,72 @@ describe Groups::EpicIssuesController do
       end
     end
   end
+
+  describe 'PUT #order' do
+    let(:issue2) { create(:issue, project: project) }
+    let!(:epic_issue1) { create(:epic_issue, epic: epic, issue: issue, position: 1) }
+    let!(:epic_issue2) { create(:epic_issue, epic: epic, issue: issue2, position: 2) }
+
+    subject do
+      put :order, group_id: group, epic_id: epic.to_param, id: epic_issue1.id, position: 2
+    end
+
+    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.position }.from(1).to(2)
+      end
+    end
+
+    context 'when user does not have permissions to admin the epic' do
+      it 'returns status 404' do
+        subject
+
+        expect(response.status).to eq(403)
+      end
+
+      it 'does not change the position value' do
+        expect { subject }.not_to change { epic_issue1.reload.position }.from(1)
+      end
+    end
+
+    context 'when the epic from the association does not equal epic from the path' do
+      subject do
+        put :order, group_id: group, epic_id: another_epic.to_param, id: epic_issue1.id, position: 2
+      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
+
+      it 'does not change the position value' do
+        expect { subject }.not_to change { epic_issue1.position }.from(1)
+      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
+
+        expect(response.status).to eq(403)
+      end
+    end
+  end
 end

spec/ee/spec/services/epic_issues/order_service_spec.rb

+require 'spec_helper'
+
+describe EpicIssues::OrderService 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], position: 1) }
+    let!(:epic_issue2) { create(:epic_issue, epic: epic, issue: issues[1], position: 2) }
+    let!(:epic_issue3) { create(:epic_issue, epic: epic, issue: issues[2], position: 3) }
+    let!(:epic_issue4) { create(:epic_issue, epic: epic, issue: issues[3], position: 4) }
+
+    before do
+      group.add_developer(user)
+    end
+
+    def order_issue(issue, new_order)
+      described_class.new(issue, user, { position: new_order } ).execute
+    end
+
+    def ordered_epics
+      EpicIssue.all.order(:position)
+    end
+
+    context 'moving issue to the first position' do
+      it 'orders issues correctly' do
+        order_issue(epic_issue3, 1)
+
+        expect(ordered_epics).to eq([epic_issue3, epic_issue1, epic_issue2, epic_issue4])
+      end
+
+      context 'when some order values are missing ' do
+        before do
+          epic_issue1.update_attribute(:position, 3)
+          epic_issue2.update_attribute(:position, 6)
+          epic_issue3.update_attribute(:position, 10)
+          epic_issue4.update_attribute(:position, 15)
+        end
+
+        it 'orders issues correctly' do
+          order_issue(epic_issue3, 1)
+
+          expect(ordered_epics).to eq([epic_issue3, epic_issue1, epic_issue2, epic_issue4])
+        end
+
+        it 'updates all affected issues positions by 1' do
+          order_issue(epic_issue3, 1)
+
+          expect(epic_issue3.reload.position).to eq(1)
+          expect(epic_issue1.reload.position).to eq(4)
+          expect(epic_issue2.reload.position).to eq(7)
+          expect(epic_issue4.reload.position).to eq(15)
+        end
+      end
+    end
+
+    context 'moving issue to the third position' do
+      it 'orders issues correctly' do
+        order_issue(epic_issue1, 3)
+
+        expect(ordered_epics).to eq([epic_issue2, epic_issue3, epic_issue1, epic_issue4])
+      end
+
+      context 'when some order values are missing ' do
+        before do
+          epic_issue1.update_attribute(:position, 3)
+          epic_issue2.update_attribute(:position, 6)
+          epic_issue3.update_attribute(:position, 10)
+          epic_issue4.update_attribute(:position, 15)
+        end
+
+        it 'orders issues correctly' do
+          order_issue(epic_issue1, 10)
+
+          expect(ordered_epics).to eq([epic_issue2, epic_issue3, epic_issue1, epic_issue4])
+        end
+
+        it 'updates all affected issues positions by 1' do
+          order_issue(epic_issue3, 1)
+
+          expect(epic_issue3.reload.position).to eq(1)
+          expect(epic_issue1.reload.position).to eq(4)
+          expect(epic_issue2.reload.position).to eq(7)
+          expect(epic_issue4.reload.position).to eq(15)
+        end
+      end
+    end
+  end
+end

spec/factories/epic_issues.rb

@@ -2,5 +2,6 @@ FactoryBot.define do
   factory :epic_issue do
     epic
     issue
+    position 1
   end
 end