Merge branch '198324-avoid_subqueries' into 'master'

Speed up NOT Issuable filters

See merge request gitlab-org/gitlab!27639

app/assets/javascripts/filtered_search/constants.js

@@ -4,3 +4,8 @@ export const DROPDOWN_TYPE = {
   hint: 'hint',
   operator: 'operator',
 };
+
+export const FILTER_TYPE = {
+  none: 'none',
+  any: 'any',
+};

app/assets/javascripts/filtered_search/dropdown_operator.js

@@ -47,13 +47,17 @@ export default class DropdownOperator extends FilteredSearchDropdown {
         title: '=',
         help: __('is'),
       },
-      {
+    ];
+
+    if (gon.features?.notIssuableQueries) {
+      dropdownData.push({
         tag: 'not-equal',
         type: 'string',
         title: '!=',
         help: __('is not'),
-      },
-    ];
+      });
+    }
+
     this.droplab.changeHookList(this.hookId, this.dropdown, [Filter], this.config);
     this.droplab.setData(this.hookId, dropdownData);
     super.renderContent(forceShowList);

app/assets/javascripts/filtered_search/filtered_search_dropdown.js

 import DropdownUtils from './dropdown_utils';
 import FilteredSearchDropdownManager from './filtered_search_dropdown_manager';
 import FilteredSearchVisualTokens from './filtered_search_visual_tokens';
+import { FILTER_TYPE } from './constants';
 
 const DATA_DROPDOWN_TRIGGER = 'data-dropdown-trigger';
 
@@ -74,6 +75,9 @@ export default class FilteredSearchDropdown {
 
   renderContent(forceShowList = false) {
     const currentHook = this.getCurrentHook();
+
+    FilteredSearchDropdown.hideDropdownItemsforNotOperator(currentHook);
+
     if (forceShowList && currentHook && currentHook.list.hidden) {
       currentHook.list.show();
     }
@@ -138,4 +142,41 @@ export default class FilteredSearchDropdown {
       hook.list.render(results);
     }
   }
+
+  /**
+   * Hide None & Any options from the current dropdown.
+   * Hiding happens only for NOT operator.
+   */
+  static hideDropdownItemsforNotOperator(currentHook) {
+    const lastOperator = FilteredSearchVisualTokens.getLastTokenOperator();
+
+    if (lastOperator === '!=') {
+      const { list: dropdownEl } = currentHook.list;
+
+      let shouldHideDivider = true;
+
+      // Iterate over all the static dropdown values,
+      // then hide `None` and `Any` items.
+      Array.from(dropdownEl.querySelectorAll('li[data-value]')).forEach(itemEl => {
+        const {
+          dataset: { value },
+        } = itemEl;
+
+        if (value.toLowerCase() === FILTER_TYPE.none || value.toLowerCase() === FILTER_TYPE.any) {
+          itemEl.classList.add('hidden');
+        } else {
+          // If we encountered any element other than None/Any, then
+          // we shouldn't hide the divider
+          shouldHideDivider = false;
+        }
+      });
+
+      if (shouldHideDivider) {
+        const divider = dropdownEl.querySelector('li.divider');
+        if (divider) {
+          divider.classList.add('hidden');
+        }
+      }
+    }
+  }
 }

app/controllers/boards/issues_controller.rb

@@ -20,6 +20,9 @@ module Boards
     skip_before_action :authenticate_user!, only: [:index]
     before_action :validate_id_list, only: [:bulk_move]
     before_action :can_move_issues?, only: [:bulk_move]
+    before_action do
+      push_frontend_feature_flag(:not_issuable_queries, board.group, default_enabled: true)
+    end
 
     def index
       list_service = Boards::Issues::ListService.new(board_parent, current_user, filter_params)

app/controllers/concerns/issuable_actions.rb

@@ -11,6 +11,9 @@ module IssuableActions
     before_action only: :show do
       push_frontend_feature_flag(:scoped_labels, default_enabled: true)
     end
