FIX: bug with multiselect user field validation (#17498)
* FIX: properly validate multiselect user fields on user creation * Add test cases * FIX: don't check multiselect user fields for watched words * Clarifiy/simplify tests * Roll back apply_watched_words changes Since this method no longer needs to deal with arrays for now. If/when we add new user fields which uses them, we can deal with it then.
This commit is contained in:
parent
3690953a3f
commit
40222eb524
|
@ -120,7 +120,7 @@ class User < ActiveRecord::Base
|
||||||
validates :name, user_full_name: true, if: :will_save_change_to_name?, length: { maximum: 255 }
|
validates :name, user_full_name: true, if: :will_save_change_to_name?, length: { maximum: 255 }
|
||||||
validates :ip_address, allowed_ip_address: { on: :create, message: :signup_not_allowed }
|
validates :ip_address, allowed_ip_address: { on: :create, message: :signup_not_allowed }
|
||||||
validates :primary_email, presence: true
|
validates :primary_email, presence: true
|
||||||
validates :public_user_field_values, watched_words: true, unless: :custom_fields_clean?
|
validates :validatable_user_fields_values, watched_words: true, unless: :custom_fields_clean?
|
||||||
validates_associated :primary_email, message: -> (_, user_email) { user_email[:value]&.errors[:email]&.first }
|
validates_associated :primary_email, message: -> (_, user_email) { user_email[:value]&.errors[:email]&.first }
|
||||||
|
|
||||||
after_initialize :add_trust_level
|
after_initialize :add_trust_level
|
||||||
|
@ -1257,7 +1257,7 @@ class User < ActiveRecord::Base
|
||||||
|
|
||||||
USER_FIELD_PREFIX ||= "user_field_"
|
USER_FIELD_PREFIX ||= "user_field_"
|
||||||
|
|
||||||
def user_fields(field_ids = nil)
|
def user_fields(field_ids = nil, exclude_types: [])
|
||||||
if field_ids.nil?
|
if field_ids.nil?
|
||||||
field_ids = (@all_user_field_ids ||= UserField.pluck(:id))
|
field_ids = (@all_user_field_ids ||= UserField.pluck(:id))
|
||||||
end
|
end
|
||||||
|
@ -1273,8 +1273,8 @@ class User < ActiveRecord::Base
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def public_user_field_values
|
def validatable_user_fields_values
|
||||||
public_user_fields.values.join(" ")
|
validatable_user_fields.values.join(" ")
|
||||||
end
|
end
|
||||||
|
|
||||||
def set_user_field(field_id, value)
|
def set_user_field(field_id, value)
|
||||||
|
@ -1282,13 +1282,16 @@ class User < ActiveRecord::Base
|
||||||
end
|
end
|
||||||
|
|
||||||
def apply_watched_words
|
def apply_watched_words
|
||||||
public_user_fields.each do |id, value|
|
validatable_user_fields.each do |id, value|
|
||||||
set_user_field(id, PrettyText.cook(value).gsub(/^<p>(.*)<\/p>$/, "\\1"))
|
set_user_field(id, PrettyText.cook(value).gsub(/^<p>(.*)<\/p>$/, "\\1"))
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def public_user_fields
|
def validatable_user_fields
|
||||||
@public_user_field_ids ||= UserField.public_fields.pluck(:id)
|
# ignore multiselect fields since they are admin-set and thus not UGC
|
||||||
|
relation = UserField.public_fields.where.not(field_type: 'multiselect')
|
||||||
|
|
||||||
|
@public_user_field_ids ||= relation.pluck(:id)
|
||||||
user_fields(@public_user_field_ids)
|
user_fields(@public_user_field_ids)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -223,6 +223,41 @@ RSpec.describe User do
|
||||||
|
|
||||||
it { is_expected.to be_valid }
|
it { is_expected.to be_valid }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context "with a multiselect user field" do
|
||||||
|
fab!(:user_field) do
|
||||||
|
Fabricate(:user_field, field_type: 'multiselect', show_on_profile: true) do
|
||||||
|
user_field_options do
|
||||||
|
[
|
||||||
|
Fabricate(:user_field_option, value: 'Axe'),
|
||||||
|
Fabricate(:user_field_option, value: 'Sword')
|
||||||
|
]
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
let(:user_field_value) { user.reload.user_fields[user_field.id.to_s] }
|
||||||
|
|
||||||
|
context "with a blocked word" do
|
||||||
|
let(:value) { %w{ Axe bad Sword } }
|
||||||
|
|
||||||
|
it "does not block the word since it is not user generated-content" do
|
||||||
|
user.save!
|
||||||
|
expect(user_field_value).to eq %w{ Axe bad Sword }
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context "with a censored word" do
|
||||||
|
let(:value) { %w{ Axe bad Sword } }
|
||||||
|
before { watched_word.action = WatchedWord.actions[:censor] }
|
||||||
|
|
||||||
|
it "does not censor the word since it is not user generated-content" do
|
||||||
|
user.save!
|
||||||
|
expect(user_field_value).to eq %w{ Axe bad Sword }
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue