Merge branch 'sy-replace-issuable-sort-dropdown' into 'master'

Replace issuable sort dropdown

See merge request gitlab-org/gitlab!81430

app/helpers/sorting_helper.rb

@@ -243,6 +243,51 @@ module SortingHelper
     sort_options_hash[sort_value]
   end
 
+  def issuable_sort_options(viewing_issues, viewing_merge_requests)
+    options = [
+      { value: sort_value_priority, text: sort_title_priority, href: page_filter_path(sort: sort_value_priority) },
+      { value: sort_value_created_date, text: sort_title_created_date, href: page_filter_path(sort: sort_value_created_date) },
+      { value: sort_value_recently_updated, text: sort_title_recently_updated, href: page_filter_path(sort: sort_value_recently_updated) },
+      { value: sort_value_milestone, text: sort_title_milestone, href: page_filter_path(sort: sort_value_milestone) }
+    ]
+
+    options.concat([due_date_option]) if viewing_issues
+
+    options.concat([popularity_option, label_priority_option])
+    options.concat([merged_option, closed_option]) if viewing_merge_requests
+    options.concat([relative_position_option]) if viewing_issues
+
+    options.concat([title_option])
+  end
+
+  def due_date_option
+    { value: sort_value_due_date, text: sort_title_due_date, href: page_filter_path(sort: sort_value_due_date) }
+  end
+
+  def popularity_option
+    { value: sort_value_popularity, text: sort_title_popularity, href: page_filter_path(sort: sort_value_popularity) }
+  end
+
+  def label_priority_option
+    { value: sort_value_label_priority, text: sort_title_label_priority, href: page_filter_path(sort: sort_value_label_priority) }
+  end
+
+  def merged_option
+    { value: sort_value_merged_date, text: sort_title_merged_date, href: page_filter_path(sort: sort_value_merged_date) }
+  end
+
+  def closed_option
+    { value: sort_value_closed_date, text: sort_title_closed_date, href: page_filter_path(sort: sort_value_closed_date) }
+  end
+
+  def relative_position_option
+    { value: sort_value_relative_position, text: sort_title_relative_position, href: page_filter_path(sort: sort_value_relative_position) }
+  end
+
+  def title_option
+    { value: sort_value_title, text: sort_title_title, href: page_filter_path(sort: sort_value_title) }
+  end
+
   def sort_direction_icon(sort_value)
     case sort_value
     when sort_value_milestone, sort_value_due_date, sort_value_merged_date, sort_value_closed_date, /_asc\z/

app/views/shared/issuable/_sort_dropdown.html.haml

-- sort_value = @sort
-- sort_title = issuable_sort_option_title(sort_value)
 - viewing_issues = controller.controller_name == 'issues' || controller.action_name == 'issues'
 - viewing_merge_requests = controller.controller_name == 'merge_requests'
+- items = issuable_sort_options(viewing_issues, viewing_merge_requests)
+- selected = issuable_sort_option_overrides[@sort] || @sort
 
-.dropdown.inline.gl-ml-3.issue-sort-dropdown
+.gl-ml-3
   .btn-group{ role: 'group' }
-    .btn-group{ role: 'group' }
-      %button.dropdown-menu-toggle{ type: 'button', data: { toggle: 'dropdown', display: 'static' }, class: 'gl-button btn btn-default' }
-        = sort_title
-        = sprite_icon('chevron-down', css_class: "dropdown-menu-toggle-icon gl-top-3")
-      %ul.dropdown-menu.dropdown-menu-right.dropdown-menu-selectable.dropdown-menu-sort
-        %li
-          = sortable_item(sort_title_priority,          page_filter_path(sort: sort_value_priority),          sort_title)
-          = sortable_item(sort_title_created_date,      page_filter_path(sort: sort_value_created_date),      sort_title)
-          = sortable_item(sort_title_recently_updated,  page_filter_path(sort: sort_value_recently_updated),  sort_title)
-          = sortable_item(sort_title_milestone,         page_filter_path(sort: sort_value_milestone),         sort_title)
-          = sortable_item(sort_title_due_date,          page_filter_path(sort: sort_value_due_date),          sort_title) if viewing_issues
-          = sortable_item(sort_title_popularity,        page_filter_path(sort: sort_value_popularity),        sort_title)
-          = sortable_item(sort_title_label_priority,    page_filter_path(sort: sort_value_label_priority),    sort_title)
-          = sortable_item(sort_title_merged_date,       page_filter_path(sort: sort_value_merged_date),       sort_title) if viewing_merge_requests
-          = sortable_item(sort_title_closed_date,       page_filter_path(sort: sort_value_closed_date),       sort_title) if viewing_merge_requests
-          = sortable_item(sort_title_relative_position, page_filter_path(sort: sort_value_relative_position), sort_title) if viewing_issues
-          = sortable_item(sort_title_title,             page_filter_path(sort: sort_value_title),             sort_title)
-          = render_if_exists('shared/ee/issuable/sort_dropdown', viewing_issues: viewing_issues, sort_title: sort_title)
-    = issuable_sort_direction_button(sort_value)
+    = gl_redirect_listbox_tag(items, selected, data: { right: true })
+    = issuable_sort_direction_button(@sort)

