Merge branch '66023-starrers-count-do-not-match-after-searching' into 'master'

Fix starrers counts after searching

See merge request gitlab-org/gitlab-ce!31823

app/controllers/projects/starrers_controller.rb

@@ -5,23 +5,9 @@ class Projects::StarrersController < Projects::ApplicationController
 
   def index
     @starrers = UsersStarProjectsFinder.new(@project, params, current_user: @current_user).execute
-
-    # Normally the number of public starrers is equal to the number of visible
-    # starrers. We need to fix the counts in two cases: when the current user
-    # is an admin (and can see everything) and when the current user has a
-    # private profile and has starred the project (and can see itself).
-    @public_count =
-      if @current_user&.admin?
-        @starrers.with_public_profile.count
-      elsif @current_user&.private_profile && has_starred_project?(@starrers)
-        @starrers.size - 1
-      else
-        @starrers.size
-      end
-
-    @total_count = @project.starrers.size
+    @public_count  = @project.starrers.with_public_profile.size
+    @total_count   = @project.starrers.size
     @private_count = @total_count - @public_count
-
     @sort = params[:sort].presence || sort_value_name
     @starrers = @starrers.sort_by_attribute(@sort).page(params[:page])
   end

changelogs/unreleased/66023-starrers-count-do-not-match-after-searching.yml

+---
+title: Fix starrers counts after searching
+merge_request: 31823
+author:
+type: fixed

spec/controllers/projects/starrers_controller_spec.rb

@@ -3,23 +3,33 @@
 require 'spec_helper'
 
 describe Projects::StarrersController do
-  let(:user) { create(:user) }
-  let(:private_user) { create(:user, private_profile: true) }
+  let(:user_1) { create(:user, name: 'John') }
+  let(:user_2) { create(:user, name: 'Michael') }
+  let(:private_user) { create(:user, name: 'Michael Douglas', private_profile: true) }
   let(:admin) { create(:user, admin: true) }
-  let(:project) { create(:project, :public, :repository) }
+  let(:project) { create(:project, :public) }
 
   before do
-    user.toggle_star(project)
+    user_1.toggle_star(project)
+    user_2.toggle_star(project)
     private_user.toggle_star(project)
   end
 
   describe 'GET index' do
-    def get_starrers
-      get :index,
-        params: {
-          namespace_id: project.namespace,
-          project_id: project
-        }
+    def get_starrers(search: nil)
+      get :index, params: { namespace_id: project.namespace, project_id: project, search: search }
+    end
+
+    def user_ids
+      assigns[:starrers].map { |s| s['user_id'] }
+    end
+
+    shared_examples 'starrers counts' do
+      it 'starrers counts are correct' do
+        expect(assigns[:total_count]).to eq(3)
+        expect(assigns[:public_count]).to eq(2)
+        expect(assigns[:private_count]).to eq(1)
+      end
     end
 
     context 'when project is public' do
@@ -28,55 +38,118 @@ describe Projects::StarrersController do
       end
 
       context 'when no user is logged in' do
+        context 'with no searching' do
+          before do
+            get_starrers
+          end
+
+          it 'only users with public profiles are visible' do
+            expect(user_ids).to contain_exactly(user_1.id, user_2.id)
+          end
+
+          include_examples 'starrers counts'
+        end
+
+        context 'when searching by user' do
+          before do
+            get_starrers(search: 'Michael')
+          end
+
+          it 'only users with public profiles are visible' do
+            expect(user_ids).to contain_exactly(user_2.id)
+          end
+
+          include_examples 'starrers counts'
+        end
+      end
+
+      context 'when public user is logged in' do
         before do
-          get_starrers
+          sign_in(user_1)
         end
 
-        it 'only public starrers are visible' do
-          user_ids = assigns[:starrers].map { |s| s['user_id'] }
-          expect(user_ids).to include(user.id)
-          expect(user_ids).not_to include(private_user.id)
+        context 'with no searching' do
+          before do
+            get_starrers
+          end
+
+          it 'their star is also visible' do
+            expect(user_ids).to contain_exactly(user_1.id, user_2.id)
+          end
+
+          include_examples 'starrers counts'
         end
 
-        it 'public/private starrers counts are correct' do
-          expect(assigns[:public_count]).to eq(1)
-          expect(assigns[:private_count]).to eq(1)
+        context 'when searching by user' do
+          before do
+            get_starrers(search: 'Michael')
+          end
+
+          it 'only users with public profiles are visible' do
+            expect(user_ids).to contain_exactly(user_2.id)
+          end
+
+          include_examples 'starrers counts'
         end
       end
 
       context 'when private user is logged in' do
         before do
           sign_in(private_user)
-
-          get_starrers
         end
 
-        it 'their star is also visible' do
-          user_ids = assigns[:starrers].map { |s| s['user_id'] }
-          expect(user_ids).to include(user.id, private_user.id)
+        context 'with no searching' do
+          before do
+            get_starrers
+          end
+
+          it 'their star is also visible' do
+            expect(user_ids).to contain_exactly(user_1.id, user_2.id, private_user.id)
+          end
+
+          include_examples 'starrers counts'
         end
 
-        it 'public/private starrers counts are correct' do
-          expect(assigns[:public_count]).to eq(1)
-          expect(assigns[:private_count]).to eq(1)
+        context 'when searching by user' do
+          before do
+            get_starrers(search: 'Michael')
+          end
+
+          it 'only users with public profiles are visible' do
+            expect(user_ids).to contain_exactly(user_2.id, private_user.id)
+          end
+
+          include_examples 'starrers counts'
         end
       end
 
       context 'when admin is logged in' do
         before do
           sign_in(admin)
-
-          get_starrers
         end
 
-        it 'all stars are visible' do
-          user_ids = assigns[:starrers].map { |s| s['user_id'] }
-          expect(user_ids).to include(user.id, private_user.id)
+        context 'with no searching' do
+          before do
+            get_starrers
+          end
+
+          it 'all users are visible' do
+            expect(user_ids).to include(user_1.id, user_2.id, private_user.id)
+          end
+
+          include_examples 'starrers counts'
         end
 
-        it 'public/private starrers counts are correct' do
-          expect(assigns[:public_count]).to eq(1)
-          expect(assigns[:private_count]).to eq(1)
+        context 'when searching by user' do
+          before do
+            get_starrers(search: 'Michael')
+          end
+
+          it 'public and private starrers are visible' do
+            expect(user_ids).to contain_exactly(user_2.id, private_user.id)
+          end
+
+          include_examples 'starrers counts'
         end
       end
     end
@@ -95,15 +168,14 @@ describe Projects::StarrersController do
       context 'when user is logged in' do
         before do
           sign_in(project.creator)
-        end
-
-        it 'only public starrers are visible' do
           get_starrers
+        end
 
-          user_ids = assigns[:starrers].map { |s| s['user_id'] }
-          expect(user_ids).to include(user.id)
-          expect(user_ids).not_to include(private_user.id)
+        it 'only users with public profiles are visible' do
+          expect(user_ids).to contain_exactly(user_1.id, user_2.id)
         end
+
+        include_examples 'starrers counts'
       end
     end
   end