Add organization or application name in emails' From header

decidim-core/app/mailers/decidim/application_mailer.rb

@@ -7,6 +7,7 @@ class ApplicationMailer < ActionMailer::Base
     include LocalisedMailer
     include MultitenantAssetHost
     after_action :set_smtp
+    after_action :set_from
 
     default from: Decidim.config.mailer_sender
     layout "decidim/mailer"
@@ -16,7 +17,6 @@ class ApplicationMailer < ActionMailer::Base
     def set_smtp
       return if @organization.nil? || @organization.smtp_settings.blank?
 
-      mail.from = @organization.smtp_settings["from"].presence || mail.from
       mail.reply_to = mail.reply_to || Decidim.config.mailer_reply
       mail.delivery_method.settings.merge!(
         address: @organization.smtp_settings["address"],
@@ -25,5 +25,32 @@ def set_smtp
         password: Decidim::AttributeEncryptor.decrypt(@organization.smtp_settings["encrypted_password"])
       ) { |_k, o, v| v.presence || o }.compact_blank!
     end
+
+    def set_from
+      return if @organization.nil?
+      return if already_defined_name_in_mail?(mail.from.first)
+
+      mail.from = get_sender
+    end
+
+    def get_sender
+      return default_sender if @organization.smtp_settings.blank?
+      return default_sender if @organization.smtp_settings["from"].nil?
+      return default_sender if @organization.smtp_settings["from"].empty?
+
+      smtp_settings_from = @organization.smtp_settings["from"]
+      return smtp_settings_from if already_defined_name_in_mail?(smtp_settings_from)
+
+      email_address_with_name(smtp_settings_from, @organization.name)
+    end
+
+    def default_sender
+      email_address_with_name(mail.from.first, @organization.name)
+    end
+
+    def already_defined_name_in_mail?(mail_address)
+      # if there is an space, there is already a name in the address
+      mail_address.match?(/ /)
+    end
   end
 end

decidim-core/spec/mailers/application_mailer_spec.rb

@@ -6,10 +6,10 @@ module Decidim
   describe Decidim::Dev::DummyResourceMailer do
     describe "smtp_settings" do
       let(:user) { create(:user, organization:) }