ee/app/helpers/ee/sorting_helper.rb

@@ -37,5 +37,26 @@ module EE
         super
       end
     end
+
+    override :issuable_sort_options
+    def issuable_sort_options(viewing_issues, viewing_merge_requests)
+      options = super
+
+      if viewing_issues && (@project || @group)&.licensed_feature_available?(:issue_weights)
+        options.concat([weight_option])
+      end
+
+      options.concat([blocking_option]) if viewing_issues
+
+      options
+    end
+
+    def weight_option
+      { value: sort_value_weight, text: sort_title_weight, href: page_filter_path(sort: sort_value_weight) }
+    end
+
+    def blocking_option
+      { value: sort_value_blocking_desc, text: sort_title_blocking, href: page_filter_path(sort: sort_value_blocking_desc) }
+    end
   end
 end

ee/app/views/shared/ee/issuable/_sort_dropdown.html.haml

-- allow_weight_sort = viewing_issues && (@project || @group)&.feature_available?(:issue_weights)
-= sortable_item(sort_title_weight, page_filter_path(sort: sort_value_weight), sort_title) if allow_weight_sort
-
-- allow_blocking_sort = viewing_issues
-= sortable_item(sort_title_blocking, page_filter_path(sort: sort_value_blocking_desc), sort_title) if allow_blocking_sort

spec/features/dashboard/issues_filter_spec.rb

@@ -78,14 +78,14 @@ RSpec.describe 'Dashboard Issues filtering', :js do
     end
 
     it 'remembers last sorting value' do
-      sort_by('Created date')
+      pajamas_sort_by(s_('SortOptions|Created date'))
       visit_issues(assignee_username: user.username)
 
       expect(page).to have_button('Created date')
     end
 
     it 'keeps sorting issues after visiting Projects Issues page' do
-      sort_by('Created date')
+      pajamas_sort_by(s_('SortOptions|Created date'))
       visit project_issues_path(project)
 
       expect(page).to have_button('Created date')

spec/features/dashboard/merge_requests_spec.rb

@@ -115,6 +115,9 @@ RSpec.describe 'Dashboard Merge Requests' do
       within("span[aria-label='#{n_("%d merge request", "%d merge requests", 3) % 3}']") do
         expect(page).to have_content('3')
       end
+
+      find('.dashboard-shortcuts-merge_requests').click
+
       expect(find('.js-assigned-mr-count')).to have_content('2')
       expect(find('.js-reviewer-mr-count')).to have_content('1')
     end
@@ -165,16 +168,16 @@ RSpec.describe 'Dashboard Merge Requests' do
       expect(page).to have_content('Please select at least one filter to see results')
     end
 
-    it 'shows sorted merge requests' do
-      sort_by('Created date')
+    it 'shows sorted merge requests', :js do
+      pajamas_sort_by(s_('SortOptions|Created date'))
 
       visit merge_requests_dashboard_path(assignee_username: current_user.username)
 
       expect(find('.issues-filters')).to have_content('Created date')
     end
 
-    it 'keeps sorting merge requests after visiting Projects MR page' do
-      sort_by('Created date')
+    it 'keeps sorting merge requests after visiting Projects MR page', :js do
+      pajamas_sort_by(s_('SortOptions|Created date'))
 
       visit project_merge_requests_path(project)
 

spec/features/issuables/sorting_list_spec.rb

@@ -88,14 +88,14 @@ RSpec.describe 'Sort Issuable List' do
       end
     end
 
