Refactor assignee filtering

Extract issuable assignee filters to separate class

app/finders/issuable_finder.rb

@@ -148,7 +148,6 @@ class IssuableFinder
 
   # Negates all params found in `negatable_params`
   def filter_negated_items(items)
-    items = by_negated_assignee(items)
     items = by_negated_label(items)
     items = by_negated_milestone(items)
     items = by_negated_release(items)
@@ -365,32 +364,21 @@ class IssuableFinder
 
   def by_author(items)
     Issuables::AuthorFilter.new(
-      items,
       params: original_params,
       or_filters_enabled: or_filters_enabled?
-    ).filter
+    ).filter(items)
   end
 
   def by_assignee(items)
-    if params.filter_by_no_assignee?
-      items.unassigned
-    elsif params.filter_by_any_assignee?
-      items.assigned
-    elsif params.assignee
-      items.assigned_to(params.assignee)
-    elsif params.assignee_id? || params.assignee_username? # assignee not found
-      items.none
-    else
-      items
-    end
+    assignee_filter.filter(items)
   end
 
-  def by_negated_assignee(items)
-    # We want CE users to be able to say "Issues not assigned to either PersonA nor PersonB"
-    if not_params.assignees.present?
-      items.not_assigned_to(not_params.assignees)
-    else
-      items
+  def assignee_filter
+    strong_memoize(:assignee_filter) do
+      Issuables::AssigneeFilter.new(
+        params: original_params,
+        or_filters_enabled: or_filters_enabled?
+      )
     end
   end
 

app/finders/issuable_finder/params.rb

@@ -27,14 +27,6 @@ class IssuableFinder
       params.present?
     end
 
-    def filter_by_no_assignee?
-      params[:assignee_id].to_s.downcase == FILTER_NONE
-    end
-
-    def filter_by_any_assignee?
-      params[:assignee_id].to_s.downcase == FILTER_ANY
-    end
-
     def filter_by_no_label?
       downcased = label_names.map(&:downcase)
 
@@ -156,24 +148,6 @@ class IssuableFinder
       end
     end
 
-    # rubocop: disable CodeReuse/ActiveRecord
-    def assignees
-      strong_memoize(:assignees) do
-        if assignee_id?
-          User.where(id: params[:assignee_id])
-        elsif assignee_username?
-          User.where(username: params[:assignee_username])
-        else
-          User.none
-        end
-      end
-    end
-    # rubocop: enable CodeReuse/ActiveRecord
-
-    def assignee
-      assignees.first
-    end
-
     def label_names
       if labels?
         params[:label_name].is_a?(String) ? params[:label_name].split(',') : params[:label_name]

app/finders/issuables/assignee_filter.rb

+# frozen_string_literal: true
+
+module Issuables
+  class AssigneeFilter < BaseFilter
+    def filter(issuables)
+      filtered = by_assignee(issuables)
+      by_negated_assignee(filtered)
+    end
+
+    def includes_user?(user)
+      Array(params[:assignee_ids]).include?(user.id) ||
+        Array(params[:assignee_id]).include?(user.id) ||
+        Array(params[:assignee_username]).include?(user.username)
+    end
+
+    private
+
+    def by_assignee(issuables)
+      if filter_by_no_assignee?
+        issuables.unassigned
+      elsif filter_by_any_assignee?
+        issuables.assigned
+      elsif has_assignee_param?(params)
+        filter_by_assignees(issuables)
+      else
+        issuables
+      end
+    end
+
+    def by_negated_assignee(issuables)
+      return issuables unless has_assignee_param?(not_params)
+
+      issuables.not_assigned_to(assignee_ids(not_params))
+    end
+
+    def filter_by_no_assignee?
+      params[:assignee_id].to_s.downcase == FILTER_NONE
+    end
+
+    def filter_by_any_assignee?
+      params[:assignee_id].to_s.downcase == FILTER_ANY
+    end
+
+    def filter_by_assignees(issuables)
+      assignee_ids = assignee_ids(params)
+
+      return issuables.none if assignee_ids.blank?
+
+      assignee_ids.each do |assignee_id|
+        issuables = issuables.assigned_to(assignee_id)
+      end
+
+      issuables
+    end
+
+    def has_assignee_param?(specific_params)
+      return if specific_params.nil?
+
+      specific_params[:assignee_ids].present? || specific_params[:assignee_id].present? || specific_params[:assignee_username].present?
+    end
+
+    def assignee_ids(specific_params)
+      if specific_params[:assignee_ids].present?
+        Array(specific_params[:assignee_ids])
+      elsif specific_params[:assignee_id].present?
+        Array(specific_params[:assignee_id])
+      elsif specific_params[:assignee_username].present?
+        User.by_username(specific_params[:assignee_username]).select(:id)
+      end
+    end
+  end
+end

app/finders/issuables/author_filter.rb

@@ -2,7 +2,7 @@
 
 module Issuables
   class AuthorFilter < BaseFilter
-    def filter
+    def filter(issuables)
       filtered = by_author(issuables)
       filtered = by_author_union(filtered)
       by_negated_author(filtered)

app/finders/issuables/base_filter.rb

@@ -2,10 +2,12 @@
 
 module Issuables
   class BaseFilter
-    attr_reader :issuables, :params
+    attr_reader :params
 
-    def initialize(issuables, params:, or_filters_enabled: false)
-      @issuables = issuables
+    FILTER_NONE = 'none'
+    FILTER_ANY = 'any'
+
+    def initialize(params:, or_filters_enabled: false)
       @params = params
       @or_filters_enabled = or_filters_enabled
     end

app/finders/issues_finder.rb

@@ -52,7 +52,7 @@ class IssuesFinder < IssuableFinder
     # can always see confidential issues assigned to them. This is just an
     # optimization since a very common usecase of this Finder is to load the
     # count of issues assigned to the user for the header bar.
-    return Issue.all if current_user && params.assignees.include?(current_user)
+    return Issue.all if current_user && assignee_filter.includes_user?(current_user)
 
     return Issue.where('issues.confidential IS NOT TRUE') if params.user_cannot_see_confidential_issues?
 

ee/app/finders/ee/issues_finder.rb

@@ -43,19 +43,6 @@ module EE
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
-    override :by_assignee
-    def by_assignee(items)
-      if params.assignees.any?
-        params.assignees.each do |assignee|
-          items = items.assigned_to(assignee)
-        end
-
-        return items
-      end
-
-      super
-    end
-
     def by_epic(items)
       return items unless params.by_epic?
 

ee/app/finders/ee/issues_finder/params.rb

@@ -30,19 +30,6 @@ module EE
         params[:weight].to_s.downcase == ::IssuableFinder::Params::FILTER_ANY
       end
 
-      override :assignees
-      # rubocop: disable CodeReuse/ActiveRecord
-      def assignees
-        strong_memoize(:assignees) do
-          if assignee_ids?
-            ::User.where(id: params[:assignee_ids])
-          else
-            super
-          end
-        end
-      end
-      # rubocop: enable CodeReuse/ActiveRecord
-
       def epics
         if params[:include_subepics]
           ::Gitlab::ObjectHierarchy.new(::Epic.id_in(params[:epic_id])).base_and_descendants.select(:id)