Merge branch 'revert-open-issues-count' into 'master'

Revert open issues count MR

See merge request !85243

app/finders/issues_finder.rb

@@ -20,7 +20,6 @@
 #     sort: string
 #     my_reaction_emoji: string
 #     public_only: boolean
-#     include_hidden: boolean
 #     due_date: date or '0', '', 'overdue', 'week', or 'month'
 #     created_after: datetime
 #     created_before: datetime
@@ -48,6 +47,8 @@ def params_class
 
   # rubocop: disable CodeReuse/ActiveRecord
   def with_confidentiality_access_check
+    return Issue.all if params.user_can_see_all_issues?
+
     # Only admins can see hidden issues, so for non-admins, we filter out any hidden issues
     issues = Issue.without_hidden
 
@@ -75,9 +76,7 @@ def with_confidentiality_access_check
   private
 
   def init_collection
-    if params.include_hidden?
-      Issue.all
-    elsif params.public_only?
+    if params.public_only?
       Issue.public_only
     else
       with_confidentiality_access_check

app/finders/issues_finder/params.rb

@@ -6,10 +6,6 @@ def public_only?
       params.fetch(:public_only, false)
     end
 
-    def include_hidden?
-      user_can_see_all_issues?
-    end
-
     def filter_by_any_due_date?
       due_date? && params[:due_date] == Issue::AnyDueDate.name
     end

app/models/issue.rb

@@ -142,9 +142,7 @@ def most_recent
   scope :with_issue_type, ->(types) { where(issue_type: types) }
   scope :without_issue_type, ->(types) { where.not(issue_type: types) }
 
-  scope :public_only, -> {
-    without_hidden.where(confidential: false)
-  }
+  scope :public_only, -> { where(confidential: false) }
 
   scope :confidential_only, -> { where(confidential: true) }
 

app/services/groups/open_issues_count_service.rb

@@ -3,15 +3,11 @@
 module Groups
   # Service class for counting and caching the number of open issues of a group.
   class OpenIssuesCountService < Groups::CountService
-    # TOTAL_COUNT_KEY includes confidential and hidden issues (admin)
-    # TOTAL_COUNT_WITHOUT_HIDDEN_KEY includes confidential issues but not hidden issues (reporter and above)
-    # PUBLIC_COUNT_WITHOUT_HIDDEN_KEY does not include confidential or hidden issues (guest)
-    TOTAL_COUNT_KEY = 'group_open_issues_including_hidden_count'
-    TOTAL_COUNT_WITHOUT_HIDDEN_KEY = 'group_open_issues_without_hidden_count'
-    PUBLIC_COUNT_WITHOUT_HIDDEN_KEY = 'group_open_public_issues_without_hidden_count'
+    PUBLIC_COUNT_KEY = 'group_public_open_issues_count'
+    TOTAL_COUNT_KEY = 'group_total_open_issues_count'
 
     def clear_all_cache_keys
-      [cache_key(TOTAL_COUNT_KEY), cache_key(TOTAL_COUNT_WITHOUT_HIDDEN_KEY), cache_key(PUBLIC_COUNT_WITHOUT_HIDDEN_KEY)].each do |key|
+      [cache_key(PUBLIC_COUNT_KEY), cache_key(TOTAL_COUNT_KEY)].each do |key|
         Rails.cache.delete(key)
       end
     end
@@ -19,19 +15,7 @@ def clear_all_cache_keys
     private
 
     def cache_key_name
-      if include_hidden?
-        TOTAL_COUNT_KEY
-      elsif public_only?
-        PUBLIC_COUNT_WITHOUT_HIDDEN_KEY
-      else
-        TOTAL_COUNT_WITHOUT_HIDDEN_KEY
-      end
-    end
-
-    def include_hidden?
-      strong_memoize(:user_is_admin) do
-        user&.can_admin_all_resources?
-      end
+      public_only? ? PUBLIC_COUNT_KEY : TOTAL_COUNT_KEY
     end
 
     def public_only?
@@ -51,8 +35,7 @@ def relation_for_count
         state: 'opened',
         non_archived: true,
         include_subgroups: true,
-        public_only: public_only?,
-        include_hidden: include_hidden?
+        public_only: public_only?
       ).execute
     end
 

app/services/projects/open_issues_count_service.rb

@@ -7,12 +7,8 @@ class OpenIssuesCountService < Projects::CountService
     include Gitlab::Utils::StrongMemoize
 
     # Cache keys used to store issues count
