Added tests for sort icon current

Refactor sort direction icon

2 files changed, 57 insertions, 89 deletions

diff --git a/app/helpers/sorting_helper.rb b/app/helpers/sorting_helper.rb
index 10ab6123467..e4d2bd91cc4 100644
--- a/app/helpers/sorting_helper.rb
+++ b/app/helpers/sorting_helper.rb
@@ -31,7 +31,7 @@ module SortingHelper
   end
 
   def projects_sort_options_hash
-    use_old_sorting = !Feature.enabled?(:project_list_filter_bar) || current_controller?('admin/projects') 
+    use_old_sorting = !Feature.enabled?(:project_list_filter_bar) || current_controller?('admin/projects')
 
     options = {
       sort_value_latest_activity  => sort_title_latest_activity,
@@ -200,48 +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
 
-  # TODO: dedupicate issuable and project sort direction
+  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 93e7c2c5840..0a31b8ad8d6 100644
--- a/spec/helpers/sorting_helper_spec.rb
+++ b/spec/helpers/sorting_helper_spec.rb
@@ -6,6 +6,10 @@ describe SortingHelper do
   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
       expect(issuable_sort_option_title('created_asc')).to eq('Created date')
@@ -22,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
@@ -46,8 +50,6 @@ describe SortingHelper do
     end
   end
 
-  # TODO: need separate tests for /admin/projects and /projects
-  # TODO: should this be renamed to `projects_sort_option_title??` ... maybe not
   def stub_controller_path(value)
     allow(helper.controller).to receive(:controller_path).and_return(value)
   end
@@ -63,14 +65,14 @@ describe SortingHelper do
 
   describe 'with `admin/projects` controller' do
     before do
-      stub_controller_path('admin/projects')
+      stub_controller_path 'admin/projects'
     end
 
     describe '#projects_sort_options_hash' do
       it 'returns a hash of available sorting options' do
         hash = projects_sort_options_hash
 
-        admin_options = project_common_options.merge(    {
+        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,
@@ -84,7 +86,7 @@ describe SortingHelper do
 
   describe 'with `projects` controller' do
     before do
-      stub_controller_path('projects')
+      stub_controller_path 'projects'
     end
 
     describe '#projects_sort_options_hash' do
@@ -99,7 +101,7 @@ describe SortingHelper do
       end
     end
 
-    describe '#projects_reverse_sort_options_hash' do 
+    describe '#projects_reverse_sort_options_hash' do
       it 'returns a reversed hash of available sorting options' do
         reverse_hash = projects_reverse_sort_options_hash
 
@@ -122,24 +124,37 @@ describe SortingHelper do
     end
 
     describe '#project_sort_direction_button' do
-      # TODO: add more of these
-      # TODO: Not too sure about these
-      # Take a look at the ui and just go with that
-      # it 'returns icon with sort-highest when sort is created_date' do
-      #   expect(project_sort_direction_button('created_date')).to include('sort-highest')
-      # end
+      it 'returns the correct icon for each sort option' do
+        sort_lowest_icon = 'sort-lowest'
+        sort_highest_icon = 'sort-highest'
+
+        sort_options_icons = {
+          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
+        }
+
+        sort_options_icons.each do |selected_sort, icon|
+          set_sorting_url selected_sort
+
+          expect(project_sort_direction_button(selected_sort)).to include(icon)
+        end
+      end
 
-      # it 'returns icon with sort-lowest when sort is asc' do
-      #   expect(project_sort_direction_button('created_asc')).to include('sort-lowest')
-      # end
+      it 'returns the correct link to reverse the current sort option' do
+        sort_options_links = projects_reverse_sort_options_hash
 
-      # it 'returns icon with sort-lowest when sorting by milestone' do
-      #   expect(project_sort_direction_button('milestone')).to include('sort-lowest')
-      # end
+        sort_options_links.each do |selected_sort, reverse_sort|
+          set_sorting_url selected_sort
 
-      # it 'returns icon with sort-lowest when sorting by due_date' do
-      #   expect(project_sort_direction_button('due_date')).to include('sort-lowest')
-      # end
+          expect(project_sort_direction_button(selected_sort)).to include(reverse_sort)
+        end
+      end
     end
 
     describe '#projects_sort_option_titles' do
@@ -187,47 +202,6 @@ describe SortingHelper do
           end
         end
       end
-
-      describe '#projects_reverse_sort_options_hash' do 
-        it 'returns a reversed hash of available sorting options' do
-          reverse_hash = projects_reverse_sort_options_hash
-
-          options = {
-            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
-          }
-
-          options.each do |key, opt|
-            expect(reverse_hash).to include(key)
-            expect(reverse_hash[key]).to eq(opt)
-          end
-        end
-      end
-
-      describe '#project_sort_direction_button' do
-        # TODO: these don't look correct
-        # it 'returns icon with sort-highest when sort is created_date' do
-        #   expect(project_sort_direction_button('created_date')).to include('sort-highest')
-        # end
-
-        # it 'returns icon with sort-lowest when sort is asc' do
-        #   expect(project_sort_direction_button('created_asc')).to include('sort-lowest')
-        # end
-
-        # it 'returns icon with sort-lowest when sorting by milestone' do
-        #   expect(project_sort_direction_button('milestone')).to include('sort-lowest')
-        # end
-
-        # it 'returns icon with sort-lowest when sorting by due_date' do
-        #   expect(project_sort_direction_button('due_date')).to include('sort-lowest')
-        # end
-      end
     end
   end
 end