diff options
author | Camil Staps <info@camilstaps.nl> | 2019-08-03 13:57:33 +0300 |
---|---|---|
committer | Camil Staps <info@camilstaps.nl> | 2019-08-07 21:49:37 +0300 |
commit | e726ed5e128893158f102b87e1407ec0a36fc3ce (patch) | |
tree | 279a9ccc0f22a461da6a4298bc803d60cffc32b1 | |
parent | d2b2486afb5a50ace08bc416be87a1ead12abe3c (diff) |
Handle reviewer comments on !24690
-rw-r--r-- | app/finders/starred_projects_finder.rb | 8 | ||||
-rw-r--r-- | app/finders/users_star_projects_finder.rb | 2 | ||||
-rw-r--r-- | app/models/user.rb | 14 | ||||
-rw-r--r-- | app/models/users_star_project.rb | 4 | ||||
-rw-r--r-- | spec/controllers/projects/starrers_controller_spec.rb | 2 | ||||
-rw-r--r-- | spec/finders/starred_projects_finder_spec.rb | 8 | ||||
-rw-r--r-- | spec/finders/users_star_projects_finder_spec.rb | 8 |
7 files changed, 28 insertions, 18 deletions
diff --git a/app/finders/starred_projects_finder.rb b/app/finders/starred_projects_finder.rb index e38cb8c4569..fcb469d1d17 100644 --- a/app/finders/starred_projects_finder.rb +++ b/app/finders/starred_projects_finder.rb @@ -2,8 +2,10 @@ class StarredProjectsFinder < ProjectsFinder def initialize(user, params: {}, current_user: nil) - project_ids = user.starred_projects.select(:id) - - super(params: params, current_user: current_user, project_ids_relation: project_ids) + super( + params: params, + current_user: current_user, + project_ids_relation: user.starred_projects.select(:id) + ) end end diff --git a/app/finders/users_star_projects_finder.rb b/app/finders/users_star_projects_finder.rb index 5b19b98451c..49c4e087b4b 100644 --- a/app/finders/users_star_projects_finder.rb +++ b/app/finders/users_star_projects_finder.rb @@ -12,7 +12,7 @@ class UsersStarProjectsFinder end def execute - stars = UsersStarProject.all.order_id_desc + stars = UsersStarProject.all stars = by_project(stars) stars = by_search(stars) stars = filter_visible_profiles(stars) diff --git a/app/models/user.rb b/app/models/user.rb index cfb8cd3819d..ac83c8e3256 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -282,15 +282,17 @@ class User < ApplicationRecord scope :for_todos, -> (todos) { where(id: todos.select(:user_id)) } scope :with_emails, -> { preload(:emails) } scope :with_dashboard, -> (dashboard) { where(dashboard: dashboard) } - scope :with_visible_profile, -> (user) { - if user.nil? - where(private_profile: false) - elsif user.admin? + scope :with_public_profile, -> { where(private_profile: false) } + + def self.with_visible_profile(user) + return with_public_profile if user.nil? + + if user.admin? all else - where(private_profile: false).or where(id: user.id) + with_public_profile.or(where(id: user.id)) end - } + end # Limits the users to those that have TODOs, optionally in the given state. # diff --git a/app/models/users_star_project.rb b/app/models/users_star_project.rb index 04a48739d93..baf055dd6dd 100644 --- a/app/models/users_star_project.rb +++ b/app/models/users_star_project.rb @@ -12,8 +12,8 @@ class UsersStarProject < ApplicationRecord alias_attribute :starred_since, :created_at - scope :order_user_name_asc, -> { joins(:user).reorder('users.name ASC') } - scope :order_user_name_desc, -> { joins(:user).reorder('users.name DESC') } + scope :order_user_name_asc, -> { joins(:user).merge(User.order_name_asc) } + scope :order_user_name_desc, -> { joins(:user).merge(User.order_name_desc) } scope :by_project, -> (project) { where(project_id: project.id) } scope :with_visible_profile, -> (user) { joins(:user).merge(User.with_visible_profile(user)) } diff --git a/spec/controllers/projects/starrers_controller_spec.rb b/spec/controllers/projects/starrers_controller_spec.rb index 2ea80da08ca..863929a2465 100644 --- a/spec/controllers/projects/starrers_controller_spec.rb +++ b/spec/controllers/projects/starrers_controller_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Projects::StarrersController do diff --git a/spec/finders/starred_projects_finder_spec.rb b/spec/finders/starred_projects_finder_spec.rb index 6bc19af7a57..7aa8251c3ab 100644 --- a/spec/finders/starred_projects_finder_spec.rb +++ b/spec/finders/starred_projects_finder_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe StarredProjectsFinder do @@ -21,19 +23,19 @@ describe StarredProjectsFinder do describe 'as same user' do let(:current_user) { user } - it { is_expected.to eq([project2, project1]) } + it { is_expected.to contain_exactly(project1, project2) } end describe 'as other user' do let(:current_user) { other_user } - it { is_expected.to eq([project2, project1]) } + it { is_expected.to contain_exactly(project1, project2) } end describe 'as no user' do let(:current_user) { nil } - it { is_expected.to eq([project2, project1]) } + it { is_expected.to contain_exactly(project1, project2) } end end end diff --git a/spec/finders/users_star_projects_finder_spec.rb b/spec/finders/users_star_projects_finder_spec.rb index 4c0aa3f8f77..fb1d8088f44 100644 --- a/spec/finders/users_star_projects_finder_spec.rb +++ b/spec/finders/users_star_projects_finder_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe UsersStarProjectsFinder do @@ -22,19 +24,19 @@ describe UsersStarProjectsFinder do describe 'as same user' do let(:current_user) { private_user } - it { is_expected.to eq(private_stars + public_stars) } + it { is_expected.to match_array(private_stars + public_stars) } end describe 'as other user' do let(:current_user) { other_user } - it { is_expected.to eq(public_stars) } + it { is_expected.to match_array(public_stars) } end describe 'as no user' do let(:current_user) { nil } - it { is_expected.to eq(public_stars) } + it { is_expected.to match_array(public_stars) } end end end |