Truncate count when over thousand

- Redefine count to check for cached threshold
- Add Group helper to truncale count if over 1k
- Add service specs

app/helpers/groups_helper.rb

@@ -68,6 +68,16 @@ module GroupsHelper
       .count
   end
 
+  def cached_open_group_issues_count
+    issues_count = @group.open_issues_count(current_user)
+
+    if issues_count > 1000
+      ActiveSupport::NumberHelper.number_to_human(issues_count, units: { thousand: 'K' }, precision: 1)
+    else
+      number_with_delimiter(issues_count)
+    end
+  end
+
   def group_merge_requests_count(state:)
     MergeRequestsFinder
       .new(current_user, group_id: @group.id, state: state, non_archived: true, include_subgroups: true)

app/models/group.rb

@@ -626,12 +626,6 @@ class Group < Namespace
   end
   # rubocop: enable CodeReuse/ServiceClass
 
-  # rubocop: disable CodeReuse/ServiceClass
-  def update_group_issues_counter_cache
-    Groups::OpenIssuesCountService.new(self).refresh_cache
-  end
-  # rubocop: enable CodeReuse/ServiceClass
-
   private
 
   def update_two_factor_requirement

app/services/groups/open_issues_count_service.rb

@@ -2,14 +2,57 @@
 
 module Groups
   # Service class for counting and caching the number of open issues of a group.
-  class OpenIssuesCountService < OpenIssuesCountBaseService
+  # class OpenIssuesCountService < IssuablesCountService
+  class OpenIssuesCountService < BaseCountService
     include Gitlab::Utils::StrongMemoize
 
+    VERSION = 1
     PUBLIC_COUNT_KEY = 'group_public_open_issues_count'
     TOTAL_COUNT_KEY = 'group_total_open_issues_count'
+    CACHED_COUNT_THRESHOLD = 1000
+    EXPIRATION_TIME = 24.hours
+
+    def initialize(group, user = nil)
+      @group = group
+      @user = user
+    end
+
+    def count
+      cached_count = Rails.cache.read(cache_key)
+
+      if cached_count && cached_count >= CACHED_COUNT_THRESHOLD
+        cached_count
+      else
+        new_count = uncached_count
+        update_cache_for_key(cache_key) { new_count }
+        new_count
+      end
+    end
 
     def cache_options
-      super.merge({ expires_in: 24.hours })
+      super.merge({ expires_in: EXPIRATION_TIME })
+    end
+
+    def cache_key(key = nil)
+      ['groups', 'open_issues_count_service', VERSION, @group.id, cache_key_name]
+    end
+
+    def cache_key_name
+      public_only? ? PUBLIC_COUNT_KEY : TOTAL_COUNT_KEY
+    end
+
+    def public_only?
+      !user_is_at_least_reporter?
+    end
+
+    def user_is_at_least_reporter?
+      strong_memoize(:user_is_at_least_reporter) do
+        @user && @group.member?(@user, Gitlab::Access::REPORTER)
+      end
+    end
+
+    def relation_for_count
+      self.class.query(@group, user: @user, public_only: public_only?)
     end
 
     def self.query(group, user: nil, public_only: true)

app/services/issuables_count_service.rb

-# frozen_string_literal: true
-
-class IssuablesCountService < BaseCountService
-  # The version of the cache format. This should be bumped whenever the
-  # underlying logic changes. This removes the need for explicitly flushing
-  # all caches.
-  VERSION = 1
-
-  def initialize(parent)
-    @parent = parent
-  end
-
-  def relation_for_count
-    self.class.query(@parent.id)
-  end
-
-  def cache_key_name
-    raise(
-      NotImplementedError,
-      '"cache_key_name" must be implemented and return a String'
-    )
-  end
-
-  def cache_key(key = nil)
-    cache_key = key || cache_key_name
-
-    ['parents', 'count_service', VERSION, @parent.id, cache_key]
-  end
-
-  def self.query(parent_ids)
-    raise(
-      NotImplementedError,
-      '"query" must be implemented and return an ActiveRecord::Relation'
-    )
-  end
-end

app/services/open_issues_count_base_service.rb

