Merge branch 'issue_14189' into 'master'

Ability to prioritize labels

Closes #14189 

See merge request !4009

15 files changed, 255 insertions, 29 deletions

diff --git a/app/assets/javascripts/LabelManager.js.coffee b/app/assets/javascripts/LabelManager.js.coffee
new file mode 100644
index 00000000000..365a062bb81
--- /dev/null
+++ b/app/assets/javascripts/LabelManager.js.coffee
@@ -0,0 +1,84 @@
+class @LabelManager
+  errorMessage: 'Unable to update label prioritization at this time'
+
+  constructor: (opts = {}) ->
+    # Defaults
+    {
+      @togglePriorityButton = $('.js-toggle-priority')
+      @prioritizedLabels = $('.js-prioritized-labels')
+      @otherLabels = $('.js-other-labels')
+    } = opts
+
+    @prioritizedLabels.sortable(
+      items: 'li'
+      placeholder: 'list-placeholder'
+      axis: 'y'
+      update: @onPrioritySortUpdate.bind(@)
+    )
+
+    @bindEvents()
+
+  bindEvents: ->
+    @togglePriorityButton.on 'click', @, @onTogglePriorityClick
+
+  onTogglePriorityClick: (e) ->
+    e.preventDefault()
+    _this = e.data
+    $btn = $(e.currentTarget)
+    $label = $("##{$btn.data('domId')}")
+    action = if $btn.parents('.js-prioritized-labels').length then 'remove' else 'add'
+    _this.toggleLabelPriority($label, action)
+
+  toggleLabelPriority: ($label, action, persistState = true) ->
+    _this = @
+    url = $label.find('.js-toggle-priority').data 'url'
+
+    $target = @prioritizedLabels
+    $from = @otherLabels
+
+    # Optimistic update
+    if action is 'remove'
+      $target = @otherLabels
+      $from = @prioritizedLabels
+
+    if $from.find('li').length is 1
+      $from.find('.empty-message').show()
+
+    if not $target.find('li').length
+      $target.find('.empty-message').hide()
+
+    $label.detach().appendTo($target)
+
+    # Return if we are not persisting state
+    return unless persistState
+
+    if action is 'remove'
+      xhr = $.ajax url: url, type: 'DELETE'
+    else
+      xhr = @savePrioritySort($label, action)
+
+    xhr.fail @rollbackLabelPosition.bind(@, $label, action)
+
+  onPrioritySortUpdate: ->
+    xhr = @savePrioritySort()
+
+    xhr.fail ->
+      new Flash(@errorMessage, 'alert')
+
+  savePrioritySort: () ->
+    $.post
+      url: @prioritizedLabels.data('url')
+      data:
+        label_ids: @getSortedLabelsIds()
+
+  rollbackLabelPosition: ($label, originalAction)->
+    action = if originalAction is 'remove' then 'add' else 'remove'
+    @toggleLabelPriority($label, action, false)
+
+    new Flash(@errorMessage, 'alert')
+
+  getSortedLabelsIds: ->
+    sortedIds = []
+    @prioritizedLabels.find('li').each ->
+      sortedIds.push $(@).data 'id'
+    sortedIds
diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee
index ec540060457..5d6ac6e757e 100644
--- a/app/assets/javascripts/dispatcher.js.coffee
+++ b/app/assets/javascripts/dispatcher.js.coffee
@@ -100,6 +100,8 @@ class Dispatcher
         shortcut_handler = new ShortcutsNavigation()
       when 'projects:labels:new', 'projects:labels:edit'
         new Labels()
+      when 'projects:labels:index'
+        new LabelManager() if $('.prioritized-labels').length
       when 'projects:network:show'
         # Ensure we don't create a particular shortcut handler here. This is
         # already created, where the network graph is created.
