Add dropdown sort to project milestones

8 files changed, 130 insertions, 5 deletions

diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb
index be52b0fa7cf..5922e686cd0 100644
--- a/app/controllers/projects/milestones_controller.rb
+++ b/app/controllers/projects/milestones_controller.rb
@@ -13,11 +13,14 @@ class Projects::MilestonesController < Projects::ApplicationController
   def index
     @milestones =
       case params[:state]
-      when 'all' then @project.milestones.reorder(due_date: :desc, title: :asc)
-      when 'closed' then @project.milestones.closed.reorder(due_date: :desc, title: :asc)
-      else @project.milestones.active.reorder(due_date: :asc, title: :asc)
+      when 'all' then @project.milestones
+      when 'closed' then @project.milestones.closed
+      else @project.milestones.active
       end
 
+    @sort = params[:sort] || 'due_date_asc'
+    @milestones = @milestones.sort(@sort)
+
     @milestones = @milestones.includes(:project)
     respond_to do |format|
       format.html do
diff --git a/app/helpers/sorting_helper.rb b/app/helpers/sorting_helper.rb
index 959ee310867..5c89cbea3fc 100644
--- a/app/helpers/sorting_helper.rb
+++ b/app/helpers/sorting_helper.rb
@@ -2,6 +2,7 @@ module SortingHelper
   def sort_options_hash
     {
       sort_value_name => sort_title_name,
+      sort_value_name_desc => sort_title_name_desc,
       sort_value_recently_updated => sort_title_recently_updated,
       sort_value_oldest_updated => sort_title_oldest_updated,
       sort_value_recently_created => sort_title_recently_created,
@@ -50,6 +51,17 @@ module SortingHelper
     }
   end
 
+  def milestone_sort_options_hash
+    {
+      sort_value_name => sort_title_name_asc,
+      sort_value_name_desc => sort_title_name_desc,
+      sort_value_due_date_soon => sort_title_due_date_soon,
+      sort_value_due_date_later => sort_title_due_date_later,
+      sort_value_start_date_soon => sort_title_start_date_soon,
+      sort_value_start_date_later => sort_title_start_date_later,
+    }
+  end
+
   def sort_title_priority
     'Priority'
   end
@@ -90,6 +102,14 @@ module SortingHelper
     'Due later'
   end
 
+  def sort_title_start_date_soon
+    'Start soon'
+  end
+
+  def sort_title_start_date_later
+    'Start later'
+  end
+
   def sort_title_name
     'Name'
   end
@@ -202,6 +222,14 @@ module SortingHelper
     'due_date_desc'
   end
 
+  def sort_value_start_date_soon
+    'start_date_asc'
+  end
+
+  def sort_value_start_date_later
+    'start_date_desc'
+  end
+
   def sort_value_name
     'name_asc'
   end
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index c0deb59ec4c..e85d5709624 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -107,6 +107,21 @@ class Milestone < ActiveRecord::Base
     end
   end
 
+  def self.sort(method)
+    case method.to_s
+    when 'due_date_asc'
+      reorder(Gitlab::Database.nulls_last_order('due_date', 'ASC'))
+    when 'due_date_desc'
+      reorder(Gitlab::Database.nulls_last_order('due_date', 'DESC'))
+    when 'start_date_asc'
+      reorder(Gitlab::Database.nulls_last_order('start_date', 'ASC'))
+    when 'start_date_desc'
+      reorder(Gitlab::Database.nulls_last_order('start_date', 'DESC'))
+    else
+      order_by(method)
+    end
+  end
+
   ##
   # Returns the String necessary to reference this Milestone in Markdown
   #
diff --git a/app/views/projects/milestones/index.html.haml b/app/views/projects/milestones/index.html.haml
index 918f5d161bb..b6340a00b29 100644
--- a/app/views/projects/milestones/index.html.haml
+++ b/app/views/projects/milestones/index.html.haml
@@ -7,6 +7,7 @@
     = render 'shared/milestones_filter', counts: milestone_counts(@project.milestones)
 
     .nav-controls
+      = render 'shared/milestones_sort_dropdown'
       - if can?(current_user, :admin_milestone, @project)
         = link_to new_namespace_project_milestone_path(@project.namespace, @project), class: 'btn btn-new', title: 'New Milestone' do
           New Milestone
diff --git a/app/views/shared/_milestones_filter.html.haml b/app/views/shared/_milestones_filter.html.haml
index 57a0eaa919e..db2ac1e1d12 100644
--- a/app/views/shared/_milestones_filter.html.haml
+++ b/app/views/shared/_milestones_filter.html.haml
@@ -4,10 +4,10 @@
       Open
       %span.badge= counts[:opened]
   %li{ class: milestone_class_for_state(params[:state], 'closed') }>