-# frozen_string_literal: true
-
-# Service class for counting and caching the number of open issues of a group or a project.
-class OpenIssuesCountBaseService < IssuablesCountService
-  include Gitlab::Utils::StrongMemoize
-
-  # Cache keys used to store issues count
-  PUBLIC_COUNT_KEY = ''
-  TOTAL_COUNT_KEY = ''
-
-  def initialize(group, user = nil)
-    @user = user
-
-    super(group)
-  end
-
-  def cache_key_name
-    public_only? ? PUBLIC_COUNT_KEY : TOTAL_COUNT_KEY
-  end
-
-  def public_only?
-    !user_is_at_least_reporter?
-  end
-
-  def relation_for_count
-    self.class.query(@parent, user: @user, public_only: public_only?)
-  end
-
-  def user_is_at_least_reporter?
-    strong_memoize(:user_is_at_least_reporter) do
-      @user && @parent.member?(@user, Gitlab::Access::REPORTER)
-    end
-  end
-
-  def public_count_cache_key
-    cache_key(PUBLIC_COUNT_KEY)
-  end
-
-  def total_count_cache_key
-    cache_key(TOTAL_COUNT_KEY)
-  end
-
-  # rubocop: disable CodeReuse/ActiveRecord
-  def refresh_cache(&block)
-    if block_given?
-      super(&block)
-    else
-      count_grouped_by_confidential = self.class.query(@parent, user: @user, public_only: false).group(:confidential).count
-      public_count = count_grouped_by_confidential[false] || 0
-      total_count = public_count + (count_grouped_by_confidential[true] || 0)
-
-      update_cache_for_key(group_public_count_cache_key) do
-        public_count
-      end
-
-      update_cache_for_key(group_total_count_cache_key) do
-        total_count
-      end
-    end
-  end
-
-  def self.query(parent, user: nil, public_only: true)
-    raise(
-      NotImplementedError,
-      '"query" must be implemented and return an ActiveRecord::Relation'
-    )
-  end
-end

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

@@ -54,8 +54,7 @@
             %span.nav-item-name
               = _('Issues')
             %span.badge.badge-pill.count
-              = number_with_delimiter(@group.open_issues_count(current_user))
-
+              = cached_open_group_issues_count
           %ul.sidebar-sub-level-items{ data: { qa_selector: 'group_issues_sidebar_submenu'} }
             = nav_link(path: ['groups#issues', 'labels#index', 'milestones#index', 'iterations#index'], html_options: { class: "fly-out-top-item" } ) do
               = link_to issues_group_path(@group) do

spec/services/groups/open_issues_count_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_caching do
+  let_it_be(:group) { create(:group, :public)}
+  let_it_be(:project) { create(:project, :public, namespace: group) }
+  let_it_be(:user) { create(:user) }
+  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) }
+
+  describe '#self.query' do
+    before do
+      allow(IssuesFinder).to receive(:new).and_call_original
+    end
+
+    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)
+
+      subject.count
+    end
+  end
+
+  describe '#count' do
+    context 'when user is nil' do
+      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
+      let(:total_count_key) { subject.cache_key(described_class::TOTAL_COUNT_KEY) }
+
+      context 'when user can read confidential issues' do
+        before do
+          group.add_reporter(user)
+          project.add_reporter(user)
+        end
+
+        it 'returns the right count with confidential issues' do
+          expect(subject.count).to eq(2)
+        end
+
+        it 'uses total_open_issues_count cache key' do
+          expect(subject.cache_key_name).to eq('group_total_open_issues_count')
+        end
+
+        context 'when cache is empty' do
+          before do
+            Rails.cache.delete(total_count_key)
+          end
+
+          it 'refreshes cache keys correctly' do
+            subject.count
+
+            expect(Rails.cache.read(total_count_key)).to eq(2)
+          end
+        end
+
+        context 'when count is over the threshold value' do
+          before do
+            Rails.cache.write(total_count_key, 12345)
+          end
+
+          it 'does not refresh cache' do
+            expect(Rails.cache).not_to receive(:write)
+
+            subject.count
+          end
+        end
+      end
+
+      context 'when user cannot read confidential issues' do
+        before do
+          group.add_guest(user)
+          project.add_guest(user)
+        end
+
+        it 'does not include confidential issues' do
+          expect(subject.count).to eq(1)
+        end
+
+        it 'uses public_open_issues_count cache key' do
+          expect(subject.cache_key_name).to eq('group_public_open_issues_count')
+        end
+
+        context 'when cache is empty' do
+          let(:public_count_key) { subject.cache_key(described_class::PUBLIC_COUNT_KEY) }
+
+          before do
+            Rails.cache.delete(public_count_key)
+          end
+
+          it 'refreshes cache keys correctly' do
+            subject.count
+
+            expect(Rails.cache.read(public_count_key)).to eq(1)
+          end
+        end
+      end
+    end
+  end
+end