-      let(:organization) { create(:organization, smtp_settings:) }
+      let(:organization) { create(:organization, name: "My Organization", smtp_settings:) }
       let(:smtp_settings) do
         {
-          "address" => "mail.gotham.gov",
+          "address" => "mail.example.org",
           "port" => "25",
           "user_name" => "f.laguardia",
           "encrypted_password" => Decidim::AttributeEncryptor.encrypt("password"),
@@ -24,7 +24,7 @@ module Decidim
       let(:mail_subject) { "Test subject" }
 
       it "update correctly mail.delivery_method.settings" do
-        expect(mail.delivery_method.settings[:address]).to eq("mail.gotham.gov")
+        expect(mail.delivery_method.settings[:address]).to eq("mail.example.org")
         expect(mail.delivery_method.settings[:port]).to eq("25")
         expect(mail.delivery_method.settings[:user_name]).to eq("f.laguardia")
         expect(mail.delivery_method.settings[:password]).to eq("password")
@@ -46,6 +46,10 @@ module Decidim
         it "returns default values" do
           expect(mail.from).to eq(["change-me@example.org"])
         end
+
+        it "returns the organization with the name" do
+          expect(mail.header[:from].value).to eq("My Organization <change-me@example.org>")
+        end
       end
 
       context "when from is not set" do
@@ -56,12 +60,28 @@ module Decidim
         end
       end
 
-      context "when from is set" do
+      context "when from is set with a name" do
         let(:from) { "Bruce Wayne <decide@gotham.org>" }
 
         it "set default values for mail.from and mail.reply_to" do
           expect(mail.from).to eq(["decide@gotham.org"])
         end
+
+        it "returns the organization with the name" do
+          expect(mail.header[:from].value).to eq("Bruce Wayne <decide@gotham.org>")
+        end
+      end
+
+      context "when from is set without a name" do
+        let(:from) { "decide@gotham.org" }
+
+        it "set default values for mail.from and mail.reply_to" do
+          expect(mail.from).to eq(["decide@gotham.org"])
+        end
+
+        it "returns the organization with the name" do
+          expect(mail.header[:from].value).to eq("My Organization <decide@gotham.org>")
+        end
       end
 
       context "when reply_to is set" do

decidim-system/config/locales/en.yml

@@ -246,7 +246,7 @@ en:
           fieldsets:
             sender: Sender
           instructions:
-            from_label: 'Email sender will be: "your-organization-name <your-organization@example.org>". Leave blank to use the ''Email address'' as label.'
+            from_label: 'Email sender will be: "your-organization-name <your-organization@example.org>". Leave blank to use the same name as the defined for the organization.'
           placeholder:
             from_email: your-organization@example.org
             from_label: your-organization-name

decidim-system/spec/commands/decidim/system/register_organization_spec.rb

@@ -17,21 +17,21 @@ module System
           let(:params) do
             {
               name: "Gotham City",
-              host: "decide.gotham.gov",
-              secondary_hosts: "foo.gotham.gov\r\n\r\nbar.gotham.gov",
+              host: "decide.example.org",
+              secondary_hosts: "foo.example.org\r\n\r\nbar.example.org",
               reference_prefix: "JKR",
               organization_admin_name: "Fiorello Henry La Guardia",
-              organization_admin_email: "f.laguardia@gotham.gov",
+              organization_admin_email: "f.laguardia@example.org",
               available_locales: ["en"],
               default_locale: "en",
               users_registration_mode: "enabled",
               force_users_to_authenticate_before_access_organization: "false",
               smtp_settings: {
-                "address" => "mail.gotham.gov",
+                "address" => "mail.example.org",
                 "port" => "25",
                 "user_name" => "f.laguardia",
                 "password" => Decidim::AttributeEncryptor.encrypt("password"),
-                "from_email" => "decide@gotham.gov",
+                "from_email" => "decide@example.org",
                 "from_label" => from_label
               },
               omniauth_settings_facebook_enabled: true,
@@ -53,11 +53,11 @@ module System
             organization = Organization.last
 
             expect(organization.name).to eq("Gotham City")
-            expect(organization.host).to eq("decide.gotham.gov")
-            expect(organization.secondary_hosts).to contain_exactly("foo.gotham.gov", "bar.gotham.gov")
+            expect(organization.host).to eq("decide.example.org")
+            expect(organization.secondary_hosts).to contain_exactly("foo.example.org", "bar.example.org")
             expect(organization.external_domain_allowlist).to contain_exactly("decidim.org", "github.com")
-            expect(organization.smtp_settings["from"]).to eq("Decide Gotham <decide@gotham.gov>")
-            expect(organization.smtp_settings["from_email"]).to eq("decide@gotham.gov")
+            expect(organization.smtp_settings["from"]).to eq("Decide Gotham <decide@example.org>")
+            expect(organization.smtp_settings["from_email"]).to eq("decide@example.org")
             expect(organization.omniauth_settings["omniauth_settings_facebook_enabled"]).to be(true)
             expect(organization.file_upload_settings).to eq(upload_settings)
             expect(
@@ -72,7 +72,7 @@ module System
             expect { command.call }.to change(User, :count).by(1)
             admin = User.last
 
-            expect(admin.email).to eq("f.laguardia@gotham.gov")
+            expect(admin.email).to eq("f.laguardia@example.org")
             expect(admin.organization.name).to eq("Gotham City")
             expect(admin).to be_admin
             expect(admin).to be_created_by_invite
@@ -127,10 +127,10 @@ module System
 
               organization = Organization.last
 
-              expect(organization.smtp_settings["from"]).to eq("Decide Gotham <decide@gotham.gov>")
+              expect(organization.smtp_settings["from"]).to eq("Decide Gotham <decide@example.org>")
               expect(organization.smtp_settings["from_label"]).to eq("Decide Gotham")
-              expect(organization.smtp_settings["from_email"]).to eq("decide@gotham.gov")
-              expect(last_email.From.value).to eq("Decide Gotham <decide@gotham.gov>")
+              expect(organization.smtp_settings["from_email"]).to eq("decide@example.org")
+              expect(last_email.From.value).to eq("Decide Gotham <decide@example.org>")
             end
 
             context "when from_label is empty" do
@@ -143,9 +143,9 @@ module System
 
                 organization = Organization.last
 
-                expect(organization.smtp_settings["from"]).to eq("decide@gotham.gov")
-                expect(organization.smtp_settings["from_email"]).to eq("decide@gotham.gov")
-                expect(last_email.From.value).to eq("decide@gotham.gov")
+                expect(organization.smtp_settings["from"]).to eq("decide@example.org")
+                expect(organization.smtp_settings["from_email"]).to eq("decide@example.org")
+                expect(last_email.From.value).to eq("decide@example.org")
               end
             end
           end

decidim-system/spec/commands/decidim/system/update_organization_spec.rb

@@ -18,16 +18,16 @@ module System
           let(:params) do
             {
               name: "Gotham City",
-              host: "decide.gotham.gov",
-              secondary_hosts: "foo.gotham.gov\r\n\r\nbar.gotham.gov",
+              host: "decide.example.org",
+              secondary_hosts: "foo.example.org\r\n\r\nbar.example.org",
               force_users_to_authenticate_before_access_organization: false,
               users_registration_mode: "existing",
               smtp_settings: {
-                "address" => "mail.gotham.gov",
+                "address" => "mail.example.org",
                 "port" => "25",
                 "user_name" => "f.laguardia",
                 "password" => Decidim::AttributeEncryptor.encrypt("password"),
-                "from_email" => "decide@gotham.gov",
+                "from_email" => "decide@example.org",
                 "from_label" => from_label
               },
               omniauth_settings_facebook_enabled: true,
@@ -49,11 +49,11 @@ module System
             organization = Organization.last
 
             expect(organization.name).to eq("Gotham City")
-            expect(organization.host).to eq("decide.gotham.gov")
-            expect(organization.secondary_hosts).to contain_exactly("foo.gotham.gov", "bar.gotham.gov")
+            expect(organization.host).to eq("decide.example.org")
+            expect(organization.secondary_hosts).to contain_exactly("foo.example.org", "bar.example.org")
             expect(organization.users_registration_mode).to eq("existing")
-            expect(organization.smtp_settings["from"]).to eq("Decide Gotham <decide@gotham.gov>")
-            expect(organization.smtp_settings["from_email"]).to eq("decide@gotham.gov")
+            expect(organization.smtp_settings["from"]).to eq("Decide Gotham <decide@example.org>")
+            expect(organization.smtp_settings["from_email"]).to eq("decide@example.org")
             expect(organization.omniauth_settings["omniauth_settings_facebook_enabled"]).to be(true)
             expect(organization.file_upload_settings).to eq(upload_settings)
             expect(
@@ -72,8 +72,8 @@ module System
                 command.call
                 organization = Organization.last
 
-                expect(organization.smtp_settings["from"]).to eq("decide@gotham.gov")
-                expect(organization.smtp_settings["from_email"]).to eq("decide@gotham.gov")
+                expect(organization.smtp_settings["from"]).to eq("decide@example.org")
+                expect(organization.smtp_settings["from_email"]).to eq("decide@example.org")
               end
             end
           end

decidim-system/spec/forms/decidim/system/update_organization_form_spec.rb

@@ -7,11 +7,11 @@ module Decidim::System
     subject do
       described_class.new(
         name: "Gotham City",
-        host: "decide.gotham.gov",
-        secondary_hosts: "foo.gotham.gov\r\n\r\nbar.gotham.gov",
+        host: "decide.example.org",
+        secondary_hosts: "foo.example.org\r\n\r\nbar.example.org",
         reference_prefix: "JKR",
         organization_admin_name: "Fiorello Henry La Guardia",
-        organization_admin_email: "f.laguardia@gotham.gov",
+        organization_admin_email: "f.laguardia@example.org",
         available_locales: ["en"],
         default_locale: "en",
         users_registration_mode: "enabled",
@@ -27,11 +27,11 @@ module Decidim::System
 
     let(:smtp_settings) do
       {
-        "address" => "mail.gotham.gov",
+        "address" => "mail.example.org",
         "port" => 25,
         "user_name" => "f.laguardia",
         "password" => password,
-        "from_email" => "decide@gotham.gov",
+        "from_email" => "decide@example.org",
         "from_label" => from_label
       }
     end
@@ -69,7 +69,7 @@ module Decidim::System
         it "concatenates from_label and from_email" do
           from = subject.set_from
 
-          expect(from).to eq("Decide Gotham <decide@gotham.gov>")
+          expect(from).to eq("Decide Gotham <decide@example.org>")
         end
 
         context "when from_label is empty" do
@@ -78,7 +78,7 @@ module Decidim::System
           it "returns the email" do
             from = subject.set_from
 
-            expect(from).to eq("decide@gotham.gov")
+            expect(from).to eq("decide@example.org")
           end
         end
       end