Merge branch 'refactor-issuable-finder-to-use-inheritance' into 'master'

Refactor IssuableFinder to extract model-specific logic

See merge request gitlab-org/gitlab-ce!17236

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)