Merge branch '31914-support-multiple-query-on-todos-finder-pd' into 'master'

Support query for multiple groups

See merge request gitlab-org/gitlab!18487

app/finders/todos_finder.rb

@@ -180,11 +180,9 @@ class TodosFinder
   end
 
   def by_group(items)
-    if group?
-      items.for_group_and_descendants(group)
-    else
-      items
-    end
+    return items unless group?
+
+    items.for_group_ids_and_descendants(params[:group_id])
   end
 
   def by_state(items)

app/models/group.rb

@@ -473,6 +473,12 @@ class Group < Namespace
 
     errors.add(:visibility_level, "#{visibility} is not allowed since there are sub-groups with higher visibility.")
   end
+
+  def self.groups_including_descendants_by(group_ids)
+    Gitlab::ObjectHierarchy
+      .new(Group.where(id: group_ids))
+      .base_and_descendants
+  end
 end
 
 Group.prepend_if_ee('EE::Group')

app/models/todo.rb

@@ -75,13 +75,13 @@ class Todo < ApplicationRecord
   after_save :keep_around_commit, if: :commit_id
 
   class << self
-    # Returns all todos for the given group and its descendants.
+    # Returns all todos for the given group ids and their descendants.
     #
-    # group - A `Group` to retrieve todos for.
+    # group_ids - Group Ids to retrieve todos for.
     #
     # Returns an `ActiveRecord::Relation`.
-    def for_group_and_descendants(group)
-      groups = group.self_and_descendants
+    def for_group_ids_and_descendants(group_ids)
+      groups = Group.groups_including_descendants_by(group_ids)
 
       from_union([
         for_project(Project.for_group(groups)),

spec/finders/todos_finder_spec.rb

@@ -97,14 +97,39 @@ describe TodosFinder do
           end
         end
 
-        context 'with subgroups' do
-          let(:subgroup) { create(:group, parent: group) }
-          let!(:todo3) { create(:todo, user: user, group: subgroup, target: issue) }
+        context 'by groups' do
+          context 'with subgroups' do
+            let(:subgroup) { create(:group, parent: group) }
+            let!(:todo3) { create(:todo, user: user, group: subgroup, target: issue) }
 
-          it 'returns todos from subgroups when filtered by a group' do
-            todos = finder.new(user, { group_id: group.id }).execute
+            it 'returns todos from subgroups when filtered by a group' do
+              todos = finder.new(user, { group_id: group.id }).execute
 
-            expect(todos).to match_array([todo1, todo2, todo3])
+              expect(todos).to match_array([todo1, todo2, todo3])
+            end
+          end
+
+          context 'filtering for multiple groups' do
+            let_it_be(:group2) { create(:group) }
+            let_it_be(:group3) { create(:group) }
+
+            let!(:todo1) { create(:todo, user: user, project: project, target: issue) }
+            let!(:todo2) { create(:todo, user: user, group: group, target: merge_request) }
+            let!(:todo3) { create(:todo, user: user, group: group2, target: merge_request) }
+
+            let(:subgroup1) { create(:group, parent: group) }
+            let!(:todo4) { create(:todo, user: user, group: subgroup1, target: issue) }
+
+            let(:subgroup2) { create(:group, parent: group2) }
+            let!(:todo5) { create(:todo, user: user, group: subgroup2, target: issue) }
+
+            let!(:todo6) { create(:todo, user: user, group: group3, target: issue) }
+
+            it 'returns the expected groups' do
+              todos = finder.new(user, { group_id: [group.id, group2.id] }).execute
+
+              expect(todos).to match_array([todo1, todo2, todo3, todo4, todo5])
+            end
           end
         end
       end

spec/models/group_spec.rb

@@ -1042,4 +1042,21 @@ describe Group do
       expect(group.access_request_approvers_to_be_notified).to eq(active_owners_in_recent_sign_in_desc_order)
     end
   end
+
+  describe '.groups_including_descendants_by' do
+    it 'returns the expected groups for a group and its descendants' do
+      parent_group1 = create(:group)
+      child_group1 = create(:group, parent: parent_group1)
+      child_group2 = create(:group, parent: parent_group1)
+
+      parent_group2 = create(:group)
+      child_group3 = create(:group, parent: parent_group2)
+
+      create(:group)
+
+      groups = described_class.groups_including_descendants_by([parent_group2.id, parent_group1.id])
+
+      expect(groups).to contain_exactly(parent_group1, parent_group2, child_group1, child_group2, child_group3)
+    end
+  end
 end

spec/models/todo_spec.rb

@@ -253,14 +253,14 @@ describe Todo do
     end
   end
 
-  describe '.for_group_and_descendants' do
+  describe '.for_group_ids_and_descendants' do
     it 'returns the todos for a group and its descendants' do
       parent_group = create(:group)
       child_group = create(:group, parent: parent_group)
 
       todo1 = create(:todo, group: parent_group)
       todo2 = create(:todo, group: child_group)
-      todos = described_class.for_group_and_descendants(parent_group)
+      todos = described_class.for_group_ids_and_descendants([parent_group.id])
 
       expect(todos).to contain_exactly(todo1, todo2)
     end