Refactor whitelisting of filter params

5 files changed, 27 insertions, 55 deletions

diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb
index 5217b4be928..34a8c50fcbd 100644
--- a/app/controllers/concerns/issuable_collections.rb
+++ b/app/controllers/concerns/issuable_collections.rb
@@ -81,36 +81,36 @@ module IssuableCollections
   end
 
   def issuable_finder_for(finder_class)
-    finder_class.new(current_user, filter_params)
+    finder_class.new(current_user, finder_options)
   end
 
   # rubocop:disable Gitlab/ModuleWithInstanceVariables
-  # rubocop: disable CodeReuse/ActiveRecord
-  def filter_params
-    set_sort_order_from_cookie
-    set_default_state
+  def finder_options
+    params[:state] = default_state if params[:state].blank?
 
-    # Skip irrelevant Rails routing params
-    @filter_params = params.dup.except(:controller, :action, :namespace_id)
-    @filter_params[:sort] ||= default_sort_order
+    options = {
+      scope: params[:scope],
+      state: params[:state],
+      sort: set_sort_order_from_cookie || default_sort_order
+    }
 
-    @sort = @filter_params[:sort]
+    # Used by view to highlight active option
+    @sort = options[:sort]
 
     if @project
-      @filter_params[:project_id] = @project.id
+      options[:project_id] = @project.id
     elsif @group
-      @filter_params[:group_id] = @group.id
-      @filter_params[:include_subgroups] = true
-      @filter_params[:use_cte_for_search] = true
+      options[:group_id] = @group.id
+      options[:include_subgroups] = true
+      options[:use_cte_for_search] = true
     end
 
-    @filter_params.permit(finder_type.valid_params)
+    params.permit(finder_type.valid_params).merge(options)
   end
-  # rubocop: enable CodeReuse/ActiveRecord
   # rubocop:enable Gitlab/ModuleWithInstanceVariables
 
-  def set_default_state
-    params[:state] = 'opened' if params[:state].blank?
+  def default_state
+    'opened'
   end
 
   def set_sort_order_from_cookie
@@ -121,7 +121,7 @@ module IssuableCollections
 
     sort_value = update_cookie_value(sort_param)
     set_secure_cookie(remember_sorting_key, sort_value)
-    params[:sort] = sort_value
+    sort_value
   end
 
   def remember_sorting_key
diff --git a/app/controllers/concerns/merge_requests_action.rb b/app/controllers/concerns/merge_requests_action.rb
index 285f2c3a8a0..ed10f32512e 100644
--- a/app/controllers/concerns/merge_requests_action.rb
+++ b/app/controllers/concerns/merge_requests_action.rb
@@ -19,7 +19,7 @@ module MergeRequestsAction
       (MergeRequestsFinder if action_name == 'merge_requests')
   end
 
-  def filter_params
+  def finder_options
     super.merge(non_archived: true)
   end
 end
diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb
index 5076c3da324..4ce9be44403 100644
--- a/app/controllers/dashboard_controller.rb
+++ b/app/controllers/dashboard_controller.rb
@@ -4,17 +4,6 @@ class DashboardController < Dashboard::ApplicationController
   include IssuesAction
   include MergeRequestsAction
 
-  FILTER_PARAMS = [
-    # author_id and assignee_id are kept so old RSS links still work
-    :author_id,
-    :assignee_id,
-    :author_username,
-    :assignee_username,
-    :milestone_title,
-    :label_name,
-    :my_reaction_emoji
-  ].freeze
-
   before_action :event_filter, only: :activity
   before_action :projects, only: [:issues, :merge_requests]
   before_action :set_show_full_reference, only: [:issues, :merge_requests]
@@ -55,10 +44,13 @@ class DashboardController < Dashboard::ApplicationController
   end
 
   def check_filters_presence!
-    @no_filters_set = FILTER_PARAMS.none? { |k| params.key?(k) }
+    @no_filters_set = finder_type.scalar_params.none? { |k| params.key?(k) }
 
     return unless @no_filters_set
 
+    # Call to set selected `state` and `sort` options in view
+    finder_options
+
     respond_to do |format|
       format.html { render }
       format.atom { head :bad_request }
diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index 93bef592c65..fdc630cbf72 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -14,7 +14,9 @@
 #     project_id: integer
 #     milestone_title: string
 #     author_id: integer
+#     author_username: string
 #     assignee_id: integer or 'None' or 'Any'
+#     assignee_username: string
 #     search: string
 #     label_name: string
 #     sort: string
@@ -49,25 +51,15 @@ class IssuableFinder
       assignee_username
       author_id
       author_username
-      authorized_only
-      group_id
-      iids
       label_name
       milestone_title
       my_reaction_emoji
-      non_archived
-      project_id
-      scope
       search
-      sort
-      state
-      include_subgroups
-      use_cte_for_search
     ]
   end
 
   def self.array_params
-    @array_params ||= { label_name: [], iids: [], assignee_username: [] }
+    @array_params ||= { label_name: [], assignee_username: [] }
   end
 
   def self.valid_params
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 61d4386fb9f..74042f0bae8 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -173,19 +173,7 @@ module ApplicationHelper
     without = options.delete(:without)
     add_label = options.delete(:label)
 
-    exist_opts = {
-      state: params[:state],
-      scope: params[:scope],
-      milestone_title: params[:milestone_title],
-      assignee_username: params[:assignee_username],
-      author_username: params[:author_username],
-      search: params[:search],
-      label_name: params[:label_name],
-      my_reaction_emoji: params[:my_reaction_emoji],
-      wip: params[:wip]
-    }
-
-    options = exist_opts.merge(options)
+    options = request.query_parameters.merge(options)
 
     if without.present?
       without.each do |key|