diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index 0d7ee06deb6..f7ba305a59f 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -103,7 +103,7 @@ module IssuableCollections # @filter_params[:authorized_only] = true end - @filter_params.permit(IssuableFinder::VALID_PARAMS) + @filter_params.permit(finder_type.valid_params) end # rubocop:enable Gitlab/ModuleWithInstanceVariables @@ -146,7 +146,7 @@ module IssuableCollections def finder strong_memoize(:finder) do - issuable_finder_for(@finder_type) # rubocop:disable Gitlab/ModuleWithInstanceVariables + issuable_finder_for(finder_type) end end diff --git a/app/controllers/concerns/issues_action.rb b/app/controllers/concerns/issues_action.rb index 3ba1235cee0..3b11a373368 100644 --- a/app/controllers/concerns/issues_action.rb +++ b/app/controllers/concerns/issues_action.rb @@ -4,7 +4,6 @@ module IssuesAction # rubocop:disable Gitlab/ModuleWithInstanceVariables def issues - @finder_type = IssuesFinder @issues = issuables_collection .non_archived .page(params[:page]) @@ -17,4 +16,11 @@ module IssuesAction end end # rubocop:enable Gitlab/ModuleWithInstanceVariables + + private + + def finder_type + (super if defined?(super)) || + (IssuesFinder if action_name == 'issues') + end end diff --git a/app/controllers/concerns/merge_requests_action.rb b/app/controllers/concerns/merge_requests_action.rb index a9cc13038bf..b70db99b157 100644 --- a/app/controllers/concerns/merge_requests_action.rb +++ b/app/controllers/concerns/merge_requests_action.rb @@ -4,8 +4,6 @@ module MergeRequestsAction # rubocop:disable Gitlab/ModuleWithInstanceVariables def merge_requests - @finder_type = MergeRequestsFinder - @merge_requests = issuables_collection.page(params[:page]) @issuable_meta_data = issuable_meta_data(@merge_requests, collection_type) @@ -14,6 +12,11 @@ module MergeRequestsAction private + def finder_type + (super if defined?(super)) || + (MergeRequestsFinder if action_name == 'merge_requests') + end + def filter_params super.merge(non_archived: true) end diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 33fced99132..73806454525 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -243,9 +243,8 @@ class Projects::IssuesController < Projects::ApplicationController Issues::UpdateService.new(project, current_user, update_params) end - def set_issuables_index - @finder_type = IssuesFinder - super + def finder_type + IssuesFinder end def whitelist_query_limiting diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 8eed957d9fe..a1af125547c 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -323,9 +323,8 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo @target_branches = @merge_request.target_project.repository.branch_names end - def set_issuables_index - @finder_type = MergeRequestsFinder - super + def finder_type + MergeRequestsFinder end def check_user_can_push_to_source_branch! diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 72573e0765d..0370edc6e20 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -279,7 +279,6 @@ class ProjectsController < Projects::ApplicationController @project_wiki = @project.wiki @wiki_home = @project_wiki.find_page('home', params[:version_id]) elsif @project.feature_available?(:issues, current_user) - @finder_type = IssuesFinder @issues = issuables_collection.page(params[:page]) @collection_type = 'Issue' @issuable_meta_data = issuable_meta_data(@issues, @collection_type) @@ -289,6 +288,10 @@ class ProjectsController < Projects::ApplicationController end end + def finder_type + IssuesFinder + end + def determine_layout if [:new, :create].include?(action_name.to_sym) 'application' diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 0fe3000ca01..384a336e2bb 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -25,32 +25,38 @@ class IssuableFinder NONE = '0'.freeze - SCALAR_PARAMS = %i[ - assignee_id - assignee_username - author_id - author_username - authorized_only - due_date - group_id - iids - label_name - milestone_title - my_reaction_emoji - non_archived - project_id - scope - search - sort - state - include_subgroups - ].freeze - ARRAY_PARAMS = { label_name: [], iids: [], assignee_username: [] }.freeze - - VALID_PARAMS = (SCALAR_PARAMS + [ARRAY_PARAMS]).freeze - attr_accessor :current_user, :params + def self.scalar_params + @scalar_params ||= %i[ + assignee_id + 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 + ] + end + + def self.array_params + @array_params ||= { label_name: [], iids: [], assignee_username: [] } + end + + def self.valid_params + @valid_params ||= scalar_params + [array_params] + end + def initialize(current_user, params = {}) @current_user = current_user @params = params @@ -58,6 +64,15 @@ class IssuableFinder def execute items = init_collection + items = filter_items(items) + + # Filtering by project HAS TO be the last because we use the project IDs yielded by the issuable query thus far + items = by_project(items) + + sort(items) + end + + def filter_items(items) items = by_scope(items) items = by_created_at(items) items = by_state(items) @@ -65,16 +80,11 @@ class IssuableFinder items = by_search(items) items = by_assignee(items) items = by_author(items) - items = by_due_date(items) items = by_non_archived(items) items = by_iids(items) items = by_milestone(items) items = by_label(items) - items = by_my_reaction_emoji(items) - - # Filtering by project HAS TO be the last because we use the project IDs yielded by the issuable query thus far - items = by_project(items) - sort(items) + by_my_reaction_emoji(items) end def find(*params) @@ -396,42 +406,6 @@ class IssuableFinder items end - def by_due_date(items) - if due_date? - if filter_by_no_due_date? - items = items.without_due_date - elsif filter_by_overdue? - items = items.due_before(Date.today) - elsif filter_by_due_this_week? - items = items.due_between(Date.today.beginning_of_week, Date.today.end_of_week) - elsif filter_by_due_this_month? - items = items.due_between(Date.today.beginning_of_month, Date.today.end_of_month) - end - end - - items - end - - def filter_by_no_due_date? - due_date? && params[:due_date] == Issue::NoDueDate.name - end - - def filter_by_overdue? - due_date? && params[:due_date] == Issue::Overdue.name - end - - def filter_by_due_this_week? - due_date? && params[:due_date] == Issue::DueThisWeek.name - end - - def filter_by_due_this_month? - due_date? && params[:due_date] == Issue::DueThisMonth.name - end - - def due_date? - params[:due_date].present? && klass.column_names.include?('due_date') - end - def label_names if labels? params[:label_name].is_a?(String) ? params[:label_name].split(',') : params[:label_name] diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb index 83245aadf6e..d65c620e75a 100644 --- a/app/finders/issues_finder.rb +++ b/app/finders/issues_finder.rb @@ -16,10 +16,15 @@ # sort: string # my_reaction_emoji: string # public_only: boolean +# due_date: date or '0', '', 'overdue', 'week', or 'month' # class IssuesFinder < IssuableFinder CONFIDENTIAL_ACCESS_LEVEL = Gitlab::Access::REPORTER + def self.scalar_params + @scalar_params ||= super + [:due_date] + end + def klass Issue.includes(:author) end @@ -52,6 +57,46 @@ class IssuesFinder < IssuableFinder params.fetch(:public_only, false) end + def filter_items(items) + by_due_date(super) + end + + def by_due_date(items) + if due_date? + if filter_by_no_due_date? + items = items.without_due_date + elsif filter_by_overdue? + items = items.due_before(Date.today) + elsif filter_by_due_this_week? + items = items.due_between(Date.today.beginning_of_week, Date.today.end_of_week) + elsif filter_by_due_this_month? + items = items.due_between(Date.today.beginning_of_month, Date.today.end_of_month) + end + end + + items + end + + def filter_by_no_due_date? + due_date? && params[:due_date] == Issue::NoDueDate.name + end + + def filter_by_overdue? + due_date? && params[:due_date] == Issue::Overdue.name + end + + def filter_by_due_this_week? + due_date? && params[:due_date] == Issue::DueThisWeek.name + end + + def filter_by_due_this_month? + due_date? && params[:due_date] == Issue::DueThisMonth.name + end + + def due_date? + params[:due_date].present? + end + def user_can_see_all_confidential_issues? return @user_can_see_all_confidential_issues if defined?(@user_can_see_all_confidential_issues) diff --git a/spec/controllers/concerns/issuable_collections_spec.rb b/spec/controllers/concerns/issuable_collections_spec.rb index d7825364ed5..c1f42bbb9d7 100644 --- a/spec/controllers/concerns/issuable_collections_spec.rb +++ b/spec/controllers/concerns/issuable_collections_spec.rb @@ -8,6 +8,10 @@ describe IssuableCollections do def self.helper_method(name); end include IssuableCollections + + def finder_type + IssuesFinder + end end controller = klass.new