Merge branch '299540-cache-epics-count-in-group-pages-sidebar' into 'master'

Cache epics count in sidebar at the group level [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!58064

config/feature_flags/development/cached_sidebar_open_epics_count.yml

+---
+name: cached_sidebar_open_epics_count
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/58064
+rollout_issue_url:
+milestone: '13.11'
+type: development
+group: group::product planning
+default_enabled: true

doc/user/group/epics/manage_epics.md

@@ -323,3 +323,36 @@ To remove a child epic from a parent epic:
 
 1. Select the <kbd>x</kbd> button in the parent epic's list of epics.
 1. Select **Remove** in the **Remove epic** warning message.
+
+## Cached epic count
+
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/299540) in GitLab 13.11.
+> - It's [deployed behind a feature flag](../../feature_flags.md), enabled by default.
+> - It's enabled on GitLab.com.
+> - It's recommended for production use.
+> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#enable-or-disable-cached-epic-count).
+
+WARNING:
+This feature might not be available to you. Check the **version history** note above for details.
+
+In a group, the sidebar displays the total count of open epics and this value is cached if higher
+than 1000. The cached value is rounded to thousands (or millions) and updated every 24 hours.
+
+### Enable or disable cached epic count **(PREMIUM SELF)**
+
+Cached epic count in the left sidebar is under development but ready for production use. It is
+deployed behind a feature flag that is **enabled by default**.
+[GitLab administrators with access to the GitLab Rails console](../../../administration/feature_flags.md)
+can disable it.
+
+To disable it:
+
+```ruby
+Feature.disable(:cached_sidebar_open_epics_count)
+```
+
+To enable it:
+
+```ruby
+Feature.enable(:cached_sidebar_open_epics_count)
+```

ee/app/helpers/ee/groups_helper.rb

@@ -11,6 +11,21 @@ module EE
         .count
     end
 
+    def open_epics_count(group)
+      if ::Feature.enabled?(:cached_sidebar_open_epics_count, group, default_enabled: :yaml)
+        cached_issuables_count(group, type: :epics)
+      else
+        number_with_delimiter(group_epics_count(state: 'opened'))
+      end
+    end
+
+    override :issuables_count_service_class
+    def issuables_count_service_class(type)
+      return super unless type == :epics
+
+      ::Groups::EpicsCountService
+    end
+
     def group_nav_link_paths
       %w[saml_providers#show usage_quotas#index billings#index]
     end

ee/app/services/groups/epics_count_service.rb

+# frozen_string_literal: true
+
+module Groups
+  class EpicsCountService < Groups::CountService
+    private
+
+    def cache_key_name
+      'open_epics_count'
+    end
+
+    def relation_for_count
+      EpicsFinder
+        .new(user, group_id: group.id, state: 'opened')
+        .execute(skip_visibility_check: true)
+    end
+
+    def issuable_key
+      'open_epics'
+    end
+  end
+end

ee/app/views/layouts/nav/ee/_epic_link.html.haml

 - return unless group_sidebar_link?(:epics)
 
-- epics_count = group_epics_count(state: 'opened')
+- epics_count = open_epics_count(group)
 - epics_items = ['epics#show', 'epics#index', 'epic_boards#index', 'epic_boards#show', 'roadmap#show']
 
 = nav_link(path: epics_items) do
@@ -20,7 +20,7 @@
       = link_to group_epics_path(group), title: 'List' do
         %span= _('List')
 
-    - if Feature.enabled?(:epic_boards, @group)
+    - if Feature.enabled?(:epic_boards, group)
       = nav_link(path: ['epic_boards#index', 'epic_boards#show'], html_options: { class: "home" }) do
         = link_to group_epic_boards_path(group), title: 'Boards' do
           %span= _('Boards')

ee/changelogs/unreleased/299540-cache-epics-count-in-group-pages-sidebar.yml

+---
+title: Cache open issues count in group sidebar
+merge_request: 58064
+author:
+type: performance

ee/spec/helpers/ee/groups_helper_spec.rb

@@ -49,6 +49,43 @@ RSpec.describe GroupsHelper do
     end
   end
 
+  describe '#open_epics_count' do
+    let_it_be(:count_service) { ::Groups::EpicsCountService }
+
+    before do
+      allow(helper).to receive(:current_user) { current_user }
+    end
+
+    it 'returns count value from cache' do
+      allow_next_instance_of(count_service) do |service|
+        allow(service).to receive(:count).and_return(2500)
+      end
+
+      expect(helper.open_epics_count(group)).to eq('2.5k')
+    end
+
+    context 'when cached_sidebar_open_epics_count feature flag is disabled' do
+      before do
+        stub_feature_flags(cached_sidebar_open_epics_count: false)
+      end
+
+      it 'returns not cached epics count' do
+        allow(helper).to receive(:group_epics_count).and_return(2500)
+
+        expect(helper.open_epics_count(group)).to eq('2,500')
+      end
+    end
+  end
+
+  describe '#cached_issuables_count' do
+    context 'with epics type' do
+      let(:type) { :epics }
+      let(:count_service) { ::Groups::EpicsCountService }
+
+      it_behaves_like 'cached issuables count'
+    end
+  end
+
   describe '#group_sidebar_links' do
     before do
       allow(helper).to receive(:can?) { |*args| Ability.allowed?(*args) }

ee/spec/services/groups/epics_count_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Groups::EpicsCountService, :use_clean_rails_memory_store_caching do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:group) { create(:group, :public)}
+  let_it_be(:epic) { create(:epic, group: group) }
+
+  subject { described_class.new(group, user) }
+
+  describe '#relation_for_count' do
+    before do
+      group.add_reporter(user)
+      allow(EpicsFinder).to receive(:new).and_call_original
+    end
+
+    it 'uses the EpicsFinder to scope epics' do
+      expect(EpicsFinder)
+        .to receive(:new)
+        .with(user, group_id: group.id, state: 'opened')
+
+      subject.count
+    end
+  end
+
+  it_behaves_like 'a counter caching service with threshold'
+end

