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

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