Skip to content

Commit

Permalink
Merge pull request #16404 from opf/bug/57077-admin-only-user-custom-f…
Browse files Browse the repository at this point in the history
…ield-is-visible-to-non-admin-users

[#57077] Admin only User Custom field is visible to non-admin users
  • Loading branch information
aaron-contreras authored Aug 13, 2024
2 parents 68327df + 2f7f173 commit 6309cd0
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 22 deletions.
24 changes: 9 additions & 15 deletions app/models/deleted_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,13 @@ def self.first
end

# Overrides a few properties
def logged?; false end

def builtin?; true end

def admin; false end

def name(*_args); I18n.t("user.deleted") end

def mail; nil end

def time_zone; nil end

def rss_key; nil end

def destroy; false end
def available_custom_fields = []
def logged? = false
def builtin? = true
def admin = false
def name(*_args) = I18n.t("user.deleted")
def mail = nil
def time_zone = nil
def rss_key = nil
def destroy = false
end
9 changes: 9 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,15 @@ def mail=(arg)
write_attribute(:mail, arg.to_s.strip)
end

def self.available_custom_fields(_user)
user = User.current
RequestStore.fetch(:"#{name.underscore}_custom_fields_#{user.id}_#{user.admin?}") do
scope = CustomField.where(type: "#{name}CustomField").order(:position)
scope = scope.where(admin_only: false) if !user.admin?
scope
end
end

def self.search_in_project(query, options)
options.fetch(:project).users.like(query)
end
Expand Down
13 changes: 7 additions & 6 deletions spec/models/principals/scopes/ordered_by_name_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

RSpec.describe Principals::Scopes::OrderedByName do
describe ".ordered_by_name" do
shared_let(:anonymous) { User.anonymous }
shared_let(:alice) { create(:user, login: "alice", firstname: "Alice", lastname: "Zetop") }
shared_let(:eve) { create(:user, login: "eve", firstname: "Eve", lastname: "Baddie") }

Expand All @@ -56,37 +57,37 @@

context "with default user sort", with_settings: { user_format: :firstname_lastname } do
it_behaves_like "sorted results" do
let(:order) { [alice.id, group.id, placeholder_user.id, eve.id] }
let(:order) { [alice.id, anonymous.id, group.id, placeholder_user.id, eve.id] }
end
end

context "with lastname_firstname user sort", with_settings: { user_format: :lastname_firstname } do
it_behaves_like "sorted results" do
let(:order) { [eve.id, group.id, placeholder_user.id, alice.id] }
let(:order) { [anonymous.id, eve.id, group.id, placeholder_user.id, alice.id] }
end
end

context "with lastname_n_firstname user sort", with_settings: { user_format: :lastname_n_firstname } do
it_behaves_like "sorted results" do
let(:order) { [eve.id, group.id, placeholder_user.id, alice.id] }
let(:order) { [anonymous.id, eve.id, group.id, placeholder_user.id, alice.id] }
end
end

context "with lastname_coma_firstname user sort", with_settings: { user_format: :lastname_coma_firstname } do
it_behaves_like "sorted results" do
let(:order) { [eve.id, group.id, placeholder_user.id, alice.id] }
let(:order) { [anonymous.id, eve.id, group.id, placeholder_user.id, alice.id] }
end
end

context "with firstname user sort", with_settings: { user_format: :firstname } do
it_behaves_like "sorted results" do
let(:order) { [alice.id, group.id, placeholder_user.id, eve.id] }
let(:order) { [alice.id, anonymous.id, group.id, placeholder_user.id, eve.id] }
end
end

context "with login user sort", with_settings: { user_format: :username } do
it_behaves_like "sorted results" do
let(:order) { [alice.id, group.id, placeholder_user.id, eve.id] }
let(:order) { [alice.id, anonymous.id, group.id, placeholder_user.id, eve.id] }
end
end
end
Expand Down
26 changes: 26 additions & 0 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1016,4 +1016,30 @@ def build_user_double_with_expired_password(is_expired)
let(:model_instance) { user }
let(:custom_field) { create(:user_custom_field, :string) }
end

describe ".available_custom_fields" do
let(:admin) { build_stubbed(:admin) }
let(:user) { build_stubbed(:user) }

shared_let(:user_cf) { create(:user_custom_field) }
shared_let(:admin_user_cf) { create(:user_custom_field, admin_only: true) }

context "for an admin" do
current_user { admin }

it "returns all fields including admin-only" do
expect(user.available_custom_fields)
.to contain_exactly(user_cf, admin_user_cf)
end
end

context "for a member" do
current_user { user }

it "does not return admin-only field" do
expect(user.available_custom_fields)
.to contain_exactly(user_cf)
end
end
end
end
8 changes: 7 additions & 1 deletion spec/models/users/scopes/with_time_zone_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
preferences: { time_zone: "" }
)
end
shared_let(:anonymous) { User.anonymous }

describe ".with_time_zone" do
it "returns user having set a time zone in their preference matching the specified time zone(s)" do
Expand Down Expand Up @@ -119,7 +120,12 @@
it "assumes Etc/UTC as default time zone",
with_settings: { user_default_timezone: nil } do
expect(User.with_time_zone("Etc/UTC"))
.to contain_exactly(user_without_preferences, user_without_time_zone, user_with_empty_time_zone)
.to contain_exactly(
user_without_preferences,
user_without_time_zone,
user_with_empty_time_zone,
anonymous
)
end
end
end
Expand Down

0 comments on commit 6309cd0

Please sign in to comment.