-    context 'custom sorting' do
+    context 'custom sorting', :js do
       let(:issuable_type) { :merge_request }
 
       it 'supports sorting in asc and desc order' do
         visit_merge_requests_with_state(project, 'open')
 
         click_button('Created date')
-        click_link('Updated date')
+        find('.dropdown-item', text: 'Updated date').click
 
         expect(first_merge_request).to include(last_updated_issuable.title)
         expect(last_merge_request).to include(first_updated_issuable.title)

spec/features/issues/filtered_search/filter_issues_spec.rb

@@ -556,7 +556,7 @@ RSpec.describe 'Filter issues', :js do
         sort_toggle = find('.filter-dropdown-container .dropdown')
         sort_toggle.click
 
-        find('.filter-dropdown-container .dropdown-menu li a', text: 'Created date').click
+        find('.dropdown-item', text: 'Created date').click
         wait_for_requests
 
         expect(find('.issues-list .issue:first-of-type .issue-title-text a')).to have_content(new_issue.title)

spec/features/issues/user_sorts_issues_spec.rb

@@ -5,6 +5,7 @@ require "spec_helper"
 RSpec.describe "User sorts issues" do
   include SortingHelper
   include IssueHelpers
+  include Spec::Support::Helpers::Features::SortingHelpers
 
   let_it_be(:user) { create(:user) }
   let_it_be(:group) { create(:group) }
@@ -24,26 +25,22 @@ RSpec.describe "User sorts issues" do
     sign_in(user)
   end
 
-  it 'keeps the sort option' do
+  it 'keeps the sort option', :js do
     visit(project_issues_path(project))
 
-    find('.filter-dropdown-container .dropdown').click
-
-    page.within('ul.dropdown-menu.dropdown-menu-right li') do
-      click_link('Milestone')
-    end
+    pajamas_sort_by(s_('SortOptions|Milestone'))
 
     visit(issues_dashboard_path(assignee_username: user.username))
 
-    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
+    expect(find('.filter-dropdown-container button.dropdown-toggle')).to have_content('Milestone')
 
     visit(project_issues_path(project))
 
-    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
+    expect(find('.filter-dropdown-container button.dropdown-toggle')).to have_content('Milestone')
 
     visit(issues_group_path(group))
 
-    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
+    expect(find('.filter-dropdown-container button.dropdown-toggle')).to have_content('Milestone')
   end
 
   it 'sorts by popularity', :js do

spec/features/merge_requests/user_lists_merge_requests_spec.rb

@@ -109,7 +109,7 @@ RSpec.describe 'Merge requests > User lists merge requests' do
     expect(count_merge_requests).to eq(4)
   end
 
-  it 'sorts by milestone' do
+  it 'sorts by milestone due date' do
     visit_merge_requests(project, sort: sort_value_milestone)
 
     expect(first_merge_request).to include('fix')
@@ -130,12 +130,12 @@ RSpec.describe 'Merge requests > User lists merge requests' do
     expect(count_merge_requests).to eq(4)
   end
 
-  it 'filters on one label and sorts by due date' do
+  it 'filters on one label and sorts by milestone due date' do
     label = create(:label, project: project)
     create(:label_link, label: label, target: @fix)
 
     visit_merge_requests(project, label_name: [label.name],
-                                  sort: sort_value_due_date)
+                                  sort: sort_value_milestone)
 
     expect(first_merge_request).to include('fix')
     expect(count_merge_requests).to eq(1)
@@ -150,19 +150,19 @@ RSpec.describe 'Merge requests > User lists merge requests' do
       create(:label_link, label: label2, target: @fix)
     end
 
-    it 'sorts by due date' do
+    it 'sorts by milestone due date' do
       visit_merge_requests(project, label_name: [label.name, label2.name],
-                                    sort: sort_value_due_date)
+                                    sort: sort_value_milestone)
 
       expect(first_merge_request).to include('fix')
       expect(count_merge_requests).to eq(1)
     end
 
     context 'filter on assignee and' do
-      it 'sorts by due soon' do
+      it 'sorts by milestone due date' do
         visit_merge_requests(project, label_name: [label.name, label2.name],
                                       assignee_id: user.id,
-                                      sort: sort_value_due_date)
+                                      sort: sort_value_milestone)
 
         expect(first_merge_request).to include('fix')
         expect(count_merge_requests).to eq(1)

spec/features/merge_requests/user_sorts_merge_requests_spec.rb

