From 543d7890fd818ed8871b0f8e6adbf19b605edf70 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Wed, 10 Jan 2024 08:36:06 -0500 Subject: [PATCH] Use normalizes to prepare `User` values (#28650) Co-authored-by: Claire --- app/models/user.rb | 22 ++++-------------- spec/models/user_spec.rb | 50 ++++++++++++++++++++++++++++++---------- 2 files changed, 42 insertions(+), 30 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 55f654137a..0c589f8068 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -118,14 +118,15 @@ class User < ApplicationRecord scope :matches_email, ->(value) { where(arel_table[:email].matches("#{value}%")) } scope :matches_ip, ->(value) { left_joins(:ips).where('user_ips.ip <<= ?', value).group('users.id') } - before_validation :sanitize_languages before_validation :sanitize_role - before_validation :sanitize_time_zone - before_validation :sanitize_locale before_create :set_approved after_commit :send_pending_devise_notifications after_create_commit :trigger_webhooks + normalizes :locale, with: ->(locale) { I18n.available_locales.exclude?(locale.to_sym) ? nil : locale } + normalizes :time_zone, with: ->(time_zone) { ActiveSupport::TimeZone[time_zone].nil? ? nil : time_zone } + normalizes :chosen_languages, with: ->(chosen_languages) { chosen_languages.compact_blank.presence } + # This avoids a deprecation warning from Rails 5.1 # It seems possible that a future release of devise-two-factor will # handle this itself, and this can be removed from our User class. @@ -447,25 +448,10 @@ class User < ApplicationRecord @bypass_invite_request_check end - def sanitize_languages - return if chosen_languages.nil? - - chosen_languages.compact_blank! - self.chosen_languages = nil if chosen_languages.empty? - end - def sanitize_role self.role = nil if role.present? && role.everyone? end - def sanitize_time_zone - self.time_zone = nil if time_zone.present? && ActiveSupport::TimeZone[time_zone].nil? - end - - def sanitize_locale - self.locale = nil if locale.present? && I18n.available_locales.exclude?(locale.to_sym) - end - def prepare_new_user! BootstrapTimelineWorker.perform_async(account_id) ActivityTracker.increment('activity:accounts:local') diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c8415df92a..7f68671df4 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -38,23 +38,49 @@ RSpec.describe User do user.save(validate: false) expect(user.valid?).to be true end + end - it 'cleans out invalid locale' do - user = Fabricate.build(:user, locale: 'toto') - expect(user.valid?).to be true - expect(user.locale).to be_nil + describe 'Normalizations' do + describe 'locale' do + it 'preserves valid locale' do + user = Fabricate.build(:user, locale: 'en') + + expect(user.locale).to eq('en') + end + + it 'cleans out invalid locale' do + user = Fabricate.build(:user, locale: 'toto') + + expect(user.locale).to be_nil + end end - it 'cleans out invalid timezone' do - user = Fabricate.build(:user, time_zone: 'toto') - expect(user.valid?).to be true - expect(user.time_zone).to be_nil + describe 'time_zone' do + it 'preserves valid timezone' do + user = Fabricate.build(:user, time_zone: 'UTC') + + expect(user.time_zone).to eq('UTC') + end + + it 'cleans out invalid timezone' do + user = Fabricate.build(:user, time_zone: 'toto') + + expect(user.time_zone).to be_nil + end end - it 'cleans out empty string from languages' do - user = Fabricate.build(:user, chosen_languages: ['']) - user.valid? - expect(user.chosen_languages).to be_nil + describe 'languages' do + it 'preserves valid options for languages' do + user = Fabricate.build(:user, chosen_languages: ['en', 'fr', '']) + + expect(user.chosen_languages).to eq(['en', 'fr']) + end + + it 'cleans out empty string from languages' do + user = Fabricate.build(:user, chosen_languages: ['']) + + expect(user.chosen_languages).to be_nil + end end end