remove authentication, user_name and password keys if set to blank or…

… nil #2048

app/controllers/admin_controller.rb

@@ -539,11 +539,11 @@ def test_email_configuration
     smtp_hash_new = { address: params[:address],
                       enable_starttls_auto: params[:enable_starttls_auto] == '1',
                       domain: params[:domain],
-                      authentication: params[:authentication],
+                      authentication: (params[:authentication].blank? ? nil : params[:authentication] ),
                       user_name: (params[:smtp_user_name].blank? ? nil : params[:smtp_user_name]),
                       password: (params[:smtp_password].blank? ? nil : params[:smtp_password]) }
     smtp_hash_new[:port] = params[:port] if only_integer params[:port], 'port'
-    ActionMailer::Base.smtp_settings = smtp_hash_new
+    ActionMailer::Base.smtp_settings = smtp_hash_new.compact
     raise_delivery_errors_setting = ActionMailer::Base.raise_delivery_errors
     ActionMailer::Base.raise_delivery_errors = true
     begin

lib/seek/config.rb

@@ -45,7 +45,7 @@ def smtp_propagate
         new_hash[key.to_sym] = smtp_hash[key]
       end
 
-      ActionMailer::Base.smtp_settings = new_hash
+      ActionMailer::Base.smtp_settings = new_hash.compact
     end
 
     def bioportal_api_key_propagate

test/functional/admin_controller_test.rb

@@ -89,7 +89,7 @@ def setup
       with_config_value(:smtp, { address: '255.255.255.255', 'address' => '0.0.0.0' }) do
         assert_equal 'Hash', Seek::Config.smtp.class.name
 
-        post :update_features_enabled, params: { email_enabled: '1', address: '127.0.0.1', port: '25', domain: 'email.example.com', authentication: 'plain', smtp_user_name: '', smtp_password: '', enable_starttls_auto: '1' }
+        post :update_features_enabled, params: { email_enabled: '1', address: '127.0.0.1', port: '25', domain: 'email.example.com', authentication: 'plain', smtp_user_name: 'fred', smtp_password: 'bbb', enable_starttls_auto: '1' }
 
         assert_equal 'ActiveSupport::HashWithIndifferentAccess', Seek::Config.smtp.class.name
         assert Seek::Config.email_enabled
@@ -99,13 +99,32 @@ def setup
         assert_equal '25', mailer_settings[:port]
         assert_equal 'email.example.com', mailer_settings[:domain]
         assert_equal 'plain', mailer_settings[:authentication]
-        assert_nil mailer_settings[:user_name]
-        assert_nil mailer_settings[:password]
+        assert_equal 'fred', mailer_settings[:user_name]
+        assert_equal 'bbb', mailer_settings[:password]
         assert mailer_settings[:enable_starttls_auto]
       end
     end
   end
 
+  test 'update SMTP settings nil authentication details removed' do
+    with_config_value(:email_enabled, false) do
+      with_config_value(:smtp, { address: '255.255.255.255', 'address' => '0.0.0.0' }) do
+        assert_equal 'Hash', Seek::Config.smtp.class.name
+
+        post :update_features_enabled, params: { email_enabled: '1', address: '127.0.0.1', port: '25', domain: 'email.example.com', authentication: '', smtp_user_name: '', smtp_password: '', enable_starttls_auto: '0' }
+
+        mailer_settings = ActionMailer::Base.smtp_settings
+        assert_equal '127.0.0.1', mailer_settings[:address]
+        assert_equal '25', mailer_settings[:port]
+        assert_equal 'email.example.com', mailer_settings[:domain]
+        refute mailer_settings[:enable_starttls_auto]
+        refute mailer_settings.has_key?(:authentication)
+        refute mailer_settings.has_key?(:user_name)
+        refute mailer_settings.has_key?(:password)
+      end
+    end
+  end
+
   test 'should read SMTP setting as a HashWithIndifferentAccess' do
     with_config_value(:smtp, { address: '255.255.255.255', 'domain' => 'email.example.com' }) do
       assert_equal 'Hash', Seek::Config.smtp.class.name