-    # TOTAL_COUNT_KEY includes confidential and hidden issues (admin)
-    # TOTAL_COUNT_WITHOUT_HIDDEN_KEY includes confidential issues but not hidden issues (reporter and above)
-    # PUBLIC_COUNT_WITHOUT_HIDDEN_KEY does not include confidential or hidden issues (guest)
-    TOTAL_COUNT_KEY = 'project_open_issues_including_hidden_count'
-    TOTAL_COUNT_WITHOUT_HIDDEN_KEY = 'project_open_issues_without_hidden_count'
-    PUBLIC_COUNT_WITHOUT_HIDDEN_KEY = 'project_open_public_issues_without_hidden_count'
+    PUBLIC_COUNT_KEY = 'public_open_issues_count'
+    TOTAL_COUNT_KEY = 'total_open_issues_count'
 
     def initialize(project, user = nil)
       @user = user
@@ -20,98 +16,59 @@ def initialize(project, user = nil)
       super(project)
     end
 
-    # rubocop: disable CodeReuse/ActiveRecord
-    def refresh_cache(&block)
-      if block_given?
-        super(&block)
-      else
-        update_cache_for_key(total_count_cache_key) do
-          issues_with_hidden
-        end
-
-        update_cache_for_key(public_count_without_hidden_cache_key) do
-          issues_without_hidden_without_confidential
-        end
-
-        update_cache_for_key(total_count_without_hidden_cache_key) do
-          issues_without_hidden_with_confidential
-        end
-      end
-    end
-
-    private
-
-    def relation_for_count
-      self.class.query(@project, public_only: public_only?, include_hidden: include_hidden?)
-    end
-
     def cache_key_name
-      if include_hidden?
-        TOTAL_COUNT_KEY
-      elsif public_only?
-        PUBLIC_COUNT_WITHOUT_HIDDEN_KEY
-      else
-        TOTAL_COUNT_WITHOUT_HIDDEN_KEY
-      end
-    end
-
-    def include_hidden?
-      user_is_admin?
+      public_only? ? PUBLIC_COUNT_KEY : TOTAL_COUNT_KEY
     end
 
     def public_only?
       !user_is_at_least_reporter?
     end
 
-    def user_is_admin?
-      strong_memoize(:user_is_admin) do
-        @user&.can_admin_all_resources?
-      end
-    end
-
     def user_is_at_least_reporter?
       strong_memoize(:user_is_at_least_reporter) do
         @user && @project.team.member?(@user, Gitlab::Access::REPORTER)
       end
     end
 
-    def total_count_without_hidden_cache_key
-      cache_key(TOTAL_COUNT_WITHOUT_HIDDEN_KEY)
+    def relation_for_count
+      self.class.query(@project, public_only: public_only?)
     end
 
-    def public_count_without_hidden_cache_key
-      cache_key(PUBLIC_COUNT_WITHOUT_HIDDEN_KEY)
+    def public_count_cache_key
+      cache_key(PUBLIC_COUNT_KEY)
     end
 
     def total_count_cache_key
       cache_key(TOTAL_COUNT_KEY)
     end
 
-    def issues_with_hidden
-      self.class.query(@project, public_only: false, include_hidden: true).count
-    end
+    # rubocop: disable CodeReuse/ActiveRecord
+    def refresh_cache(&block)
+      count_grouped_by_confidential = self.class.query(@project, public_only: false).group(:confidential).count
+      public_count = count_grouped_by_confidential[false] || 0
+      total_count = public_count + (count_grouped_by_confidential[true] || 0)
 
-    def issues_without_hidden_without_confidential
-      self.class.query(@project, public_only: true, include_hidden: false).count
-    end
+      update_cache_for_key(public_count_cache_key) do
+        public_count
+      end
 
-    def issues_without_hidden_with_confidential
-      self.class.query(@project, public_only: false, include_hidden: false).count
+      update_cache_for_key(total_count_cache_key) do
+        total_count
+      end
     end
 
-    # We only show total issues count for admins, who are allowed to view hidden issues.
-    # We also only show issues count including confidential for reporters, who are allowed to view confidential issues.
+    # We only show issues count including confidential for reporters, who are allowed to view confidential issues.
     # This will still show a discrepancy on issues number but should be less than before.
     # Check https://gitlab.com/gitlab-org/gitlab-foss/issues/38418 description.
+
     # rubocop: disable CodeReuse/ActiveRecord
+    def self.query(projects, public_only: true)
+      issues_filtered_by_type = Issue.opened.with_issue_type(Issue::TYPES_FOR_LIST)
 
-    def self.query(projects, public_only: true, include_hidden: false)
-      if include_hidden
-        Issue.opened.with_issue_type(Issue::TYPES_FOR_LIST).where(project: projects)
-      elsif public_only
-        Issue.public_only.opened.with_issue_type(Issue::TYPES_FOR_LIST).where(project: projects)
+      if public_only
+        issues_filtered_by_type.public_only.where(project: projects)
       else
-        Issue.without_hidden.opened.with_issue_type(Issue::TYPES_FOR_LIST).where(project: projects)
+        issues_filtered_by_type.where(project: projects)
       end
     end
     # rubocop: enable CodeReuse/ActiveRecord