+    before_action do
+      push_frontend_feature_flag(:not_issuable_queries, @project, default_enabled: true)
+    end
   end
 
   def permitted_keys

app/controllers/concerns/issuable_collections_action.rb

@@ -32,6 +32,10 @@ module IssuableCollectionsAction
 
   private
 
+  def set_not_query_feature_flag(object = nil)
+    push_frontend_feature_flag(:not_issuable_queries, object, default_enabled: true)
+  end
+
   def sorting_field
     case action_name
     when 'issues'

app/controllers/dashboard_controller.rb

@@ -10,6 +10,7 @@ class DashboardController < Dashboard::ApplicationController
   before_action :projects, only: [:issues, :merge_requests]
   before_action :set_show_full_reference, only: [:issues, :merge_requests]
   before_action :check_filters_presence!, only: [:issues, :merge_requests]
+  before_action :set_not_query_feature_flag
 
   respond_to :html
 

app/controllers/groups_controller.rb

@@ -31,6 +31,10 @@ class GroupsController < Groups::ApplicationController
     push_frontend_feature_flag(:vue_issuables_list, @group)
   end
 
+  before_action do
+    set_not_query_feature_flag(@group)
+  end
+
   before_action :export_rate_limit, only: [:export, :download_export]
 
   skip_cross_project_access_check :index, :new, :create, :edit, :update,

app/finders/issuable_finder.rb

@@ -40,7 +40,7 @@ class IssuableFinder
 
   requires_cross_project_access unless: -> { params.project? }
 
-  NEGATABLE_PARAMS_HELPER_KEYS = %i[include_subgroups in].freeze
+  NEGATABLE_PARAMS_HELPER_KEYS = %i[project_id scope status include_subgroups].freeze
 
   attr_accessor :current_user, :params
 
@@ -68,7 +68,7 @@ class IssuableFinder
 
     # This should not be used in controller strong params!
     def negatable_scalar_params
-      @negatable_scalar_params ||= scalar_params + %i[project_id group_id]
+      @negatable_scalar_params ||= scalar_params - %i[search in]
     end
 
     # This should not be used in controller strong params!
@@ -100,7 +100,7 @@ class IssuableFinder
     items = filter_items(items)
 
     # Let's see if we have to negate anything
-    items = by_negation(items)
+    items = filter_negated_items(items)
 
     # This has to be last as we use a CTE as an optimization fence
     # for counts by passing the force_cte param and enabling the
@@ -132,6 +132,22 @@ class IssuableFinder
     by_my_reaction_emoji(items)
   end
 
+  # Negates all params found in `negatable_params`
+  def filter_negated_items(items)
+    return items unless Feature.enabled?(:not_issuable_queries, params.group || params.project, default_enabled: true)
+
+    # API endpoints send in `nil` values so we test if there are any non-nil
+    return items unless not_params.present? && not_params.values.any?
+
+    items = by_negated_author(items)
+    items = by_negated_assignee(items)
+    items = by_negated_label(items)
+    items = by_negated_milestone(items)
+    items = by_negated_release(items)
+    items = by_negated_my_reaction_emoji(items)
+    by_negated_iids(items)
+  end
+
   def row_count
     Gitlab::IssuablesCountForState.new(self).for_state_or_opened(params[:state])
   end
@@ -189,6 +205,21 @@ class IssuableFinder
 
   private
 
+  def not_params
+    strong_memoize(:not_params) do
+      params_class.new(params[:not].dup, current_user, klass).tap do |not_params|
+        next unless not_params.present?
+
+        # These are "helper" params that modify the results, like :in and :search. They usually come in at the top-level
+        # params, but if they do come in inside the `:not` params, the inner ones should take precedence.
+        not_helpers = params.slice(*NEGATABLE_PARAMS_HELPER_KEYS).merge(params[:not].slice(*NEGATABLE_PARAMS_HELPER_KEYS))
+        not_helpers.each do |key, value|
+          not_params[key] = value unless not_params[key].present?
+        end
+      end
+    end
+  end
+
   def force_cte?
     !!params[:force_cte]
   end
