Merge branch '25413-group-dashboard-reveals-number-of-projects-and-subgroups' into 'master'

Group dashboard reveals number of projects and subgroups

See merge request gitlab-org/gitlab!67867

app/assets/javascripts/groups/components/item_stats.vue

@@ -40,24 +40,31 @@ export default {
       return this.item.type === ITEM_TYPE.GROUP;
     },
   },
+  methods: {
+    displayValue(value) {
+      return this.isGroup && value !== undefined;
+    },
+  },
 };
 </script>
 
 <template>
   <div class="stats gl-text-gray-500">
     <item-stats-value
-      v-if="isGroup"
+      v-if="displayValue(item.subgroupCount)"
       :title="__('Subgroups')"
       :value="item.subgroupCount"
       css-class="number-subgroups gl-ml-5"
       icon-name="folder-o"
+      data-testid="subgroups-count"
     />
     <item-stats-value
-      v-if="isGroup"
+      v-if="displayValue(item.projectCount)"
       :title="__('Projects')"
       :value="item.projectCount"
       css-class="number-projects gl-ml-5"
       icon-name="bookmark"
+      data-testid="projects-count"
     />
     <item-stats-value
       v-if="isGroup"

app/policies/group_policy.rb

@@ -111,8 +111,11 @@ class GroupPolicy < BasePolicy
     enable :read_issue_board
     enable :read_group_member
     enable :read_custom_emoji
+    enable :read_counts
   end
 
+  rule { ~public_group & ~has_access }.prevent :read_counts
+
   rule { ~can?(:read_group) }.policy do
     prevent :read_design_activity
   end

app/serializers/group_child_entity.rb

@@ -37,9 +37,13 @@ class GroupChildEntity < Grape::Entity
          if: lambda { |_instance, _options| project? }
 
   # Group only attributes
-  expose :children_count, :parent_id, :project_count, :subgroup_count,
+  expose :children_count, :parent_id,
          unless: lambda { |_instance, _options| project? }
 
+  expose :subgroup_count, if: lambda { |group| access_group_counts?(group) }
+
+  expose :project_count, if: lambda { |group| access_group_counts?(group) }
+
   expose :leave_path, unless: lambda { |_instance, _options| project? } do |instance|
     leave_group_members_path(instance)
   end
@@ -52,10 +56,6 @@ class GroupChildEntity < Grape::Entity
     end
   end
 
-  expose :number_projects_with_delimiter, unless: lambda { |_instance, _options| project? } do |instance|
-    number_with_delimiter(instance.project_count)
-  end
-
   expose :number_users_with_delimiter, unless: lambda { |_instance, _options| project? } do |instance|
     number_with_delimiter(instance.member_count)
   end
@@ -66,6 +66,10 @@ class GroupChildEntity < Grape::Entity
 
   private
 
+  def access_group_counts?(group)
+    !project? && can?(request.current_user, :read_counts, group)
+  end
+
   # rubocop: disable CodeReuse/ActiveRecord
   def membership
     return unless request.current_user

app/serializers/group_entity.rb

@@ -40,10 +40,6 @@ class GroupEntity < Grape::Entity
     GroupsFinder.new(request.current_user, parent: group).execute.any?
   end
 
-  expose :number_projects_with_delimiter do |group|
-    number_with_delimiter(GroupProjectsFinder.new(group: group, current_user: request.current_user).execute.count)
-  end
-
   expose :number_users_with_delimiter do |group|
     number_with_delimiter(group.users.count)
   end

spec/controllers/groups/children_controller_spec.rb

@@ -227,8 +227,8 @@ RSpec.describe Groups::ChildrenController do
 
         context 'when rendering hierarchies' do
           # When loading hierarchies we load the all the ancestors for matched projects
-          # in 1 separate query
-          let(:extra_queries_for_hierarchies) { 1 }
+          # in 2 separate queries
+          let(:extra_queries_for_hierarchies) { 2 }
 
           def get_filtered_list
             get :index, params: { group_id: group.to_param, filter: 'filter' }, format: :json

spec/frontend/groups/components/item_stats_spec.js

-import { shallowMount } from '@vue/test-utils';
+import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
 import ItemStats from '~/groups/components/item_stats.vue';
 import ItemStatsValue from '~/groups/components/item_stats_value.vue';
 