ee/spec/finders/issues_finder_spec.rb

@@ -325,29 +325,43 @@
           end
         end
       end
+    end
+  end
 
-      context 'with hidden issues' do
-        let_it_be(:banned_user) { create(:user, :banned) }
-        let_it_be(:hidden_issue) { create(:issue, project: project1, author: banned_user) }
+  describe '#with_confidentiality_access_check' do
+    let_it_be(:guest) { create(:user) }
 
-        context 'for an auditor' do
-          let(:user) { create(:user, :auditor) }
+    let_it_be(:authorized_user) { create(:user) }
+    let_it_be(:banned_user) { create(:user, :banned) }
+    let_it_be(:project) { create(:project, namespace: authorized_user.namespace) }
+    let_it_be(:public_issue) { create(:issue, project: project) }
+    let_it_be(:confidential_issue) { create(:issue, project: project, confidential: true) }
+    let_it_be(:hidden_issue) { create(:issue, project: project, author: banned_user) }
 
-          context 'when no project filter is given' do
-            let(:params) { {} }
+    context 'when no project filter is given' do
+      let(:params) { {} }
 
-            it 'returns all issues' do
-              expect(issues).to contain_exactly(issue1, issue2, issue3, issue4, issue5, hidden_issue)
-            end
-          end
+      context 'for an auditor' do
+        let(:auditor_user) { create(:user, :auditor) }
 
-          context 'when searching within a specific project' do
-            let(:params) { { project_id: project1.id } }
+        subject { described_class.new(auditor_user, params).with_confidentiality_access_check }
 
-            it 'returns all issues' do
-              expect(issues).to contain_exactly(issue1, issue5, hidden_issue)
-            end
-          end
+        it 'returns all issues' do
+          expect(subject).to include(public_issue, confidential_issue, hidden_issue)
+        end
+      end
+    end
+
+    context 'when searching within a specific project' do
+      let(:params) { { project_id: project.id } }
+
+      context 'for an auditor' do
+        let(:auditor_user) { create(:user, :auditor) }
+
+        subject { described_class.new(auditor_user, params).with_confidentiality_access_check }
+
+        it 'returns all issues' do
+          expect(subject).to include(public_issue, confidential_issue, hidden_issue)
         end
       end
     end

spec/finders/issues_finder/params_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe IssuesFinder::Params do
-  describe '#include_hidden' do
-    subject { described_class.new(params, user, IssuesFinder) }
-
-    context 'when param is not set' do
-      let(:params) { {} }
-
-      context 'with an admin', :enable_admin_mode do
-        let(:user) { create(:user, :admin) }
-
-        it 'returns true' do
-          expect(subject.include_hidden?).to be_truthy
-        end
-      end
-
-      context 'with a regular user' do
-        let(:user) { create(:user) }
-
-        it 'returns false' do
-          expect(subject.include_hidden?).to be_falsey
-        end
-      end
-    end
-
-    context 'when param is set' do
-      let(:params) { { include_hidden: true } }
-
-      context 'with an admin', :enable_admin_mode do
-        let(:user) { create(:user, :admin) }
-
-        it 'returns true' do
-          expect(subject.include_hidden?).to be_truthy
-        end
-      end
-
-      context 'with a regular user' do
-        let(:user) { create(:user) }
-
-        it 'returns false' do
-          expect(subject.include_hidden?).to be_falsey
-        end
-      end
-    end
-  end
-end

spec/finders/issues_finder_spec.rb

@@ -12,52 +12,8 @@
     context 'scope: all' do
       let(:scope) { 'all' }
 
-      context 'include_hidden and public_only params' do
-        let_it_be(:banned_user) { create(:user, :banned) }
-        let_it_be(:hidden_issue) { create(:issue, project: project1, author: banned_user) }
-        let_it_be(:confidential_issue) { create(:issue, project: project1, confidential: true) }
-
-        context 'when user is an admin', :enable_admin_mode do
-          let(:user) { create(:user, :admin) }
-
-          it 'returns all issues' do
-            expect(issues).to contain_exactly(issue1, issue2, issue3, issue4, issue5, hidden_issue, confidential_issue)
-          end
-        end
-
-        context 'when user is not an admin' do
-          context 'when public_only is true' do
-            let(:params) { { public_only: true } }
-
-            it 'returns public issues' do
-              expect(issues).to contain_exactly(issue1, issue2, issue3, issue4, issue5)
-            end
-          end
-
-          context 'when public_only is false' do
-            let(:params) { { public_only: false } }
-
-            it 'returns public and confidential issues' do
-              expect(issues).to contain_exactly(issue1, issue2, issue3, issue4, issue5, confidential_issue)
-            end
-          end
-
-          context 'when public_only is not set' do
-            it 'returns public and confidential issue' do
-              expect(issues).to contain_exactly(issue1, issue2, issue3, issue4, issue5, confidential_issue)
-            end
-          end
-
-          context 'when ban_user_feature_flag is false' do
-            before do
-              stub_feature_flags(ban_user_feature_flag: false)
-            end
-
-            it 'returns all issues' do
-              expect(issues).to contain_exactly(issue1, issue2, issue3, issue4, issue5, hidden_issue, confidential_issue)
-            end
-          end
-        end
+      it 'returns all issues' do
+        expect(issues).to contain_exactly(issue1, issue2, issue3, issue4, issue5)
       end
 
       context 'user does not have read permissions' do
