Refactoring: Set ordering preference from IssuableCollections

app/controllers/concerns/issuable_collections.rb

@@ -91,7 +91,7 @@ module IssuableCollections
     options = {
       scope: params[:scope],
       state: params[:state],
-      sort: params[:sort] || set_sort_order_from_cookie || default_sort_order
+      sort: set_sort_order_from_user_preference || set_sort_order_from_cookie || default_sort_order
     }
 
     # Used by view to highlight active option
@@ -113,6 +113,28 @@ module IssuableCollections
     'opened'
   end
 
+  def set_sort_order_from_user_preference
+    return unless current_user
+    return unless issuable_sorting_field
+
+    user_preference = current_user.user_preference
+
+    sort_param = params[:sort]
+    sort_param ||= user_preference[issuable_sorting_field]
+
+    if user_preference[issuable_sorting_field] != sort_param
+      user_preference.update_attribute(issuable_sorting_field, sort_param)
+    end
+
+    sort_param
+  end
+
+  # Implement default_sorting_field method on controllers
+  # to choose which column to store the sorting parameter.
+  def issuable_sorting_field
+    nil
+  end
+
   def set_sort_order_from_cookie
     sort_param = params[:sort] if params[:sort].present?
     # fallback to legacy cookie value for backward compatibility

ee/app/controllers/concerns/epics_sorting.rb

-module EpicsSorting
-  def set_epics_sorting
-    if current_user
-      set_sort_order_from_user_preference
-    else
-      set_sort_order_from_cookie
-    end
-  end
-
-  def set_sort_order_from_user_preference
-    return params[:sort] unless current_user
-
-    user_preference = current_user.user_preference
-
-    sort_param = params[:sort]
-    sort_param ||= user_preference.epics_sort
-
-    if user_preference.epics_sort != sort_param
-      user_preference.update(epics_sort: sort_param)
-    end
-
-    params[:sort] = sort_param
-  end
-end

ee/app/controllers/groups/epics_controller.rb

@@ -5,15 +5,9 @@ class Groups::EpicsController < Groups::ApplicationController
   include ToggleSubscriptionAction
   include RendersNotes
   include EpicsActions
-  include EpicsSorting
 
   before_action :check_epics_available!
   before_action :epic, except: [:index, :create]
-
-  # This callback should be executed before the :set_issuables_index
-  # otherwise sorting will be ignored for epics.
-  before_action :set_epics_sorting, only: :index
-
   before_action :set_issuables_index, only: :index
   before_action :authorize_update_issuable!, only: :update
   before_action :authorize_create_epic!, only: [:create]
@@ -99,6 +93,10 @@ class Groups::EpicsController < Groups::ApplicationController
     EpicsFinder
   end
 
+  def issuable_sorting_field
+    :epics_sort
+  end
+
   def preload_for_collection
     @preload_for_collection ||= [:group, :author]
   end

ee/app/controllers/groups/roadmap_controller.rb

@@ -2,16 +2,16 @@ module Groups
   class RoadmapController < Groups::ApplicationController
     include IssuableCollections
     include EpicsActions
-    include EpicsSorting
 
     before_action :check_epics_available!
     before_action :group
     before_action :persist_roadmap_layout, only: [:show]
-    before_action :set_epics_sorting, only: :show
 
+    # show roadmap for a group
     def show
-      # show roadmap for a group
-      @sort = params[:sort] || default_sort_order
+      # Used only to show to correct sort dropdown option on filter bar
+      @sort = params[:sort] || current_user&.user_preference&.epics_sort || default_sort_order
+
       @epics_count = EpicsFinder.new(current_user, group_id: @group.id).execute.count
     end
 

ee/spec/controllers/groups/epics_controller_spec.rb

@@ -57,11 +57,25 @@ describe Groups::EpicsController do
         expect(response).to have_gitlab_http_status(200)
       end
 
-      it 'stores sorting param in a cookie' do
-        get :index, group_id: group, sort: 'start_date_asc'
+      context 'when there is no logged user' do
+        it 'stores sorting param in a cookie' do
+          group.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+          sign_out(user)
 
-        expect(cookies['epic_sort']).to eq('start_date_asc')
-        expect(response).to have_gitlab_http_status(200)
+          get :index, group_id: group, sort: 'start_date_asc'
+
+          expect(cookies['epic_sort']).to eq('start_date_asc')
+          expect(response).to have_gitlab_http_status(200)
+        end
+      end
+
+      context 'when there is a logged user' do
+        it 'stores sorting param in user preferences' do
+          get :index, group_id: group, sort: 'start_date_asc'
+
+          expect(user.user_preference.epics_sort).to eq('start_date_asc')
+          expect(response).to have_gitlab_http_status(200)
+        end
       end
 
       context 'with page param' do