@@ -215,33 +246,6 @@ class IssuableFinder
     klass.available_states.key(value)
   end
 
-  # Negates all params found in `negatable_params`
-  # rubocop: disable CodeReuse/ActiveRecord
-  def by_negation(items)
-    not_params = params[:not].dup
-    # API endpoints send in `nil` values so we test if there are any non-nil
-    return items unless not_params.present? && not_params.values.any?
-
-    not_params.keep_if { |_k, v| v.present? }.each do |(key, value)|
-      # These aren't negatable params themselves, but rather help other searches, so we skip them.
-      # They will be added into all the NOT searches.
-      next if NEGATABLE_PARAMS_HELPER_KEYS.include?(key.to_sym)
-      next unless self.class.negatable_params.include?(key.to_sym)
-
-      # These are "helper" params that are required inside the NOT to get the right results. They usually come in
-      # at the top-level params, but if they do come in inside the `:not` params, they should take precedence.
-      not_helpers = params.slice(*NEGATABLE_PARAMS_HELPER_KEYS).merge(params[:not].slice(*NEGATABLE_PARAMS_HELPER_KEYS))
-      not_param = { key => value }.with_indifferent_access.merge(not_helpers).merge(not_query: true)
-
-      items_to_negate = self.class.new(current_user, not_param).execute
-
-      items = items.where.not(id: items_to_negate)
-    end
-
-    items
-  end
-  # rubocop: enable CodeReuse/ActiveRecord
-
   # rubocop: disable CodeReuse/ActiveRecord
   def by_scope(items)
     return items.none if params.current_user_related? && !current_user
@@ -325,6 +329,12 @@ class IssuableFinder
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
+  # rubocop: disable CodeReuse/ActiveRecord
+  def by_negated_iids(items)
+    not_params[:iids].present? ? items.where.not(iid: not_params[:iids]) : items
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
+
   # rubocop: disable CodeReuse/ActiveRecord
   def sort(items)
     # Ensure we always have an explicit sort order (instead of inheriting
@@ -347,9 +357,19 @@ class IssuableFinder
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
-  def by_assignee(items)
-    return items.assigned_to(params.assignees) if not_query? && params.assignees.any?
+  # rubocop: disable CodeReuse/ActiveRecord
+  def by_negated_author(items)
+    if not_params.author
+      items.where.not(author_id: not_params.author.id)
+    elsif not_params.author_id? || not_params.author_username? # author not found
+      items.none
+    else
+      items
+    end
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
 
+  def by_assignee(items)
     if params.filter_by_no_assignee?
       items.unassigned
     elsif params.filter_by_any_assignee?
@@ -363,6 +383,17 @@ class IssuableFinder
     end
   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)
+    elsif not_params.assignee_id? || not_params.assignee_username? # assignee not found
+      items.none
+    else
+      items
+    end
+  end
+
   # rubocop: disable CodeReuse/ActiveRecord
   def by_milestone(items)
     return items unless params.milestones?
@@ -382,6 +413,20 @@ class IssuableFinder
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
+  # rubocop: disable CodeReuse/ActiveRecord
+  def by_negated_milestone(items)
+    return items unless not_params.milestones?
+
+    if not_params.filter_by_upcoming_milestone?
+      items.joins(:milestone).merge(Milestone.not_upcoming)
+    elsif not_params.filter_by_started_milestone?
+      items.joins(:milestone).merge(Milestone.not_started)
+    else
+      items.without_particular_milestone(not_params[:milestone_title])
+    end
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
+
   def by_release(items)
     return items unless params.releases?
 
@@ -394,6 +439,12 @@ class IssuableFinder
     end
   end
 
+  def by_negated_release(items)
+    return items unless not_params.releases?
+
+    items.without_particular_release(not_params[:release_tag], not_params[:project_id])
+  end
+
   def by_label(items)
     return items unless params.labels?
 
@@ -402,10 +453,16 @@ class IssuableFinder
     elsif params.filter_by_any_label?
       items.any_label
     else
