Merge branch 'master' into 'ce-upstream'

# Conflicts:
#   doc/api/v3_to_v4.md

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

@@ -56,11 +56,6 @@ import boardCard from './board_card';
         });
       }
     },
-    computed: {
-      orderedIssues () {
-        return _.sortBy(this.issues, 'priority');
-      },
-    },
     methods: {
       listHeight () {
         return this.$refs.list.getBoundingClientRect().height;
@@ -92,9 +87,9 @@ import boardCard from './board_card';
       const options = gl.issueBoards.getBoardSortableDefaultOptions({
         scroll: document.querySelectorAll('.boards-list')[0],
         group: 'issues',
-        sort: false,
         disabled: this.disabled,
         filter: '.board-list-count, .is-disabled',
+        dataIdAttr: 'data-issue-id',
         onStart: (e) => {
           const card = this.$refs.issue[e.oldIndex];
 
@@ -111,6 +106,13 @@ import boardCard from './board_card';
             e.item.remove();
           });
         },
+        onUpdate: (e) => {
+          const sortedArray = this.sortable.toArray().filter(id => id !== '-1');
+          gl.issueBoards.BoardsStore.moveIssueInList(this.list, Store.moving.issue, e.oldIndex, e.newIndex, sortedArray);
+        },
+        onMove(e) {
+          return !e.related.classList.contains('board-list-count');
+        }
       });
 
       this.sortable = Sortable.create(this.$refs.list, options);

app/assets/javascripts/boards/models/issue.js