@@ -12,7 +12,7 @@ describe('ItemStats', () => {
   };
 
   const createComponent = (props = {}) => {
-    wrapper = shallowMount(ItemStats, {
+    wrapper = shallowMountExtended(ItemStats, {
       propsData: { ...defaultProps, ...props },
     });
   };
@@ -46,5 +46,31 @@ describe('ItemStats', () => {
       expect(findItemStatsValue().props('cssClass')).toBe('project-stars');
       expect(wrapper.find('.last-updated').exists()).toBe(true);
     });
+
+    describe('group specific rendering', () => {
+      describe.each`
+        provided | state                 | data
+        ${true}  | ${'displays'}         | ${null}
+        ${false} | ${'does not display'} | ${{ subgroupCount: undefined, projectCount: undefined }}
+      `('when provided = $provided', ({ provided, state, data }) => {
+        beforeEach(() => {
+          const item = {
+            ...mockParentGroupItem,
+            ...data,
+            type: ITEM_TYPE.GROUP,
+          };
+
+          createComponent({ item });
+        });
+
+        it.each`
+          entity         | testId
+          ${'subgroups'} | ${'subgroups-count'}
+          ${'projects'}  | ${'projects-count'}
+        `(`${state} $entity count`, ({ testId }) => {
+          expect(wrapper.findByTestId(testId).exists()).toBe(provided);
+        });
+      });
+    });
   });
 });

spec/policies/group_policy_spec.rb

@@ -11,6 +11,7 @@ RSpec.describe GroupPolicy do
 
     it do
       expect_allowed(:read_group)
+      expect_allowed(:read_counts)
       expect_allowed(*read_group_permissions)
       expect_disallowed(:upload_file)
       expect_disallowed(*reporter_permissions)
@@ -30,6 +31,7 @@ RSpec.describe GroupPolicy do
     end
 
     it { expect_disallowed(:read_group) }
+    it { expect_disallowed(:read_counts) }
     it { expect_disallowed(*read_group_permissions) }
   end
 
@@ -42,6 +44,7 @@ RSpec.describe GroupPolicy do
     end
 
     it { expect_disallowed(:read_group) }
+    it { expect_disallowed(:read_counts) }
     it { expect_disallowed(*read_group_permissions) }
   end
 
@@ -245,6 +248,7 @@ RSpec.describe GroupPolicy do
       let(:current_user) { nil }
 
       it do
+        expect_disallowed(:read_counts)
         expect_disallowed(*read_group_permissions)
         expect_disallowed(*guest_permissions)
         expect_disallowed(*reporter_permissions)
@@ -258,6 +262,7 @@ RSpec.describe GroupPolicy do
       let(:current_user) { guest }
 
       it do
+        expect_allowed(:read_counts)
         expect_allowed(*read_group_permissions)
         expect_allowed(*guest_permissions)
         expect_disallowed(*reporter_permissions)
@@ -271,6 +276,7 @@ RSpec.describe GroupPolicy do
       let(:current_user) { reporter }
 
       it do
+        expect_allowed(:read_counts)
         expect_allowed(*read_group_permissions)
         expect_allowed(*guest_permissions)
         expect_allowed(*reporter_permissions)
@@ -284,6 +290,7 @@ RSpec.describe GroupPolicy do
       let(:current_user) { developer }
 
       it do
+        expect_allowed(:read_counts)
         expect_allowed(*read_group_permissions)
         expect_allowed(*guest_permissions)
         expect_allowed(*reporter_permissions)
@@ -297,6 +304,7 @@ RSpec.describe GroupPolicy do
       let(:current_user) { maintainer }
 
       it do
+        expect_allowed(:read_counts)
         expect_allowed(*read_group_permissions)
         expect_allowed(*guest_permissions)
         expect_allowed(*reporter_permissions)
@@ -310,6 +318,7 @@ RSpec.describe GroupPolicy do
       let(:current_user) { owner }
 
       it do
+        expect_allowed(:read_counts)
         expect_allowed(*read_group_permissions)
         expect_allowed(*guest_permissions)
         expect_allowed(*reporter_permissions)

spec/serializers/group_child_entity_spec.rb

@@ -87,7 +87,7 @@ RSpec.describe GroupChildEntity do
       expect(json[:children_count]).to eq(2)
     end
 
-    %w[children_count leave_path parent_id number_projects_with_delimiter number_users_with_delimiter project_count subgroup_count].each do |attribute|
+    %w[children_count leave_path parent_id number_users_with_delimiter project_count subgroup_count].each do |attribute|
       it "includes #{attribute}" do
         expect(json[attribute.to_sym]).to be_present
       end
@@ -114,6 +114,40 @@ RSpec.describe GroupChildEntity do
     it_behaves_like 'group child json'
   end
 
+  describe 'for a private group' do
+    let(:object) do
+      create(:group, :private)
+    end
+
+    describe 'user is member of the group' do
+      before do
+        object.add_owner(user)
+      end
+
+      it 'includes the counts' do
+        expect(json.keys).to include(*%i(project_count subgroup_count))
+      end
+    end
+
+    describe 'user is not a member of the group' do
+      it 'does not include the counts' do
+        expect(json.keys).not_to include(*%i(project_count subgroup_count))
+      end
+    end
+
+    describe 'user is only a member of a project in the group' do
+      let(:project) { create(:project, namespace: object) }
+
+      before do
+        project.add_guest(user)
+      end
+
+      it 'does not include the counts' do
+        expect(json.keys).not_to include(*%i(project_count subgroup_count))
+      end
+    end
+  end
+
   describe 'for a project with external authorization enabled' do
     let(:object) do
       create(:project, :with_avatar,