Implement accurate snippet counts via one query

To improve performance over using multiple
individual queries for each type of snippet,
whilst also ensuring counts are accurate for
the user's current authorisation.

app/controllers/dashboard/snippets_controller.rb

@@ -7,6 +7,10 @@ class Dashboard::SnippetsController < Dashboard::ApplicationController
   skip_cross_project_access_check :index
 
   def index
+    @snippet_counts = Snippets::CountService
+      .new(current_user, author: current_user)
+      .execute
+
     @snippets = SnippetsFinder.new(current_user, author: current_user, scope: params[:scope])
       .execute
       .page(params[:page])

app/controllers/projects/snippets_controller.rb

@@ -30,6 +30,10 @@ class Projects::SnippetsController < Projects::ApplicationController
   respond_to :html
 
   def index
+    @snippet_counts = Snippets::CountService
+      .new(current_user, project: @project)
+      .execute
+
     @snippets = SnippetsFinder.new(current_user, project: @project, scope: params[:scope])
       .execute
       .page(params[:page])

app/services/snippets/count_service.rb

+# frozen_string_literal: true
+
+# Service for calculating visible Snippet counts via one query
+# for the given user or project.
+#
+# Authorisation level checks will be included, ensuring the correct
+# counts will be returned for the given user (if any).
+#
+# Basic usage:
+#
+#   user = User.find(1)
+#
+#   Snippets::CountService.new(user, author: user).execute
+#   #=> {
+#     are_public: 1,
+#     are_internal: 1,
+#     are_private: 1,
+#     all: 3
+#   }
+#
+# Counts can be scoped to a project:
+#
+#   user = User.find(1)
+#   project = Project.find(1)
+#
+#   Snippets::CountService.new(user, project: project).execute
+#   #=> {
+#     are_public: 1,
+#     are_internal: 1,
+#     are_private: 0,
+#     all: 2
+#   }
+#
+# Either a project or an author *must* be supplied.
+module Snippets
+  class CountService
+    def initialize(current_user, author: nil, project: nil)
+      if !author && !project
+        raise(
+          ArgumentError, 'Must provide either an author or a project'
+        )
+      end
+
+      @snippets_finder = SnippetsFinder.new(current_user, author: author, project: project)
+    end
+
+    def execute
+      counts = snippet_counts
+      return {} unless counts
+
+      counts.slice(
+        :are_public,
+        :are_private,
+        :are_internal,
+        :are_public_or_internal,
+        :total
+      )
+    end
+
+    private
+
+    # rubocop: disable CodeReuse/ActiveRecord
+    def snippet_counts
+      @snippets_finder.execute
+        .reorder(nil)
+        .select("
+          count(case when snippets.visibility_level=#{Snippet::PUBLIC} and snippets.secret is FALSE then 1 else null end) as are_public,
+          count(case when snippets.visibility_level=#{Snippet::INTERNAL} then 1 else null end) as are_internal,
+          count(case when snippets.visibility_level=#{Snippet::PRIVATE} then 1 else null end) as are_private,
+          count(case when visibility_level=#{Snippet::PUBLIC} OR visibility_level=#{Snippet::INTERNAL} then 1 else null end) as are_public_or_internal,
+          count(*) as total
+        ")
+        .first
+    end
+    # rubocop: enable CodeReuse/ActiveRecord
+  end
+end

app/views/dashboard/snippets/index.html.haml

@@ -5,7 +5,7 @@
 
 = render 'dashboard/snippets_head'
 - if current_user.snippets.exists?
-  = render partial: 'snippets/snippets_scope_menu', locals: { include_private: true }
+  = render partial: 'snippets/snippets_scope_menu', locals: { include_private: true, counts: @snippet_counts }
 
 - if current_user.snippets.exists?
   = render partial: 'shared/snippets/list', locals: { link_project: true }

app/views/projects/snippets/index.html.haml

@@ -5,7 +5,7 @@
   - if current_user
     .top-area
       - include_private = @project.team.member?(current_user) || current_user.admin?
-      = render partial: 'snippets/snippets_scope_menu', locals: { subject: @project, include_private: include_private }
+      = render partial: 'snippets/snippets_scope_menu', locals: { subject: @project, include_private: include_private, counts: @snippet_counts }
 
       - if new_project_snippet_link.present?
         .nav-controls

app/views/snippets/_snippets_scope_menu.html.haml

@@ -7,25 +7,25 @@
       = _("All")
       %span.badge.badge-pill
         - if include_private
-          = subject.snippets.count
+          = counts[:total]
         - else
-          = subject.snippets.public_and_internal_only.count
+          = counts[:are_public_or_internal]
 
   - if include_private
     %li{ class: active_when(params[:scope] == "are_private") }
       = link_to subject_snippets_path(subject, scope: 'are_private') do
         = _("Private")
         %span.badge.badge-pill
-          = subject.snippets.are_private.count
+          = counts[:are_private]
 
   %li{ class: active_when(params[:scope] == "are_internal") }
     = link_to subject_snippets_path(subject, scope: 'are_internal') do
       = _("Internal")
       %span.badge.badge-pill
-        = subject.snippets.are_internal.count
+        = counts[:are_internal]
 
   %li{ class: active_when(params[:scope] == "are_public") }
     = link_to subject_snippets_path(subject, scope: 'are_public') do
       = _("Public")
       %span.badge.badge-pill
-        = subject.snippets.are_public.count
+        = counts[:are_public]

changelogs/unreleased/21705-number-of-snippets-in-the-snippets-page-is-wrong.yml

+---
+title: Fix bug with snippet counts not being scoped to current authorisation
+merge_request: 21705
+author:
+type: fixed

spec/controllers/dashboard/snippets_controller_spec.rb

@@ -17,5 +17,14 @@ describe Dashboard::SnippetsController do
         create(:personal_snippet, :public, author: user)
       end
     end
+
+    it 'fetches snippet counts via the snippet count service' do
+      service = double(:count_service, execute: {})
+      expect(Snippets::CountService)
+        .to receive(:new).with(user, author: user)
+        .and_return(service)
+
+      get :index
+    end
   end
 end

spec/controllers/projects/snippets_controller_spec.rb

@@ -27,6 +27,15 @@ describe Projects::SnippetsController do
       end
     end
 
+    it 'fetches snippet counts via the snippet count service' do
+      service = double(:count_service, execute: {})
+      expect(Snippets::CountService)
+        .to receive(:new).with(nil, project: project)
+        .and_return(service)
+
+      get :index, params: { namespace_id: project.namespace, project_id: project }
+    end
+
     context 'when the project snippet is private' do
       let!(:project_snippet) { create(:project_snippet, :private, project: project, author: user) }
 

spec/features/dashboard/snippets_spec.rb

@@ -59,6 +59,10 @@ describe 'Dashboard snippets' do
       visit dashboard_snippets_path
     end
 
+    it_behaves_like 'tabs with counts' do
+      let_it_be(:counts) { { all: '3', public: '1', private: '1', internal: '1' } }
+    end
+
     it 'contains all snippets of logged user' do
       expect(page).to have_selector('.snippet-row', count: 3)
 

spec/features/projects/snippets/user_views_snippets_spec.rb

@@ -31,6 +31,16 @@ describe 'Projects > Snippets > User views snippets' do
       it_behaves_like 'paginated snippets'
     end
 
+    context 'filtering by visibility' do
+      before do
+        visit_project_snippets
+      end
+
+      it_behaves_like 'tabs with counts' do
+        let_it_be(:counts) { { all: '1', public: '0', private: '1', internal: '0' } }
+      end
+    end
+
     it 'shows snippets' do
       visit_project_snippets
 

spec/services/snippets/count_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Snippets::CountService do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project, :public) }
+
+  describe '#new' do
+    it 'raises an error if no author or project' do
+      expect { described_class.new(user) }.to raise_error(ArgumentError)
+    end
+
+    it 'uses the SnippetsFinder to scope snippets by user' do
+      expect(SnippetsFinder)
+        .to receive(:new)
+        .with(user, author: user, project: nil)
+
+      described_class.new(user, author: user)
+    end
+
+    it 'allows scoping to project' do
+      expect(SnippetsFinder)
+        .to receive(:new)
+        .with(user, author: nil, project: project)
+
+      described_class.new(user, project: project)
+    end
+  end
+
+  describe '#execute' do
+    subject { described_class.new(user, author: user).execute }
+
+    it 'returns a hash of counts' do
+      expect(subject).to match({
+        are_public: 0,
+        are_internal: 0,
+        are_private: 0,
+        are_public_or_internal: 0,
+        total: 0
+      })
+    end
+
+    it 'only counts snippets the user has access to' do
+      create(:personal_snippet, :private, author: user)
+      create(:project_snippet, :private, author: user)
+      create(:project_snippet, :private, author: create(:user))
+
+      expect(subject).to match({
+        are_public: 0,
+        are_internal: 0,
+        are_private: 1,
+        are_public_or_internal: 0,
+        total: 1
+      })
+    end
+
+    it 'returns an empty hash if select returns nil' do
+      allow_next_instance_of(described_class) do |instance|
+        allow(instance).to receive(:snippet_counts).and_return(nil)
+      end
+
+      expect(subject).to match({})
+    end
+  end
+end

spec/support/shared_examples/features/snippets_shared_examples.rb

@@ -18,3 +18,35 @@ RSpec.shared_examples 'paginated snippets' do |remote: false|
     end
   end
 end
+
+RSpec.shared_examples 'tabs with counts' do
+  let(:tabs) { page.all('.snippet-scope-menu li') }
+
+  it 'shows a tab for All snippets and count' do
+    tab = tabs[0]
+
+    expect(tab.text).to include('All')
+    expect(tab.find('.badge').text).to eq(counts[:all])
+  end
+
+  it 'shows a tab for Private snippets and count' do
+    tab = tabs[1]
+
+    expect(tab.text).to include('Private')
+    expect(tab.find('.badge').text).to eq(counts[:private])
+  end
+
+  it 'shows a tab for Internal snippets and count' do
+    tab = tabs[2]
+
+    expect(tab.text).to include('Internal')
+    expect(tab.find('.badge').text).to eq(counts[:internal])
+  end
+
+  it 'shows a tab for Public snippets and count' do
+    tab = tabs[3]
+
+    expect(tab.text).to include('Public')
+    expect(tab.find('.badge').text).to eq(counts[:public])
+  end
+end