Add group milestone to feature spec and minor changes

app/views/groups/milestones/index.html.haml

@@ -4,7 +4,7 @@
   = render 'shared/milestones_filter', counts: @milestone_states
 
   .nav-controls
-    = render 'shared/group_milestones_sort_dropdown'
+    = render 'shared/milestones_sort_dropdown'
     - if can?(current_user, :admin_milestones, @group)
       = link_to "New milestone", new_group_milestone_path(@group), class: "btn btn-new"
 

app/views/shared/_group_milestones_sort_dropdown.html.haml

@@ -1,22 +0,0 @@
-.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

lib/milestone_array.rb

@@ -13,7 +13,7 @@ module MilestoneArray
       when 'name_asc'
         sort_asc(array, 'title')
       when 'name_desc'
-        sort_desc(array, 'title')
+        sort_asc(array, 'title').reverse
       else
         array
       end
@@ -22,19 +22,19 @@ module MilestoneArray
     private
 
     def sort_asc_nulls_last(array, attribute)
-      array.select(&attribute.to_sym).sort_by(&attribute.to_sym) + array.reject(&attribute.to_sym)
+      attribute = attribute.to_sym
+
+      array.select(&attribute).sort_by(&attribute) + array.reject(&attribute)
     end
 
     def sort_desc_nulls_last(array, attribute)
-      array.select(&attribute.to_sym).sort_by(&attribute.to_sym).reverse + array.reject(&attribute.to_sym)
+      attribute = attribute.to_sym
+
+      array.select(&attribute).sort_by(&attribute).reverse + array.reject(&attribute)
     end
 
     def sort_asc(array, attribute)
       array.sort_by(&attribute.to_sym)
     end
-
-    def sort_desc(array, attribute)
-      array.sort_by(&attribute.to_sym).reverse
-    end
   end
 end

spec/features/groups/milestones_sorting_spec.rb

@@ -8,6 +8,7 @@ feature 'Milestones sorting', :js do
   let!(:other_project_milestone1) { create(:milestone, project: other_project, title: 'v1.0', due_date: 10.days.from_now) }
   let!(:project_milestone2) { create(:milestone, project: project, title: 'v2.0', due_date: 5.days.from_now) }
   let!(:other_project_milestone2) { create(:milestone, project: other_project, title: 'v2.0', due_date: 5.days.from_now) }
+  let!(:group_milestone) { create(:milestone, group: group, title: 'v3.0', due_date: 7.days.from_now) }
   let(:user) { create(:group_member, :master, user: create(:user), group: group ).user }
 
   before do
@@ -22,6 +23,7 @@ feature 'Milestones sorting', :js do
     # assert default sorting
     within '.milestones' do
       expect(page.all('ul.content-list > li').first.text).to include('v2.0')
+      expect(page.all('ul.content-list > li')[1].text).to include('v3.0')
       expect(page.all('ul.content-list > li').last.text).to include('v1.0')
     end
 
@@ -33,6 +35,8 @@ feature 'Milestones sorting', :js do
     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'
 
@@ -40,6 +44,7 @@ feature 'Milestones sorting', :js do
 
     within '.milestones' do
       expect(page.all('ul.content-list > li').first.text).to include('v1.0')
+      expect(page.all('ul.content-list > li')[1].text).to include('v3.0')
       expect(page.all('ul.content-list > li').last.text).to include('v2.0')
     end
   end