Merge branch '43510-merge-requests-and-issues-don-t-show-for-all-subgroups' into 'master'

Resolve "Merge requests and issues don't show for all subgroups"

Closes #43510

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

app/controllers/groups/application_controller.rb

@@ -18,10 +18,6 @@ class Groups::ApplicationController < ApplicationController
     @projects ||= GroupProjectsFinder.new(group: group, current_user: current_user).execute
   end
 
-  def group_merge_requests
-    @group_merge_requests = MergeRequestsFinder.new(current_user, group_id: @group.id).execute
-  end
-
   def authorize_admin_group!
     unless can?(current_user, :admin_group, group)
       return render_404

app/controllers/groups_controller.rb

@@ -14,7 +14,6 @@ class GroupsController < Groups::ApplicationController
   before_action :authorize_create_group!, only: [:new]
 
   before_action :group_projects, only: [:projects, :activity, :issues, :merge_requests]
-  before_action :group_merge_requests, only: [:merge_requests]
   before_action :event_filter, only: [:activity]
 
   before_action :user_actions, only: [:show, :subgroups]

app/helpers/groups_helper.rb

@@ -19,6 +19,20 @@ module GroupsHelper
     can?(current_user, :change_share_with_group_lock, group)
   end
 
+  def group_issues_count(state:)
+    IssuesFinder
+      .new(current_user, group_id: @group.id, state: state, non_archived: true, include_subgroups: true)
+      .execute
+      .count
+  end
+
+  def group_merge_requests_count(state:)
+    MergeRequestsFinder
+      .new(current_user, group_id: @group.id, state: state, non_archived: true, include_subgroups: true)
+      .execute
+      .count
+  end
+
   def group_icon(group, options = {})
     img_path = group_icon_url(group, options)
     image_tag img_path, options
@@ -77,10 +91,6 @@ module GroupsHelper
     end
   end
 
-  def group_issues(group)
-    IssuesFinder.new(current_user, group_id: group.id).execute
-  end
-
   def remove_group_message(group)
     _("You are going to remove %{group_name}. Removed groups CANNOT be restored! Are you ABSOLUTELY sure?") %
       { group_name: group.name }

app/views/groups/issues.html.haml

 - page_title "Issues"
-- group_issues_exists = group_issues(@group).exists?
 = content_for :meta_tags do
   = auto_discovery_link_tag(:atom, params.merge(rss_url_options), title: "#{@group.name} issues")
 
 - content_for :page_specific_javascripts do
   = webpack_bundle_tag 'common_vue'
 
-- if group_issues_exists
+- if group_issues_count(state: 'all').zero?
+  = render 'shared/empty_states/issues', project_select_button: true
+- else
   .top-area
     = render 'shared/issuable/nav', type: :issues
     .nav-controls
@@ -19,5 +20,3 @@
   = render 'shared/issuable/search_bar', type: :issues
 
   = render 'shared/issues'
-- else
-  = render 'shared/empty_states/issues', project_select_button: true

app/views/groups/merge_requests.html.haml

@@ -3,7 +3,7 @@
 - content_for :page_specific_javascripts do
   = webpack_bundle_tag 'common_vue'
 
-- if @group_merge_requests.empty?
+- if group_merge_requests_count(state: 'all').zero?
   = render 'shared/empty_states/merge_requests', project_select_button: true
 - else
   .top-area

app/views/layouts/nav/sidebar/_group.html.haml

-- issues_count = IssuesFinder.new(current_user, group_id: @group.id, state: 'opened').execute.count
-- merge_requests_count = MergeRequestsFinder.new(current_user, group_id: @group.id, state: 'opened', non_archived: true).execute.count
-
+- issues_count = group_issues_count(state: 'opened')
+- merge_requests_count = group_merge_requests_count(state: 'opened')
 - issues_sub_menu_items = ['groups#issues', 'labels#index', 'milestones#index']
 
 .nav-sidebar{ class: ("sidebar-collapsed-desktop" if collapsed_sidebar?) }

changelogs/unreleased/43510-merge-requests-and-issues-don-t-show-for-all-subgroups.yml

