Merge branch '49993-fix-remember-sorting-issue-mr' into 'master'29398-support-kubernetes-rbac-for-gitlab-managed-apps-3

Resolve "Separate remembering sorting issues/merge requests"

Closes #49993

See merge request gitlab-org/gitlab-ce!21153

4 files changed, 77 insertions, 17 deletions

diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb
index 2ef2ee76855..22b39f47bf0 100644
--- a/app/controllers/concerns/issuable_collections.rb
+++ b/app/controllers/concerns/issuable_collections.rb
@@ -107,11 +107,15 @@ module IssuableCollections
   end
 
   def set_sort_order_from_cookie
-    key = 'issuable_sort'
+    cookies[remember_sorting_key] = params[:sort] if params[:sort].present?
+    # fallback to legacy cookie value for backward compatibility
+    cookies[remember_sorting_key] ||= cookies['issuable_sort']
+    cookies[remember_sorting_key] = update_cookie_value(cookies[remember_sorting_key])
+    params[:sort] = cookies[remember_sorting_key]
+  end
 
-    cookies[key] = params[:sort] if params[:sort].present?
-    cookies[key] = update_cookie_value(cookies[key])
-    params[:sort] = cookies[key]
+  def remember_sorting_key
+    @remember_sorting_key ||= "#{collection_type.downcase}_sort"
   end
 
   def default_sort_order
@@ -140,16 +144,14 @@ module IssuableCollections
   end
 
   def finder
-    strong_memoize(:finder) do
-      issuable_finder_for(finder_type)
-    end
+    @finder ||= issuable_finder_for(finder_type)
   end
 
   def collection_type
-    @collection_type ||= case finder
-                         when IssuesFinder
+    @collection_type ||= case finder_type.name
+                         when 'IssuesFinder'
                            'Issue'
-                         when MergeRequestsFinder
+                         when 'MergeRequestsFinder'
                            'MergeRequest'
                          end
   end
diff --git a/changelogs/unreleased/49993-fix-remember-sorting-issue-mr.yml b/changelogs/unreleased/49993-fix-remember-sorting-issue-mr.yml
new file mode 100644
index 00000000000..df05bf3f3e2
--- /dev/null
+++ b/changelogs/unreleased/49993-fix-remember-sorting-issue-mr.yml
@@ -0,0 +1,5 @@
+---
+title: Split remembering sorting for issues and merge requests
+merge_request: 21153
+author: Jacopo Beschi @jacopo-beschi
+type: fixed
diff --git a/spec/features/projects/issues/user_sorts_issues_spec.rb b/spec/features/projects/issues/user_sorts_issues_spec.rb
index db5936a30cb..7d261ec7dae 100644
--- a/spec/features/projects/issues/user_sorts_issues_spec.rb
+++ b/spec/features/projects/issues/user_sorts_issues_spec.rb
@@ -1,7 +1,9 @@
 require "spec_helper"
 
 describe "User sorts issues" do
-  set(:project) { create(:project_empty_repo, :public) }
+  set(:user) { create(:user) }
+  set(:group) { create(:group) }
+  set(:project) { create(:project_empty_repo, :public, group: group) }
   set(:issue1) { create(:issue, project: project) }
   set(:issue2) { create(:issue, project: project) }
   set(:issue3) { create(:issue, project: project) }
@@ -12,7 +14,29 @@ describe "User sorts issues" do
     create(:award_emoji, :downvote, awardable: issue1)
     create(:award_emoji, :upvote, awardable: issue2)
 
+    sign_in(user)
+
+    visit(project_issues_path(project))
+  end
+
+  it 'keeps the sort option' do
+    find('button.dropdown-toggle').click
+
+    page.within('.content ul.dropdown-menu.dropdown-menu-right li') do
+      click_link('Milestone')
+    end
+
+    visit(issues_dashboard_path(assignee_id: user.id))
+
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
+
     visit(project_issues_path(project))
+
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
+
+    visit(issues_group_path(group))
+
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
   end
 
   it "sorts by popularity" do
diff --git a/spec/features/projects/merge_requests/user_sorts_merge_requests_spec.rb b/spec/features/projects/merge_requests/user_sorts_merge_requests_spec.rb
index e401933aed2..82cfe600d52 100644
--- a/spec/features/projects/merge_requests/user_sorts_merge_requests_spec.rb
+++ b/spec/features/projects/merge_requests/user_sorts_merge_requests_spec.rb
@@ -1,15 +1,18 @@
 require 'spec_helper'
 
 describe 'User sorts merge requests' do
+  include CookieHelper
+
   let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
   let!(:merge_request2) do
     create(:merge_request_with_diffs, source_project: project, target_project: project, source_branch: 'merge-test')
   end
-  let(:project) { create(:project, :public, :repository) }
-  let(:user) { create(:user) }
+  set(:user) { create(:user) }
+  set(:group) { create(:group) }
+  set(:group_member) { create(:group_member, :maintainer, user: user, group: group) }
+  set(:project) { create(:project, :public, group: group) }
 
   before do
-    project.add_maintainer(user)
     sign_in(user)
 
     visit(project_merge_requests_path(project))
@@ -19,16 +22,42 @@ describe 'User sorts merge requests' do
     find('button.dropdown-toggle').click
 
     page.within('.content ul.dropdown-menu.dropdown-menu-right li') do
-      click_link('Last updated')
+      click_link('Milestone')
     end
 
     visit(merge_requests_dashboard_path(assignee_id: user.id))
 
-    expect(find('.issues-filters')).to have_content('Last updated')
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
 
     visit(project_merge_requests_path(project))
 
-    expect(find('.issues-filters')).to have_content('Last updated')
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
+
+    visit(merge_requests_group_path(group))
+
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
+  end
+
+  it 'fallbacks to issuable_sort cookie key when remembering the sorting option' do
+    set_cookie('issuable_sort', 'milestone')
+
+    visit(merge_requests_dashboard_path(assignee_id: user.id))
+
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
+  end
+
+  it 'separates remember sorting with issues' do
+    create(:issue, project: project)
+
+    find('button.dropdown-toggle').click
+
+    page.within('.content ul.dropdown-menu.dropdown-menu-right li') do
+      click_link('Milestone')
+    end
+
+    visit(project_issues_path(project))
+
+    expect(find('.issues-filters a.is-active')).not_to have_content('Milestone')
   end
 
   context 'when merge requests have awards' do