@@ -1148,64 +1104,132 @@
   end
 
   describe '#with_confidentiality_access_check' do
-    let(:user) { create(:user) }
+    let(:guest) { create(:user) }
 
     let_it_be(:authorized_user) { create(:user) }
+    let_it_be(:banned_user) { create(:user, :banned) }
     let_it_be(:project) { create(:project, namespace: authorized_user.namespace) }
     let_it_be(:public_issue) { create(:issue, project: project) }
     let_it_be(:confidential_issue) { create(:issue, project: project, confidential: true) }
+    let_it_be(:hidden_issue) { create(:issue, project: project, author: banned_user) }
 
-    shared_examples 'returns public, does not return confidential' do
+    shared_examples 'returns public, does not return hidden or confidential' do
       it 'returns only public issues' do
         expect(subject).to include(public_issue)
-        expect(subject).not_to include(confidential_issue)
+        expect(subject).not_to include(confidential_issue, hidden_issue)
       end
     end
 
-    shared_examples 'returns public and confidential' do
-      it 'returns public and confidential issues' do
+    shared_examples 'returns public and confidential, does not return hidden' do
+      it 'returns only public and confidential issues' do
         expect(subject).to include(public_issue, confidential_issue)
+        expect(subject).not_to include(hidden_issue)
       end
     end
 
-    subject { described_class.new(user, params).with_confidentiality_access_check }
+    shared_examples 'returns public and hidden, does not return confidential' do
+      it 'returns only public and hidden issues' do
+        expect(subject).to include(public_issue, hidden_issue)
+        expect(subject).not_to include(confidential_issue)
+      end
+    end
+
+    shared_examples 'returns public, confidential, and hidden' do
+      it 'returns all issues' do
+        expect(subject).to include(public_issue, confidential_issue, hidden_issue)
+      end
+    end
 
     context 'when no project filter is given' do
       let(:params) { {} }
 
       context 'for an anonymous user' do
-        it_behaves_like 'returns public, does not return confidential'
+        subject { described_class.new(nil, params).with_confidentiality_access_check }
+
+        it_behaves_like 'returns public, does not return hidden or confidential'
+
+        context 'when feature flag is disabled' do
+          before do
+            stub_feature_flags(ban_user_feature_flag: false)
+          end
+
+          it_behaves_like 'returns public and hidden, does not return confidential'
+        end
       end
 
       context 'for a user without project membership' do
-        it_behaves_like 'returns public, does not return confidential'
+        subject { described_class.new(user, params).with_confidentiality_access_check }
+
+        it_behaves_like 'returns public, does not return hidden or confidential'
+
+        context 'when feature flag is disabled' do
+          before do
+            stub_feature_flags(ban_user_feature_flag: false)
+          end
+
+          it_behaves_like 'returns public and hidden, does not return confidential'
+        end
       end
 
       context 'for a guest user' do
+        subject { described_class.new(guest, params).with_confidentiality_access_check }
+
         before do
-          project.add_guest(user)
+          project.add_guest(guest)
         end
 
-        it_behaves_like 'returns public, does not return confidential'
+        it_behaves_like 'returns public, does not return hidden or confidential'
+
+        context 'when feature flag is disabled' do
+          before do
+            stub_feature_flags(ban_user_feature_flag: false)
+          end
+
+          it_behaves_like 'returns public and hidden, does not return confidential'
+        end
       end
 
       context 'for a project member with access to view confidential issues' do
-        before do
-          project.add_reporter(user)
-        end
+        subject { described_class.new(authorized_user, params).with_confidentiality_access_check }
 
-        it_behaves_like 'returns public and confidential'
+        it_behaves_like 'returns public and confidential, does not return hidden'
+
+        context 'when feature flag is disabled' do
+          before do
+            stub_feature_flags(ban_user_feature_flag: false)
+          end
+
+          it_behaves_like 'returns public, confidential, and hidden'
+        end
       end
 
       context 'for an admin' do
-        let(:user) { create(:user, :admin) }
+        let(:admin_user) { create(:user, :admin) }
+
+        subject { described_class.new(admin_user, params).with_confidentiality_access_check }
 
         context 'when admin mode is enabled', :enable_admin_mode do
