From d8263b285193d9163089683eb77825f1cd673b14 Mon Sep 17 00:00:00 2001 From: Thijs Wouters Date: Mon, 14 Mar 2016 10:46:26 +0100 Subject: [PATCH] Sort by label priority MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- app/controllers/projects/labels_controller.rb | 2 +- app/finders/issuable_finder.rb | 8 +++++-- app/helpers/sorting_helper.rb | 11 ++++++++- app/models/concerns/issuable.rb | 9 +++++++- app/models/label.rb | 23 +++++++++++++++++++ app/views/projects/labels/_form.html.haml | 4 ++++ app/views/shared/_sort_dropdown.html.haml | 2 ++ config/initializers/nulls_last.rb | 15 ++++++++++++ .../20160314094147_add_priority_to_label.rb | 6 +++++ db/schema.rb | 2 ++ 10 files changed, 77 insertions(+), 5 deletions(-) create mode 100644 config/initializers/nulls_last.rb create mode 100644 db/migrate/20160314094147_add_priority_to_label.rb diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb index 88d745e6bae..0a60a802430 100644 --- a/app/controllers/projects/labels_controller.rb +++ b/app/controllers/projects/labels_controller.rb @@ -100,7 +100,7 @@ class Projects::LabelsController < Projects::ApplicationController end def label_params - params.require(:label).permit(:title, :description, :color) + params.require(:label).permit(:title, :description, :color, :priority) end def label diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 7d8c56f4c22..68ab6e87684 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], 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..8871cb8a6cd 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -48,6 +48,12 @@ module Issuable scope :non_archived, -> { join_project.where(projects: { archived: false }) } + def self.order_priority(labels) + select("#{table_name}.*, (#{Label.high_priority(name, table_name, labels).to_sql}) AS highest_priority") + .group("#{table_name}.id") + .reorder(nulls_last('highest_priority', 'ASC')) + end + delegate :name, :email, to: :author, @@ -105,12 +111,13 @@ module Issuable where(t[:title].matches(pattern).or(t[:description].matches(pattern))) end - def sort(method) + def sort(method, 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_priority(labels) else order_by(method) end diff --git a/app/models/label.rb b/app/models/label.rb index 59e7afe53f3..4437ca393ed 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -27,11 +27,28 @@ class Label < ActiveRecord::Base format: { with: /\A[^&\?,]+\z/ }, uniqueness: { scope: :project_id } + before_save :nillify_priority + default_scope { order(title: :asc) } scope :templates, -> { where(template: true) } scope :prioritized, ->(value = true) { where(priority: value) } + def self.high_priority(name, table_name, labels) + unfiltered = unscoped + .select("MIN(labels.priority)") + .joins("INNER JOIN label_links ON label_links.label_id = labels.id") + .where("label_links.target_type = '#{name}'") + .where("label_links.target_id = #{table_name}.id") + .where("labels.project_id = #{table_name}.project_id") + + if labels.empty? + unfiltered + else + unfiltered.where("labels.title NOT IN (?)", labels) + end + end + alias_attribute :name, :title def self.reference_prefix @@ -120,4 +137,10 @@ class Label < ActiveRecord::Base id end end + + def nillify_priority + unless self.priority.present? + self.priority = nil + end + end end diff --git a/app/views/projects/labels/_form.html.haml b/app/views/projects/labels/_form.html.haml index aa143e54ffe..227ce5d2317 100644 --- a/app/views/projects/labels/_form.html.haml +++ b/app/views/projects/labels/_form.html.haml @@ -24,6 +24,10 @@ - suggested_colors.each do |color| = link_to '#', style: "background-color: #{color}", data: { color: color } do   + .form-group + = f.label :priority, "Priority", class: 'control-label' + .col-sm-10 + = f.text_field :priority, class: "form-control" .form-actions - if @label.persisted? 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/initializers/nulls_last.rb b/config/initializers/nulls_last.rb new file mode 100644 index 00000000000..47b7b0bb3d2 --- /dev/null +++ b/config/initializers/nulls_last.rb @@ -0,0 +1,15 @@ +module ActiveRecord + class Base + def self.nulls_last(field, direction = 'ASC') + if Gitlab::Database.postgresql? + "#{field} #{direction} NULLS LAST" + else + if direction.upcase == 'ASC' + "-#{field} DESC" + else + "#{field} DESC" + end + end + end + 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|