-      items.with_label(params.label_names, params[:sort], not_query: not_query?)
+      items.with_label(params.label_names, params[:sort])
     end
   end
 
+  def by_negated_label(items)
+    return items unless not_params.labels?
+
+    items.without_particular_labels(not_params.label_names)
+  end
+
   def by_my_reaction_emoji(items)
     return items unless params[:my_reaction_emoji] && current_user
 
@@ -418,11 +475,13 @@ class IssuableFinder
     end
   end
 
-  def by_non_archived(items)
-    params[:non_archived].present? ? items.non_archived : items
+  def by_negated_my_reaction_emoji(items)
+    return items unless not_params[:my_reaction_emoji] && current_user
+
+    items.not_awarded(current_user, not_params[:my_reaction_emoji])
   end
 
-  def not_query?
-    !!params[:not_query]
+  def by_non_archived(items)
+    params[:non_archived].present? ? items.non_archived : items
   end
 end

app/finders/issuable_finder/params.rb

@@ -132,6 +132,8 @@ class IssuableFinder
 
     def project
       strong_memoize(:project) do
+        next nil unless params[:project_id].present?
+
         project = Project.find(params[:project_id])
         project = nil unless Ability.allowed?(current_user, :"read_#{klass.to_ability_name}", project)
 

app/finders/issues_finder/params.rb

@@ -50,4 +50,4 @@ class IssuesFinder
   end
 end
 
-IssuableFinder::Params.prepend_if_ee('EE::IssuesFinder::Params')
+IssuesFinder::Params.prepend_if_ee('EE::IssuesFinder::Params')

app/models/concerns/awardable.rb

@@ -14,32 +14,29 @@ module Awardable
 
   class_methods do
     def awarded(user, name = nil)
-      sql = <<~EOL
-        EXISTS (
-          SELECT TRUE
-          FROM award_emoji
-          WHERE user_id = :user_id AND
-                #{"name = :name AND" if name.present?}
-                awardable_type = :awardable_type AND
-                awardable_id = #{self.arel_table.name}.id
-        )
-      EOL
+      award_emoji_table = Arel::Table.new('award_emoji')
+      inner_query = award_emoji_table
+                .project('true')
+                .where(award_emoji_table[:user_id].eq(user.id))
+                .where(award_emoji_table[:awardable_type].eq(self.name))
+                .where(award_emoji_table[:awardable_id].eq(self.arel_table[:id]))
+
+      inner_query = inner_query.where(award_emoji_table[:name].eq(name)) if name.present?
 
-      where(sql, user_id: user.id, name: name, awardable_type: self.name)
+      where(inner_query.exists)
     end
 
-    def not_awarded(user)
-      sql = <<~EOL
-        NOT EXISTS (
-          SELECT TRUE
-          FROM award_emoji
-          WHERE user_id = :user_id AND
-                awardable_type = :awardable_type AND
-                awardable_id = #{self.arel_table.name}.id
-        )
-      EOL
+    def not_awarded(user, name = nil)
+      award_emoji_table = Arel::Table.new('award_emoji')
+      inner_query = award_emoji_table
+                .project('true')
+                .where(award_emoji_table[:user_id].eq(user.id))
+                .where(award_emoji_table[:awardable_type].eq(self.name))
+                .where(award_emoji_table[:awardable_id].eq(self.arel_table[:id]))
+
+      inner_query = inner_query.where(award_emoji_table[:name].eq(name)) if name.present?
 
-      where(sql, user_id: user.id, awardable_type: self.name)
+      where(inner_query.exists.not)
     end
 
     def order_upvotes_desc

app/models/concerns/issuable.rb

@@ -115,9 +115,31 @@ module Issuable
     end
     # rubocop:enable GitlabSecurity/SqlInjection
 