-          it_behaves_like 'returns public and confidential'
+          it_behaves_like 'returns public, confidential, and hidden'
+
+          context 'when feature flag is disabled' do
+            before do
+              stub_feature_flags(ban_user_feature_flag: false)
+            end
+
+            it_behaves_like 'returns public, confidential, and hidden'
+          end
         end
 
         context 'when admin mode is disabled' do
-          it_behaves_like 'returns public, does not return confidential'
+          it_behaves_like 'returns public, does not return hidden or confidential'
+
+          context 'when feature flag is disabled' do
+            before do
+              stub_feature_flags(ban_user_feature_flag: false)
+            end
+
+            it_behaves_like 'returns public and hidden, does not return confidential'
+          end
         end
       end
     end
@@ -1214,9 +1238,17 @@
       let(:params) { { project_id: project.id } }
 
       context 'for an anonymous user' do
-        let(:user) { nil }
+        subject { described_class.new(nil, params).with_confidentiality_access_check }
 
-        it_behaves_like 'returns public, does not return confidential'
+        it_behaves_like 'returns public, does not return hidden or confidential'
+
+        context 'when feature flag is disabled' do
+          before do
+            stub_feature_flags(ban_user_feature_flag: false)
+          end
+
+          it_behaves_like 'returns public and hidden, does not return confidential'
+        end
 
         it 'does not filter by confidentiality' do
           expect(Issue).not_to receive(:where).with(a_string_matching('confidential'), anything)
@@ -1225,7 +1257,17 @@
       end
 
       context 'for a user without project membership' do
-        it_behaves_like 'returns public, does not return confidential'
+        subject { described_class.new(user, params).with_confidentiality_access_check }
+
+        it_behaves_like 'returns public, does not return hidden or confidential'
+
+        context 'when feature flag is disabled' do
+          before do
+            stub_feature_flags(ban_user_feature_flag: false)
+          end
+
+          it_behaves_like 'returns public and hidden, does not return confidential'
+        end
 
         it 'filters by confidentiality' do
           expect(subject.to_sql).to match("issues.confidential")
@@ -1233,11 +1275,21 @@
       end
 
       context 'for a guest user' do
+        subject { described_class.new(guest, params).with_confidentiality_access_check }
+
         before do
-          project.add_guest(user)
+          project.add_guest(guest)
         end
 
-        it_behaves_like 'returns public, does not return confidential'
+        it_behaves_like 'returns public, does not return hidden or confidential'
+
+        context 'when feature flag is disabled' do
+          before do
+            stub_feature_flags(ban_user_feature_flag: false)
+          end
+
+          it_behaves_like 'returns public and hidden, does not return confidential'
+        end
 
         it 'filters by confidentiality' do
           expect(subject.to_sql).to match("issues.confidential")
@@ -1245,18 +1297,40 @@
       end
 
       context 'for a project member with access to view confidential issues' do
-        before do
-          project.add_reporter(user)
+        subject { described_class.new(authorized_user, params).with_confidentiality_access_check }
+
+        it_behaves_like 'returns public and confidential, does not return hidden'
+
+        context 'when feature flag is disabled' do
+          before do
+            stub_feature_flags(ban_user_feature_flag: false)
+          end
+
+          it_behaves_like 'returns public, confidential, and hidden'
         end
 
-        it_behaves_like 'returns public and confidential'
+        it 'does not filter by confidentiality' do
+          expect(Issue).not_to receive(:where).with(a_string_matching('confidential'), anything)
+
+          subject
+        end
       end
 
       context 'for an admin' do
-        let(:user) { create(:user, :admin) }
+        let(:admin_user) { create(:user, :admin) }
+
+        subject { described_class.new(admin_user, params).with_confidentiality_access_check }
 
         context 'when admin mode is enabled', :enable_admin_mode do
-          it_behaves_like 'returns public and confidential'
+          it_behaves_like 'returns public, confidential, and hidden'
+
+          context 'when feature flag is disabled' do
+            before do
+              stub_feature_flags(ban_user_feature_flag: false)
+            end
+
+            it_behaves_like 'returns public, confidential, and hidden'
+          end
 
           it 'does not filter by confidentiality' do
             expect(Issue).not_to receive(:where).with(a_string_matching('confidential'), anything)
@@ -1266,7 +1340,19 @@
         end
 
         context 'when admin mode is disabled' do
-          it_behaves_like 'returns public, does not return confidential'
+          it_behaves_like 'returns public, does not return hidden or confidential'
+
+          context 'when feature flag is disabled' do
+            before do
+              stub_feature_flags(ban_user_feature_flag: false)
+            end
+
+            it_behaves_like 'returns public and hidden, does not return confidential'
+          end
+
+          it 'filters by confidentiality' do
+            expect(subject.to_sql).to match("issues.confidential")
+          end
         end
       end
     end

spec/models/issue_spec.rb

@@ -1241,23 +1241,11 @@
   end
 
   describe '.public_only' do
