diff --git a/ee/app/finders/epics_finder.rb b/ee/app/finders/epics_finder.rb index 5ba31aba9384605eb7f0a77116fe271e0edbe2ae..e5b437c364311100b9f8a347034716cd6e154305 100644 --- a/ee/app/finders/epics_finder.rb +++ b/ee/app/finders/epics_finder.rb @@ -50,7 +50,9 @@ def klass Epic end - def execute + def execute(skip_visibility_check: false) + @skip_visibility_check = skip_visibility_check + raise ArgumentError, 'group_id argument is missing' unless params[:group_id] return Epic.none unless Ability.allowed?(current_user, :read_epic, group) @@ -194,6 +196,7 @@ def groups_with_confidential_access(groups) end def can_read_all_epics_in_related_groups?(groups) + return true if skip_visibility_check? return false unless current_user # If a user is a member of a group, he also inherits access to all subgroups, @@ -209,4 +212,8 @@ def can_read_all_epics_in_related_groups?(groups) parent = params.fetch(:include_ancestor_groups, false) ? groups.first.root_ancestor : group Ability.allowed?(current_user, :read_confidential_epic, parent) end + + def skip_visibility_check? + @skip_visibility_check && Feature.enabled?(:skip_epic_count_visibility_check, group, default_enabled: true) + end end diff --git a/ee/app/helpers/ee/groups_helper.rb b/ee/app/helpers/ee/groups_helper.rb index 8cf314e0c6d46854c15791a9b62006f0737f770d..d319cfa6eb6006ad88680583669a48b49fe25db9 100644 --- a/ee/app/helpers/ee/groups_helper.rb +++ b/ee/app/helpers/ee/groups_helper.rb @@ -7,7 +7,7 @@ module GroupsHelper def group_epics_count(state:) EpicsFinder .new(current_user, group_id: @group.id, state: state) - .execute + .execute(skip_visibility_check: true) .count end diff --git a/ee/changelogs/unreleased/epics_count_all.yml b/ee/changelogs/unreleased/epics_count_all.yml new file mode 100644 index 0000000000000000000000000000000000000000..121c2abe99acab3107df3cee2a8487d515b8f254 --- /dev/null +++ b/ee/changelogs/unreleased/epics_count_all.yml @@ -0,0 +1,5 @@ +--- +title: Show count of all epics on group page no matter if user can see them or not. +merge_request: 35129 +author: +type: performance diff --git a/ee/spec/finders/epics_finder_spec.rb b/ee/spec/finders/epics_finder_spec.rb index a8a608c3868147342372323fa2797ed8a9955cd8..20bfd7a035279ef1303a29fa7c6e8b7677b51313 100644 --- a/ee/spec/finders/epics_finder_spec.rb +++ b/ee/spec/finders/epics_finder_spec.rb @@ -308,13 +308,32 @@ def epics(params = {}) let_it_be(:public_group1) { create(:group, :public, parent: base_group) } let_it_be(:public_epic1) { create(:epic, group: public_group1) } let_it_be(:public_epic2) { create(:epic, :confidential, group: public_group1) } + let(:execute_params) { {} } - subject { described_class.new(search_user, group_id: base_group.id).execute } + subject { described_class.new(search_user, group_id: base_group.id).execute(execute_params) } it 'returns only public epics' do expect(subject).to match_array([base_epic2, public_epic1]) end + context 'when skip_visibility_check is true' do + let(:execute_params) { { skip_visibility_check: true } } + + it 'returns all epics' do + expect(subject).to match_array([base_epic1, base_epic2, private_epic1, private_epic2, public_epic1, public_epic2]) + end + + context 'when skip_epic_count_visibility_check is disabled' do + before do + stub_feature_flags(skip_epic_count_visibility_check: false) + end + + it 'returns only public epics' do + expect(subject).to match_array([base_epic2, public_epic1]) + end + end + end + context 'when user is member of ancestor group' do before do ancestor_group.add_developer(search_user) diff --git a/ee/spec/helpers/ee/groups_helper_spec.rb b/ee/spec/helpers/ee/groups_helper_spec.rb index d0ab20f7fcc14bb4524ebdcb1fcd05df127db1fd..85468acdb04d92aa010b71ab41dc999402026640 100644 --- a/ee/spec/helpers/ee/groups_helper_spec.rb +++ b/ee/spec/helpers/ee/groups_helper_spec.rb @@ -19,17 +19,33 @@ describe '#group_epics_count' do before do stub_licensed_features(epics: true) - - create_list(:epic, 3, :opened, group: group) - create_list(:epic, 2, :closed, group: group) end - it 'returns open epics count' do - expect(helper.group_epics_count(state: 'opened')).to eq(3) + describe 'filtering by state' do + before do + create_list(:epic, 3, :opened, group: group) + create_list(:epic, 2, :closed, group: group) + end + + it 'returns open epics count' do + expect(helper.group_epics_count(state: 'opened')).to eq(3) + end + + it 'returns closed epics count' do + expect(helper.group_epics_count(state: 'closed')).to eq(2) + end end - it 'returns closed epics count' do - expect(helper.group_epics_count(state: 'closed')).to eq(2) + it 'counts also epics from subgroups not visible to user' do + parent_group = create(:group, :public) + subgroup = create(:group, :private, parent: parent_group) + create(:epic, :opened, group: parent_group) + create(:epic, :opened, group: subgroup) + helper.instance_variable_set(:@group, parent_group) + + expect(Ability.allowed?(owner, :read_epic, parent_group)).to be_truthy + expect(Ability.allowed?(owner, :read_epic, subgroup)).to be_falsey + expect(helper.group_epics_count(state: 'opened')).to eq(2) end end