diff options
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | app/controllers/dashboard/todos_controller.rb | 1 | ||||
| -rw-r--r-- | app/finders/todos_finder.rb | 6 | ||||
| -rw-r--r-- | app/models/concerns/issuable.rb | 19 | ||||
| -rw-r--r-- | app/models/concerns/sortable.rb | 14 | ||||
| -rw-r--r-- | app/models/todo.rb | 19 | ||||
| -rw-r--r-- | app/views/dashboard/todos/index.html.haml | 19 | ||||
| -rw-r--r-- | spec/features/todos/todos_sorting_spec.rb | 67 | ||||
| -rw-r--r-- | spec/finders/todos_finder_spec.rb | 70 |
9 files changed, 200 insertions, 16 deletions
diff --git a/CHANGELOG b/CHANGELOG index 6d691fd2652..c8c3727873f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -45,6 +45,7 @@ v 8.11.0 (unreleased) - Show member roles to all users on members page - Project.visible_to_user is instrumented again - Fix awardable button mutuality loading spinners (ClemMakesApps) + - Sort todos by date and priority - Add support for using RequestStore within Sidekiq tasks via SIDEKIQ_REQUEST_STORE env variable - Optimize maximum user access level lookup in loading of notes - Send notification emails to users newly mentioned in issue and MR edits !5800 diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb index c8390af3b36..d425d0f9014 100644 --- a/app/controllers/dashboard/todos_controller.rb +++ b/app/controllers/dashboard/todos_controller.rb @@ -2,6 +2,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController before_action :find_todos, only: [:index, :destroy_all] def index + @sort = params[:sort] @todos = @todos.page(params[:page]) end diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index 37bad596a16..06b3e8a9502 100644 --- a/app/finders/todos_finder.rb +++ b/app/finders/todos_finder.rb @@ -33,7 +33,7 @@ class TodosFinder # the project IDs yielded by the todos query thus far items = by_project(items) - items.reorder(id: :desc) + sort(items) end private @@ -106,6 +106,10 @@ class TodosFinder params[:type] end + def sort(items) + params[:sort] ? items.sort(params[:sort]) : items.reorder(id: :desc) + end + def by_action(items) if action? items = items.where(action: to_action_id) diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index cbae1cd439b..afb5ce37c06 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -131,7 +131,10 @@ module Issuable end def order_labels_priority(excluded_labels: []) - select("#{table_name}.*, (#{highest_label_priority(excluded_labels).to_sql}) AS highest_priority"). + condition_field = "#{table_name}.id" + highest_priority = highest_label_priority(name, condition_field, excluded_labels: excluded_labels).to_sql + + select("#{table_name}.*, (#{highest_priority}) AS highest_priority"). group(arel_table[:id]). reorder(Gitlab::Database.nulls_last_order('highest_priority', 'ASC')) end @@ -159,20 +162,6 @@ 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/concerns/sortable.rb b/app/models/concerns/sortable.rb index 8b47b9e0abd..1ebecd86af9 100644 --- a/app/models/concerns/sortable.rb +++ b/app/models/concerns/sortable.rb @@ -35,5 +35,19 @@ module Sortable all end end + + private + + def highest_label_priority(object_types, condition_field, excluded_labels: []) + query = Label.select(Label.arel_table[:priority].minimum). + joins(:label_links). + where(label_links: { target_type: object_types }). + where("label_links.target_id = #{condition_field}"). + reorder(nil) + + query.where.not(title: excluded_labels) if excluded_labels.present? + + query + end end end diff --git a/app/models/todo.rb b/app/models/todo.rb index 8d7a5965aa1..6ae9956ade5 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -1,4 +1,6 @@ class Todo < ActiveRecord::Base + include Sortable + ASSIGNED = 1 MENTIONED = 2 BUILD_FAILED = 3 @@ -41,6 +43,23 @@ class Todo < ActiveRecord::Base after_save :keep_around_commit + class << self + def sort(method) + method == "priority" ? order_by_labels_priority : order_by(method) + end + + # Order by priority depending on which issue/merge request the Todo belongs to + # Todos with highest priority first then oldest todos + # Need to order by created_at last because of differences on Mysql and Postgres when joining by type "Merge_request/Issue" + def order_by_labels_priority + highest_priority = highest_label_priority(["Issue", "MergeRequest"], "todos.target_id").to_sql + + select("#{table_name}.*, (#{highest_priority}) AS highest_priority"). + order(Gitlab::Database.nulls_last_order('highest_priority', 'ASC')). + order('todos.created_at') + end + end + def build_failed? action == BUILD_FAILED end diff --git a/app/views/dashboard/todos/index.html.haml b/app/views/dashboard/todos/index.html.haml index 4e340b6ec16..d320d3bcc1e 100644 --- a/app/views/dashboard/todos/index.html.haml +++ b/app/views/dashboard/todos/index.html.haml @@ -43,6 +43,25 @@ class: 'select2 trigger-submit', include_blank: true, data: {placeholder: 'Action'}) + .pull-right + .dropdown.inline.prepend-left-10 + %button.dropdown-toggle.btn{type: 'button', 'data-toggle' => 'dropdown'} + %span.light + - if @sort.present? + = sort_options_hash[@sort] + - else + = sort_title_recently_created + %b.caret + %ul.dropdown-menu.dropdown-menu-align-right.dropdown-menu-sort + %li + = link_to todos_filter_path(sort: sort_value_priority) do + = sort_title_priority + = link_to todos_filter_path(sort: sort_value_recently_created) do + = sort_title_recently_created + = link_to todos_filter_path(sort: sort_value_oldest_created) do + = sort_title_oldest_created + + .prepend-top-default - if @todos.any? .js-todos-options{ data: {per_page: @todos.limit_value, current_page: @todos.current_page, total_pages: @todos.total_pages} } diff --git a/spec/features/todos/todos_sorting_spec.rb b/spec/features/todos/todos_sorting_spec.rb new file mode 100644 index 00000000000..e74a51acede --- /dev/null +++ b/spec/features/todos/todos_sorting_spec.rb @@ -0,0 +1,67 @@ +require 'spec_helper' + +describe "Dashboard > User sorts todos", feature: true do + let(:user) { create(:user) } + let(:project) { create(:empty_project) } + + 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(:issue_1) { create(:issue, title: 'issue_1', project: project) } + let(:issue_2) { create(:issue, title: 'issue_2', project: project) } + let(:issue_3) { create(:issue, title: 'issue_3', project: project) } + let(:issue_4) { create(:issue, title: 'issue_4', project: project) } + + let!(:merge_request_1) { create(:merge_request, source_project: project, title: "merge_request_1") } + + before do + create(:todo, user: user, project: project, target: issue_4, created_at: 5.hours.ago) + create(:todo, user: user, project: project, target: issue_2, created_at: 4.hours.ago) + create(:todo, user: user, project: project, target: issue_3, created_at: 3.hours.ago) + create(:todo, user: user, project: project, target: issue_1, created_at: 2.hours.ago) + create(:todo, user: user, project: project, target: merge_request_1, created_at: 1.hour.ago) + + merge_request_1.labels << label_1 + issue_3.labels << label_1 + issue_2.labels << label_3 + issue_1.labels << label_2 + + project.team << [user, :developer] + login_as(user) + visit dashboard_todos_path + end + + it "sorts with oldest created todos first" do + click_link "Last created" + + results_list = page.find('.todos-list') + expect(results_list.all('p')[0]).to have_content("merge_request_1") + expect(results_list.all('p')[1]).to have_content("issue_1") + expect(results_list.all('p')[2]).to have_content("issue_3") + expect(results_list.all('p')[3]).to have_content("issue_2") + expect(results_list.all('p')[4]).to have_content("issue_4") + end + + it "sorts with newest created todos first" do + click_link "Oldest created" + + results_list = page.find('.todos-list') + expect(results_list.all('p')[0]).to have_content("issue_4") + expect(results_list.all('p')[1]).to have_content("issue_2") + expect(results_list.all('p')[2]).to have_content("issue_3") + expect(results_list.all('p')[3]).to have_content("issue_1") + expect(results_list.all('p')[4]).to have_content("merge_request_1") + end + + it "sorts by priority" do + click_link "Priority" + + results_list = page.find('.todos-list') + expect(results_list.all('p')[0]).to have_content("issue_3") + expect(results_list.all('p')[1]).to have_content("merge_request_1") + expect(results_list.all('p')[2]).to have_content("issue_1") + expect(results_list.all('p')[3]).to have_content("issue_2") + expect(results_list.all('p')[4]).to have_content("issue_4") + end +end diff --git a/spec/finders/todos_finder_spec.rb b/spec/finders/todos_finder_spec.rb new file mode 100644 index 00000000000..f7e7e733cf7 --- /dev/null +++ b/spec/finders/todos_finder_spec.rb @@ -0,0 +1,70 @@ +require 'spec_helper' + +describe TodosFinder do + describe '#execute' do + let(:user) { create(:user) } + let(:project) { create(:empty_project) } + let(:finder) { described_class } + + before { project.team << [user, :developer] } + + describe '#sort' do + context 'by date' do + let!(:todo1) { create(:todo, user: user, project: project) } + let!(:todo2) { create(:todo, user: user, project: project) } + let!(:todo3) { create(:todo, user: user, project: project) } + + it 'sorts with oldest created first' do + todos = finder.new(user, { sort: 'id_asc' }).execute + + expect(todos.first).to eq(todo1) + expect(todos.second).to eq(todo2) + expect(todos.third).to eq(todo3) + end + + it 'sorts with newest created first' do + todos = finder.new(user, { sort: 'id_desc' }).execute + + expect(todos.first).to eq(todo3) + expect(todos.second).to eq(todo2) + expect(todos.third).to eq(todo1) + end + end + + it "sorts by priority" do + label_1 = create(:label, title: 'label_1', project: project, priority: 1) + label_2 = create(:label, title: 'label_2', project: project, priority: 2) + label_3 = create(:label, title: 'label_3', project: project, priority: 3) + + 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) + merge_request_1 = create(:merge_request, source_project: project) + + merge_request_1.labels << label_1 + + # Covers the case where Todo has more than one label + issue_3.labels << label_1 + issue_3.labels << label_3 + + issue_2.labels << label_3 + issue_1.labels << label_2 + + todo_1 = create(:todo, user: user, project: project, target: issue_4) + todo_2 = create(:todo, user: user, project: project, target: issue_2) + todo_3 = create(:todo, user: user, project: project, target: issue_3, created_at: 2.hours.ago) + todo_4 = create(:todo, user: user, project: project, target: issue_1) + todo_5 = create(:todo, user: user, project: project, target: merge_request_1, created_at: 1.hour.ago) + + todos = finder.new(user, { sort: 'priority' }).execute + + expect(todos.first).to eq(todo_3) + expect(todos.second).to eq(todo_5) + expect(todos.third).to eq(todo_4) + expect(todos.fourth).to eq(todo_2) + expect(todos.fifth).to eq(todo_1) + end + end + end +end |