From 063d44fecc55f42712e98726f7b6fed925e05d7e Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Mon, 25 Jan 2021 12:54:58 +0100 Subject: [PATCH 1/3] Disable mail methods form fields except mails_from This moves a step closer to having a simple and straightforward way to configure the app's mail delivery which doesn't require to be a nuclear engineer to troubleshoot mail issues. It happens way too often that servers have mail config broken when restarted or redeployed and it takes too much brain power to fix it. No doubt; it's way too complex. I chose to leave this page's form fields but "Send mails as" as read-only. This other field is still used by instance manager to troubleshoot mail issues. --- .../admin/components/messages.scss | 13 ++++++++++++- app/assets/stylesheets/admin/variables.scss | 2 ++ .../spree/admin/mail_methods/_form.html.haml | 6 ++++-- config/locales/en.yml | 1 + .../admin/configuration/mail_methods_spec.rb | 19 +++++++++---------- 5 files changed, 28 insertions(+), 13 deletions(-) diff --git a/app/assets/stylesheets/admin/components/messages.scss b/app/assets/stylesheets/admin/components/messages.scss index 0b353da214..b7950db01f 100644 --- a/app/assets/stylesheets/admin/components/messages.scss +++ b/app/assets/stylesheets/admin/components/messages.scss @@ -42,4 +42,15 @@ &.notice { background-color: rgba($color-notice, 0.8) } &.success { background-color: rgba($color-success, 0.8) } &.error { background-color: rgba($color-error, 0.8) } -} \ No newline at end of file +} + +.notice { + padding: 1rem; + margin-bottom: 1.5rem; + background-color: $spree-light-blue; + border-radius: $border-radius; + + a { + font-weight: bold; + } +} diff --git a/app/assets/stylesheets/admin/variables.scss b/app/assets/stylesheets/admin/variables.scss index 72f806af7d..8767b65aa3 100644 --- a/app/assets/stylesheets/admin/variables.scss +++ b/app/assets/stylesheets/admin/variables.scss @@ -16,3 +16,5 @@ $admin-table-border: $pale-blue; $modal-close-button-color: #de6060; $modal-close-button-hover-color: #bf4545; $disabled-button: $light-grey; + +$border-radius: 3px; diff --git a/app/views/spree/admin/mail_methods/_form.html.haml b/app/views/spree/admin/mail_methods/_form.html.haml index f987a5b28b..740e276e0a 100644 --- a/app/views/spree/admin/mail_methods/_form.html.haml +++ b/app/views/spree/admin/mail_methods/_form.html.haml @@ -1,6 +1,8 @@ %div .row %fieldset.no-border-bottom + %div.notice + = t('spree.mail_settings_notice_html') %legend{align: "center"}= t("spree.general") .field = label_tag :mails_from, t("spree.send_mails_as") @@ -12,14 +14,14 @@ .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' + = text_field_tag :mail_bcc, Spree::Config[:mail_bcc], disabled: true, 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' + = text_field_tag :intercept_email, Spree::Config[:intercept_email], disabled: true, class: 'fullwidth' %br/ %span.info = t("spree.intercept_email_instructions") diff --git a/config/locales/en.yml b/config/locales/en.yml index a39ac74bd8..1ee179abd9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3101,6 +3101,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using choose_currency: "Choose Currency" mail_method_settings: "Mail Method Settings" + mail_settings_notice_html: "Some of the following settings can't be edited and are listed here just for debugging purposes. Changes can be made by updating the instance's secrets and provisioning them using ofn-install. Reach out to the OFN global team for further details." general: "General" enable_mail_delivery: "Enable Mail Delivery" send_mails_as: "Send Mails As" diff --git a/spec/features/admin/configuration/mail_methods_spec.rb b/spec/features/admin/configuration/mail_methods_spec.rb index e15b13502b..14dbcdbc77 100644 --- a/spec/features/admin/configuration/mail_methods_spec.rb +++ b/spec/features/admin/configuration/mail_methods_spec.rb @@ -5,19 +5,18 @@ require 'spec_helper' describe "Mail Methods" do include AuthenticationHelper - before(:each) do - login_as_admin_and_visit spree.edit_admin_general_settings_path - end + before { login_as_admin_and_visit spree.edit_admin_general_settings_path } context "edit" do - before(:each) do - click_link "Mail Method Settings" - end + before { click_link "Mail Method Settings" } - it "should be able to edit mail method settings" do - fill_in "mail_bcc", with: "ofn@example.com" - click_button "Update" - expect(page).to have_content("successfully updated!") + it "only allows changing the mails_from setting" do + fill_in 'mails_from', with: 'ofn@example.com' + expect(page).to have_field('mail_bcc', disabled: true) + expect(page).to have_field('intercept_email', disabled: true) + + click_button 'Update' + expect(page).to have_content('successfully updated!') end end end From 20b9ac89b899c4fe1ab09ee4fbe079d5678bede7 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Mon, 1 Feb 2021 18:41:22 +0100 Subject: [PATCH 2/3] Fix missing translation The used key doesn't exist. --- app/controllers/spree/admin/mail_methods_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/spree/admin/mail_methods_controller.rb b/app/controllers/spree/admin/mail_methods_controller.rb index 35ef212511..860f98bb49 100644 --- a/app/controllers/spree/admin/mail_methods_controller.rb +++ b/app/controllers/spree/admin/mail_methods_controller.rb @@ -10,7 +10,7 @@ module Spree Spree::Config[name] = value end - flash[:success] = Spree.t(:successfully_updated, resource: Spree.t(:mail_methods)) + flash[:success] = Spree.t(:successfully_updated, resource: Spree.t(:mail_method_settings)) render :edit end From cdcda46bff88078acc93c921d781adc549ff27ee Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Tue, 2 Feb 2021 12:16:55 +0100 Subject: [PATCH 3/3] Re-enable mail_bcc field in mail method settings This feels safer because we don't risk messing up with any instance's operations while still moving us towards removing this page. --- app/views/spree/admin/mail_methods/_form.html.haml | 2 +- spec/features/admin/configuration/mail_methods_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/spree/admin/mail_methods/_form.html.haml b/app/views/spree/admin/mail_methods/_form.html.haml index 740e276e0a..327c0874a0 100644 --- a/app/views/spree/admin/mail_methods/_form.html.haml +++ b/app/views/spree/admin/mail_methods/_form.html.haml @@ -14,7 +14,7 @@ .field = label_tag :mail_bcc, t("spree.send_copy_of_all_mails_to") %br/ - = text_field_tag :mail_bcc, Spree::Config[:mail_bcc], disabled: true, class: 'fullwidth' + = text_field_tag :mail_bcc, Spree::Config[:mail_bcc], maxlength: 256, class: 'fullwidth' %br/ %span.info = t("spree.smtp_send_copy_to_this_addresses") diff --git a/spec/features/admin/configuration/mail_methods_spec.rb b/spec/features/admin/configuration/mail_methods_spec.rb index 14dbcdbc77..6b37b2f39d 100644 --- a/spec/features/admin/configuration/mail_methods_spec.rb +++ b/spec/features/admin/configuration/mail_methods_spec.rb @@ -12,7 +12,7 @@ describe "Mail Methods" do it "only allows changing the mails_from setting" do fill_in 'mails_from', with: 'ofn@example.com' - expect(page).to have_field('mail_bcc', disabled: true) + fill_in 'mail_bcc', with: 'bcc@example.com' expect(page).to have_field('intercept_email', disabled: true) click_button 'Update'