-    let_it_be(:banned_user) { create(:user, :banned) }
-    let_it_be(:public_issue) { create(:issue, project: reusable_project) }
-    let_it_be(:confidential_issue) { create(:issue, project: reusable_project, confidential: true) }
-    let_it_be(:hidden_issue) { create(:issue, project: reusable_project, author: banned_user) }
-
     it 'only returns public issues' do
-      expect(described_class.public_only).to eq([public_issue])
-    end
-
-    context 'when feature flag is disabled' do
-      before do
-        stub_feature_flags(ban_user_feature_flag: false)
-      end
+      public_issue = create(:issue, project: reusable_project)
+      create(:issue, project: reusable_project, confidential: true)
 
-      it 'returns public and hidden issues' do
-        expect(described_class.public_only).to contain_exactly(public_issue, hidden_issue)
-      end
+      expect(described_class.public_only).to eq([public_issue])
     end
   end
 

spec/services/groups/open_issues_count_service_spec.rb

@@ -3,18 +3,12 @@
 require 'spec_helper'
 
 RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_caching do
-  let_it_be(:group) { create(:group, :public) }
+  let_it_be(:group) { create(:group, :public)}
   let_it_be(:project) { create(:project, :public, namespace: group) }
-  let_it_be(:admin) { create(:user, :admin) }
   let_it_be(:user) { create(:user) }
-  let_it_be(:banned_user) { create(:user, :banned) }
-
-  before do
-    create(:issue, :opened, project: project)
-    create(:issue, :opened, confidential: true, project: project)
-    create(:issue, :opened, author: banned_user, project: project)
-    create(:issue, :closed, project: project)
-  end
+  let_it_be(:issue) { create(:issue, :opened, project: project) }
+  let_it_be(:confidential) { create(:issue, :opened, confidential: true, project: project) }
+  let_it_be(:closed) { create(:issue, :closed, project: project) }
 
   subject { described_class.new(group, user) }
 
@@ -26,27 +20,17 @@
     it 'uses the IssuesFinder to scope issues' do
       expect(IssuesFinder)
         .to receive(:new)
-        .with(user, group_id: group.id, state: 'opened', non_archived: true, include_subgroups: true, public_only: true, include_hidden: false)
+        .with(user, group_id: group.id, state: 'opened', non_archived: true, include_subgroups: true, public_only: true)
 
       subject.count
     end
   end
 
   describe '#count' do
-    shared_examples 'counts public issues, does not count hidden or confidential' do
-      it 'counts only public issues' do
-        expect(subject.count).to eq(1)
-      end
-
-      it 'uses PUBLIC_COUNT_WITHOUT_HIDDEN_KEY cache key' do
-        expect(subject.cache_key).to include('group_open_public_issues_without_hidden_count')
-      end
-    end
-
     context 'when user is nil' do
-      let(:user) { nil }
-
-      it_behaves_like 'counts public issues, does not count hidden or confidential'
+      it 'does not include confidential issues in the issue count' do
+        expect(described_class.new(group).count).to eq(1)
+      end
     end
 
     context 'when user is provided' do
@@ -55,13 +39,9 @@
           group.add_reporter(user)
         end
 
-        it 'includes confidential issues and does not include hidden issues in count' do
+        it 'returns the right count with confidential issues' do
           expect(subject.count).to eq(2)
         end
-
-        it 'uses TOTAL_COUNT_WITHOUT_HIDDEN_KEY cache key' do
-          expect(subject.cache_key).to include('group_open_issues_without_hidden_count')
-        end
       end
 
       context 'when user cannot read confidential issues' do
@@ -69,24 +49,8 @@
           group.add_guest(user)
         end
 
-        it_behaves_like 'counts public issues, does not count hidden or confidential'
-      end
-
-      context 'when user is an admin' do
-        let(:user) { admin }
-
-        context 'when admin mode is enabled', :enable_admin_mode do
-          it 'includes confidential and hidden issues in count' do
-            expect(subject.count).to eq(3)
-          end
-
-          it 'uses TOTAL_COUNT_KEY cache key' do
-            expect(subject.cache_key).to include('group_open_issues_including_hidden_count')
-          end
-        end
-
-        context 'when admin mode is disabled' do
-          it_behaves_like 'counts public issues, does not count hidden or confidential'
+        it 'does not include confidential issues' do
+          expect(subject.count).to eq(1)
         end
       end
 
@@ -97,13 +61,11 @@
   describe '#clear_all_cache_keys' do
     it 'calls `Rails.cache.delete` with the correct keys' do
       expect(Rails.cache).to receive(:delete)
-        .with(['groups', 'open_issues_count_service', 1, group.id, described_class::PUBLIC_COUNT_WITHOUT_HIDDEN_KEY])
+        .with(['groups', 'open_issues_count_service', 1, group.id, described_class::PUBLIC_COUNT_KEY])
       expect(Rails.cache).to receive(:delete)
         .with(['groups', 'open_issues_count_service', 1, group.id, described_class::TOTAL_COUNT_KEY])
