diff options
author | Heinrich Lee Yu <hleeyu@gmail.com> | 2018-11-09 17:29:45 +0300 |
---|---|---|
committer | Heinrich Lee Yu <hleeyu@gmail.com> | 2018-11-14 20:12:07 +0300 |
commit | 2a5319832454fe40cfea151dd7b9e2aa520b3450 (patch) | |
tree | 4e996385d7f70ead102fe361f28e43ada80c30da /app | |
parent | 2ade9b4479d7ce1f9b1bebd1e9ca47efb4b6f423 (diff) |
Refactor whitelisting of filter params
Diffstat (limited to 'app')
-rw-r--r-- | app/controllers/concerns/issuable_collections.rb | 36 | ||||
-rw-r--r-- | app/controllers/concerns/merge_requests_action.rb | 2 | ||||
-rw-r--r-- | app/controllers/dashboard_controller.rb | 16 | ||||
-rw-r--r-- | app/finders/issuable_finder.rb | 14 | ||||
-rw-r--r-- | app/helpers/application_helper.rb | 14 |
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| |