diff --git a/app/assets/stylesheets/framework/lists.scss b/app/assets/stylesheets/framework/lists.scss
index b17c8bcbb1e..96e7aa4fb15 100644
--- a/app/assets/stylesheets/framework/lists.scss
+++ b/app/assets/stylesheets/framework/lists.scss
@@ -141,6 +141,18 @@ ul.content-list {
         padding: 10px 14px;
       }
     }
+
+    // When dragging a list item
+    &.ui-sortable-helper {
+      border-bottom: none;
+    }
+
+    &.list-placeholder {
+      background-color: $gray-light;
+      border: dotted 1px $gray-dark;
+      margin: 1px 0;
+      min-height: 30px;
+    }
   }
 }
 
diff --git a/app/assets/stylesheets/pages/labels.scss b/app/assets/stylesheets/pages/labels.scss
index e179bdf0048..2cd9d74b2de 100644
--- a/app/assets/stylesheets/pages/labels.scss
+++ b/app/assets/stylesheets/pages/labels.scss
@@ -51,7 +51,7 @@
 .label-row {
   .label-name {
     display: inline-block;
-    width: 200px;
+    width: 170px;
 
     @media (max-width: $screen-xs-min) {
       display: block;
@@ -138,3 +138,34 @@
     }
   }
 }
+
+.prioritized-labels {
+  margin-bottom: 30px;
+
+  .add-priority {
+    display: none;
+    color: $gray-light;
+  }
+}
+
+.other-labels {
+  .remove-priority {
+    display: none;
+  }
+}
+
+.toggle-priority {
+  display: inline-block;
+  vertical-align: middle;
+
+  button {
+    border-color: transparent;
+    padding: 5px 8px;
+    vertical-align: top;
+    font-size: 14px;
+
+    &:hover {
+      border-color: transparent;
+    }
+  }
+}
diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb
index ff771ea6d9c..0ca675623e5 100644
--- a/app/controllers/projects/labels_controller.rb
+++ b/app/controllers/projects/labels_controller.rb
@@ -5,13 +5,14 @@ class Projects::LabelsController < Projects::ApplicationController
   before_action :label, only: [:edit, :update, :destroy]
   before_action :authorize_read_label!
   before_action :authorize_admin_labels!, only: [
-    :new, :create, :edit, :update, :generate, :destroy
+    :new, :create, :edit, :update, :generate, :destroy, :remove_priority, :set_priorities
   ]
 
   respond_to :js, :html
 
   def index
-    @labels = @project.labels.page(params[:page])
+    @labels = @project.labels.unprioritized.page(params[:page])
+    @prioritized_labels = @project.labels.prioritized
 
     respond_to do |format|
       format.html
@@ -71,6 +72,30 @@ class Projects::LabelsController < Projects::ApplicationController
     end
   end
 
+  def remove_priority
+    respond_to do |format|
+      if label.update_attribute(:priority, nil)
+        format.json { render json: label }
+      else
+        message = label.errors.full_messages.uniq.join('. ')
+        format.json { render json: { message: message }, status: :unprocessable_entity }
+      end
+    end
+  end
+
+  def set_priorities
+    Label.transaction do
+      params[:label_ids].each_with_index do |label_id, index|
+        label = @project.labels.find_by_id(label_id)
+        label.update_attribute(:priority, index) if label
+      end
+    end
+
+    respond_to do |format|
+      format.json { render json: { message: 'success' } }
+    end
+  end
+
   protected
 
   def module_enabled
diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index 7d8c56f4c22..a0932712bd0 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -224,7 +224,7 @@ class IssuableFinder
   def sort(items)
     # Ensure we always have an explicit sort order (instead of inheriting
     # multiple orders when combining ActiveRecord::Relation objects).
-    params[:sort] ? items.sort(params[:sort]) : items.reorder(id: :desc)
+    params[:sort] ? items.sort(params[:sort], excluded_labels: label_names) : items.reorder(id: :desc)
   end
 
   def by_assignee(items)
