Handle reviewer comments on !24690

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