+---
+title: Ensure group issues and merge requests pages show results from subgroups when
+  there are no results from the current group
+merge_request: 17312
+author:
+type: fixed

spec/features/groups/empty_states_spec.rb

 require 'spec_helper'
 
-feature 'Groups Merge Requests Empty States' do
+feature 'Group empty states' do
   let(:group) { create(:group) }
   let(:user) { create(:group_member, :developer, user: create(:user), group: group ).user }
 
@@ -8,62 +8,100 @@ feature 'Groups Merge Requests Empty States' do
     sign_in(user)
   end
 
-  context 'group has a project' do
-    let(:project) { create(:project, namespace: group) }
+  [:issue, :merge_request].each do |issuable|
+    issuable_name = issuable.to_s.humanize.downcase
+    project_relation = issuable == :issue ? :project : :source_project
 
-    before do
-      project.add_master(user)
-    end
+    context "for #{issuable_name}s" do
+      let(:path) { public_send(:"#{issuable}s_group_path", group) }
 
-    context 'the project has a merge request' do
-      before do
-        create(:merge_request, source_project: project)
+      context 'group has a project' do
+        let(:project) { create(:project, namespace: group) }
 
-        visit merge_requests_group_path(group)
-      end
+        before do
+          project.add_master(user)
+        end
 
-      it 'should not display an empty state' do
-        expect(page).not_to have_selector('.empty-state')
-      end
-    end
+        context "the project has #{issuable_name}s" do
+          before do
+            create(issuable, project_relation => project)
 
-    context 'the project has no merge requests', :js do
-      before do
-        visit merge_requests_group_path(group)
-      end
+            visit path
+          end
 
-      it 'should display an empty state' do
-        expect(page).to have_selector('.empty-state')
-      end
+          it 'does not display an empty state' do
+            expect(page).not_to have_selector('.empty-state')
+          end
+        end
+
+        context "the project has no #{issuable_name}s", :js do
+          before do
+            visit path
+          end
+
+          it 'displays an empty state' do
+            expect(page).to have_selector('.empty-state')
+          end
+
+          it "shows a new #{issuable_name} button" do
+            within '.empty-state' do
+              expect(page).to have_content("create #{issuable_name}")
+            end
+          end
+
+          it "the new #{issuable_name} button opens a project dropdown" do
+            within '.empty-state' do
+              find('.new-project-item-select-button').click
+            end
 
-      it 'should show a new merge request button' do
-        within '.empty-state' do
-          expect(page).to have_content('create merge request')
+            expect(page).to have_selector('.ajax-project-dropdown')
+          end
         end
       end
 
-      it 'the new merge request button opens a project dropdown' do
-        within '.empty-state' do
-          find('.new-project-item-select-button').click
-        end
+      context 'group without a project' do
+        context 'group has a subgroup', :nested_groups do
+          let(:subgroup) { create(:group, parent: group) }
+          let(:subgroup_project) { create(:project, namespace: subgroup) }
 
-        expect(page).to have_selector('.ajax-project-dropdown')
-      end
-    end
-  end
+          context "the project has #{issuable_name}s" do
+            before do
+              create(issuable, project_relation => subgroup_project)
 
-  context 'group without a project' do
-    before do
-      visit merge_requests_group_path(group)
-    end
+              visit path
+            end
 
-    it 'should display an empty state' do
-      expect(page).to have_selector('.empty-state')
-    end
+            it 'does not display an empty state' do
+              expect(page).not_to have_selector('.empty-state')
+            end
+          end
 
-    it 'should not show a new merge request button' do
-      within '.empty-state' do
-        expect(page).not_to have_link('create merge request')
+          context "the project has no #{issuable_name}s" do
+            before do
+              visit path
+            end
+
+            it 'displays an empty state' do
+              expect(page).to have_selector('.empty-state')
+            end
+          end
+        end
+
+        context 'group has no subgroups' do
+          before do
+            visit path
+          end
+
+          it 'displays an empty state' do
+            expect(page).to have_selector('.empty-state')
+          end
+
+          it "shows a new #{issuable_name} button" do
+            within '.empty-state' do
+              expect(page).not_to have_link("create #{issuable_name}")
+            end
+          end
+        end
       end
     end
   end