@@ -318,7 +318,11 @@ class IssuableFinder
   end
 
   def label_names
-    params[:label_name].is_a?(String) ? params[:label_name].split(',') : params[:label_name]
+    if labels?
+      params[:label_name].is_a?(String) ? params[:label_name].split(',') : params[:label_name]
+    else
+      []
+    end
   end
 
   def current_user_related?
diff --git a/app/helpers/sorting_helper.rb b/app/helpers/sorting_helper.rb
index 630e10ea892..d86f1999f5c 100644
--- a/app/helpers/sorting_helper.rb
+++ b/app/helpers/sorting_helper.rb
@@ -14,7 +14,8 @@ module SortingHelper
       sort_value_recently_signin => sort_title_recently_signin,
       sort_value_oldest_signin => sort_title_oldest_signin,
       sort_value_downvotes => sort_title_downvotes,
-      sort_value_upvotes => sort_title_upvotes
+      sort_value_upvotes => sort_title_upvotes,
+      sort_value_priority => sort_title_priority
     }
   end
 
@@ -28,6 +29,10 @@ module SortingHelper
     }
   end
 
+  def sort_title_priority
+    'Priority'
+  end
+
   def sort_title_oldest_updated
     'Oldest updated'
   end
@@ -84,6 +89,10 @@ module SortingHelper
     'Most popular'
   end
 
+  def sort_value_priority
+    'priority'
+  end
+
   def sort_value_oldest_updated
     'updated_asc'
   end
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 5d279ae602a..92526a99147 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -105,17 +105,24 @@ module Issuable
       where(t[:title].matches(pattern).or(t[:description].matches(pattern)))
     end
 
-    def sort(method)
+    def sort(method, excluded_labels: [])
       case method.to_s
       when 'milestone_due_asc' then order_milestone_due_asc
       when 'milestone_due_desc' then order_milestone_due_desc
       when 'downvotes_desc' then order_downvotes_desc
       when 'upvotes_desc' then order_upvotes_desc
+      when 'priority' then order_labels_priority(excluded_labels: excluded_labels)
       else
         order_by(method)
       end
     end
 
+    def order_labels_priority(excluded_labels: [])
+      select("#{table_name}.*, (#{highest_label_priority(excluded_labels).to_sql}) AS highest_priority").
+        group(arel_table[:id]).
+        reorder(Gitlab::Database.nulls_last_order('highest_priority', 'ASC'))
+    end
+
     def with_label(title, sort = nil)
       if title.is_a?(Array) && title.size > 1
         joins(:labels).where(labels: { title: title }).group(*grouping_columns(sort)).having("COUNT(DISTINCT labels.title) = #{title.size}")
@@ -139,6 +146,20 @@ module Issuable
 
       grouping_columns
     end
+
+    private
+
+    def highest_label_priority(excluded_labels)
+      query = Label.select(Label.arel_table[:priority].minimum).
+        joins(:label_links).
+        where(label_links: { target_type: name }).
+        where("label_links.target_id = #{table_name}.id").
+        reorder(nil)
+
+      query.where.not(title: excluded_labels) if excluded_labels.present?
+
+      query
+    end
   end
 
   def today?
diff --git a/app/models/issue.rb b/app/models/issue.rb
index bd0fbc96d18..235922710ad 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -75,10 +75,10 @@ class Issue < ActiveRecord::Base
     @link_reference_pattern ||= super("issues", /(?<issue>\d+)/)
   end
 
-  def self.sort(method)
+  def self.sort(method, excluded_labels: [])
     case method.to_s
     when 'due_date_asc' then order_due_date_asc
-    when 'due_date_desc'  then order_due_date_desc
+    when 'due_date_desc' then order_due_date_desc
     else
       super
     end
