diff options
author | Sean McGivern <sean@gitlab.com> | 2018-02-20 15:33:49 +0300 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2018-02-21 13:31:29 +0300 |
commit | c2fc40668c34215a7e727e60647114f1b178eb8c (patch) | |
tree | 75736437f7657fa617dd3a48376b73bd46e0c9fc /app/finders | |
parent | 5048c8d5050cd432381d845997c5e7991e6590f1 (diff) |
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.
Diffstat (limited to 'app/finders')
-rw-r--r-- | app/finders/issuable_finder.rb | 106 | ||||
-rw-r--r-- | app/finders/issues_finder.rb | 45 |
2 files changed, 85 insertions, 66 deletions
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) |