+    scope :not_assigned_to, ->(users) do
+      assignees_table = Arel::Table.new("#{to_ability_name}_assignees")
+      sql = assignees_table.project('true')
+                .where(assignees_table[:user_id].in(users))
+                .where(Arel::Nodes::SqlLiteral.new("#{to_ability_name}_id = #{to_ability_name}s.id"))
+      where(sql.exists.not)
+    end
+
+    scope :without_particular_labels, ->(label_names) do
+      labels_table = Label.arel_table
+      label_links_table = LabelLink.arel_table
+      issuables_table = klass.arel_table
+      inner_query = label_links_table.project('true')
+                        .join(labels_table, Arel::Nodes::InnerJoin).on(labels_table[:id].eq(label_links_table[:label_id]))
+                        .where(label_links_table[:target_type].eq(name)
+                                   .and(label_links_table[:target_id].eq(issuables_table[:id]))
+                                   .and(labels_table[:title].in(label_names)))
+                        .exists.not
+
+      where(inner_query)
+    end
+
     scope :without_label, -> { joins("LEFT OUTER JOIN label_links ON label_links.target_type = '#{name}' AND label_links.target_id = #{table_name}.id").where(label_links: { id: nil }) }
     scope :with_label_ids, ->(label_ids) { joins(:label_links).where(label_links: { label_id: label_ids }) }
-    scope :any_label, -> { joins(:label_links).group(:id) }
+    scope :any_label, -> { joins(:label_links).distinct }
     scope :join_project, -> { joins(:project) }
     scope :inc_notes_with_associations, -> { includes(notes: [:project, :author, :award_emoji]) }
     scope :references_project, -> { references(:project) }
@@ -286,9 +308,8 @@ module Issuable
         .reorder(Gitlab::Database.nulls_last_order('highest_priority', direction))
     end
 
-    def with_label(title, sort = nil, not_query: false)
-      multiple_labels = title.is_a?(Array) && title.size > 1
-      if multiple_labels && !not_query
+    def with_label(title, sort = nil)
+      if title.is_a?(Array) && title.size > 1
         joins(:labels).where(labels: { title: title }).group(*grouping_columns(sort)).having("COUNT(DISTINCT labels.title) = #{title.size}")
       else
         joins(:labels).where(labels: { title: title })

app/models/concerns/milestoneable.rb

@@ -17,8 +17,10 @@ module Milestoneable
     scope :of_milestones, ->(ids) { where(milestone_id: ids) }
     scope :any_milestone, -> { where('milestone_id IS NOT NULL') }
     scope :with_milestone, ->(title) { left_joins_milestones.where(milestones: { title: title }) }
+    scope :without_particular_milestone, ->(title) { left_outer_joins(:milestone).where("milestones.title != ? OR milestone_id IS NULL", title) }
     scope :any_release, -> { joins_milestone_releases }
     scope :with_release, -> (tag, project_id) { joins_milestone_releases.where( milestones: { releases: { tag: tag, project_id: project_id } } ) }
+    scope :without_particular_release, -> (tag, project_id) { joins_milestone_releases.where.not( milestones: { releases: { tag: tag, project_id: project_id } } ) }
 
     scope :left_joins_milestones,    -> { joins("LEFT OUTER JOIN milestones ON #{table_name}.milestone_id = milestones.id") }
     scope :order_milestone_due_desc, -> { left_joins_milestones.reorder(Arel.sql('milestones.due_date IS NULL, milestones.id IS NULL, milestones.due_date DESC')) }

app/models/milestone.rb

@@ -19,6 +19,12 @@ class Milestone < ApplicationRecord
   has_many :events, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
 
   scope :started, -> { active.where('milestones.start_date <= CURRENT_DATE') }
+  scope :not_started, -> { active.where('milestones.start_date > CURRENT_DATE') }
+  scope :not_upcoming, -> do
+    active
+        .where('milestones.due_date <= CURRENT_DATE')
+        .order(:project_id, :group_id, :due_date)
+  end
 
   scope :order_by_name_asc, -> { order(Arel::Nodes::Ascending.new(arel_table[:title].lower)) }
   scope :reorder_by_due_date_asc, -> { reorder(Gitlab::Database.nulls_last_order('due_date', 'ASC')) }

changelogs/unreleased/198324-avoid_subqueries.yml

