From 38ba59bedd8b7e2f1a4de08725d6e000bf4fde42 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 14 Aug 2019 17:29:41 -0300 Subject: Fix starrers counters after searching --- app/controllers/projects/starrers_controller.rb | 18 +-- .../projects/starrers_controller_spec.rb | 152 +++++++++++++++------ 2 files changed, 114 insertions(+), 56 deletions(-) diff --git a/app/controllers/projects/starrers_controller.rb b/app/controllers/projects/starrers_controller.rb index c8facea1d70..e4093bed0ef 100644 --- a/app/controllers/projects/starrers_controller.rb +++ b/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 diff --git a/spec/controllers/projects/starrers_controller_spec.rb b/spec/controllers/projects/starrers_controller_spec.rb index 59d258e99ce..7085cba08d5 100644 --- a/spec/controllers/projects/starrers_controller_spec.rb +++ b/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 -- cgit v1.2.3 From 4596d5d1a2eb8f918f9f59d85aed53aa232f1fcf Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 14 Aug 2019 17:48:13 -0300 Subject: Add CHANGELOG entry --- .../unreleased/66023-starrers-count-do-not-match-after-searching.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/66023-starrers-count-do-not-match-after-searching.yml diff --git a/changelogs/unreleased/66023-starrers-count-do-not-match-after-searching.yml b/changelogs/unreleased/66023-starrers-count-do-not-match-after-searching.yml new file mode 100644 index 00000000000..1caa5fa84ce --- /dev/null +++ b/changelogs/unreleased/66023-starrers-count-do-not-match-after-searching.yml @@ -0,0 +1,5 @@ +--- +title: Fix starrers counts after searching +merge_request: 31823 +author: +type: fixed -- cgit v1.2.3