Commit 5a5e75a8 authored by Sean McGivern's avatar Sean McGivern
Browse files

Refactor IssuableFinder to extract model-specific logic

By extracting a new `filter_items` method, we can override that in the
IssuesFinder and MergeRequestsFinder separately, so we don't need checks that
the model is the correct one, because we can just use the class we're in to know
that.

We can do the same for the VALID_PARAMS constant, by making it a class method.
parent c3ee1cfbb803
......@@ -103,7 +103,7 @@ def filter_params
# @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 @@ def update_cookie_value(value)
def finder
strong_memoize(:finder) do
issuable_finder_for(@finder_type) # rubocop:disable Gitlab/ModuleWithInstanceVariables
issuable_finder_for(finder_type)
end
end
......
......@@ -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 @@ def issues
end
end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
private
def finder_type
(super if defined?(super)) ||
(IssuesFinder if action_name == 'issues')
end
end
......@@ -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 @@ def merge_requests
private
def finder_type
(super if defined?(super)) ||
(MergeRequestsFinder if action_name == 'merge_requests')
end
def filter_params
super.merge(non_archived: true)
end
......
......@@ -243,9 +243,8 @@ def update_service
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
......
......@@ -323,9 +323,8 @@ def define_edit_vars
@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!
......
......@@ -279,7 +279,6 @@ def render_landing_page
@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 @@ def render_landing_page
end
end
def finder_type
IssuesFinder
end
def determine_layout
if [:new, :create].include?(action_name.to_sym)
'application'
......
......@@ -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 @@ def initialize(current_user, params = {})
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 @@ def execute
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 @@ def by_my_reaction_emoji(items)
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]
......
......@@ -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 @@ def public_only?
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)
......
......@@ -8,6 +8,10 @@
def self.helper_method(name); end
include IssuableCollections
def finder_type
IssuesFinder
end
end
controller = klass.new
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment