diff options
| -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 | ||||
| -rw-r--r-- | spec/controllers/concerns/issuable_collections_spec.rb | 15 |
6 files changed, 32 insertions, 65 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| diff --git a/spec/controllers/concerns/issuable_collections_spec.rb b/spec/controllers/concerns/issuable_collections_spec.rb index d16a3464495..e93c923fd39 100644 --- a/spec/controllers/concerns/issuable_collections_spec.rb +++ b/spec/controllers/concerns/issuable_collections_spec.rb @@ -60,7 +60,7 @@ describe IssuableCollections do end end - describe '#filter_params' do + describe '#finder_options' do let(:params) do { assignee_id: '1', @@ -84,25 +84,20 @@ describe IssuableCollections do } end - it 'filters params' do + it 'only allows whitelisted params' do allow(controller).to receive(:cookies).and_return({}) - filtered_params = controller.send(:filter_params) + finder_options = controller.send(:finder_options) - expect(filtered_params).to eq({ + expect(finder_options).to eq({ 'assignee_id' => '1', 'assignee_username' => 'user1', 'author_id' => '2', 'author_username' => 'user2', - 'authorized_only' => 'true', - 'due_date' => '2017-01-01', - 'group_id' => '3', - 'iids' => '4', 'label_name' => 'foo', 'milestone_title' => 'bar', 'my_reaction_emoji' => 'thumbsup', - 'non_archived' => 'true', - 'project_id' => '5', + 'due_date' => '2017-01-01', 'scope' => 'all', 'search' => 'baz', 'sort' => 'priority', |