Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove smtp config from admin config page #5235

Merged
merged 2 commits into from Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 0 additions & 4 deletions app/controllers/spree/admin/mail_methods_controller.rb
Expand Up @@ -4,10 +4,6 @@ class MailMethodsController < Spree::Admin::BaseController
after_filter :initialize_mail_settings

def update
if params[:smtp_password].blank?
params.delete(:smtp_password)
end

params.each do |name, value|
next unless Spree::Config.has_preference? name

Expand Down
84 changes: 26 additions & 58 deletions app/views/spree/admin/mail_methods/_form.html.haml
@@ -1,60 +1,28 @@
%div
.row
.alpha.six.columns
%fieldset.no-border-bottom
%legend{align: "center"}= t("spree.general")
.field
= preference_field_tag("enable_mail_delivery", Spree::Config[:enable_mail_delivery], type: :boolean)
= label_tag :enable_mail_delivery, t("spree.enable_mail_delivery")
.field
= label_tag :mails_from, t("spree.send_mails_as")
%br/
= text_field_tag :mails_from, Spree::Config[:mails_from], maxlength: 256, class: 'fullwidth'
%br/
%span.info
= t("spree.smtp_send_all_emails_as_from_following_address")
.field
= label_tag :mail_bcc, t("spree.send_copy_of_all_mails_to")
%br/
= text_field_tag :mail_bcc, Spree::Config[:mail_bcc], maxlength: 256, class: 'fullwidth'
%br/
%span.info
= t("spree.smtp_send_copy_to_this_addresses")
.field
= label_tag :intercept_email, t("spree.intercept_email_address")
%br/
= text_field_tag :intercept_email, Spree::Config[:intercept_email], maxlength: 256, class: 'fullwidth'
%br/
%span.info
= t("spree.intercept_email_instructions")
.six.columns.omega
%fieldset.no-border-bottom
%legend{align: "center"}= t("spree.smtp")
.field
= label_tag :mail_domain, t("spree.smtp_domain")
%br/
= text_field_tag :mail_domain, Spree::Config[:mail_domain], class: 'fullwidth'
.field
= label_tag :mail_host, t("spree.smtp_mail_host")
%br/
= text_field_tag :mail_host, Spree::Config[:mail_host], class: 'fullwidth'
.field
= label_tag :mail_port, t("spree.smtp_port")
%br/
= text_field_tag :mail_port, Spree::Config[:mail_port], class: 'fullwidth'
.field
= label_tag :secure_connection_type, t("spree.secure_connection_type")
%br/
= select_tag(:secure_connection_type, options_from_collection_for_select(Spree::Core::MailSettings::SECURE_CONNECTION_TYPES.map{|w| Spree.t(w.downcase.to_sym, default: w)}, :to_s, :to_s, Spree::Config[:secure_connection_type]), class: 'select2 fullwidth')
.field
= label_tag :mail_auth_type, t("spree.smtp_authentication_type")
%br/
= select_tag(:mail_auth_type, options_from_collection_for_select(Spree::Core::MailSettings::MAIL_AUTH.map{|w| Spree.t(w.downcase.to_sym, default: w)}, :to_s, :to_s, Spree::Config[:mail_auth_type]), class: 'select2 fullwidth')
.field
= label_tag :smtp_username, t("spree.smtp_username")
%br/
= text_field_tag :smtp_username, Spree::Config[:smtp_username], class: 'fullwidth'
.field
= label_tag :preferred_smtp_password, t("spree.smtp_password")
%br/
= password_field_tag :smtp_password, Spree::Config[:smtp_password], class: 'fullwidth'
%fieldset.no-border-bottom
%legend{align: "center"}= t("spree.general")
.field
= preference_field_tag("enable_mail_delivery", Spree::Config[:enable_mail_delivery], type: :boolean)
= label_tag :enable_mail_delivery, t("spree.enable_mail_delivery")
.field
= label_tag :mails_from, t("spree.send_mails_as")
%br/
= text_field_tag :mails_from, Spree::Config[:mails_from], maxlength: 256, class: 'fullwidth'
%br/
%span.info
= t("spree.smtp_send_all_emails_as_from_following_address")
.field
= label_tag :mail_bcc, t("spree.send_copy_of_all_mails_to")
%br/
= text_field_tag :mail_bcc, Spree::Config[:mail_bcc], maxlength: 256, class: 'fullwidth'
%br/
%span.info
= t("spree.smtp_send_copy_to_this_addresses")
.field
= label_tag :intercept_email, t("spree.intercept_email_address")
%br/
= text_field_tag :intercept_email, Spree::Config[:intercept_email], maxlength: 256, class: 'fullwidth'
%br/
%span.info
= t("spree.intercept_email_instructions")
8 changes: 0 additions & 8 deletions config/locales/en.yml
Expand Up @@ -2978,14 +2978,6 @@ See the %{link} to find out more about %{sitename}'s features and to start using
smtp_send_copy_to_this_addresses: "Sends a copy of all outgoing mails to this address. For multiple addresses, separate with commas."
intercept_email_address: "Intercept Email Address"
intercept_email_instructions: "Override email recipient and replace with this address."
smtp: "SMTP"
smtp_domain: "SMTP Domain"
smtp_mail_host: "SMTP Mail Host"
smtp_port: "SMTP Port"
secure_connection_type: "Secure Connection Type"
smtp_authentication_type: "SMTP Authentication Type"
smtp_username: "SMTP Username"
smtp_password: "SMTP Password"

