diff --git a/lib/spree/core/mail_settings.rb b/lib/spree/core/mail_settings.rb index 07d8dbee7a..0373f7fac7 100644 --- a/lib/spree/core/mail_settings.rb +++ b/lib/spree/core/mail_settings.rb @@ -20,18 +20,14 @@ module Spree private def mail_server_settings - settings = if need_authentication? - basic_settings.merge(user_credentials) - else - basic_settings - end + settings = basic_settings.merge(user_credentials) settings.merge(enable_starttls_auto: secure_connection?) end def user_credentials - { user_name: Config.smtp_username, - password: Config.smtp_password } + { user_name: Config.smtp_username.presence, + password: Config.smtp_password.presence } end def basic_settings @@ -42,13 +38,10 @@ module Spree end def authentication - return unless need_authentication? - - Config.mail_auth_type.presence - end - - def need_authentication? - Config.mail_auth_type != 'None' + # "None" is an option in the UI but not a real authentication type. + # We should remove it from our host configurations but I'm keeping + # this check for backwards-compatibility for now. + Config.mail_auth_type.presence unless Config.mail_auth_type == "None" end def secure_connection? diff --git a/spec/lib/spree/core/mail_settings_spec.rb b/spec/lib/spree/core/mail_settings_spec.rb index d1a2fc99b4..6a40495695 100644 --- a/spec/lib/spree/core/mail_settings_spec.rb +++ b/spec/lib/spree/core/mail_settings_spec.rb @@ -8,12 +8,12 @@ module Spree let!(:subject) { MailSettings.new } context "overrides appplication defaults" do - context "authentication method is none" do + context "authentication method is login" do before do Config.mail_host = "smtp.example.com" Config.mail_domain = "example.com" Config.mail_port = 123 - Config.mail_auth_type = MailSettings::SECURE_CONNECTION_TYPES[0] + Config.mail_auth_type = "login" Config.smtp_username = "schof" Config.smtp_password = "hellospree!" Config.secure_connection_type = "TLS" @@ -23,31 +23,22 @@ module Spree it { expect(ActionMailer::Base.smtp_settings[:address]).to eq "smtp.example.com" } it { expect(ActionMailer::Base.smtp_settings[:domain]).to eq "example.com" } it { expect(ActionMailer::Base.smtp_settings[:port]).to eq 123 } - it { expect(ActionMailer::Base.smtp_settings[:authentication]).to eq nil } + it { expect(ActionMailer::Base.smtp_settings[:authentication]).to eq "login" } it { expect(ActionMailer::Base.smtp_settings[:enable_starttls_auto]).to be_truthy } - - it "doesnt touch user name config" do - expect(ActionMailer::Base.smtp_settings[:user_name]).to be_nil - end - - it "doesnt touch password config" do - expect(ActionMailer::Base.smtp_settings[:password]).to be_nil - end - end - end - - context "when mail_auth_type is other than none" do - before do - Config.mail_auth_type = "login" - Config.smtp_username = "schof" - Config.smtp_password = "hellospree!" - subject.override! - end - - context "overrides user credentials" do it { expect(ActionMailer::Base.smtp_settings[:user_name]).to eq "schof" } it { expect(ActionMailer::Base.smtp_settings[:password]).to eq "hellospree!" } end + + context "authentication method is none" do + before do + Config.mail_auth_type = "None" + subject.override! + end + + it "doesn't store 'None' as auth method" do + expect(ActionMailer::Base.smtp_settings[:authentication]).to eq nil + end + end end end end