Merge branch...

Merge branch '24779-super-group-milestone-view-doesn-t-include-milestones-from-projects-in-subgroups' into 'master'

Include milestones from subgroups in Milestones view

Closes #24779

See merge request gitlab-org/gitlab!22922

app/controllers/groups/application_controller.rb

@@ -20,6 +20,14 @@ class Groups::ApplicationController < ApplicationController
     @projects ||= GroupProjectsFinder.new(group: group, current_user: current_user).execute
   end
 
+  def group_projects_with_subgroups
+    @group_projects_with_subgroups ||= GroupProjectsFinder.new(
+      group: group,
+      current_user: current_user,
+      options: { include_subgroups: true }
+    ).execute
+  end
+
   def authorize_admin_group!
     unless can?(current_user, :admin_group, group)
       return render_404

app/controllers/groups/milestones_controller.rb

@@ -103,8 +103,15 @@ class Groups::MilestonesController < Groups::ApplicationController
   end
 
   def group_projects_with_access
-    group_projects.with_issues_available_for_user(current_user)
-      .or(group_projects.with_merge_requests_available_for_user(current_user))
+    group_projects_with_subgroups.with_issues_or_mrs_available_for_user(current_user)
+  end
+
+  def group_ids(include_ancestors: false)
+    if include_ancestors
+      group.self_and_hierarchy.public_or_visible_to_user(current_user).select(:id)
+    else
+      group.self_and_descendants.public_or_visible_to_user(current_user).select(:id)
+    end
   end
 
   def milestone
@@ -119,7 +126,7 @@ class Groups::MilestonesController < Groups::ApplicationController
   end
 
   def search_params
-    groups = request.format.json? ? group.self_and_ancestors.select(:id) : group.id
+    groups = request.format.json? ? group_ids(include_ancestors: true) : group_ids
 
     params.permit(:state, :search_title).merge(group_ids: groups)
   end

app/models/project.rb

@@ -453,6 +453,9 @@ class Project < ApplicationRecord
   scope :with_issues_enabled, -> { with_feature_enabled(:issues) }
   scope :with_issues_available_for_user, ->(current_user) { with_feature_available_for_user(:issues, current_user) }
   scope :with_merge_requests_available_for_user, ->(current_user) { with_feature_available_for_user(:merge_requests, current_user) }
+  scope :with_issues_or_mrs_available_for_user, -> (user) do
+    with_issues_available_for_user(user).or(with_merge_requests_available_for_user(user))
+  end
   scope :with_merge_requests_enabled, -> { with_feature_enabled(:merge_requests) }
   scope :with_remote_mirrors, -> { joins(:remote_mirrors).where(remote_mirrors: { enabled: true }).distinct }
   scope :with_limit, -> (maximum) { limit(maximum) }

changelogs/unreleased/24779-super-group-milestone-view-doesn-t-include-milestones-from-projects.yml

+---
+title: Include milestones from subgroups in the list of Group Milestones.
+merge_request: 22922
+author:
+type: fixed

spec/controllers/groups/milestones_controller_spec.rb

@@ -130,6 +130,40 @@ describe Groups::MilestonesController do
           end
         end
       end
+
+      context 'when subgroup milestones are present' do
+        let(:subgroup) { create(:group, :private, parent: group) }
+        let(:sub_project) { create(:project, :private, group: subgroup) }
+        let!(:group_milestone) { create(:milestone, group: group, title: 'Group milestone') }
+        let!(:sub_project_milestone) { create(:milestone, project: sub_project, title: 'Sub Project Milestone') }
+        let!(:subgroup_milestone) { create(:milestone, title: 'Subgroup Milestone', group: subgroup) }
+
+        it 'shows subgroup milestones that user has access to' do
+          get :index, params: { group_id: group.to_param }
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(response.body).to include(group_milestone.title)
+          expect(response.body).to include(sub_project_milestone.title)
+          expect(response.body).to include(subgroup_milestone.title)
+        end
+
+        context 'when user has no access to subgroups' do
+          let(:non_member) { create(:user) }
+
+          before do
+            sign_in(non_member)
+          end
+
+          it 'does not show subgroup milestones' do
+            get :index, params: { group_id: group.to_param }
+
+            expect(response).to have_gitlab_http_status(200)
+            expect(response.body).to include(group_milestone.title)
+            expect(response.body).not_to include(sub_project_milestone.title)
+            expect(response.body).not_to include(subgroup_milestone.title)
+          end
+        end
+      end
     end
 
     context 'as JSON' do
@@ -149,6 +183,19 @@ describe Groups::MilestonesController do
         expect(response.content_type).to eq 'application/json'
       end
 
+      context 'with subgroup milestones' do
+        it 'lists descendants group milestones' do
+          subgroup = create(:group, :public, parent: group)
+          create(:milestone, group: subgroup, title: 'subgroup milestone')
+
+          get :index, params: { group_id: group.to_param }, format: :json
+          milestones = json_response
+
+          expect(milestones.count).to eq(3)
+          expect(milestones.second["title"]).to eq("subgroup milestone")
+        end
+      end
+
       context 'for a subgroup' do
         let(:subgroup) { create(:group, parent: group) }
 

spec/models/project_spec.rb

@@ -5587,6 +5587,25 @@ describe Project do
     end
   end
 
+  describe 'with_issues_or_mrs_available_for_user' do
+    before do
+      Project.delete_all
+    end
+
+    it 'returns correct projects' do
+      user = create(:user)
+      project1 = create(:project, :public, :merge_requests_disabled, :issues_enabled)
+      project2 = create(:project, :public, :merge_requests_disabled, :issues_disabled)
+      project3 = create(:project, :public, :issues_enabled, :merge_requests_enabled)
+      project4 = create(:project, :private, :issues_private, :merge_requests_private)
+
+      [project1, project2, project3, project4].each { |project| project.add_developer(user) }
+
+      expect(described_class.with_issues_or_mrs_available_for_user(user))
+        .to contain_exactly(project1, project3, project4)
+    end
+  end
+
   def rugged_config
     rugged_repo(project.repository).config
   end