+---
+title: Speed up NOT Issue filters
+merge_request: 27639
+author:
+type: performance

ee/app/finders/ee/issues_finder.rb

@@ -39,7 +39,7 @@ module EE
 
     override :by_assignee
     def by_assignee(items)
-      if params.assignees.any? && !not_query?
+      if params.assignees.any?
         params.assignees.each do |assignee|
           items = items.assigned_to(assignee)
         end

spec/controllers/concerns/issuable_actions_spec.rb

@@ -14,7 +14,7 @@ describe IssuableActions do
     klass = Class.new do
       attr_reader :current_user, :project, :issuable
 
-      def self.before_action(action, params = nil)
+      def self.before_action(action = nil, params = nil)
       end
 
       include IssuableActions

spec/features/issues/filtered_search/filter_issues_spec.rb

@@ -81,6 +81,26 @@ describe 'Filter issues', :js do
     expect_filtered_search_input(search_term)
   end
 
+  context 'with the NOT queries feature flag disabled' do
+    before do
+      stub_feature_flags(not_issuable_queries: false)
+      visit project_issues_path(project)
+    end
+
+    it 'does not have the != option' do
+      input_filtered_search("label:", submit: false)
+
+      wait_for_requests
+      within('#js-dropdown-operator') do
+        tokens = all(:css, 'li.filter-dropdown-item')
+        expect(tokens.count).to eq(1)
+        button = tokens[0].find('button')
+        expect(button).to have_content('=')
+        expect(button).not_to have_content('!=')
+      end
+    end
+  end
+
   describe 'filter issues by author' do
     context 'only author' do
       it 'filters issues by searched author' do
@@ -153,16 +173,16 @@ describe 'Filter issues', :js do
         expect_filtered_search_input_empty
       end
 
-      it 'filters issues by no label' do
-        input_filtered_search('label:=none')
+      it 'filters issues by any label' do
+        input_filtered_search('label:=any')
 
-        expect_tokens([label_token('None', false)])
+        expect_tokens([label_token('Any', false)])
         expect_issues_list_count(4)
         expect_filtered_search_input_empty
       end
 
       it 'filters issues by no label' do
-        input_filtered_search('label:!=none')
+        input_filtered_search('label:=none')
 
         expect_tokens([label_token('None', false)])
         expect_issues_list_count(4)
@@ -351,14 +371,6 @@ describe 'Filter issues', :js do
         expect_filtered_search_input_empty
       end
 
-      it 'filters issues by negation of no milestone' do
-        input_filtered_search("milestone:!=none ")
-
-        expect_tokens([milestone_token('None', false, '!=')])
-        expect_issues_list_count(5)
-        expect_filtered_search_input_empty
-      end
-
       it 'filters issues by upcoming milestones' do
         create(:milestone, project: project, due_date: 1.month.from_now) do |future_milestone|
           create(:issue, project: project, milestone: future_milestone, author: user)
@@ -376,10 +388,14 @@ describe 'Filter issues', :js do
           create(:issue, project: project, milestone: future_milestone, author: user)
         end
 
+        create(:milestone, project: project, due_date: 3.days.ago) do |past_milestone|
+          create(:issue, project: project, milestone: past_milestone, author: user)
+        end
+
         input_filtered_search("milestone:!=upcoming")
 
         expect_tokens([milestone_token('Upcoming', false, '!=')])
-        expect_issues_list_count(8)
+        expect_issues_list_count(1)
         expect_filtered_search_input_empty
       end
 
@@ -392,10 +408,13 @@ describe 'Filter issues', :js do
       end
 
       it 'filters issues by negation of started milestones' do
+        milestone2 = create(:milestone, title: "9", project: project, start_date: 2.weeks.from_now)
+        create(:issue, project: project, author: user, title: "something else", milestone: milestone2)
+
         input_filtered_search("milestone:!=started")
 
         expect_tokens([milestone_token('Started', false, '!=')])
-        expect_issues_list_count(3)
+        expect_issues_list_count(1)
         expect_filtered_search_input_empty
       end
 