@@ -2,8 +2,9 @@
 
 require 'spec_helper'
 
-RSpec.describe 'User sorts merge requests' do
+RSpec.describe 'User sorts merge requests', :js do
   include CookieHelper
+  include Spec::Support::Helpers::Features::SortingHelpers
 
   let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
   let!(:merge_request2) do
@@ -22,23 +23,19 @@ RSpec.describe 'User sorts merge requests' do
   end
 
   it 'keeps the sort option' do
-    find('.filter-dropdown-container .dropdown').click
-
-    page.within('ul.dropdown-menu.dropdown-menu-right li') do
-      click_link('Milestone')
-    end
+    pajamas_sort_by(s_('SortOptions|Milestone'))
 
     visit(merge_requests_dashboard_path(assignee_username: user.username))
 
-    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
+    expect(find('.filter-dropdown-container button.dropdown-toggle')).to have_content('Milestone')
 
     visit(project_merge_requests_path(project))
 
-    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
+    expect(find('.filter-dropdown-container button.dropdown-toggle')).to have_content('Milestone')
 
     visit(merge_requests_group_path(group))
 
-    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
+    expect(find('.filter-dropdown-container button.dropdown-toggle')).to have_content('Milestone')
   end
 
   it 'fallbacks to issuable_sort cookie key when remembering the sorting option' do
@@ -46,17 +43,13 @@ RSpec.describe 'User sorts merge requests' do
 
     visit(merge_requests_dashboard_path(assignee_username: user.username))
 
-    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
+    expect(find('.filter-dropdown-container button.dropdown-toggle')).to have_content('Milestone')
   end
 
   it 'separates remember sorting with issues' do
     create(:issue, project: project)
 
-    find('.filter-dropdown-container .dropdown').click
-
-    page.within('ul.dropdown-menu.dropdown-menu-right li') do
-      click_link('Milestone')
-    end
+    pajamas_sort_by(s_('SortOptions|Milestone'))
 
     visit(project_issues_path(project))
 
@@ -73,11 +66,7 @@ RSpec.describe 'User sorts merge requests' do
     end
 
     it 'sorts by popularity' do
-      find('.filter-dropdown-container .dropdown').click
-
-      page.within('ul.dropdown-menu.dropdown-menu-right li') do
-        click_link('Popularity')
-      end
+      pajamas_sort_by(s_('SortOptions|Popularity'))
 
       page.within('.mr-list') do
         page.within('li.merge-request:nth-child(1)') do

spec/features/user_sorts_things_spec.rb

@@ -6,7 +6,7 @@ require "spec_helper"
 # to check if the sorting option set by user is being kept persisted while going through pages.
 # The `it`s are named here by convention `starting point -> some pages -> final point`.
 # All those specs are moved out to this spec intentionally to keep them all in one place.
-RSpec.describe "User sorts things" do
+RSpec.describe "User sorts things", :js do
   include Spec::Support::Helpers::Features::SortingHelpers
   include DashboardHelper
 
@@ -21,11 +21,11 @@ RSpec.describe "User sorts things" do
   end
 
   it "issues -> project home page -> issues" do
-    sort_option = 'Updated date'
+    sort_option = s_('SortOptions|Updated date')
 
     visit(project_issues_path(project))
 
-    sort_by(sort_option)
+    pajamas_sort_by(sort_option)
 
     visit(project_path(project))
     visit(project_issues_path(project))
@@ -34,11 +34,11 @@ RSpec.describe "User sorts things" do
   end
 
   it "merge requests -> dashboard merge requests" do
-    sort_option = 'Updated date'
+    sort_option = s_('SortOptions|Updated date')
 
     visit(project_merge_requests_path(project))
 
-    sort_by(sort_option)
+    pajamas_sort_by(sort_option)
 
     visit(assigned_mrs_dashboard_path)
 

spec/support/helpers/features/sorting_helpers.rb

@@ -21,6 +21,14 @@ module Spec
               click_link(value)
             end
           end
+
+          # pajamas_sort_by is used to sort new pajamas dropdowns. When
+          # all of the dropdowns are converted, pajamas_sort_by can be renamed to sort_by
+          # https://gitlab.com/groups/gitlab-org/-/epics/7551
+          def pajamas_sort_by(value)
+            find('.filter-dropdown-container .dropdown').click
+            find('.dropdown-item', text: value).click
+          end
         end
       end
     end