diff --git a/app/models/label.rb b/app/models/label.rb
index e5ad11983be..49c352cc239 100644
--- a/app/models/label.rb
+++ b/app/models/label.rb
@@ -26,10 +26,20 @@ class Label < ActiveRecord::Base
             format: { with: /\A[^&\?,]+\z/ },
             uniqueness: { scope: :project_id }
 
+  before_save :nullify_priority
+
   default_scope { order(title: :asc) }
 
   scope :templates, ->  { where(template: true) }
 
+  def self.prioritized
+    where.not(priority: nil).reorder(:priority, :title)
+  end
+
+  def self.unprioritized
+    where(priority: nil)
+  end
+
   alias_attribute :name, :title
 
   def self.reference_prefix
@@ -118,4 +128,8 @@ class Label < ActiveRecord::Base
       id
     end
   end
+
+  def nullify_priority
+    self.priority = nil if priority.blank?
+  end
 end
diff --git a/app/views/projects/labels/_label.html.haml b/app/views/projects/labels/_label.html.haml
index 294fec422c5..1c51ea676c7 100644
--- a/app/views/projects/labels/_label.html.haml
+++ b/app/views/projects/labels/_label.html.haml
@@ -1,4 +1,5 @@
-%li{ id: dom_id(label), data: { id: label.id } }
+- label_css_id = dom_id(label)
+%li{id: label_css_id, data: { id: label.id } }
   = render "shared/label_row", label: label
   .pull-info-right
     %span.append-right-20
@@ -10,18 +11,18 @@
         = pluralize label.open_issues_count(current_user), 'open issue'
 
     - if current_user
-      .label-subscription{data: {url: toggle_subscription_namespace_project_label_path(@project.namespace, @project, label)}}
-        .subscription-status{data: {status: label_subscription_status(label)}}
+      .label-subscription{ data: { url: toggle_subscription_namespace_project_label_path(@project.namespace, @project, label) } }
+        .subscription-status{ data: { status: label_subscription_status(label) } }
 
         %button.js-subscribe-button.label-subscribe-button.btn.action-buttons{ type: "button", data: { toggle: "tooltip" } }
           %span= label_subscription_toggle_button_text(label)
 
-    - if can? current_user, :admin_label, @project
-      = link_to edit_namespace_project_label_path(@project.namespace, @project, label), title: "Edit", class: 'btn action-buttons', data: {toggle: "tooltip"} do
+    - if can?(current_user, :admin_label, @project)
+      = link_to edit_namespace_project_label_path(@project.namespace, @project, label), title: "Edit", class: 'btn action-buttons', data: { toggle: 'tooltip' } do
         %i.fa.fa-pencil-square-o
-      = link_to namespace_project_label_path(@project.namespace, @project, label), title: "Delete", class: 'btn action-buttons remove-row', method: :delete, remote: true, data: {confirm: "Remove this label? Are you sure?", toggle: "tooltip"} do
+      = link_to namespace_project_label_path(@project.namespace, @project, label), title: "Delete", class: 'btn action-buttons remove-row', method: :delete, remote: true, data: { confirm: 'Remove this label? Are you sure?', toggle: 'tooltip' } do
         %i.fa.fa-trash-o
 
 - if current_user
   :javascript
-    new Subscription('##{dom_id(label)} .label-subscription');
+    new Subscription('##{label_css_id} .label-subscription');
diff --git a/app/views/projects/labels/index.html.haml b/app/views/projects/labels/index.html.haml
index 2557d1a4d5b..c72eddba37f 100644
--- a/app/views/projects/labels/index.html.haml
+++ b/app/views/projects/labels/index.html.haml
@@ -1,22 +1,36 @@
 - page_title "Labels"
+- hide_class = ''
 
 .top-area
   .nav-text
     Labels can be applied to issues and merge requests.
   .nav-controls
-    - if can? current_user, :admin_label, @project
+    - if can?(current_user, :admin_label, @project)
       = link_to new_namespace_project_label_path(@project.namespace, @project), class: "btn btn-new" do
         = icon('plus')
         New label
 
 .labels