@@ -15,6 +15,7 @@ class ListIssue {
     this.labels = [];
     this.selected = false;
     this.assignee = false;
+    this.position = obj.relative_position || Infinity;
 
     if (obj.assignee) {
       this.assignee = new ListUser(obj.assignee);
@@ -27,10 +28,6 @@ class ListIssue {
     obj.labels.forEach((label) => {
       this.labels.push(new ListLabel(label));
     });
-
-    this.priority = this.labels.reduce((max, label) => {
-      return (label.priority < max) ? label.priority : max;
-    }, Infinity);
   }
 
   addLabel (label) {

app/assets/javascripts/boards/models/list.js

@@ -110,9 +110,20 @@ class List {
   }
 
   addIssue (issue, listFrom, newIndex) {
+    let moveBeforeIid = null;
+    let moveAfterIid = null;
+
     if (!this.findIssue(issue.id)) {
       if (newIndex !== undefined) {
         this.issues.splice(newIndex, 0, issue);
+
+        if (this.issues[newIndex - 1]) {
+          moveBeforeIid = this.issues[newIndex - 1].id;
+        }
+
+        if (this.issues[newIndex + 1]) {
+          moveAfterIid = this.issues[newIndex + 1].id;
+        }
       } else {
         this.issues.push(issue);
       }
@@ -123,13 +134,21 @@ class List {
 
       if (listFrom) {
         this.issuesSize += 1;
-        this.updateIssueLabel(issue, listFrom);
+
+        this.updateIssueLabel(issue, listFrom, moveBeforeIid, moveAfterIid);
       }
     }
   }
 
-  updateIssueLabel(issue, listFrom) {
-    gl.boardService.moveIssue(issue.id, listFrom.id, this.id)
+  moveIssue (issue, oldIndex, newIndex, moveBeforeIid, moveAfterIid) {
+    this.issues.splice(oldIndex, 1);
+    this.issues.splice(newIndex, 0, issue);
+
+    gl.boardService.moveIssue(issue.id, null, null, moveBeforeIid, moveAfterIid);
+  }
+
+  updateIssueLabel(issue, listFrom, moveBeforeIid, moveAfterIid) {
+    gl.boardService.moveIssue(issue.id, listFrom.id, this.id, moveBeforeIid, moveAfterIid)
       .then(() => {
         listFrom.getIssues(false);
       });

app/assets/javascripts/boards/services/board_service.js

@@ -76,10 +76,12 @@ class BoardService {
     return this.issues.get(data);
   }
 
-  moveIssue (id, from_list_id, to_list_id) {
+  moveIssue (id, from_list_id = null, to_list_id = null, move_before_iid = null, move_after_iid = null) {
     return this.issue.update({ id }, {
       from_list_id,
-      to_list_id
+      to_list_id,
+      move_before_iid,
+      move_after_iid,
     });
   }
 

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

@@ -114,6 +114,12 @@
         listFrom.removeIssue(issue);
       }
     },
+    moveIssueInList (list, issue, oldIndex, newIndex, idArray) {
+      const beforeId = parseInt(idArray[newIndex - 1], 10) || null;
+      const afterId = parseInt(idArray[newIndex + 1], 10) || null;
+
+      list.moveIssue(issue, oldIndex, newIndex, beforeId, afterId);
+    },
     findList (key, val, type = 'label') {
       return this.state.lists.filter((list) => {
         const byType = type ? list['type'] === type : true;

app/assets/javascripts/filtered_search/dropdown_utils.js

@@ -79,6 +79,7 @@
       return dataValue !== null;
     }
 
+    // Determines the full search query (visual tokens + input)
     static getSearchQuery() {
       const tokensContainer = document.querySelector('.tokens-container');
       const values = [];

app/assets/javascripts/filtered_search/filtered_search_manager.js

@@ -39,6 +39,7 @@
       this.checkForBackspaceWrapper = this.checkForBackspace.bind(this);
       this.removeSelectedTokenWrapper = this.removeSelectedToken.bind(this);
       this.unselectEditTokensWrapper = this.unselectEditTokens.bind(this);
+      this.editTokenWrapper = this.editToken.bind(this);
       this.tokenChange = this.tokenChange.bind(this);
 
       this.filteredSearchInput.form.addEventListener('submit', this.handleFormSubmit);
@@ -51,7 +52,7 @@
       this.filteredSearchInput.addEventListener('click', this.tokenChange);
       this.filteredSearchInput.addEventListener('keyup', this.tokenChange);
       this.tokensContainer.addEventListener('click', FilteredSearchManager.selectToken);
-      this.tokensContainer.addEventListener('dblclick', FilteredSearchManager.editToken);
+      this.tokensContainer.addEventListener('dblclick', this.editTokenWrapper);
       this.clearSearchButton.addEventListener('click', this.clearSearchWrapper);
       document.addEventListener('click', gl.FilteredSearchVisualTokens.unselectTokens);
       document.addEventListener('click', this.unselectEditTokensWrapper);
@@ -69,7 +70,7 @@
       this.filteredSearchInput.removeEventListener('click', this.tokenChange);
       this.filteredSearchInput.removeEventListener('keyup', this.tokenChange);
       this.tokensContainer.removeEventListener('click', FilteredSearchManager.selectToken);
-      this.tokensContainer.removeEventListener('dblclick', FilteredSearchManager.editToken);
+      this.tokensContainer.removeEventListener('dblclick', this.editTokenWrapper);
       this.clearSearchButton.removeEventListener('click', this.clearSearchWrapper);
       document.removeEventListener('click', gl.FilteredSearchVisualTokens.unselectTokens);
       document.removeEventListener('click', this.unselectEditTokensWrapper);
@@ -138,11 +139,12 @@
       }
     }
 
-    static editToken(e) {
+    editToken(e) {
       const token = e.target.closest('.js-visual-token');
 
       if (token) {
         gl.FilteredSearchVisualTokens.editToken(token);
+        this.tokenChange();
       }
     }
 

app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js

@@ -178,9 +178,9 @@ class FilteredSearchVisualTokens {
     const inputLi = input.parentElement;
     const tokenContainer = document.querySelector('.tokens-container');
 
-    if (!tokenContainer.lastElementChild.isEqualNode(inputLi)) {
-      FilteredSearchVisualTokens.tokenizeInput();
+    FilteredSearchVisualTokens.tokenizeInput();
 
+    if (!tokenContainer.lastElementChild.isEqualNode(inputLi)) {
       const { isLastVisualTokenValid } =
         gl.FilteredSearchVisualTokens.getLastVisualTokenBeforeInput();
 

app/assets/javascripts/test_utils/simulate_drag.js

@@ -43,7 +43,14 @@
     return event;
   }
 
-  function getTraget(target) {
+  function isLast(target) {
+    var el = typeof target.el === 'string' ? document.getElementById(target.el.substr(1)) : target.el;
+    var children = el.children;
+
+    return children.length - 1 === target.index;
+  }
+
+  function getTarget(target) {
     var el = typeof target.el === 'string' ? document.getElementById(target.el.substr(1)) : target.el;
     var children = el.children;
 
@@ -75,12 +82,22 @@
   function simulateDrag(options, callback) {
     options.to.el = options.to.el || options.from.el;
 
-    var fromEl = getTraget(options.from);
-    var toEl = getTraget(options.to);
+    var fromEl = getTarget(options.from);
+    var toEl = getTarget(options.to);
+    var firstEl = getTarget({
+      el: options.to.el,
+      index: 'first'
+    });
+    var lastEl = getTarget({
+      el: options.to.el,
+      index: 'last'
+    });
     var scrollable = options.scrollable;
 
     var fromRect = getRect(fromEl);
     var toRect = getRect(toEl);
+    var firstRect = getRect(firstEl);
+    var lastRect = getRect(lastEl);
 
     var startTime = new Date().getTime();
     var duration = options.duration || 1000;
@@ -88,6 +105,12 @@
     options.ontap && options.ontap();
     window.SIMULATE_DRAG_ACTIVE = 1;
 
+    if (options.to.index === 0) {
+      toRect.cy = firstRect.y;
+    } else if (isLast(options.to)) {
+      toRect.cy = lastRect.y + lastRect.h + 50;
+    }
+
     var dragInterval = setInterval(function loop() {
       var progress = (new Date().getTime() - startTime) / duration;
       var x = (fromRect.cx + (toRect.cx - fromRect.cx) * progress) - scrollable.scrollLeft;

app/assets/stylesheets/framework/dropdowns.scss

@@ -159,12 +159,12 @@
   li {
     text-align: left;
     list-style: none;
-    padding: 0 8px;
+    padding: 0 10px;
   }
 
   .divider {
     height: 1px;
-    margin: 8px;
+    margin: 6px 10px;
     padding: 0;
     background-color: $dropdown-divider-color;
   }
@@ -181,7 +181,7 @@
     display: block;
     position: relative;
     padding: 5px 8px;
-    color: $dropdown-link-color;
+    color: $gl-text-color;
     line-height: initial;
     text-overflow: ellipsis;
     border-radius: 2px;
@@ -222,10 +222,12 @@
   }
 
   .dropdown-header {
-    color: $gl-text-color-secondary;
+    color: $gl-text-color;
     font-size: 13px;
+    font-weight: 600;
     line-height: 22px;
-    padding: 0 10px;
+    text-transform: capitalize;
+    padding: 0 16px;
   }
 
   .separator + .dropdown-header {
@@ -328,14 +330,17 @@
 
 .dropdown-menu-selectable {
   a {
-    padding-left: 25px;
+    padding-left: 26px;
 
     &.is-indeterminate,
     &.is-active {
+      font-weight: 600;
+      color: $gl-text-color;
+
       &::before {
         position: absolute;
-        left: 5px;
-        top: 8px;
+        left: 6px;
+        top: 6px;
         font: normal normal normal 14px/1 FontAwesome;
         font-size: inherit;
         text-rendering: auto;
@@ -357,7 +362,7 @@
 
 .dropdown-title {
   position: relative;
-  padding: 0 25px 10px;
+  padding: 2px 25px 10px;
   margin: 0 10px 10px;
   font-weight: 600;
   line-height: 1;
@@ -387,7 +392,7 @@
   right: 5px;
   width: 20px;
   height: 20px;
-  top: -3px;
+  top: -1px;
 }
 
 .dropdown-menu-back {

app/controllers/groups/hooks_controller.rb

@@ -53,8 +53,19 @@ class Groups::HooksController < Groups::ApplicationController
   end
 
   def hook_params
-    params.require(:hook).permit(:url, :push_events, :issues_events,
-      :merge_requests_events, :tag_push_events, :note_events,
-      :build_events, :enable_ssl_verification)
+    params.require(:hook).permit(
+      :build_events,
+      :confidential_issues_events,
+      :enable_ssl_verification,
+      :issues_events,
+      :merge_requests_events,
+      :note_events,
+      :pipeline_events,
+      :push_events,
+      :tag_push_events,
+      :token,
+      :url,
+      :wiki_page_events
+    )
   end
 end

app/controllers/projects/boards/issues_controller.rb

@@ -8,6 +8,7 @@ module Projects
       def index
         issues = ::Boards::Issues::ListService.new(project, current_user, filter_params).execute
         issues = issues.page(params[:page]).per(params[:per] || 20)
+        make_sure_position_is_set(issues) unless Gitlab::Geo.secondary?
 
         render json: {
           issues: serialize_as_json(issues),
@@ -38,6 +39,12 @@ module Projects
 
       private
 
+      def make_sure_position_is_set(issues)
+        issues.each do |issue|
+          issue.move_to_end && issue.save unless issue.relative_position
+        end
+      end
+
       def issue
         @issue ||=
           IssuesFinder.new(current_user, project_id: project.id)
@@ -63,7 +70,7 @@ module Projects
       end
 
       def move_params
-        params.permit(:board_id, :id, :from_list_id, :to_list_id)
+        params.permit(:board_id, :id, :from_list_id, :to_list_id, :move_before_iid, :move_after_iid)
       end
 
       def issue_params
@@ -73,7 +80,7 @@ module Projects
       def serialize_as_json(resource)
         resource.as_json(
           labels: true,
-          only: [:id, :iid, :title, :confidential, :due_date],
+          only: [:id, :iid, :title, :confidential, :due_date, :relative_position],
           include: {
             assignee: { only: [:id, :name, :username], methods: [:avatar_url] },
             milestone: { only: [:id, :title] }

app/models/concerns/relative_positioning.rb

+module RelativePositioning
+  extend ActiveSupport::Concern
+
+  MIN_POSITION = 0
+  MAX_POSITION = Gitlab::Database::MAX_INT_VALUE
+
+  included do
+    after_save :save_positionable_neighbours
+  end
+
+  def min_relative_position
+    self.class.in_projects(project.id).minimum(:relative_position)
+  end
+
+  def max_relative_position
+    self.class.in_projects(project.id).maximum(:relative_position)
+  end
+
+  def prev_relative_position
+    prev_pos = nil
+
+    if self.relative_position
+      prev_pos = self.class.
+        in_projects(project.id).
+        where('relative_position < ?', self.relative_position).
+        maximum(:relative_position)
+    end
+
+    prev_pos || MIN_POSITION
+  end
+
+  def next_relative_position
+    next_pos = nil
+
+    if self.relative_position
+      next_pos = self.class.
+        in_projects(project.id).
+        where('relative_position > ?', self.relative_position).
+        minimum(:relative_position)
+    end
+
+    next_pos || MAX_POSITION
+  end
+
+  def move_between(before, after)
+    return move_after(before) unless after
+    return move_before(after) unless before
+
+    pos_before = before.relative_position
+    pos_after = after.relative_position
+
+    if pos_after && (pos_before == pos_after)
+      self.relative_position = pos_before
+      before.move_before(self)
+      after.move_after(self)
+
+      @positionable_neighbours = [before, after]
+    else
+      self.relative_position = position_between(pos_before, pos_after)
+    end
+  end
+
+  def move_before(after)
+    self.relative_position = position_between(after.prev_relative_position, after.relative_position)
+  end
+
+  def move_after(before)
+    self.relative_position = position_between(before.relative_position, before.next_relative_position)
+  end
+
+  def move_to_end
+    self.relative_position = position_between(max_relative_position, MAX_POSITION)
+  end
+
+  private
+
+  # This method takes two integer values (positions) and
+  # calculates some random position between them. The range is huge as
+  # the maximum integer value is 2147483647. Ideally, the calculated value would be
+  # exactly between those terminating values, but this will introduce possibility of a race condition
+  # so two or more issues can get the same value, we want to avoid that and we also want to avoid
+  # using a lock here. If we have two issues with distance more than one thousand, we are OK.
+  # Given the huge range of possible values that integer can fit we shoud never face a problem.
+  def position_between(pos_before, pos_after)
+    pos_before ||= MIN_POSITION
+    pos_after ||= MAX_POSITION
+
+    pos_before, pos_after = [pos_before, pos_after].sort
+
+    rand(pos_before.next..pos_after.pred)
+  end
+
+  def save_positionable_neighbours
+    return unless @positionable_neighbours
+
+    status = @positionable_neighbours.all?(&:save)
+    @positionable_neighbours = nil
+
+    status
+  end
+end

app/models/issue.rb

@@ -8,6 +8,7 @@ class Issue < ActiveRecord::Base
   include Spammable
   include Elastic::IssuesSearch
   include FasterCacheKeys
+  include RelativePositioning
 
   WEIGHT_RANGE = 1..9
   WEIGHT_ALL = 'Everything'.freeze

app/services/boards/issues/list_service.rb

@@ -5,7 +5,7 @@ module Boards
         issues = IssuesFinder.new(current_user, filter_params).execute
         issues = without_board_labels(issues) unless movable_list?
         issues = with_list_label(issues) if movable_list?
-        issues
+        issues.reorder(Gitlab::Database.nulls_last_order('relative_position', 'ASC'))
       end
 
       private
@@ -26,7 +26,6 @@ module Boards
 
       def filter_params
         set_default_scope
-        set_default_sort
         set_project
         set_state
 
@@ -37,10 +36,6 @@ module Boards
         params[:scope] = 'all'
       end
 
-      def set_default_sort
-        params[:sort] = 'priority'
-      end
-
       def set_project
         params[:project_id] = project.id
       end

app/services/boards/issues/move_service.rb

@@ -3,7 +3,7 @@ module Boards
     class MoveService < BaseService
       def execute(issue)
         return false unless can?(current_user, :update_issue, issue)
-        return false unless valid_move?
+        return false if issue_params.empty?
 
         update_service.execute(issue)
       end
@@ -14,7 +14,7 @@ module Boards
         @board ||= project.boards.find(params[:board_id])
       end
 
-      def valid_move?
+      def move_between_lists?
         moving_from_list.present? && moving_to_list.present? &&
           moving_from_list != moving_to_list
       end
@@ -32,11 +32,19 @@ module Boards
       end
 
       def issue_params
-        {
-          add_label_ids: add_label_ids,
-          remove_label_ids: remove_label_ids,
-          state_event: issue_state
-        }
+        attrs = {}
+
+        if move_between_lists?
+          attrs.merge!(
+            add_label_ids: add_label_ids,
+            remove_label_ids: remove_label_ids,
+            state_event: issue_state,
+          )
+        end
+
+        attrs[:move_between_iids] = move_between_iids if move_between_iids
+
+        attrs
       end
 
       def issue_state
@@ -58,6 +66,12 @@ module Boards
 
         Array(label_ids).compact
       end
+
+      def move_between_iids
+        return unless params[:move_after_iid] || params[:move_before_iid]
+
+        [params[:move_after_iid], params[:move_before_iid]]
+      end
     end
   end
 end

app/services/issuable_base_service.rb

@@ -211,7 +211,7 @@ class IssuableBaseService < BaseService
     label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids)
     params[:label_ids] = label_ids if labels_changing?(issuable.label_ids, label_ids)
 
-    if params.present?
+    if issuable.changed? || params.present?
       issuable.assign_attributes(params.merge(updated_by: current_user))
 
       before_update(issuable)

app/services/issues/create_service.rb

@@ -13,6 +13,7 @@ module Issues
 
     def before_create(issue)
       spam_check(issue, current_user)
+      issue.move_to_end
     end
 
     def after_create(issuable)

app/services/issues/update_service.rb

@@ -3,8 +3,8 @@ module Issues
     include SpamCheckService
 
     def execute(issue)
+      handle_move_between_iids(issue)
       filter_spam_check_params
-
       update(issue)
     end
 
@@ -37,11 +37,13 @@ module Issues
       end
 
       added_labels = issue.labels - old_labels
+
       if added_labels.present?
         notification_service.relabeled_issue(issue, added_labels, current_user)
       end
 
       added_mentions = issue.mentioned_users - old_mentioned_users
+
       if added_mentions.present?
         notification_service.new_mentions_in_issue(issue, added_mentions, current_user)
       end
@@ -55,8 +57,24 @@ module Issues
       Issues::CloseService
     end
 
+    def handle_move_between_iids(issue)
+      return unless params[:move_between_iids]
+
+      after_iid, before_iid = params.delete(:move_between_iids)
+
+      issue_before = get_issue_if_allowed(issue.project, before_iid) if before_iid
+      issue_after = get_issue_if_allowed(issue.project, after_iid) if after_iid
+
+      issue.move_between(issue_before, issue_after)
+    end
+
     private
 
+    def get_issue_if_allowed(project, iid)
+      issue = project.issues.find_by(iid: iid)
+      issue if can?(current_user, :update_issue, issue)
+    end
+
     def create_confidentiality_note(issue)
       SystemNoteService.change_issue_confidentiality(issue, issue.project, current_user)
     end

app/services/merge_requests/squash_service.rb

@@ -41,7 +41,7 @@ module MergeRequests
       )
 
       run_git_command(
-        %W(commit --no-verify -m #{merge_request.title}),
+        %W(commit --no-verify --message #{merge_request.title}),
         tree_path,
         git_env.merge('GIT_COMMITTER_NAME' => current_user.name,
                       'GIT_COMMITTER_EMAIL' => current_user.email,
@@ -50,8 +50,12 @@ module MergeRequests
         'commit squashed changes'
       )
 
+      # May print a warning for ambiguous refs, but we can ignore that with
+      # `--quiet` and just take the SHA, if present. HEAD here always refers to
+      # the current HEAD commit, even if there is another ref called HEAD.
+      #
       squash_sha = run_git_command(
-        %w(rev-parse HEAD),
+        %w(rev-parse --quiet --verify HEAD),
         tree_path,
         git_env,
         'get SHA of squashed commit'

app/views/projects/boards/components/_board_list.html.haml

@@ -8,7 +8,7 @@
     "v-show" => "!loading",
     ":data-board" => "list.id",
     ":class" => '{ "is-smaller": showIssueForm }' }
-    %board-card{ "v-for" => "(issue, index) in orderedIssues",
+    %board-card{ "v-for" => "(issue, index) in issues",
       "ref" => "issue",
       ":index" => "index",
       ":list" => "list",
@@ -17,7 +17,8 @@
       ":root-path" => "rootPath",
       ":disabled" => "disabled",
       ":key" => "issue.id" }
-    %li.board-list-count.text-center{ "v-if" => "showCount" }
+    %li.board-list-count.text-center{ "v-if" => "showCount",
+      "data-issue-id" => "-1" }
       = icon("spinner spin", "v-show" => "list.loadingMore" )
       %span{ "v-if" => "list.issues.length === list.issuesSize" }
         Showing all issues

changelogs/unreleased-ee/1853-fix-group-webhook-token.yml

+---
+title: Allow all parameters of group webhooks to be set through the UI
+merge_request: 1376
+author:

changelogs/unreleased-ee/1861-squash-commits-fails-with-older-branches.yml

+---
+title: Fix squashing MRs when the repository contains a ref named HEAD
+merge_request:
+author:

changelogs/unreleased/26202-change-dropdown-style-slightly.yml

+---
+title: Changed dropdown style slightly
+merge_request:
+author:

db/migrate/20170131221752_add_relative_position_to_issues.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddRelativePositionToIssues < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  # When a migration requires downtime you **must** uncomment the following
+  # constant and define a short and easy to understand explanation as to why the
+  # migration requires downtime.
+  # DOWNTIME_REASON = ''
+
+  # When using the methods "add_concurrent_index" or "add_column_with_default"
+  # you must disable the use of transactions as these methods can not run in an
+  # existing transaction. When using "add_concurrent_index" make sure that this
+  # method is the _only_ method called in the migration, any other changes
+  # should go in a separate migration. This ensures that upon failure _only_ the
+  # index creation fails and can be retried or reverted easily.
+  #
+  # To disable transactions uncomment the following line and remove these
+  # comments:
+  disable_ddl_transaction!
+
+  def up
+    add_column :issues, :relative_position, :integer
+
+    add_concurrent_index :issues, :relative_position
+  end
+
+  def down
+    remove_column :issues, :relative_position
+
+    remove_index :issues, :relative_position if index_exists? :issues, :relative_position
+  end
+end

db/schema.rb

@@ -615,6 +615,7 @@ ActiveRecord::Schema.define(version: 20170308015651) do
     t.text "title_html"
     t.text "description_html"
     t.integer "time_estimate"
+    t.integer "relative_position"
   end
 
   add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree
@@ -626,6 +627,7 @@ ActiveRecord::Schema.define(version: 20170308015651) do
   add_index "issues", ["due_date"], name: "index_issues_on_due_date", using: :btree
   add_index "issues", ["milestone_id"], name: "index_issues_on_milestone_id", using: :btree
   add_index "issues", ["project_id", "iid"], name: "index_issues_on_project_id_and_iid", unique: true, using: :btree
+  add_index "issues", ["relative_position"], name: "index_issues_on_relative_position", using: :btree
   add_index "issues", ["state"], name: "index_issues_on_state", using: :btree
   add_index "issues", ["title"], name: "index_issues_on_title_trigram", using: :gin, opclasses: {"title"=>"gin_trgm_ops"}
 

doc/api/v3_to_v4.md

@@ -69,9 +69,14 @@ changes are in V4:
 - Rename Build Triggers to be Pipeline Triggers API [!9713](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9713)
   - `POST /projects/:id/trigger/builds` to `POST /projects/:id/trigger/pipeline`
   - Require description when creating a new trigger `POST /projects/:id/triggers`
+- Simplify project payload exposed on Environment endpoints [!9675](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9675)
+- API uses merge request `IID`s (internal ID, as in the web UI) rather than `ID`s. This affects the merge requests, award emoji, todos, and time tracking APIs. [!9530](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9530)
+- API uses issue `IID`s (internal ID, as in the web UI) rather than `ID`s. This affects the issues, award emoji, todos, and time tracking APIs. [!9530](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9530)
+- Change initial page from `0` to `1` on `GET projects/:id/repository/commits` (like on the rest of the API) [!9679] (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9679)
+- Return correct `Link` header data for `GET projects/:id/repository/commits` [!9679] (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9679)
 
 #### EE-specific
 
 - Remove the ProjectGitHook API. Use the ProjectPushRule API instead [!1301](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1301)
 - Removed `repository_storage` from `PUT /application/settings` and `GET /application/settings` (use `repository_storages` instead) [!1307](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1307)
-- Removed `elasticsearch_host` and `elasticsearch_port` from `PUT /application/settings` (use `elasticsearch_url` instead) [!1305](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1305)
+- Removed `elasticsearch_host` and `elasticsearch_port` from `PUT /application/settings` (use `elasticsearch_url` instead) [!1305](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1305)
\ No newline at end of file

spec/controllers/groups/hooks_controller_spec.rb

+require 'spec_helper'
+
+describe Groups::HooksController do
+  let(:user)  { create(:user) }
+  let(:group) { create(:group) }
+
+  before do
+    group.add_owner(user)
+    sign_in(user)
+  end
+
+  describe 'POST #create' do
+    it 'sets all parameters' do
+      hook_params = {
+        build_events: true,
+        confidential_issues_events: true,
+        enable_ssl_verification: true,
+        issues_events: true,
+        merge_requests_events: true,
+        note_events: true,
+        pipeline_events: true,
+        push_events: true,
+        tag_push_events: true,
+        token: "TEST TOKEN",
+        url: "http://example.com",
+        wiki_page_events: true,
+      }
+
+      post :create, group_id: group.to_param, hook: hook_params
+
+      expect(response).to have_http_status(302)
+      expect(group.hooks.size).to eq(1)
+      expect(group.hooks.first).to have_attributes(hook_params)
+    end
+  end
+end

spec/controllers/projects/boards/issues_controller_spec.rb

@@ -43,6 +43,7 @@ describe Projects::Boards::IssuesController do
 
           expect(response).to match_response_schema('issues')
           expect(parsed_response.length).to eq 2
+          expect(development.issues.map(&:relative_position)).not_to include(nil)
         end
       end
 

spec/features/boards/boards_spec.rb

@@ -71,16 +71,16 @@ describe 'Issue Boards', feature: true, js: true do
     let!(:list1) { create(:list, board: board, label: planning, position: 0) }
     let!(:list2) { create(:list, board: board, label: development, position: 1) }
 
-    let!(:confidential_issue) { create(:labeled_issue, :confidential, project: project, author: user, labels: [planning]) }
-    let!(:issue1) { create(:labeled_issue, project: project, assignee: user, labels: [planning]) }
-    let!(:issue2) { create(:labeled_issue, project: project, author: user2, labels: [planning]) }
-    let!(:issue3) { create(:labeled_issue, project: project, labels: [planning]) }
-    let!(:issue4) { create(:labeled_issue, project: project, labels: [planning]) }
-    let!(:issue5) { create(:labeled_issue, project: project, labels: [planning], milestone: milestone) }
-    let!(:issue6) { create(:labeled_issue, project: project, labels: [planning, development]) }
-    let!(:issue7) { create(:labeled_issue, project: project, labels: [development]) }
+    let!(:confidential_issue) { create(:labeled_issue, :confidential, project: project, author: user, labels: [planning], relative_position: 9) }
+    let!(:issue1) { create(:labeled_issue, project: project, assignee: user, labels: [planning], relative_position: 8) }
+    let!(:issue2) { create(:labeled_issue, project: project, author: user2, labels: [planning], relative_position: 7) }
+    let!(:issue3) { create(:labeled_issue, project: project, labels: [planning], relative_position: 6) }
+    let!(:issue4) { create(:labeled_issue, project: project, labels: [planning], relative_position: 5) }
+    let!(:issue5) { create(:labeled_issue, project: project, labels: [planning], milestone: milestone, relative_position: 4) }
+    let!(:issue6) { create(:labeled_issue, project: project, labels: [planning, development], relative_position: 3) }
+    let!(:issue7) { create(:labeled_issue, project: project, labels: [development], relative_position: 2) }
     let!(:issue8) { create(:closed_issue, project: project) }
-    let!(:issue9) { create(:labeled_issue, project: project, labels: [planning, testing, bug, accepting]) }
+    let!(:issue9) { create(:labeled_issue, project: project, labels: [planning, testing, bug, accepting], relative_position: 1) }
 
     before do
       visit namespace_project_boards_path(project.namespace, project)

spec/features/boards/issue_ordering_spec.rb

+require 'rails_helper'
+
+describe 'Issue Boards', :feature, :js do
+  include WaitForVueResource
+  include DragTo
+
+  let(:project) { create(:empty_project, :public) }
+  let(:board) { create(:board, project: project) }
+  let(:user) { create(:user) }
+  let(:label) { create(:label, project: project) }
+  let!(:list1) { create(:list, board: board, label: label, position: 0) }
+  let!(:issue1) { create(:labeled_issue, project: project, title: 'testing 1', labels: [label], relative_position: 3) }
+  let!(:issue2) { create(:labeled_issue, project: project, title: 'testing 2', labels: [label], relative_position: 2) }
+  let!(:issue3) { create(:labeled_issue, project: project, title: 'testing 3', labels: [label], relative_position: 1) }
+
+  before do
+    project.team << [user, :master]
+
+    login_as(user)
+  end
+
+  context 'un-ordered issues' do
+    let!(:issue4) { create(:labeled_issue, project: project, labels: [label]) }
+
+    before do
+      visit namespace_project_board_path(project.namespace, project, board)
+      wait_for_vue_resource
+
+      expect(page).to have_selector('.board', count: 2)
+    end
+
+    it 'has un-ordered issue as last issue' do
+      page.within(first('.board')) do
+        expect(all('.card').last).to have_content(issue4.title)
+      end
+    end
+
+    it 'moves un-ordered issue to top of list' do
+      drag(from_index: 3, to_index: 0)
+
+      page.within(first('.board')) do
+        expect(first('.card')).to have_content(issue4.title)
+      end
+    end
+  end
+
+  context 'ordering in list' do
+    before do
+      visit namespace_project_board_path(project.namespace, project, board)
+      wait_for_vue_resource
+
+      expect(page).to have_selector('.board', count: 2)
+    end
+
+    it 'moves from middle to top' do
+      drag(from_index: 1, to_index: 0)
+
+      wait_for_vue_resource
+
+      expect(first('.card')).to have_content(issue2.title)
+    end
+
+    it 'moves from middle to bottom' do
+      drag(from_index: 1, to_index: 2)
+
+      wait_for_vue_resource
+
+      expect(all('.card').last).to have_content(issue2.title)
+    end
+
+    it 'moves from top to bottom' do
+      drag(from_index: 0, to_index: 2)
+
+      wait_for_vue_resource
+
+      expect(all('.card').last).to have_content(issue3.title)
+    end
+
+    it 'moves from bottom to top' do
+      drag(from_index: 2, to_index: 0)
+
+      wait_for_vue_resource
+
+      expect(first('.card')).to have_content(issue1.title)
+    end
+
+    it 'moves from top to middle' do
+      drag(from_index: 0, to_index: 1)
+
+      wait_for_vue_resource
+
+      expect(first('.card')).to have_content(issue2.title)
+    end
+
+    it 'moves from bottom to middle' do
+      drag(from_index: 2, to_index: 1)
+
+      wait_for_vue_resource
+
+      expect(all('.card').last).to have_content(issue2.title)
+    end
+  end
+
+  context 'ordering when changing list' do
+    let(:label2) { create(:label, project: project) }
+    let!(:list2) { create(:list, board: board, label: label2, position: 1) }
+    let!(:issue4) { create(:labeled_issue, project: project, title: 'testing 1', labels: [label2], relative_position: 3.0) }
+    let!(:issue5) { create(:labeled_issue, project: project, title: 'testing 2', labels: [label2], relative_position: 2.0) }
+    let!(:issue6) { create(:labeled_issue, project: project, title: 'testing 3', labels: [label2], relative_position: 1.0) }
+
+    before do
+      visit namespace_project_board_path(project.namespace, project, board)
+      wait_for_vue_resource
+
+      expect(page).to have_selector('.board', count: 3)
+    end
+
+    it 'moves to top of another list' do
+      drag(list_from_index: 0, list_to_index: 1)
+
+      wait_for_vue_resource
+
+      expect(first('.board')).to have_selector('.card', count: 2)
+      expect(all('.board')[1]).to have_selector('.card', count: 4)
+
+      page.within(all('.board')[1]) do
+        expect(first('.card')).to have_content(issue3.title)
+      end
+    end
+
+    it 'moves to bottom of another list' do
+      drag(list_from_index: 0, list_to_index: 1, to_index: 2)
+
+      wait_for_vue_resource
+
+      expect(first('.board')).to have_selector('.card', count: 2)
+      expect(all('.board')[1]).to have_selector('.card', count: 4)
+
+      page.within(all('.board')[1]) do
+        expect(all('.card').last).to have_content(issue3.title)
+      end
+    end
+
+    it 'moves to index of another list' do
+      drag(list_from_index: 0, list_to_index: 1, to_index: 1)
+
+      wait_for_vue_resource
+
+      expect(first('.board')).to have_selector('.card', count: 2)
+      expect(all('.board')[1]).to have_selector('.card', count: 4)
+
+      page.within(all('.board')[1]) do
+        expect(all('.card')[1]).to have_content(issue3.title)
+      end
+    end
+  end
+
+  def drag(selector: '.board-list', list_from_index: 0, from_index: 0, to_index: 0, list_to_index: 0)
+    drag_to(selector: selector,
+            scrollable: '#board-app',
+            list_from_index: list_from_index,
+            from_index: from_index,
+            to_index: to_index,
+            list_to_index: list_to_index)
+  end
+end

spec/features/boards/sidebar_spec.rb

@@ -11,8 +11,8 @@ describe 'Issue Boards', feature: true, js: true do
   let!(:bug)         { create(:label, project: project, name: 'Bug') }
   let!(:regression)  { create(:label, project: project, name: 'Regression') }
   let!(:stretch)     { create(:label, project: project, name: 'Stretch') }
-  let!(:issue1)      { create(:labeled_issue, project: project, assignee: user, milestone: milestone, labels: [development]) }
-  let!(:issue2)      { create(:labeled_issue, project: project, labels: [development, stretch]) }
+  let!(:issue1)      { create(:labeled_issue, project: project, assignee: user, milestone: milestone, labels: [development], relative_position: 2) }
+  let!(:issue2)      { create(:labeled_issue, project: project, labels: [development, stretch], relative_position: 1) }
   let(:board)        { create(:board, project: project) }
   let!(:list)        { create(:list, board: board, label: development, position: 0) }
   let(:card) { first('.board').first('.card') }

spec/features/issues/filtered_search/filter_issues_spec.rb

@@ -112,7 +112,7 @@ describe 'Filter issues', js: true, feature: true do
     end
 
     context 'author with other filters' do
-      search_term = 'issue'
+      let(:search_term) { 'issue' }
 
       it 'filters issues by searched author and text' do
         input_filtered_search("author:@#{user.username} #{search_term}")
@@ -536,7 +536,7 @@ describe 'Filter issues', js: true, feature: true do
     end
 
     context 'milestone with other filters' do
-      search_term = 'bug'
+      let(:search_term) { 'bug' }
 
       it 'filters issues by searched milestone and text' do
         input_filtered_search("milestone:%#{milestone.title} #{search_term}")

spec/features/issues/filtered_search/visual_tokens_spec.rb

@@ -226,6 +226,22 @@ describe 'Visual tokens', js: true, feature: true do
     end
   end
 
+  describe 'editing multiple tokens' do
+    before do
+      input_filtered_search('author:@root assignee:none', submit: false)
+      first('.tokens-container .filtered-search-token').double_click
+    end
+
+    it 'opens author dropdown' do
+      expect(page).to have_css('#js-dropdown-author', visible: true)
+    end
+
+    it 'opens assignee dropdown' do
+      find('.tokens-container .filtered-search-token', text: 'Assignee').double_click
+      expect(page).to have_css('#js-dropdown-assignee', visible: true)
+    end
+  end
+
   describe 'add new token after editing existing token' do
     before do
       input_filtered_search('author:@root assignee:none', submit: false)

spec/features/merge_requests/filter_by_labels_spec.rb

@@ -70,7 +70,7 @@ feature 'Issue filtering by Labels', feature: true, js: true do
 
   context 'filter by label enhancement and bug in issues list' do
     before do
-      input_filtered_search('label:~bug label:~enhancement ')
+      input_filtered_search('label:~bug label:~enhancement')
     end
 
     it 'applies the filters' do

spec/fixtures/api/schemas/issue.json

@@ -11,6 +11,7 @@
     "title": { "type": "string" },
     "confidential": { "type": "boolean" },
     "due_date": { "type": ["date", "null"] },
+    "relative_position": { "type": "integer" },
     "labels": {
       "type": "array",
       "items": {

spec/javascripts/boards/boards_store_spec.js

@@ -5,6 +5,7 @@
 /* global Cookies */
 /* global listObj */
 /* global listObjDuplicate */
+/* global ListIssue */
 
 require('~/lib/utils/url_utility');
 require('~/boards/models/issue');
@@ -14,6 +15,7 @@ require('~/boards/models/user');
 require('~/boards/services/board_service');
 require('~/boards/stores/boards_store');
 require('./mock_data');
+require('es6-promise').polyfill();
 
 describe('Store', () => {
   beforeEach(() => {
@@ -21,6 +23,10 @@ describe('Store', () => {
     gl.boardService = new BoardService('/test/issue-boards/board', '', '1');
     gl.issueBoards.BoardsStore.create();
 
+    spyOn(gl.boardService, 'moveIssue').and.callFake(() => new Promise((resolve) => {
+      resolve();
+    }));
+
     Cookies.set('issue_board_welcome_hidden', 'false', {
       expires: 365 * 10,
       path: ''
@@ -154,5 +160,74 @@ describe('Store', () => {
         done();
       }, 0);
     });
+
+    it('moves issue to top of another list', (done) => {
+      const listOne = gl.issueBoards.BoardsStore.addList(listObj);
+      const listTwo = gl.issueBoards.BoardsStore.addList(listObjDuplicate);
+
+      expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(2);
+
+      setTimeout(() => {
+        listOne.issues[0].id = 2;
+
+        expect(listOne.issues.length).toBe(1);
+        expect(listTwo.issues.length).toBe(1);
+
+        gl.issueBoards.BoardsStore.moveIssueToList(listOne, listTwo, listOne.findIssue(2), 0);
+
+        expect(listOne.issues.length).toBe(0);
+        expect(listTwo.issues.length).toBe(2);
+        expect(listTwo.issues[0].id).toBe(2);
+        expect(gl.boardService.moveIssue).toHaveBeenCalledWith(2, listOne.id, listTwo.id, null, 1);
+
+        done();
+      }, 0);
+    });
+
+    it('moves issue to bottom of another list', (done) => {
+      const listOne = gl.issueBoards.BoardsStore.addList(listObj);
+      const listTwo = gl.issueBoards.BoardsStore.addList(listObjDuplicate);
+
+      expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(2);
+
+      setTimeout(() => {
+        listOne.issues[0].id = 2;
+
+        expect(listOne.issues.length).toBe(1);
+        expect(listTwo.issues.length).toBe(1);
+
+        gl.issueBoards.BoardsStore.moveIssueToList(listOne, listTwo, listOne.findIssue(2), 1);
+
+        expect(listOne.issues.length).toBe(0);
+        expect(listTwo.issues.length).toBe(2);
+        expect(listTwo.issues[1].id).toBe(2);
+        expect(gl.boardService.moveIssue).toHaveBeenCalledWith(2, listOne.id, listTwo.id, 1, null);
+
+        done();
+      }, 0);
+    });
+
+    it('moves issue in list', (done) => {
+      const issue = new ListIssue({
+        title: 'Testing',
+        iid: 2,
+        confidential: false,
+        labels: []
+      });
+      const list = gl.issueBoards.BoardsStore.addList(listObj);
+
+      setTimeout(() => {
+        list.addIssue(issue);
+
+        expect(list.issues.length).toBe(2);
+
+        gl.issueBoards.BoardsStore.moveIssueInList(list, issue, 0, 1, [1, 2]);
+
+        expect(list.issues[0].id).toBe(2);
+        expect(gl.boardService.moveIssue).toHaveBeenCalledWith(2, null, null, 1, null);
+
+        done();
+      });
+    });
   });
 });

spec/javascripts/boards/issue_spec.js

@@ -79,4 +79,20 @@ describe('Issue model', () => {
     issue.removeLabels([issue.labels[0], issue.labels[1]]);
     expect(issue.labels.length).toBe(0);
   });
+
+  it('sets position to infinity if no position is stored', () => {
+    expect(issue.position).toBe(Infinity);
+  });
+
+  it('sets position', () => {
+    const relativePositionIssue = new ListIssue({
+      title: 'Testing',
+      iid: 1,
+      confidential: false,
+      relative_position: 1,
+      labels: []
+    });
+
+    expect(relativePositionIssue.position).toBe(1);
+  });
 });

spec/javascripts/boards/list_spec.js

@@ -103,6 +103,7 @@ describe('List model', () => {
 
     listDup.updateIssueLabel(list, issue);
 
-    expect(gl.boardService.moveIssue).toHaveBeenCalledWith(issue.id, list.id, listDup.id);
+    expect(gl.boardService.moveIssue)
+      .toHaveBeenCalledWith(issue.id, list.id, listDup.id, undefined, undefined);
   });
 });

spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js

@@ -533,12 +533,12 @@ describe('Filtered Search Visual Tokens', () => {
         FilteredSearchSpecHelper.createFilterVisualTokenHTML('label', 'none'),
       );
 
-      spyOn(gl.FilteredSearchVisualTokens, 'tokenizeInput').and.callThrough();
+      spyOn(gl.FilteredSearchVisualTokens, 'tokenizeInput').and.callFake(() => {});
       spyOn(gl.FilteredSearchVisualTokens, 'getLastVisualTokenBeforeInput').and.callThrough();
 
       gl.FilteredSearchVisualTokens.moveInputToTheRight();
 
-      expect(gl.FilteredSearchVisualTokens.tokenizeInput).not.toHaveBeenCalled();
+      expect(gl.FilteredSearchVisualTokens.tokenizeInput).toHaveBeenCalled();
       expect(gl.FilteredSearchVisualTokens.getLastVisualTokenBeforeInput).not.toHaveBeenCalled();
     });
 
@@ -583,5 +583,18 @@ describe('Filtered Search Visual Tokens', () => {
 
       expect(tokensContainer.children[2].querySelector('.filtered-search')).not.toEqual(null);
     });
+
+    it('tokenizes input even if input is the right most element', () => {
+      tokensContainer.innerHTML = `
+        ${FilteredSearchSpecHelper.createFilterVisualTokenHTML('label', 'none')}
+        ${FilteredSearchSpecHelper.createNameFilterVisualTokenHTML('label')}
+        ${FilteredSearchSpecHelper.createInputHTML('', '~bug')}
+      `;
+
+      gl.FilteredSearchVisualTokens.moveInputToTheRight();
+
+      const token = tokensContainer.children[1];
+      expect(token.querySelector('.value').innerText).toEqual('~bug');
+    });
   });
 });

spec/javascripts/helpers/filtered_search_spec_helper.js

@@ -33,10 +33,10 @@ class FilteredSearchSpecHelper {
     `;
   }
 
-  static createInputHTML(placeholder = '') {
+  static createInputHTML(placeholder = '', value = '') {
     return `
       <li class="input-token">
-        <input type='text' class='filtered-search' placeholder='${placeholder}' />
+        <input type='text' class='filtered-search' placeholder='${placeholder}' value='${value}'/>
       </li>
     `;
   }

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -21,6 +21,7 @@ Issue:
 - milestone_id
 - weight
 - time_estimate
+- relative_position
 Event:
 - id
 - target_type

spec/models/concerns/relative_positioning_spec.rb

+require 'spec_helper'
+
+describe Issue, 'RelativePositioning' do
+  let(:project) { create(:empty_project) }
+  let(:issue) { create(:issue, project: project) }
+  let(:issue1) { create(:issue, project: project) }
+  let(:new_issue) { create(:issue, project: project) }
+
+  before do
+    [issue, issue1].each do |issue|
+      issue.move_to_end && issue.save
+    end
+  end
+
+  describe '#min_relative_position' do
+    it 'returns maximum position' do
+      expect(issue.min_relative_position).to eq issue.relative_position
+    end
+  end
+
+  describe '#max_relative_position' do
+    it 'returns maximum position' do
+      expect(issue.max_relative_position).to eq issue1.relative_position
+    end
+  end
+
+  describe '#prev_relative_position' do
+    it 'returns previous position if there is an issue above' do
+      expect(issue1.prev_relative_position).to eq issue.relative_position
+    end
+
+    it 'returns minimum position if there is no issue above' do
+      expect(issue.prev_relative_position).to eq RelativePositioning::MIN_POSITION
+    end
+  end
+
+  describe '#next_relative_position' do
+    it 'returns next position if there is an issue below' do
+      expect(issue.next_relative_position).to eq issue1.relative_position
+    end
+
+    it 'returns next position if there is no issue below' do
+      expect(issue1.next_relative_position).to eq RelativePositioning::MAX_POSITION
+    end
+  end
+
+  describe '#move_before' do
+    it 'moves issue before' do
+      [issue1, issue].each(&:move_to_end)
+
+      issue.move_before(issue1)
+
+      expect(issue.relative_position).to be < issue1.relative_position
+    end
+  end
+
+  describe '#move_after' do
+    it 'moves issue after' do
+      [issue, issue1].each(&:move_to_end)
+
+      issue.move_after(issue1)
+
+      expect(issue.relative_position).to be > issue1.relative_position
+    end
+  end
+
+  describe '#move_to_end' do
+    it 'moves issue to the end' do
+      new_issue.move_to_end
+
+      expect(new_issue.relative_position).to be > issue1.relative_position
+    end
+  end
+
+  describe '#move_between' do
+    it 'positions issue between two other' do
+      new_issue.move_between(issue, issue1)
+
+      expect(new_issue.relative_position).to be > issue.relative_position
+      expect(new_issue.relative_position).to be < issue1.relative_position
+    end
+
+    it 'positions issue between on top' do
+      new_issue.move_between(nil, issue)
+
+      expect(new_issue.relative_position).to be < issue.relative_position
+    end
+
+    it 'positions issue between to end' do
+      new_issue.move_between(issue1, nil)
+
+      expect(new_issue.relative_position).to be > issue1.relative_position
+    end
+
+    it 'positions issues even when after and before positions are the same' do
+      issue1.update relative_position: issue.relative_position
+
+      new_issue.move_between(issue, issue1)
+
+      expect(new_issue.relative_position).to be > issue.relative_position
+      expect(issue.relative_position).to be < issue1.relative_position
+    end
+  end
+end

spec/services/boards/issues/list_service_spec.rb

@@ -43,32 +43,6 @@ describe Boards::Issues::ListService, services: true do
       described_class.new(project, user, params).execute
     end
 
-    context 'sets default order to priority' do
-      it 'returns opened issues when list id is missing' do
-        params = { board_id: board.id }
-
-        issues = described_class.new(project, user, params).execute
-
-        expect(issues).to eq [opened_issue2, reopened_issue1, opened_issue1]
-      end
-
-      it 'returns closed issues when listing issues from Done' do
-        params = { board_id: board.id, id: done.id }
-
-        issues = described_class.new(project, user, params).execute
-
-        expect(issues).to eq [closed_issue4, closed_issue2, closed_issue3, closed_issue1]
-      end
-
-      it 'returns opened issues that have label list applied when listing issues from a label list' do
-        params = { board_id: board.id, id: list1.id }
-
-        issues = described_class.new(project, user, params).execute
-
-        expect(issues).to eq [list1_issue3, list1_issue1, list1_issue2]
-      end
-    end
-
     context 'with list that does not belong to the board' do
       it 'raises an error' do
         list = create(:list)

spec/services/boards/issues/move_service_spec.rb

@@ -78,8 +78,10 @@ describe Boards::Issues::MoveService, services: true do
     end
 
     context 'when moving to same list' do
-      let(:issue)  { create(:labeled_issue, project: project, labels: [bug, development]) }
-      let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list1.id } }
+      let(:issue)   { create(:labeled_issue, project: project, labels: [bug, development]) }
+      let(:issue1)  { create(:labeled_issue, project: project, labels: [bug, development]) }
+      let(:issue2)  { create(:labeled_issue, project: project, labels: [bug, development]) }
+      let(:params)  { { board_id: board1.id, from_list_id: list1.id, to_list_id: list1.id } }
 
       it 'returns false' do
         expect(described_class.new(project, user, params).execute(issue)).to eq false
@@ -90,6 +92,18 @@ describe Boards::Issues::MoveService, services: true do
 
         expect(issue.reload.labels).to contain_exactly(bug, development)
       end
+
+      it 'sorts issues' do
+        [issue, issue1, issue2].each do |issue|
+          issue.move_to_end && issue.save!
+        end
+
+        params.merge!(move_after_iid: issue1.iid, move_before_iid: issue2.iid)
+
+        described_class.new(project, user, params).execute(issue)
+
+        expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
+      end
     end
   end
 end

spec/services/issues/update_service_spec.rb

@@ -58,6 +58,22 @@ describe Issues::UpdateService, services: true do
         expect(issue.due_date).to eq Date.tomorrow
       end
 
+      it 'sorts issues as specified by parameters' do
+        issue1 = create(:issue, project: project, assignee_id: user3.id)
+        issue2 = create(:issue, project: project, assignee_id: user3.id)
+
+        [issue, issue1, issue2].each do |issue|
+          issue.move_to_end
+          issue.save
+        end
+
+        opts[:move_between_iids] = [issue1.iid, issue2.iid]
+
+        update_issue(opts)
+
+        expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
+      end
+
       context 'when current user cannot admin issues in the project' do
         let(:guest) { create(:user) }
         before do

spec/services/merge_requests/refresh_service_spec.rb

@@ -66,18 +66,19 @@ describe MergeRequests::RefreshService, services: true do
       it 'executes hooks with update action' do
         expect(refresh_service).to have_received(:execute_hooks).
           with(@merge_request, 'update', @oldrev)
-      end
 
-      it { expect(@merge_request.notes).not_to be_empty }
-      it { expect(@merge_request).to be_open }
-      it { expect(@merge_request.approvals).to be_empty }
-      it { expect(@merge_request.merge_when_pipeline_succeeds).to be_falsey }
-      it { expect(@merge_request.diff_head_sha).to eq(@newrev) }
-      it { expect(@fork_merge_request).to be_open }
-      it { expect(@fork_merge_request.notes).to be_empty }
-      it { expect(@build_failed_todo).to be_done }
-      it { expect(@fork_build_failed_todo).to be_done }
-      it { expect(@fork_merge_request.approvals).not_to be_empty }
+        expect(@merge_request.notes).not_to be_empty
+        expect(@merge_request).to be_open
+        expect(@merge_request.merge_when_pipeline_succeeds).to be_falsey
+        expect(@merge_request.diff_head_sha).to eq(@newrev)
+        expect(@fork_merge_request).to be_open
+        expect(@fork_merge_request.notes).to be_empty
+        expect(@build_failed_todo).to be_done
+        expect(@fork_build_failed_todo).to be_done
+        # EE-only
+        expect(@merge_request.approvals).to be_empty
+        expect(@fork_merge_request.approvals).not_to be_empty
+      end
     end
 
     context 'push to origin repo target branch' do
@@ -86,14 +87,17 @@ describe MergeRequests::RefreshService, services: true do
         reload_mrs
       end
 
-      it { expect(@merge_request.notes.last.note).to include('merged') }
-      it { expect(@merge_request).to be_merged }
-      it { expect(@merge_request.approvals).not_to be_empty }
-      it { expect(@fork_merge_request).to be_merged }
-      it { expect(@fork_merge_request.notes.last.note).to include('merged') }
-      it { expect(@fork_merge_request.approvals).not_to be_empty }
-      it { expect(@build_failed_todo).to be_done }
-      it { expect(@fork_build_failed_todo).to be_done }
+      it 'updates the merge state' do
+        expect(@merge_request.notes.last.note).to include('merged')
+        expect(@merge_request).to be_merged
+        expect(@fork_merge_request).to be_merged
+        expect(@fork_merge_request.notes.last.note).to include('merged')
+        expect(@build_failed_todo).to be_done
+        expect(@fork_build_failed_todo).to be_done
+        # EE-only
+        expect(@merge_request.approvals).not_to be_empty
+        expect(@fork_merge_request.approvals).not_to be_empty
+      end
     end
 
     context 'manual merge of source branch' do
@@ -107,13 +111,15 @@ describe MergeRequests::RefreshService, services: true do
         reload_mrs
       end
 
-      it { expect(@merge_request.notes.last.note).to include('merged') }
-      it { expect(@merge_request).to be_merged }
-      it { expect(@merge_request.diffs.size).to be > 0 }
-      it { expect(@fork_merge_request).to be_merged }
-      it { expect(@fork_merge_request.notes.last.note).to include('merged') }
-      it { expect(@build_failed_todo).to be_done }
-      it { expect(@fork_build_failed_todo).to be_done }
+      it 'updates the merge state' do
+        expect(@merge_request.notes.last.note).to include('merged')
+        expect(@merge_request).to be_merged
+        expect(@merge_request.diffs.size).to be > 0
+        expect(@fork_merge_request).to be_merged
+        expect(@fork_merge_request.notes.last.note).to include('merged')
+        expect(@build_failed_todo).to be_done
+        expect(@fork_build_failed_todo).to be_done
+      end
     end
 
     context 'push to fork repo source branch' do
@@ -129,16 +135,17 @@ describe MergeRequests::RefreshService, services: true do
         it 'executes hooks with update action' do
           expect(refresh_service).to have_received(:execute_hooks).
             with(@fork_merge_request, 'update', @oldrev)
-        end
 
-        it { expect(@merge_request.notes).to be_empty }
-        it { expect(@merge_request).to be_open }
-        it { expect(@merge_request.approvals).not_to be_empty }
-        it { expect(@fork_merge_request.notes.last.note).to include('added 28 commits') }
-        it { expect(@fork_merge_request).to be_open }
-        it { expect(@build_failed_todo).to be_pending }
-        it { expect(@fork_build_failed_todo).to be_pending }
-        it { expect(@fork_merge_request.approvals).to be_empty }
+          expect(@merge_request.notes).to be_empty
+          expect(@merge_request).to be_open
+          expect(@fork_merge_request.notes.last.note).to include('added 28 commits')
+          expect(@fork_merge_request).to be_open
+          expect(@build_failed_todo).to be_pending
+          expect(@fork_build_failed_todo).to be_pending
+          # EE-only
+          expect(@merge_request.approvals).not_to be_empty
+          expect(@fork_merge_request.approvals).to be_empty
+        end
       end
 
       context 'closed fork merge request' do
@@ -153,14 +160,17 @@ describe MergeRequests::RefreshService, services: true do
           expect(refresh_service).not_to have_received(:execute_hooks)
         end
 
-        it { expect(@merge_request.notes).to be_empty }
-        it { expect(@merge_request).to be_open }
-        it { expect(@merge_request.approvals).not_to be_empty }
-        it { expect(@fork_merge_request.notes).to be_empty }
-        it { expect(@fork_merge_request).to be_closed }
-        it { expect(@build_failed_todo).to be_pending }
-        it { expect(@fork_build_failed_todo).to be_pending }
-        it { expect(@fork_merge_request.approvals).to be_empty }
+        it 'updates merge request to closed state' do
+          expect(@merge_request.notes).to be_empty
+          expect(@merge_request).to be_open
+          expect(@fork_merge_request.notes).to be_empty
+          expect(@fork_merge_request).to be_closed
+          expect(@build_failed_todo).to be_pending
+          expect(@fork_build_failed_todo).to be_pending
+          # EE-only
+          expect(@merge_request.approvals).not_to be_empty
+          expect(@fork_merge_request.approvals).to be_empty
+        end
       end
     end
 
@@ -171,14 +181,17 @@ describe MergeRequests::RefreshService, services: true do
           reload_mrs
         end
 
-        it { expect(@merge_request.notes).to be_empty }
-        it { expect(@merge_request).to be_open }
-        it { expect(@merge_request.approvals).not_to be_empty }
-        it { expect(@fork_merge_request.notes).to be_empty }
-        it { expect(@fork_merge_request).to be_open }
-        it { expect(@fork_merge_request.approvals).not_to be_empty }
-        it { expect(@build_failed_todo).to be_pending }
-        it { expect(@fork_build_failed_todo).to be_pending }
+        it 'updates the merge request state' do
+          expect(@merge_request.notes).to be_empty
+          expect(@merge_request).to be_open
+          expect(@fork_merge_request.notes).to be_empty
+          expect(@fork_merge_request).to be_open
+          expect(@build_failed_todo).to be_pending
+          expect(@fork_build_failed_todo).to be_pending
+          # EE-only
+          expect(@merge_request.approvals).not_to be_empty
+          expect(@fork_merge_request.approvals).not_to be_empty
+        end
       end
 
       describe 'merge request diff' do
@@ -197,14 +210,17 @@ describe MergeRequests::RefreshService, services: true do
         reload_mrs
       end
 
-      it { expect(@merge_request.notes.last.note).to include('merged') }
-      it { expect(@merge_request).to be_merged }
-      it { expect(@merge_request.approvals).not_to be_empty }
-      it { expect(@fork_merge_request).to be_open }
-      it { expect(@fork_merge_request.notes).to be_empty }
-      it { expect(@build_failed_todo).to be_done }
-      it { expect(@fork_build_failed_todo).to be_done }
-      it { expect(@fork_merge_request.approvals).not_to be_empty }
+      it 'updates the merge request state' do
+        expect(@merge_request.notes.last.note).to include('merged')
+        expect(@merge_request).to be_merged
+        expect(@fork_merge_request).to be_open
+        expect(@fork_merge_request.notes).to be_empty
+        expect(@build_failed_todo).to be_done
+        expect(@fork_build_failed_todo).to be_done
+        # EE-only
+        expect(@merge_request.approvals).not_to be_empty
+        expect(@fork_merge_request.approvals).not_to be_empty
+      end
     end
 
     context 'resetting approvals if they are enabled' do