summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJacob Schatz <jschatz@gitlab.com>2016-06-07 15:15:07 +0000
committerJacob Schatz <jschatz@gitlab.com>2016-06-07 15:15:07 +0000
commit5b83abcc01bd4a24268126dc52019b9f11152a7c (patch)
tree195d1b0b10dcaf5ae58472075d350812b270612e
parent25370fd158e95d9672340ac25d13ed51e8dc77af (diff)
parentee26c3cab4651c8876efc45b6a63539727e6a42e (diff)
downloadgitlab-ce-5b83abcc01bd4a24268126dc52019b9f11152a7c.tar.gz
Merge branch 'issue_14189' into 'master'
Ability to prioritize labels Closes #14189 See merge request !4009
-rw-r--r--CHANGELOG1
-rw-r--r--app/assets/javascripts/LabelManager.js.coffee84
-rw-r--r--app/assets/javascripts/dispatcher.js.coffee2
-rw-r--r--app/assets/stylesheets/framework/lists.scss12
-rw-r--r--app/assets/stylesheets/pages/labels.scss33
-rw-r--r--app/controllers/projects/labels_controller.rb29
-rw-r--r--app/finders/issuable_finder.rb8
-rw-r--r--app/helpers/sorting_helper.rb11
-rw-r--r--app/models/concerns/issuable.rb23
-rw-r--r--app/models/issue.rb4
-rw-r--r--app/models/label.rb14
-rw-r--r--app/views/projects/labels/_label.html.haml15
-rw-r--r--app/views/projects/labels/index.html.haml36
-rw-r--r--app/views/shared/_issues.html.haml2
-rw-r--r--app/views/shared/_label_row.html.haml9
-rw-r--r--app/views/shared/_sort_dropdown.html.haml2
-rw-r--r--config/routes.rb2
-rw-r--r--db/migrate/20160314094147_add_priority_to_label.rb6
-rw-r--r--db/schema.rb2
-rw-r--r--features/steps/project/issues/labels.rb10
-rw-r--r--lib/gitlab/database.rb14
-rw-r--r--spec/controllers/projects/labels_controller_spec.rb53
-rw-r--r--spec/features/projects/labels/issues_sorted_by_priority_spec.rb87
-rw-r--r--spec/features/projects/labels/update_prioritization_spec.rb115
-rw-r--r--spec/lib/gitlab/database_spec.rb16
25 files changed, 556 insertions, 34 deletions
diff --git a/CHANGELOG b/CHANGELOG
index ba834c82e98..ef4bd5fe295 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
v 8.9.0 (unreleased)
- Bulk assign/unassign labels to issues.
+ - Ability to prioritize labels !4009 / !3205 (Thijs Wouters)
- Allow enabling wiki page events from Webhook management UI
- Bump rouge to 1.11.0
- Make EmailsOnPushWorker use Sidekiq mailers queue
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
diff --git a/config/routes.rb b/config/routes.rb
index 7e735541f7f..240dcc74b06 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -719,10 +719,12 @@ Rails.application.routes.draw do
resources :labels, constraints: { id: /\d+/ } do
collection do
post :generate
+ post :set_priorities
end
member do
post :toggle_subscription
+ delete :remove_priority
end
end
diff --git a/db/migrate/20160314094147_add_priority_to_label.rb b/db/migrate/20160314094147_add_priority_to_label.rb
new file mode 100644
index 00000000000..8ddf7782972
--- /dev/null
+++ b/db/migrate/20160314094147_add_priority_to_label.rb
@@ -0,0 +1,6 @@
+class AddPriorityToLabel < ActiveRecord::Migration
+ def change
+ add_column :labels, :priority, :integer
+ add_index :labels, :priority
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 9b991f347a9..69e37470de0 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -496,8 +496,10 @@ ActiveRecord::Schema.define(version: 20160530150109) do
t.datetime "updated_at"
t.boolean "template", default: false
t.string "description"
+ t.integer "priority"
end
+ add_index "labels", ["priority"], name: "index_labels_on_priority", using: :btree
add_index "labels", ["project_id"], name: "index_labels_on_project_id", using: :btree
create_table "lfs_objects", force: :cascade do |t|
diff --git a/features/steps/project/issues/labels.rb b/features/steps/project/issues/labels.rb
index 8d87f6a7a58..e02b57bbf84 100644
--- a/features/steps/project/issues/labels.rb
+++ b/features/steps/project/issues/labels.rb
@@ -60,25 +60,25 @@ class Spinach::Features::ProjectIssuesLabels < Spinach::FeatureSteps
end
step 'I should see label \'feature\'' do
- page.within '.manage-labels-list' do
+ page.within '.other-labels .manage-labels-list' do
expect(page).to have_content 'feature'
end
end
step 'I should see label \'bug\'' do
- page.within '.manage-labels-list' do
+ page.within '.other-labels .manage-labels-list' do
expect(page).to have_content 'bug'
end
end
step 'I should not see label \'bug\'' do
- page.within '.manage-labels-list' do
+ page.within '.other-labels .manage-labels-list' do
expect(page).not_to have_content 'bug'
end
end
step 'I should see label \'support\'' do
- page.within '.manage-labels-list' do
+ page.within '.other-labels .manage-labels-list' do
expect(page).to have_content 'support'
end
end
@@ -90,7 +90,7 @@ class Spinach::Features::ProjectIssuesLabels < Spinach::FeatureSteps
end
step 'I should see label \'fix\'' do
- page.within '.manage-labels-list' do
+ page.within '.other-labels .manage-labels-list' do
expect(page).to have_content 'fix'
end
end
diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb
index 42bec913a45..04fa6a3a5de 100644
--- a/lib/gitlab/database.rb
+++ b/lib/gitlab/database.rb
@@ -16,6 +16,20 @@ module Gitlab
database_version.match(/\A(?:PostgreSQL |)([^\s]+).*\z/)[1]
end
+ def self.nulls_last_order(field, direction = 'ASC')
+ order = "#{field} #{direction}"
+
+ if Gitlab::Database.postgresql?
+ order << ' NULLS LAST'
+ else
+ # `field IS NULL` will be `0` for non-NULL columns and `1` for NULL
+ # columns. In the (default) ascending order, `0` comes first.
+ order.prepend("#{field} IS NULL, ") if direction == 'ASC'
+ end
+
+ order
+ end
+
def true_value
if Gitlab::Database.postgresql?
"'t'"
diff --git a/spec/controllers/projects/labels_controller_spec.rb b/spec/controllers/projects/labels_controller_spec.rb
new file mode 100644
index 00000000000..ab1dd34ed57
--- /dev/null
+++ b/spec/controllers/projects/labels_controller_spec.rb
@@ -0,0 +1,53 @@
+require 'spec_helper'
+
+describe Projects::LabelsController do
+ let(:project) { create(:project) }
+ let(:user) { create(:user) }
+
+ before do
+ project.team << [user, :master]
+ sign_in(user)
+ end
+
+ describe 'GET #index' do
+ def create_label(attributes)
+ create(:label, attributes.merge(project: project))
+ end
+
+ before do
+ 15.times { |i| create_label(priority: (i % 3) + 1, title: "label #{15 - i}") }
+ 5.times { |i| create_label(title: "label #{100 - i}") }
+
+
+ get :index, namespace_id: project.namespace.to_param, project_id: project.to_param
+ end
+
+ context '@prioritized_labels' do
+ let(:prioritized_labels) { assigns(:prioritized_labels) }
+
+ it 'contains only prioritized labels' do
+ expect(prioritized_labels).to all(have_attributes(priority: a_value > 0))
+ end
+
+ it 'is sorted by priority, then label title' do
+ priorities_and_titles = prioritized_labels.pluck(:priority, :title)
+
+ expect(priorities_and_titles.sort).to eq(priorities_and_titles)
+ end
+ end
+
+ context '@labels' do
+ let(:labels) { assigns(:labels) }
+
+ it 'contains only unprioritized labels' do
+ expect(labels).to all(have_attributes(priority: nil))
+ end
+
+ it 'is sorted by label title' do
+ titles = labels.pluck(:title)
+
+ expect(titles.sort).to eq(titles)
+ end
+ end
+ end
+end
diff --git a/spec/features/projects/labels/issues_sorted_by_priority_spec.rb b/spec/features/projects/labels/issues_sorted_by_priority_spec.rb
new file mode 100644
index 00000000000..461f1737928
--- /dev/null
+++ b/spec/features/projects/labels/issues_sorted_by_priority_spec.rb
@@ -0,0 +1,87 @@
+require 'spec_helper'
+
+feature 'Issue prioritization', feature: true do
+
+ let(:user) { create(:user) }
+ let(:project) { create(:project, name: 'test', namespace: user.namespace) }
+
+ # Labels
+ let(:label_1) { create(:label, title: 'label_1', project: project, priority: 1) }
+ let(:label_2) { create(:label, title: 'label_2', project: project, priority: 2) }
+ let(:label_3) { create(:label, title: 'label_3', project: project, priority: 3) }
+ let(:label_4) { create(:label, title: 'label_4', project: project, priority: 4) }
+ let(:label_5) { create(:label, title: 'label_5', project: project) } # no priority
+
+ # According to https://gitlab.com/gitlab-org/gitlab-ce/issues/14189#note_4360653
+ context 'when issues have one label' do
+ scenario 'Are sorted properly' do
+
+ # Issues
+ issue_1 = create(:issue, title: 'issue_1', project: project)
+ issue_2 = create(:issue, title: 'issue_2', project: project)
+ issue_3 = create(:issue, title: 'issue_3', project: project)
+ issue_4 = create(:issue, title: 'issue_4', project: project)
+ issue_5 = create(:issue, title: 'issue_5', project: project)
+
+ # Assign labels to issues disorderly
+ issue_4.labels << label_1
+ issue_3.labels << label_2
+ issue_5.labels << label_3
+ issue_2.labels << label_4
+ issue_1.labels << label_5
+
+ login_as user
+ visit namespace_project_issues_path(project.namespace, project, sort: 'priority')
+
+ # Ensure we are indicating that issues are sorted by priority
+ expect(page).to have_selector('.dropdown-toggle', text: 'Priority')
+
+ page.within('.issues-holder') do
+ issue_titles = all('.issues-list .issue-title-text').map(&:text)
+
+ expect(issue_titles).to eq(['issue_4', 'issue_3', 'issue_5', 'issue_2', 'issue_1'])
+ end
+ end
+ end
+
+ context 'when issues have multiple labels' do
+ scenario 'Are sorted properly' do
+
+ # Issues
+ issue_1 = create(:issue, title: 'issue_1', project: project)
+ issue_2 = create(:issue, title: 'issue_2', project: project)
+ issue_3 = create(:issue, title: 'issue_3', project: project)
+ issue_4 = create(:issue, title: 'issue_4', project: project)
+ issue_5 = create(:issue, title: 'issue_5', project: project)
+ issue_6 = create(:issue, title: 'issue_6', project: project)
+ issue_7 = create(:issue, title: 'issue_7', project: project)
+ issue_8 = create(:issue, title: 'issue_8', project: project)
+
+ # Assign labels to issues disorderly
+ issue_5.labels << label_1 # 1
+ issue_5.labels << label_2
+ issue_8.labels << label_1 # 2
+ issue_1.labels << label_2 # 3
+ issue_1.labels << label_3
+ issue_3.labels << label_2 # 4
+ issue_3.labels << label_4
+ issue_7.labels << label_2 # 5
+ issue_2.labels << label_3 # 6
+ issue_4.labels << label_4 # 7
+ issue_6.labels << label_5 # 8 - No priority
+
+ login_as user
+ visit namespace_project_issues_path(project.namespace, project, sort: 'priority')
+
+ expect(page).to have_selector('.dropdown-toggle', text: 'Priority')
+
+ page.within('.issues-holder') do
+ issue_titles = all('.issues-list .issue-title-text').map(&:text)
+
+ expect(issue_titles[0..1]).to contain_exactly('issue_5', 'issue_8')
+ expect(issue_titles[2..4]).to contain_exactly('issue_1', 'issue_3', 'issue_7')
+ expect(issue_titles[5..-1]).to eq(['issue_2', 'issue_4', 'issue_6'])
+ end
+ end
+ end
+end
diff --git a/spec/features/projects/labels/update_prioritization_spec.rb b/spec/features/projects/labels/update_prioritization_spec.rb
new file mode 100644
index 00000000000..8550d279d09
--- /dev/null
+++ b/spec/features/projects/labels/update_prioritization_spec.rb
@@ -0,0 +1,115 @@
+require 'spec_helper'
+
+feature 'Prioritize labels', feature: true do
+ include WaitForAjax
+
+ context 'when project belongs to user' do
+ let(:user) { create(:user) }
+ let(:project) { create(:project, name: 'test', namespace: user.namespace) }
+
+ scenario 'user can prioritize a label', js: true do
+ bug = create(:label, title: 'bug')
+ wontfix = create(:label, title: 'wontfix')
+
+ project.labels << bug
+ project.labels << wontfix
+
+ login_as user
+ visit namespace_project_labels_path(project.namespace, project)
+
+ expect(page).to have_content('No prioritized labels yet')
+
+ page.within('.other-labels') do
+ first('.js-toggle-priority').click
+ wait_for_ajax
+ expect(page).not_to have_content('bug')
+ end
+
+ page.within('.prioritized-labels') do
+ expect(page).not_to have_content('No prioritized labels yet')
+ expect(page).to have_content('bug')
+ end
+ end
+
+ scenario 'user can unprioritize a label', js: true do
+ bug = create(:label, title: 'bug', priority: 1)
+ wontfix = create(:label, title: 'wontfix')
+
+ project.labels << bug
+ project.labels << wontfix
+
+ login_as user
+ visit namespace_project_labels_path(project.namespace, project)
+
+ expect(page).to have_content('bug')
+
+ page.within('.prioritized-labels') do
+ first('.js-toggle-priority').click
+ wait_for_ajax
+ expect(page).not_to have_content('bug')
+ end
+
+ page.within('.other-labels') do
+ expect(page).to have_content('bug')
+ expect(page).to have_content('wontfix')
+ end
+ end
+
+ scenario 'user can sort prioritized labels and persist across reloads', js: true do
+ bug = create(:label, title: 'bug', priority: 1)
+ wontfix = create(:label, title: 'wontfix', priority: 2)
+
+ project.labels << bug
+ project.labels << wontfix
+
+ login_as user
+ visit namespace_project_labels_path(project.namespace, project)
+
+ expect(page).to have_content 'bug'
+ expect(page).to have_content 'wontfix'
+
+ # Sort labels
+ find("#label_#{bug.id}").drag_to find("#label_#{wontfix.id}")
+
+ page.within('.prioritized-labels') do
+ expect(first('li')).to have_content('wontfix')
+ expect(page.all('li').last).to have_content('bug')
+ end
+
+ visit current_url
+
+ page.within('.prioritized-labels') do
+ expect(first('li')).to have_content('wontfix')
+ expect(page.all('li').last).to have_content('bug')
+ end
+ end
+ end
+
+ context 'as a guest' do
+ it 'can not prioritize labels' do
+ user = create(:user)
+ guest = create(:user)
+ project = create(:project, name: 'test', namespace: user.namespace)
+
+ create(:label, title: 'bug')
+
+ login_as guest
+ visit namespace_project_labels_path(project.namespace, project)
+
+ expect(page).not_to have_css('.prioritized-labels')
+ end
+ end
+
+ context 'as a non signed in user' do
+ it 'can not prioritize labels' do
+ user = create(:user)
+ project = create(:project, name: 'test', namespace: user.namespace)
+
+ create(:label, title: 'bug')
+
+ visit namespace_project_labels_path(project.namespace, project)
+
+ expect(page).not_to have_css('.prioritized-labels')
+ end
+ end
+end
diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb
index d0a447753b7..3031559c613 100644
--- a/spec/lib/gitlab/database_spec.rb
+++ b/spec/lib/gitlab/database_spec.rb
@@ -39,6 +39,22 @@ describe Gitlab::Database, lib: true do
end
end
+ describe '.nulls_last_order' do
+ context 'when using PostgreSQL' do
+ before { expect(described_class).to receive(:postgresql?).and_return(true) }
+
+ it { expect(described_class.nulls_last_order('column', 'ASC')).to eq 'column ASC NULLS LAST'}
+ it { expect(described_class.nulls_last_order('column', 'DESC')).to eq 'column DESC NULLS LAST'}
+ end
+
+ context 'when using MySQL' do
+ before { expect(described_class).to receive(:postgresql?).and_return(false) }
+
+ it { expect(described_class.nulls_last_order('column', 'ASC')).to eq 'column IS NULL, column ASC'}
+ it { expect(described_class.nulls_last_order('column', 'DESC')).to eq 'column DESC'}
+ end
+ end
+
describe '#true_value' do
it 'returns correct value for PostgreSQL' do
expect(described_class).to receive(:postgresql?).and_return(true)