From c2fc40668c34215a7e727e60647114f1b178eb8c Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Tue, 20 Feb 2018 12:33:49 +0000 Subject: 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. --- app/finders/issuable_finder.rb | 106 ++++++++++++++++------------------------- 1 file changed, 40 insertions(+), 66 deletions(-) (limited to 'app/finders/issuable_finder.rb') 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] -- cgit v1.2.1