-      expect(Rails.cache).to receive(:delete)
-        .with(['groups', 'open_issues_count_service', 1, group.id, described_class::TOTAL_COUNT_WITHOUT_HIDDEN_KEY])
 
-      described_class.new(group).clear_all_cache_keys
+      subject.clear_all_cache_keys
     end
   end
 end

spec/services/issues/close_service_spec.rb

@@ -279,7 +279,7 @@
 
       it 'verifies the number of queries' do
         recorded = ActiveRecord::QueryRecorder.new { close_issue }
-        expected_queries = 32
+        expected_queries = 30
 
         expect(recorded.count).to be <= expected_queries
         expect(recorded.cached_count).to eq(0)

spec/services/projects/batch_open_issues_count_service_spec.rb

@@ -5,7 +5,6 @@
 RSpec.describe Projects::BatchOpenIssuesCountService do
   let!(:project_1) { create(:project) }
   let!(:project_2) { create(:project) }
-  let!(:banned_user) { create(:user, :banned) }
 
   let(:subject) { described_class.new([project_1, project_2]) }
 
@@ -13,41 +12,32 @@
     before do
       create(:issue, project: project_1)
       create(:issue, project: project_1, confidential: true)
-      create(:issue, project: project_1, author: banned_user)
+
       create(:issue, project: project_2)
       create(:issue, project: project_2, confidential: true)
-      create(:issue, project: project_2, author: banned_user)
     end
 
-    context 'when cache is clean', :aggregate_failures do
+    context 'when cache is clean' do
       it 'refreshes cache keys correctly' do
-        expect(get_cache_key(project_1)).to eq(nil)
-        expect(get_cache_key(project_2)).to eq(nil)
-
-        subject.count_service.new(project_1).refresh_cache
-        subject.count_service.new(project_2).refresh_cache
-
-        expect(get_cache_key(project_1)).to eq(1)
-        expect(get_cache_key(project_2)).to eq(1)
+        subject.refresh_cache_and_retrieve_data
 
-        expect(get_cache_key(project_1, true)).to eq(2)
-        expect(get_cache_key(project_2, true)).to eq(2)
+        # It does not update total issues cache
+        expect(Rails.cache.read(get_cache_key(subject, project_1))).to eq(nil)
+        expect(Rails.cache.read(get_cache_key(subject, project_2))).to eq(nil)
 
-        expect(get_cache_key(project_1, true, true)).to eq(3)
-        expect(get_cache_key(project_2, true, true)).to eq(3)
+        expect(Rails.cache.read(get_cache_key(subject, project_1, true))).to eq(1)
+        expect(Rails.cache.read(get_cache_key(subject, project_1, true))).to eq(1)
       end
     end
   end
 
-  def get_cache_key(project, with_confidential = false, with_hidden = false)
+  def get_cache_key(subject, project, public_key = false)
     service = subject.count_service.new(project)
 
-    if with_confidential && with_hidden
-      Rails.cache.read(service.cache_key(service.class::TOTAL_COUNT_KEY))
-    elsif with_confidential
-      Rails.cache.read(service.cache_key(service.class::TOTAL_COUNT_WITHOUT_HIDDEN_KEY))
+    if public_key
+      service.cache_key(service.class::PUBLIC_COUNT_KEY)
     else
-      Rails.cache.read(service.cache_key(service.class::PUBLIC_COUNT_WITHOUT_HIDDEN_KEY))
+      service.cache_key(service.class::TOTAL_COUNT_KEY)
     end
   end
 end

spec/services/projects/open_issues_count_service_spec.rb

@@ -4,102 +4,89 @@
 
 RSpec.describe Projects::OpenIssuesCountService, :use_clean_rails_memory_store_caching do
   let(:project) { create(:project) }
-  let(:user) { create(:user) }
-  let(:banned_user) { create(:user, :banned) }
 
-  subject { described_class.new(project, user) }
+  subject { described_class.new(project) }
 
   it_behaves_like 'a counter caching service'
 
-  before do
-    create(:issue, :opened, project: project)
-    create(:issue, :opened, confidential: true, project: project)
-    create(:issue, :opened, author: banned_user, project: project)
-    create(:issue, :closed, project: project)
-
-    described_class.new(project).refresh_cache
-  end
-
   describe '#count' do
-    shared_examples 'counts public issues, does not count hidden or confidential' do
-      it 'counts only public issues' do
-        expect(subject.count).to eq(1)
-      end
-
-      it 'uses PUBLIC_COUNT_WITHOUT_HIDDEN_KEY cache key' do
-        expect(subject.cache_key).to include('project_open_public_issues_without_hidden_count')
-      end
-    end
-
     context 'when user is nil' do