image_settings: "Image Settings"
image_settings_warning: "You will need to regenerate thumbnails if you update the paperclip styles. Use rake paperclip:refresh:thumbnails CLASS=Spree::Image to do this."
Expand Down
16 changes: 16 additions & 0 deletions db/migrate/20200429122446_drop_mail_methods.rb
@@ -0,0 +1,16 @@
class DropMailMethods < ActiveRecord::Migration
def up
drop_table :spree_mail_methods

# delete mail_method preferences associated with the old MailMethod model
execute "DELETE FROM spree_preferences WHERE key LIKE 'spree/mail_method%'"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will skip smtp_username and smtp_password though. Is that what we want?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I think smtp_username and smtp_password are the values set in MailConfiguration and used in MailSettings.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ok, got it. These are old Spree preference and not ours. I wasn't sure what you were aiming at.

end

def down
create_table :spree_mail_methods do |t|
t.string :environment
t.boolean :active, default: true
t.timestamps
end
end
end
9 changes: 1 addition & 8 deletions db/schema.rb
Expand Up @@ -11,7 +11,7 @@
#
# It's strongly recommended to check this file into your version control system.

ActiveRecord::Schema.define(:version => 20200406085833) do
ActiveRecord::Schema.define(:version => 20200429122446) do
create_table "adjustment_metadata", :force => true do |t|
t.integer "adjustment_id"
t.integer "enterprise_id"
Expand Down Expand Up @@ -505,13 +505,6 @@
t.datetime "updated_at", :null => false
end

create_table "spree_mail_methods", :force => true do |t|
t.string "environment"
t.boolean "active", :default => true
t.datetime "created_at", :null => false
t.datetime "updated_at", :null => false
end

create_table "spree_option_types", :force => true do |t|
t.string "name", :limit => 100
t.string "presentation", :limit => 100
Expand Down
2 changes: 0 additions & 2 deletions lib/tasks/data/anonymize_data.rake
Expand Up @@ -18,8 +18,6 @@ namespace :ofn do
anonymize_payments_accounts

Spree::TokenizedPermission.update_all("token = null")
ActiveRecord::Base.connection.execute("update spree_mail_methods
set environment = '#{Rails.env}'")

# Delete all preferences that may contain sensitive information
Spree::Preference
Expand Down
9 changes: 0 additions & 9 deletions spec/features/admin/configuration/mail_methods_spec.rb
Expand Up @@ -19,14 +19,5 @@
click_button "Update"
expect(page).to have_content("successfully updated!")
end

# Regression test for #2094
it "does not clear password if not provided" do
Spree::Config[:smtp_password] = "haxme"
click_button "Update"
expect(page).to have_content("successfully updated!")

expect(Spree::Config[:smtp_password]).not_to be_blank
end
end
end