-  - if @labels.present?
-    %ul.content-list.manage-labels-list
-      = render @labels
-    = paginate @labels, theme: 'gitlab'
-  - else
-    .nothing-here-block
-      - if can? current_user, :admin_label, @project
-        Create a label or #{link_to 'generate a default set of labels', generate_namespace_project_labels_path(@project.namespace, @project), method: :post}.
-      - else
-        No labels created
+  - if can?(current_user, :admin_label, @project)
+    -# Only show it in the first page
+    - hide = @project.labels.empty? || (params[:page].present? && params[:page] != '1')
+    .prioritized-labels{ class: ('hide' if hide) }
+      %h5 Prioritized Labels
+      %ul.content-list.manage-labels-list.js-prioritized-labels{ "data-url" => set_priorities_namespace_project_labels_path(@project.namespace, @project) }
+        - if @prioritized_labels.present?
+          = render @prioritized_labels
+        - else
+          %p.empty-message No prioritized labels yet
+  .other-labels
+    - if can?(current_user, :admin_label, @project)
+      %h5{ class: ('hide' if hide) } Other Labels
+    - if @labels.present?
+      %ul.content-list.manage-labels-list.js-other-labels
+        = render @labels
+      = paginate @labels, theme: 'gitlab'
+    - else
+      .nothing-here-block
+        - if can?(current_user, :admin_label, @project)
+          Create a label or #{link_to 'generate a default set of labels', generate_namespace_project_labels_path(@project.namespace, @project), method: :post}.
+        - else
+          No labels created
diff --git a/app/views/shared/_issues.html.haml b/app/views/shared/_issues.html.haml
index 8ff9d4c1c7f..a5df502d7b5 100644
--- a/app/views/shared/_issues.html.haml
+++ b/app/views/shared/_issues.html.haml
@@ -1,4 +1,4 @@
-- if @issues.any?
+- if @issues.reorder(nil).any?
   - @issues.group_by(&:project).each do |group|
     .panel.panel-default.panel-small
       - project = group[0]
diff --git a/app/views/shared/_label_row.html.haml b/app/views/shared/_label_row.html.haml
index 9ce5562e667..d315a3fe93b 100644
--- a/app/views/shared/_label_row.html.haml
+++ b/app/views/shared/_label_row.html.haml
@@ -1,5 +1,12 @@
 %span.label-row
+  - if can?(current_user, :admin_label, @project)
+    .js-toggle-priority.toggle-priority{ data: { url: remove_priority_namespace_project_label_path(@project.namespace, @project, label),
+      dom_id: dom_id(label) } }
+      %button.add-priority.btn.has-tooltip{ title: 'Prioritize', :'data-placement' => 'top' }
+        = icon('star-o')
+      %button.remove-priority.btn.has-tooltip{ title: 'Remove priority', :'data-placement' => 'top' }
+        = icon('star')
   %span.label-name
     = link_to_label(label, tooltip: false)
   %span.prepend-left-10
-    = markdown(label.description, pipeline: :single_line)
\ No newline at end of file
+    = markdown(label.description, pipeline: :single_line)
diff --git a/app/views/shared/_sort_dropdown.html.haml b/app/views/shared/_sort_dropdown.html.haml
index 1e0f075b303..249bce926ce 100644
--- a/app/views/shared/_sort_dropdown.html.haml
+++ b/app/views/shared/_sort_dropdown.html.haml
@@ -8,6 +8,8 @@
     %b.caret
   %ul.dropdown-menu.dropdown-menu-align-right.dropdown-menu-sort
     %li
+      = link_to page_filter_path(sort: sort_value_priority) do
+        = sort_title_priority
       = link_to page_filter_path(sort: sort_value_recently_created) do
         = sort_title_recently_created
       = link_to page_filter_path(sort: sort_value_oldest_created) do