Commit c620c405 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'issue_18135' into 'master'

Todos sorting dropdown

Implements #18135 

![todos_sorting](/uploads/bff76827c421628134dfb8b864e47c74/todos_sorting.png)  


- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

See merge request !5691
parents d6e027d9 37bf35f0
...@@ -45,6 +45,7 @@ v 8.11.0 (unreleased) ...@@ -45,6 +45,7 @@ v 8.11.0 (unreleased)
- Show member roles to all users on members page - Show member roles to all users on members page
- Project.visible_to_user is instrumented again - Project.visible_to_user is instrumented again
- Fix awardable button mutuality loading spinners (ClemMakesApps) - 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 - Add support for using RequestStore within Sidekiq tasks via SIDEKIQ_REQUEST_STORE env variable
- Optimize maximum user access level lookup in loading of notes - Optimize maximum user access level lookup in loading of notes
- Send notification emails to users newly mentioned in issue and MR edits !5800 - Send notification emails to users newly mentioned in issue and MR edits !5800
......
...@@ -2,6 +2,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController ...@@ -2,6 +2,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController
before_action :find_todos, only: [:index, :destroy_all] before_action :find_todos, only: [:index, :destroy_all]
def index def index
@sort = params[:sort]
@todos = @todos.page(params[:page]) @todos = @todos.page(params[:page])
end end
......
...@@ -33,7 +33,7 @@ class TodosFinder ...@@ -33,7 +33,7 @@ class TodosFinder
# the project IDs yielded by the todos query thus far # the project IDs yielded by the todos query thus far
items = by_project(items) items = by_project(items)
items.reorder(id: :desc) sort(items)
end end
private private
...@@ -106,6 +106,10 @@ class TodosFinder ...@@ -106,6 +106,10 @@ class TodosFinder
params[:type] params[:type]
end end
def sort(items)
params[:sort] ? items.sort(params[:sort]) : items.reorder(id: :desc)
end
def by_action(items) def by_action(items)
if action? if action?
items = items.where(action: to_action_id) items = items.where(action: to_action_id)
......
...@@ -131,7 +131,10 @@ module Issuable ...@@ -131,7 +131,10 @@ module Issuable
end end
def order_labels_priority(excluded_labels: []) 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]). group(arel_table[:id]).
reorder(Gitlab::Database.nulls_last_order('highest_priority', 'ASC')) reorder(Gitlab::Database.nulls_last_order('highest_priority', 'ASC'))
end end
...@@ -159,20 +162,6 @@ module Issuable ...@@ -159,20 +162,6 @@ module Issuable
grouping_columns grouping_columns
end 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 end
def today? def today?
......
...@@ -35,5 +35,19 @@ module Sortable ...@@ -35,5 +35,19 @@ module Sortable
all all
end end
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
end end
class Todo < ActiveRecord::Base class Todo < ActiveRecord::Base
include Sortable
ASSIGNED = 1 ASSIGNED = 1
MENTIONED = 2 MENTIONED = 2
BUILD_FAILED = 3 BUILD_FAILED = 3
...@@ -41,6 +43,23 @@ class Todo < ActiveRecord::Base ...@@ -41,6 +43,23 @@ class Todo < ActiveRecord::Base
after_save :keep_around_commit 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? def build_failed?
action == BUILD_FAILED action == BUILD_FAILED
end end
......
...@@ -43,6 +43,25 @@ ...@@ -43,6 +43,25 @@
class: 'select2 trigger-submit', include_blank: true, class: 'select2 trigger-submit', include_blank: true,
data: {placeholder: 'Action'}) 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 .prepend-top-default
- if @todos.any? - if @todos.any?
.js-todos-options{ data: {per_page: @todos.limit_value, current_page: @todos.current_page, total_pages: @todos.total_pages} } .js-todos-options{ data: {per_page: @todos.limit_value, current_page: @todos.current_page, total_pages: @todos.total_pages} }
......
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
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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment