diff options
author | Sean McGivern <sean@gitlab.com> | 2019-07-12 10:55:14 +0000 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2019-07-12 10:55:14 +0000 |
commit | a0c86c1fd47177c7c8924967d710545d5f5c57d1 (patch) | |
tree | 9eed2a1ef14b509a4ecc981f03ec8eb89491a603 | |
parent | e869ff9dad74351b70f23fa73924561aec79fa48 (diff) | |
parent | f271162880a898c4fde8ba2dfb15f5417b650a5d (diff) | |
download | gitlab-ce-a0c86c1fd47177c7c8924967d710545d5f5c57d1.tar.gz |
Merge branch '60798-follow-up-simplify-sort-direction-logic' into 'master'
Resolve "Follow up: Simplify sort direction logic"
Closes #60798
See merge request gitlab-org/gitlab-ce!30443
-rw-r--r-- | app/helpers/sorting_helper.rb | 83 | ||||
-rw-r--r-- | spec/helpers/sorting_helper_spec.rb | 148 |
2 files changed, 181 insertions, 50 deletions
diff --git a/app/helpers/sorting_helper.rb b/app/helpers/sorting_helper.rb index 26692934456..d5e5b472115 100644 --- a/app/helpers/sorting_helper.rb +++ b/app/helpers/sorting_helper.rb @@ -31,22 +31,24 @@ module SortingHelper end def projects_sort_options_hash - Feature.enabled?(:project_list_filter_bar) && !current_controller?('admin/projects') ? projects_sort_common_options_hash : old_projects_sort_options_hash - end + use_old_sorting = Feature.disabled?(:project_list_filter_bar) || current_controller?('admin/projects') - # TODO: Simplify these sorting options - # https://gitlab.com/gitlab-org/gitlab-ce/issues/60798 - # https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/11209#note_162234858 - def old_projects_sort_options_hash options = { sort_value_latest_activity => sort_title_latest_activity, + sort_value_recently_created => sort_title_created_date, sort_value_name => sort_title_name, - sort_value_oldest_activity => sort_title_oldest_activity, - sort_value_oldest_created => sort_title_oldest_created, - sort_value_recently_created => sort_title_recently_created, - sort_value_stars_desc => sort_title_most_stars + sort_value_stars_desc => sort_title_stars } + if use_old_sorting + options = options.merge({ + sort_value_oldest_activity => sort_title_oldest_activity, + sort_value_oldest_created => sort_title_oldest_created, + sort_value_recently_created => sort_title_recently_created, + sort_value_stars_desc => sort_title_most_stars + }) + end + if current_controller?('admin/projects') options[sort_value_largest_repo] = sort_title_largest_repo end @@ -54,26 +56,14 @@ module SortingHelper options end - def projects_sort_common_options_hash - { - sort_value_latest_activity => sort_title_latest_activity, - sort_value_recently_created => sort_title_created_date, - sort_value_name => sort_title_name, - sort_value_stars_desc => sort_title_stars - } - end - def projects_sort_option_titles - { - sort_value_latest_activity => sort_title_latest_activity, - sort_value_recently_created => sort_title_created_date, - sort_value_name => sort_title_name, - sort_value_stars_desc => sort_title_stars, + # Only used for the project filter search bar + projects_sort_options_hash.merge({ sort_value_oldest_activity => sort_title_latest_activity, sort_value_oldest_created => sort_title_created_date, sort_value_name_desc => sort_title_name, sort_value_stars_asc => sort_title_stars - } + }) end def projects_reverse_sort_options_hash @@ -210,47 +200,42 @@ module SortingHelper sort_options_hash[sort_value] end - def issuable_sort_icon_suffix(sort_value) + def sort_direction_icon(sort_value) case sort_value when sort_value_milestone, sort_value_due_date, /_asc\z/ - 'lowest' + 'sort-lowest' else - 'highest' + 'sort-highest' end end - # TODO: dedupicate issuable and project sort direction - # https://gitlab.com/gitlab-org/gitlab-ce/issues/60798 - def issuable_sort_direction_button(sort_value) + def sort_direction_button(reverse_url, reverse_sort, sort_value) link_class = 'btn btn-default has-tooltip reverse-sort-btn qa-reverse-sort' - reverse_sort = issuable_reverse_sort_order_hash[sort_value] + icon = sort_direction_icon(sort_value) + url = reverse_url - if reverse_sort - reverse_url = page_filter_path(sort: reverse_sort) - else - reverse_url = '#' + unless reverse_sort + url = '#' link_class += ' disabled' end - link_to(reverse_url, type: 'button', class: link_class, title: s_('SortOptions|Sort direction')) do - sprite_icon("sort-#{issuable_sort_icon_suffix(sort_value)}", size: 16) + link_to(url, type: 'button', class: link_class, title: s_('SortOptions|Sort direction')) do + sprite_icon(icon, size: 16) end end + def issuable_sort_direction_button(sort_value) + reverse_sort = issuable_reverse_sort_order_hash[sort_value] + url = page_filter_path(sort: reverse_sort) + + sort_direction_button(url, reverse_sort, sort_value) + end + def project_sort_direction_button(sort_value) - link_class = 'btn btn-default has-tooltip reverse-sort-btn qa-reverse-sort' reverse_sort = projects_reverse_sort_options_hash[sort_value] + url = filter_projects_path(sort: reverse_sort) - if reverse_sort - reverse_url = filter_projects_path(sort: reverse_sort) - else - reverse_url = '#' - link_class += ' disabled' - end - - link_to(reverse_url, type: 'button', class: link_class, title: s_('SortOptions|Sort direction')) do - sprite_icon("sort-#{issuable_sort_icon_suffix(sort_value)}", size: 16) - end + sort_direction_button(url, reverse_sort, sort_value) end # Titles. diff --git a/spec/helpers/sorting_helper_spec.rb b/spec/helpers/sorting_helper_spec.rb index f405268d198..5397a47b3dd 100644 --- a/spec/helpers/sorting_helper_spec.rb +++ b/spec/helpers/sorting_helper_spec.rb @@ -4,6 +4,11 @@ require 'spec_helper' describe SortingHelper do include ApplicationHelper include IconsHelper + include ExploreHelper + + def set_sorting_url(option) + allow(self).to receive(:request).and_return(double(path: 'http://test.com', query_parameters: { label_name: option })) + end describe '#issuable_sort_option_title' do it 'returns correct title for issuable_sort_option_overrides key' do @@ -21,7 +26,7 @@ describe SortingHelper do describe '#issuable_sort_direction_button' do before do - allow(self).to receive(:request).and_return(double(path: 'http://test.com', query_parameters: { label_name: 'test_label' })) + set_sorting_url 'test_label' end it 'keeps label filter param' do @@ -44,4 +49,145 @@ describe SortingHelper do expect(issuable_sort_direction_button('due_date')).to include('sort-lowest') end end + + def stub_controller_path(value) + allow(helper.controller).to receive(:controller_path).and_return(value) + end + + def project_common_options + { + sort_value_latest_activity => sort_title_latest_activity, + sort_value_recently_created => sort_title_created_date, + sort_value_name => sort_title_name, + sort_value_stars_desc => sort_title_stars + } + end + + describe 'with `admin/projects` controller' do + before do + stub_controller_path 'admin/projects' + end + + describe '#projects_sort_options_hash' do + it 'returns a hash of available sorting options' do + admin_options = project_common_options.merge({ + sort_value_oldest_activity => sort_title_oldest_activity, + sort_value_oldest_created => sort_title_oldest_created, + sort_value_recently_created => sort_title_recently_created, + sort_value_stars_desc => sort_title_most_stars, + sort_value_largest_repo => sort_title_largest_repo + }) + + expect(projects_sort_options_hash).to eq(admin_options) + end + end + end + + describe 'with `projects` controller' do + before do + stub_controller_path 'projects' + end + + describe '#projects_sort_options_hash' do + it 'returns a hash of available sorting options' do + expect(projects_sort_options_hash).to include(project_common_options) + end + end + + describe '#projects_reverse_sort_options_hash' do + context 'returns a reversed hash of available sorting options' do + using RSpec::Parameterized::TableSyntax + + where(:sort_key, :reverse_sort_title) do + sort_value_latest_activity | sort_value_oldest_activity + sort_value_recently_created | sort_value_oldest_created + sort_value_name | sort_value_name_desc + sort_value_stars_desc | sort_value_stars_asc + sort_value_oldest_activity | sort_value_latest_activity + sort_value_oldest_created | sort_value_recently_created + sort_value_name_desc | sort_value_name + sort_value_stars_asc | sort_value_stars_desc + end + + with_them do + it do + reverse_hash = projects_reverse_sort_options_hash + + expect(reverse_hash).to include(sort_key) + expect(reverse_hash[sort_key]).to eq(reverse_sort_title) + end + end + end + end + + describe '#project_sort_direction_button' do + context 'returns the correct icon for each sort option' do + using RSpec::Parameterized::TableSyntax + + sort_lowest_icon = 'sort-lowest' + sort_highest_icon = 'sort-highest' + + where(:selected_sort, :icon) do + sort_value_latest_activity | sort_highest_icon + sort_value_recently_created | sort_highest_icon + sort_value_name_desc | sort_highest_icon + sort_value_stars_desc | sort_highest_icon + sort_value_oldest_activity | sort_lowest_icon + sort_value_oldest_created | sort_lowest_icon + sort_value_name | sort_lowest_icon + sort_value_stars_asc | sort_lowest_icon + end + + with_them do + it do + set_sorting_url selected_sort + + expect(project_sort_direction_button(selected_sort)).to include(icon) + end + end + end + + it 'returns the correct link to reverse the current sort option' do + sort_options_links = projects_reverse_sort_options_hash + + sort_options_links.each do |selected_sort, reverse_sort| + set_sorting_url selected_sort + + expect(project_sort_direction_button(selected_sort)).to include(reverse_sort) + end + end + end + + describe '#projects_sort_option_titles' do + it 'returns a hash of titles for the sorting options' do + options = project_common_options.merge({ + sort_value_oldest_activity => sort_title_latest_activity, + sort_value_oldest_created => sort_title_created_date, + sort_value_name_desc => sort_title_name, + sort_value_stars_asc => sort_title_stars + }) + + expect(projects_sort_option_titles).to eq(options) + end + end + + describe 'with project_list_filter_bar off' do + before do + stub_feature_flags(project_list_filter_bar: false) + end + + describe '#projects_sort_options_hash' do + it 'returns a hash of available sorting options' do + options = project_common_options.merge({ + sort_value_oldest_activity => sort_title_oldest_activity, + sort_value_oldest_created => sort_title_oldest_created, + sort_value_recently_created => sort_title_recently_created, + sort_value_stars_desc => sort_title_most_stars + }) + + expect(projects_sort_options_hash).to eq(options) + end + end + end + end end |