-      let(:user) { nil }
+      it 'does not include confidential issues in the issue count' do
+        create(:issue, :opened, project: project)
+        create(:issue, :opened, confidential: true, project: project)
 
-      it_behaves_like 'counts public issues, does not count hidden or confidential'
+        expect(described_class.new(project).count).to eq(1)
+      end
     end
 
     context 'when user is provided' do
+      let(:user) { create(:user) }
+
       context 'when user can read confidential issues' do
         before do
           project.add_reporter(user)
         end
 
-        it 'includes confidential issues and does not include hidden issues in count' do
-          expect(subject.count).to eq(2)
+        it 'returns the right count with confidential issues' do
+          create(:issue, :opened, project: project)
+          create(:issue, :opened, confidential: true, project: project)
+
+          expect(described_class.new(project, user).count).to eq(2)
         end
 
-        it 'uses TOTAL_COUNT_WITHOUT_HIDDEN_KEY cache key' do
-          expect(subject.cache_key).to include('project_open_issues_without_hidden_count')
+        it 'uses total_open_issues_count cache key' do
+          expect(described_class.new(project, user).cache_key_name).to eq('total_open_issues_count')
         end
       end
 
-      context 'when user cannot read confidential or hidden issues' do
+      context 'when user cannot read confidential issues' do
         before do
           project.add_guest(user)
         end
 
-        it_behaves_like 'counts public issues, does not count hidden or confidential'
-      end
-
-      context 'when user is an admin' do
-        let_it_be(:user) { create(:user, :admin) }
-
-        context 'when admin mode is enabled', :enable_admin_mode do
-          it 'includes confidential and hidden issues in count' do
-            expect(subject.count).to eq(3)
-          end
+        it 'does not include confidential issues' do
+          create(:issue, :opened, project: project)
+          create(:issue, :opened, confidential: true, project: project)
 
-          it 'uses TOTAL_COUNT_KEY cache key' do
-            expect(subject.cache_key).to include('project_open_issues_including_hidden_count')
-          end
+          expect(described_class.new(project, user).count).to eq(1)
         end
 
-        context 'when admin mode is disabled' do
-          it_behaves_like 'counts public issues, does not count hidden or confidential'
+        it 'uses public_open_issues_count cache key' do
+          expect(described_class.new(project, user).cache_key_name).to eq('public_open_issues_count')
         end
       end
     end
-  end
-
-  describe '#refresh_cache', :aggregate_failures do
-    context 'when cache is empty' do
-      it 'refreshes cache keys correctly' do
-        expect(Rails.cache.read(described_class.new(project).cache_key(described_class::PUBLIC_COUNT_WITHOUT_HIDDEN_KEY))).to eq(1)
-        expect(Rails.cache.read(described_class.new(project).cache_key(described_class::TOTAL_COUNT_WITHOUT_HIDDEN_KEY))).to eq(2)
-        expect(Rails.cache.read(described_class.new(project).cache_key(described_class::TOTAL_COUNT_KEY))).to eq(3)
-      end
-    end
 
-    context 'when cache is outdated' do
-      it 'refreshes cache keys correctly' do
+    describe '#refresh_cache' do
+      before do
+        create(:issue, :opened, project: project)
         create(:issue, :opened, project: project)
         create(:issue, :opened, confidential: true, project: project)
-        create(:issue, :opened, author: banned_user, project: project)
+      end
 
-        described_class.new(project).refresh_cache
+      context 'when cache is empty' do
+        it 'refreshes cache keys correctly' do
+          subject.refresh_cache
 
-        expect(Rails.cache.read(described_class.new(project).cache_key(described_class::PUBLIC_COUNT_WITHOUT_HIDDEN_KEY))).to eq(2)
-        expect(Rails.cache.read(described_class.new(project).cache_key(described_class::TOTAL_COUNT_WITHOUT_HIDDEN_KEY))).to eq(4)
-        expect(Rails.cache.read(described_class.new(project).cache_key(described_class::TOTAL_COUNT_KEY))).to eq(6)
+          expect(Rails.cache.read(subject.cache_key(described_class::PUBLIC_COUNT_KEY))).to eq(2)
+          expect(Rails.cache.read(subject.cache_key(described_class::TOTAL_COUNT_KEY))).to eq(3)
+        end
+      end
+
+      context 'when cache is outdated' do
+        before do
+          subject.refresh_cache
+        end
+
+        it 'refreshes cache keys correctly' do
+          create(:issue, :opened, project: project)
+          create(:issue, :opened, confidential: true, project: project)
+
+          subject.refresh_cache
+
+          expect(Rails.cache.read(subject.cache_key(described_class::PUBLIC_COUNT_KEY))).to eq(3)
+          expect(Rails.cache.read(subject.cache_key(described_class::TOTAL_COUNT_KEY))).to eq(5)
+        end
       end
     end
   end