-    = link_to milestones_filter_path(state: 'closed') do
+    = link_to milestones_filter_path(state: 'closed', sort: 'due_date_desc') do
       Closed
       %span.badge= counts[:closed]
   %li{ class: milestone_class_for_state(params[:state], 'all') }>
-    = link_to milestones_filter_path(state: 'all') do
+    = link_to milestones_filter_path(state: 'all', sort: 'due_date_desc') do
       All
       %span.badge= counts[:all]
diff --git a/app/views/shared/_milestones_sort_dropdown.html.haml b/app/views/shared/_milestones_sort_dropdown.html.haml
new file mode 100644
index 00000000000..9b2f2fdcc93
--- /dev/null
+++ b/app/views/shared/_milestones_sort_dropdown.html.haml
@@ -0,0 +1,22 @@
+.dropdown.inline.prepend-left-10
+  %button.dropdown-toggle{ type: 'button', data: { toggle: 'dropdown' } }
+    %span.light
+    - if @sort.present?
+      = milestone_sort_options_hash[@sort]
+    - else
+      = sort_title_due_date_soon
+    = icon('chevron-down')
+  %ul.dropdown-menu.dropdown-menu-align-right.dropdown-menu-sort
+    %li
+      = link_to page_filter_path(sort: sort_value_due_date_soon, label: true) do
+        = sort_title_due_date_soon
+      = link_to page_filter_path(sort: sort_value_due_date_later, label: true) do
+        = sort_title_due_date_later
+      = link_to page_filter_path(sort: sort_value_start_date_soon, label: true) do
+        = sort_title_start_date_soon
+      = link_to page_filter_path(sort: sort_value_start_date_later, label: true) do
+        = sort_title_start_date_later
+      = link_to page_filter_path(sort: sort_value_name, label: true) do
+        = sort_title_name_asc
+      = link_to page_filter_path(sort: sort_value_name_desc, label: true) do
+        = sort_title_name_desc
diff --git a/changelogs/unreleased/4195-add-sorting-to-project-milestones.yml b/changelogs/unreleased/4195-add-sorting-to-project-milestones.yml
new file mode 100644
index 00000000000..d4104dfa772
--- /dev/null
+++ b/changelogs/unreleased/4195-add-sorting-to-project-milestones.yml
@@ -0,0 +1,4 @@
+---
+title: Add dropdown sort to project milestones
+merge_request:
+author: George Andrinopoulos
diff --git a/spec/features/projects/milestones/milestones_sorting_spec.rb b/spec/features/projects/milestones/milestones_sorting_spec.rb
new file mode 100644
index 00000000000..da3eaed707a
--- /dev/null
+++ b/spec/features/projects/milestones/milestones_sorting_spec.rb
@@ -0,0 +1,52 @@
+require 'spec_helper'
+
+feature 'Milestones sorting', :feature, :js do
+  include SortingHelper
+  let(:user)    { create(:user) }
+  let(:project) { create(:empty_project, name: 'test', namespace: user.namespace) }
+
+  before do
+    # Milestones
+    create(:milestone,
+      due_date: 10.days.from_now,
+      created_at:  2.hours.ago,
+      title: "aaa", project: project)
+    create(:milestone,
+      due_date: 11.days.from_now,
+      created_at:  1.hour.ago,
+      title: "bbb", project: project)
+    login_as(user)
+  end
+
+  scenario 'visit project milestones and sort by due_date_asc' do
+    visit namespace_project_milestones_path(project.namespace, project)
+
+    expect(page).to have_button('Due soon')
+
+    # assert default sorting
+    within '.milestones' do
+      expect(page.all('ul.content-list > li').first.text).to include('aaa')
+      expect(page.all('ul.content-list > li').last.text).to include('bbb')
+    end
+
+    click_button 'Due soon'
+
+    sort_options = find('ul.dropdown-menu-sort li').all('a').collect(&:text)
+
+    expect(sort_options[0]).to eq('Due soon')
+    expect(sort_options[1]).to eq('Due later')
+    expect(sort_options[2]).to eq('Start soon')
+    expect(sort_options[3]).to eq('Start later')
+    expect(sort_options[4]).to eq('Name, ascending')
+    expect(sort_options[5]).to eq('Name, descending')
+
+    click_link 'Due later'
+
+    expect(page).to have_button('Due later')
+
+    within '.milestones' do
+      expect(page.all('ul.content-list > li').first.text).to include('bbb')
+      expect(page.all('ul.content-list > li').last.text).to include('aaa')
+    end
+  end
+end