From bba537a7bebe11be4b1e4a7e126a34cd27b73678 Mon Sep 17 00:00:00 2001
From: Matt Jankowski <mjankowski@thoughtbot.com>
Date: Sun, 7 May 2017 21:32:52 -0400
Subject: [PATCH] Improve allowed language handling (#2897)

* Dont allow empty value in user allowed languages

* Sanitize language input to reject blank values in array
---
 app/models/user.rb                                   |  8 ++++++++
 .../settings/preferences_controller_spec.rb          |  2 +-
 spec/models/status_spec.rb                           | 12 ++++++++++++
 spec/models/user_spec.rb                             |  6 ++++++
 4 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index f8e8a2efa24..dfecb2339f3 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -52,6 +52,8 @@ class User < ApplicationRecord
   scope :admins,    -> { where(admin: true) }
   scope :confirmed, -> { where.not(confirmed_at: nil) }
 
+  before_validation :sanitize_languages
+
   def confirmed?
     confirmed_at.present?
   end
@@ -77,4 +79,10 @@ class User < ApplicationRecord
   def setting_auto_play_gif
     settings.auto_play_gif
   end
+
+  private
+
+  def sanitize_languages
+    allowed_languages.reject!(&:blank?)
+  end
 end
diff --git a/spec/controllers/settings/preferences_controller_spec.rb b/spec/controllers/settings/preferences_controller_spec.rb
index 432e35cd451..6805a2ce0c6 100644
--- a/spec/controllers/settings/preferences_controller_spec.rb
+++ b/spec/controllers/settings/preferences_controller_spec.rb
@@ -18,7 +18,7 @@ describe Settings::PreferencesController do
 
   describe 'PUT #update' do
     it 'updates the user record' do
-      put :update, params: { user: { locale: 'en', allowed_languages: ['es', 'fr'] } }
+      put :update, params: { user: { locale: 'en', allowed_languages: ['es', 'fr', ''] } }
 
       expect(response).to redirect_to(settings_preferences_path)
       user.reload
diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb
index 0c0b16829c3..721951030ed 100644
--- a/spec/models/status_spec.rb
+++ b/spec/models/status_spec.rb
@@ -265,6 +265,18 @@ RSpec.describe Status, type: :model do
           expect(results).not_to include(fr_status)
         end
 
+        it 'includes all languages when user does not have a setting' do
+          user = Fabricate(:user, allowed_languages: [])
+          @account.update(user: user)
+
+          en_status = Fabricate(:status, language: 'en')
+          es_status = Fabricate(:status, language: 'es')
+
+          results = Status.as_public_timeline(@account)
+          expect(results).to include(en_status)
+          expect(results).to include(es_status)
+        end
+
         it 'includes all languages when account does not have a user' do
           expect(@account.user).to be_nil
           en_status = Fabricate(:status, language: 'en')
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index fffd92e3d8a..04c39de3b28 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -22,6 +22,12 @@ RSpec.describe User, type: :model do
       user.valid?
       expect(user).to model_have_error_on_field(:email)
     end
+
+    it 'cleans out empty string from languages' do
+      user = Fabricate.build(:user, allowed_languages: [''])
+      user.valid?
+      expect(user.allowed_languages).to eq []
+    end
   end
 
   describe 'settings' do