From f4ed780ef5fc76b7704742d4886ac435c3e5ab98 Mon Sep 17 00:00:00 2001 From: George Andrinopoulos Date: Mon, 6 Nov 2017 19:51:24 +0200 Subject: [PATCH 1/2] Add Group Milestone sorting --- .../groups/milestones_controller.rb | 3 +- app/views/groups/milestones/index.html.haml | 1 + .../_group_milestones_sort_dropdown.html.haml | 22 +++++++++ .../39720-group-milestone-sorting.yml | 5 ++ lib/milestone_array.rb | 40 ++++++++++++++++ .../groups/milestones_sorting_spec.rb | 46 +++++++++++++++++++ spec/lib/milestone_array_spec.rb | 34 ++++++++++++++ 7 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 app/views/shared/_group_milestones_sort_dropdown.html.haml create mode 100644 changelogs/unreleased/39720-group-milestone-sorting.yml create mode 100644 lib/milestone_array.rb create mode 100644 spec/features/groups/milestones_sorting_spec.rb create mode 100644 spec/lib/milestone_array_spec.rb diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb index 7a7bcb1a3d2..f013d21275e 100644 --- a/app/controllers/groups/milestones_controller.rb +++ b/app/controllers/groups/milestones_controller.rb @@ -80,7 +80,8 @@ class Groups::MilestonesController < Groups::ApplicationController milestones = MilestonesFinder.new(search_params).execute legacy_milestones = GroupMilestone.build_collection(group, group_projects, params) - milestones + legacy_milestones + @sort = params[:sort] || 'due_date_asc' + MilestoneArray.sort(milestones + legacy_milestones, @sort) end def milestone diff --git a/app/views/groups/milestones/index.html.haml b/app/views/groups/milestones/index.html.haml index cb4fc69d5b8..6c199c52583 100644 --- a/app/views/groups/milestones/index.html.haml +++ b/app/views/groups/milestones/index.html.haml @@ -4,6 +4,7 @@ = render 'shared/milestones_filter', counts: @milestone_states .nav-controls + = render 'shared/group_milestones_sort_dropdown' - if can?(current_user, :admin_milestones, @group) = link_to "New milestone", new_group_milestone_path(@group), class: "btn btn-new" diff --git a/app/views/shared/_group_milestones_sort_dropdown.html.haml b/app/views/shared/_group_milestones_sort_dropdown.html.haml new file mode 100644 index 00000000000..9b2f2fdcc93 --- /dev/null +++ b/app/views/shared/_group_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/39720-group-milestone-sorting.yml b/changelogs/unreleased/39720-group-milestone-sorting.yml new file mode 100644 index 00000000000..15ef87fa567 --- /dev/null +++ b/changelogs/unreleased/39720-group-milestone-sorting.yml @@ -0,0 +1,5 @@ +--- +title: Add dropdown sort to group milestones +merge_request: 15230 +author: George Andrinopoulos +type: added diff --git a/lib/milestone_array.rb b/lib/milestone_array.rb new file mode 100644 index 00000000000..9b3f2acc123 --- /dev/null +++ b/lib/milestone_array.rb @@ -0,0 +1,40 @@ +module MilestoneArray + class << self + def sort(array, sort_method) + case sort_method + when 'due_date_asc' + sort_asc_nulls_last(array, 'due_date') + when 'due_date_desc' + sort_desc_nulls_last(array, 'due_date') + when 'start_date_asc' + sort_asc_nulls_last(array, 'start_date') + when 'start_date_desc' + sort_desc_nulls_last(array, 'start_date') + when 'name_asc' + sort_asc(array, 'title') + when 'name_desc' + sort_desc(array, 'title') + else + array + end + end + + private + + def sort_asc_nulls_last(array, attribute) + array.select(&attribute.to_sym).sort_by(&attribute.to_sym) + array.reject(&attribute.to_sym) + end + + def sort_desc_nulls_last(array, attribute) + array.select(&attribute.to_sym).sort_by(&attribute.to_sym).reverse + array.reject(&attribute.to_sym) + 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 diff --git a/spec/features/groups/milestones_sorting_spec.rb b/spec/features/groups/milestones_sorting_spec.rb new file mode 100644 index 00000000000..3bd59587535 --- /dev/null +++ b/spec/features/groups/milestones_sorting_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +feature 'Milestones sorting', :js do + let(:group) { create(:group) } + let!(:project) { create(:project_empty_repo, group: group) } + let!(:other_project) { create(:project_empty_repo, group: group) } + let!(:project_milestone1) { create(:milestone, project: project, title: 'v1.0', due_date: 10.days.from_now) } + 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(:user) { create(:group_member, :master, user: create(:user), group: group ).user } + + before do + sign_in(user) + end + + scenario 'visit group milestones and sort by due_date_asc' do + visit group_milestones_path(group) + + expect(page).to have_button('Due soon') + + # 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').last.text).to include('v1.0') + 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') + + 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('v1.0') + expect(page.all('ul.content-list > li').last.text).to include('v2.0') + end + end +end diff --git a/spec/lib/milestone_array_spec.rb b/spec/lib/milestone_array_spec.rb new file mode 100644 index 00000000000..df91677b925 --- /dev/null +++ b/spec/lib/milestone_array_spec.rb @@ -0,0 +1,34 @@ +require 'spec_helper' + +describe MilestoneArray do + let(:object1) { instance_double("BirdMilestone", due_date: Time.now, start_date: Time.now - 15.days, title: 'v2.0') } + let(:object2) { instance_double("CatMilestone", due_date: Time.now - 1.day, start_date: nil, title: 'v1.0') } + let(:object3) { instance_double("DogMilestone", due_date: nil, start_date: Time.now - 30.days, title: 'v3.0') } + let(:array) { [object1, object3, object2] } + + describe '#sort' do + it 'reorders array with due date in ascending order with nulls last' do + expect(described_class.sort(array, 'due_date_asc')).to eq([object2, object1, object3]) + end + + it 'reorders array with due date in desc order with nulls last' do + expect(described_class.sort(array, 'due_date_desc')).to eq([object1, object2, object3]) + end + + it 'reorders array with start date in ascending order with nulls last' do + expect(described_class.sort(array, 'start_date_asc')).to eq([object3, object1, object2]) + end + + it 'reorders array with start date in descending order with nulls last' do + expect(described_class.sort(array, 'start_date_desc')).to eq([object1, object3, object2]) + end + + it 'reorders array with title in ascending order' do + expect(described_class.sort(array, 'name_asc')).to eq([object2, object1, object3]) + end + + it 'reorders array with title in descending order' do + expect(described_class.sort(array, 'name_desc')).to eq([object3, object1, object2]) + end + end +end From a4d71cba7ef80e6f3c10f148dd1edfbef7f82893 Mon Sep 17 00:00:00 2001 From: George Andrinopoulos Date: Tue, 7 Nov 2017 20:57:30 +0200 Subject: [PATCH 2/2] Add group milestone to feature spec and minor changes --- app/views/groups/milestones/index.html.haml | 2 +- .../_group_milestones_sort_dropdown.html.haml | 22 ------------------- lib/milestone_array.rb | 14 ++++++------ .../groups/milestones_sorting_spec.rb | 5 +++++ 4 files changed, 13 insertions(+), 30 deletions(-) delete mode 100644 app/views/shared/_group_milestones_sort_dropdown.html.haml diff --git a/app/views/groups/milestones/index.html.haml b/app/views/groups/milestones/index.html.haml index 6c199c52583..f5f621507b8 100644 --- a/app/views/groups/milestones/index.html.haml +++ b/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" diff --git a/app/views/shared/_group_milestones_sort_dropdown.html.haml b/app/views/shared/_group_milestones_sort_dropdown.html.haml deleted file mode 100644 index 9b2f2fdcc93..00000000000 --- a/app/views/shared/_group_milestones_sort_dropdown.html.haml +++ /dev/null @@ -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 diff --git a/lib/milestone_array.rb b/lib/milestone_array.rb index 9b3f2acc123..4ed8485b36a 100644 --- a/lib/milestone_array.rb +++ b/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 diff --git a/spec/features/groups/milestones_sorting_spec.rb b/spec/features/groups/milestones_sorting_spec.rb index 3bd59587535..a0fe40cf1d3 100644 --- a/spec/features/groups/milestones_sorting_spec.rb +++ b/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