Use normalizes to prepare User values (#28650)

Co-authored-by: Claire <claire.github-309c@sitedethib.com>

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')

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