spec/helpers/groups_helper_spec.rb

@@ -515,64 +515,18 @@ RSpec.describe GroupsHelper do
     let_it_be(:current_user) { create(:user) }
     let_it_be(:group) { create(:group, name: 'group') }
 
-    subject { helper.cached_issuables_count(group, type: type) }
-
-    before do
-      allow(helper).to receive(:current_user) { current_user }
-      allow(count_service).to receive(:new).and_call_original
-    end
-
-    shared_examples 'caching issuables count' do
-      it 'calls the correct service class' do
-        subject
-        expect(count_service).to have_received(:new).with(group, current_user)
-      end
-
-      it 'returns all digits for count value under 1000' do
-        allow_next_instance_of(count_service) do |service|
-          allow(service).to receive(:count).and_return(999)
-        end
-
-        expect(subject).to eq('999')
-      end
-
-      it 'returns truncated digits for count value over 1000' do
-        allow_next_instance_of(count_service) do |service|
-          allow(service).to receive(:count).and_return(2300)
-        end
-
-        expect(subject).to eq('2.3k')
-      end
-
-      it 'returns truncated digits for count value over 10000' do
-        allow_next_instance_of(count_service) do |service|
-          allow(service).to receive(:count).and_return(12560)
-        end
-
-        expect(subject).to eq('12.6k')
-      end
-
-      it 'returns truncated digits for count value over 100000' do
-        allow_next_instance_of(count_service) do |service|
-          allow(service).to receive(:count).and_return(112560)
-        end
-
-        expect(subject).to eq('112.6k')
-      end
-    end
-
-    context 'with issue type' do
+    context 'with issues type' do
       let(:type) { :issues }
       let(:count_service) { Groups::OpenIssuesCountService }
 
-      it_behaves_like 'caching issuables count'
+      it_behaves_like 'cached issuables count'
     end
 
-    context 'with merge request type' do
+    context 'with merge requests type' do
       let(:type) { :merge_requests }
       let(:count_service) { Groups::MergeRequestsCountService }
 
-      it_behaves_like 'caching issuables count'
+      it_behaves_like 'cached issuables count'
     end
   end
 end

spec/support/shared_examples/helpers/groups_shared_examples.rb

+# frozen_string_literal: true
+
+# This shared_example requires the following variables:
+# - current_user
+# - group
+# - type, the issuable type (ie :issues, :merge_requests)
+# - count_service, the Service used by the specified issuable type
+
+RSpec.shared_examples 'cached issuables count' do
+  subject { helper.cached_issuables_count(group, type: type) }
+
+  before do
+    allow(helper).to receive(:current_user) { current_user }
+    allow(count_service).to receive(:new).and_call_original
+  end
+
+  it 'calls the correct service class' do
+    subject
+    expect(count_service).to have_received(:new).with(group, current_user)
+  end
+
+  it 'returns all digits for count value under 1000' do
+    allow_next_instance_of(count_service) do |service|
+      allow(service).to receive(:count).and_return(999)
+    end
+
+    expect(subject).to eq('999')
+  end
+
+  it 'returns truncated digits for count value over 1000' do
+    allow_next_instance_of(count_service) do |service|
+      allow(service).to receive(:count).and_return(2300)
+    end
+
+    expect(subject).to eq('2.3k')
+  end
+
+  it 'returns truncated digits for count value over 10000' do
+    allow_next_instance_of(count_service) do |service|
+      allow(service).to receive(:count).and_return(12560)
+    end
+
+    expect(subject).to eq('12.6k')
+  end
+
+  it 'returns truncated digits for count value over 100000' do
+    allow_next_instance_of(count_service) do |service|
+      allow(service).to receive(:count).and_return(112560)
+    end
+
+    expect(subject).to eq('112.6k')
+  end
+end