spec/features/issues/filtered_search/visual_tokens_spec.rb

@@ -175,4 +175,20 @@ describe 'Visual tokens', :js do
     expect(token.find('.name').text).to eq('Label')
     expect(token.find('.operator').text).to eq('=')
   end
+
+  describe 'Any/None option' do
+    it 'hidden when NOT operator is selected' do
+      input_filtered_search('milestone:!=', extra_space: false, submit: false)
+
+      expect(page).not_to have_selector("#js-dropdown-milestone", text: 'Any')
+      expect(page).not_to have_selector("#js-dropdown-milestone", text: 'None')
+    end
+
+    it 'shown when EQUAL operator is selected' do
+      input_filtered_search('milestone:=', extra_space: false, submit: false)
+
+      expect(page).to have_selector("#js-dropdown-milestone", text: 'Any')
+      expect(page).to have_selector("#js-dropdown-milestone", text: 'None')
+    end
+  end
 end

spec/finders/issues_finder_spec.rb

@@ -132,26 +132,6 @@ describe IssuesFinder do
         end
       end
 
-      context 'filtering by NOT group_id' do
-        let(:params) { { not: { group_id: group.id } } }
-
-        context 'when include_subgroup param not set' do
-          it 'returns all other group issues' do
-            expect(issues).to contain_exactly(issue2, issue3, issue4)
-          end
-        end
-
-        context 'when include_subgroup param is true', :nested_groups do
-          before do
-            params[:include_subgroups] = true
-          end
-
-          it 'returns all other group and subgroup issues' do
-            expect(issues).to contain_exactly(issue2, issue3)
-          end
-        end
-      end
-
       context 'filtering by author ID' do
         let(:params) { { author_id: user2.id } }
 
@@ -292,12 +272,12 @@ describe IssuesFinder do
         context 'using NOT' do
           let(:params) { { not: { milestone_title: Milestone::Upcoming.name } } }
 
-          it 'returns issues not in upcoming milestones for each project or group' do
-            target_issues = @created_issues.reject do |issue|
-              issue.milestone&.due_date && issue.milestone.due_date > Date.current
-            end + @created_issues.select { |issue| issue.milestone&.title == '8.9' }
+          it 'returns issues not in upcoming milestones for each project or group, but must have a due date' do
+            target_issues = @created_issues.select do |issue|
+              issue.milestone&.due_date && issue.milestone.due_date <= Date.current
+            end
 
-            expect(issues).to contain_exactly(issue1, issue2, issue3, issue4, *target_issues)
+            expect(issues).to contain_exactly(*target_issues)
           end
         end
       end
@@ -343,9 +323,9 @@ describe IssuesFinder do
           let(:params) { { not: { milestone_title: Milestone::Started.name } } }
 
           it 'returns issues not in the started milestones for each project' do
-            target_issues = Issue.where.not(milestone: Milestone.started)
+            target_issues = Issue.where(milestone: Milestone.not_started)
 
-            expect(issues).to contain_exactly(issue2, issue3, issue4, *target_issues)
+            expect(issues).to contain_exactly(*target_issues)
           end
         end
       end
@@ -452,14 +432,6 @@ describe IssuesFinder do
         it 'returns issues with title and description match for search term' do
           expect(issues).to contain_exactly(issue1, issue2)
         end
-
-        context 'using NOT' do
-          let(:params) { { not: { search: 'git' } } }
-
-          it 'returns issues with no title and description match for search term' do
-            expect(issues).to contain_exactly(issue3, issue4)
-          end
-        end
       end
 
       context 'filtering by issue term in title' do
@@ -468,14 +440,6 @@ describe IssuesFinder do
         it 'returns issues with title match for search term' do
           expect(issues).to contain_exactly(issue1)
         end
-
-        context 'using NOT' do
-          let(:params) { { not: { search: 'git', in: 'title' } } }
-
-          it 'returns issues with no title match for search term' do
-            expect(issues).to contain_exactly(issue2, issue3, issue4)
-          end
-        end
       end
 
       context 'filtering by issues iids' do