Fix newsletters unwanted CSS and 404 page on preview (#10354)

* Fix unwanted visible CSS definition with the newsletters

* Fix the notifications page URL in the newsletter preview

* Fix the organization link URLs in the newsletter preview

* Apply suggestions from code review

* Pass the preview option for the preview newsletter rendering

* Replace the newsletter URLs with  before it is sent

* Fix organization official URLs before the newsletter is sent

* Add  method to the fake newsletter

* Fix the port for the custom URL for mail root

Co-authored-by: Antti Hukkanen <antti.hukkanen@mainiotech.fi>

decidim-admin/app/controllers/decidim/admin/newsletter_templates_controller.rb

@@ -15,7 +15,7 @@ def index
       def show; end
 
       def preview
-        email = NewsletterMailer.newsletter(current_user, fake_newsletter)
+        email = NewsletterMailer.newsletter(current_user, fake_newsletter, preview: true)
         Premailer::Rails::Hook.perform(email)
         render html: email.html_part.body.decoded.html_safe
       end

decidim-admin/app/controllers/decidim/admin/newsletters_controller.rb

@@ -28,7 +28,7 @@ def show
       def preview
         enforce_permission_to :read, :newsletter, newsletter: newsletter
 
-        email = NewsletterMailer.newsletter(current_user, newsletter)
+        email = NewsletterMailer.newsletter(current_user, newsletter, preview: true)
         Premailer::Rails::Hook.perform(email)
         render html: email.html_part.body.decoded.html_safe
       end

decidim-admin/app/models/decidim/admin/fake_newsletter.rb

@@ -41,6 +41,26 @@ def sent_at
         nil
       end
 
+      def draft?
+        true
+      end
+
+      def url(**_)
+        "#"
+      end
+
+      def notifications_settings_url(**_)
+        "#"
+      end
+
+      def unsubscribe_newsletters_url(**_)
+        "#"
+      end
+
+      def organization_official_url
+        "#"
+      end
+
       private
 
       attr_reader :organization, :manifest

decidim-admin/spec/models/decidim/admin/fake_newsletter_spec.rb

@@ -3,8 +3,9 @@
 require "spec_helper"
 
 describe Decidim::Admin::FakeNewsletter do
-  subject { described_class.new(organization, manifest) }
+  subject { newsletter }
 
+  let(:newsletter) { described_class.new(organization, manifest) }
   let(:organization) { create :organization }
   let(:manifest) do
     Decidim
@@ -32,4 +33,28 @@
       expect(subject.template.settings.body).to include("Dummy text for body")
     end
   end
+
+  describe "#url" do
+    subject { newsletter.url }
+
+    it { is_expected.to eq("#") }
+  end
+
+  describe "#notifications_settings_url" do
+    subject { newsletter.notifications_settings_url }
+
+    it { is_expected.to eq("#") }
+  end
+
+  describe "#unsubscribe_newsletters_url" do
+    subject { newsletter.unsubscribe_newsletters_url }
+
+    it { is_expected.to eq("#") }
+  end
+
+  describe "#organization_official_url" do
+    subject { newsletter.organization_official_url }
+
+    it { is_expected.to eq("#") }
+  end
 end

decidim-admin/spec/system/admin_manages_newsletter_templates_spec.rb

@@ -59,4 +59,18 @@
       expect(page).to have_content("New newsletter")
     end
   end
+
+  describe "previewing a newsletter template iframe" do
+    shared_examples "working newsletter template iframe" do |template_id|
+      it "changes the footer links correctly" do
+        visit decidim_admin.preview_newsletter_template_path(template_id)
+        expect(page).to have_link("notifications page", href: "#")
+        expect(page).to have_link("Unsubscribe", href: "#")
+        expect(page).to have_link(organization.name, href: "#", count: 2)
+      end
+    end
+
+    it_behaves_like "working newsletter template iframe", :basic_only_text
+    it_behaves_like "working newsletter template iframe", :image_text_cta
+  end
 end

decidim-core/app/cells/decidim/newsletter_templates/base_cell.rb

@@ -25,6 +25,14 @@ def newsletter
       def recipient_user
         options[:recipient_user]
       end
+
+      def custom_url_for_mail_root
+        options[:custom_url_for_mail_root]
+      end
+
+      def decidim
+        @decidim ||= EngineRouter.new("decidim", {})
+      end
     end
   end
 end

decidim-core/app/cells/decidim/newsletter_templates/basic_only_text/show.erb

@@ -16,7 +16,7 @@
               <tr>
                 <th>
                   <center>
-                    <%= render partial: "layouts/decidim/mailer_logo.html", locals: { organization: organization } %>
+                    <%= render partial: "layouts/decidim/mailer_logo.html", locals: { organization: organization, custom_url_for_mail_root: custom_url_for_mail_root } %>
                   </center>
                 </th>
               </tr>
@@ -27,7 +27,7 @@
               <tr>
                 <th>
                   <% if organization.official_img_header.attached? %>
-                    <%= link_to organization.official_url do %>
+                    <%= link_to newsletter.organization_official_url do %>
                       <%= image_tag organization.attached_uploader(:official_img_header).path, alt: "", style: "max-height: 50px", class: "float-right" %>
                     <% end %>
                   <% end %>
@@ -72,8 +72,8 @@
           <th class="expander"></th>
           <th class="small-12 first columns cityhall-bar">
             <div class="decidim-logo" style="float: right; text-align: right; padding-right: 16px">
-              <% if @custom_url_for_mail_root.present? %>
-                <%= link_to organization.name.html_safe, @custom_url_for_mail_root %>
+              <% if custom_url_for_mail_root.present? %>
+                <%= link_to organization.name.html_safe, custom_url_for_mail_root %>
               <% else %>
                 <%= link_to organization.name.html_safe, decidim.root_url(host: organization.host) %>
               <% end %>

decidim-core/app/cells/decidim/newsletter_templates/image_text_cta/show.erb

@@ -24,7 +24,7 @@ table.button table td {
               <tr>
                 <th>
                   <center>
-                    <%= render partial: "layouts/decidim/mailer_logo.html", locals: { organization: organization } %>
+                    <%= render partial: "layouts/decidim/mailer_logo.html", locals: { organization: organization, custom_url_for_mail_root: custom_url_for_mail_root } %>
                   </center>
                 </th>
               </tr>
@@ -35,7 +35,7 @@ table.button table td {
               <tr>
                 <th>
                   <% if organization.official_img_header.attached? %>
-                    <%= link_to organization.official_url do %>
+                    <%= link_to newsletter.organization_official_url do %>
                       <%= image_tag organization.attached_uploader(:official_img_header).url(host: organization.host), alt: "", style: "max-height: 50px", class: "float-right" %>
                     <% end %>
                   <% end %>
@@ -111,8 +111,8 @@ table.button table td {
           <th class="expander"></th>
           <th class="small-12 first columns cityhall-bar">
             <div class="decidim-logo" style="float: right; text-align: right; padding-right: 16px">
-              <% if @custom_url_for_mail_root.present? %>
-                <%= link_to organization.name.html_safe, @custom_url_for_mail_root %>
+              <% if custom_url_for_mail_root.present? %>
+                <%= link_to organization.name.html_safe, custom_url_for_mail_root %>
               <% else %>
                 <%= link_to organization.name.html_safe, decidim.root_url(host: organization.host) %>
               <% end %>

decidim-core/app/helpers/decidim/newsletters_helper.rb

@@ -31,6 +31,7 @@ def parse_interpolations(content, user = nil, id = nil)
     # this method is used to generate the root link on mail with the utm_codes
     # If the newsletter_id is nil, it returns the root_url
     def custom_url_for_mail_root(organization, newsletter_id = nil)
+      decidim = EngineRouter.new("decidim", {})
       if newsletter_id.present?
         decidim.root_url(host: organization.host) + utm_codes(organization.host, newsletter_id.to_s)
       else

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

@@ -11,14 +11,20 @@ class NewsletterMailer < ApplicationMailer
 
     helper_method :cell
 
-    def newsletter(user, newsletter)
+    def newsletter(user, newsletter, preview: false)
       return if user.email.blank?
 
       @organization = user.organization
       @newsletter = newsletter
       @user = user
-
-      @custom_url_for_mail_root = custom_url_for_mail_root(@organization, @newsletter.id) if Decidim.config.track_newsletter_links
+      @preview = preview
+
+      @custom_url_for_mail_root =
+        if @preview
+          "#"
+        elsif Decidim.config.track_newsletter_links
+          custom_url_for_mail_root(@organization, @newsletter.id)
+        end
       @encrypted_token = Decidim::NewsletterEncryptor.sent_at_encrypted(@user.id, @newsletter.sent_at)
 
       with_user(user) do
@@ -40,6 +46,7 @@ def cell
         organization: @organization,
         newsletter: @newsletter,
         recipient_user: @user,
+        custom_url_for_mail_root: @custom_url_for_mail_root,
         context: {
           controller: self
         }

decidim-core/app/models/decidim/newsletter.rb

@@ -56,12 +56,40 @@ def template
                     .find_by(scoped_resource_id: id)
     end
 
+    def url(**kwargs)
+      proxy_url(:newsletter_url, id: id, **kwargs)
+    end
+
+    def notifications_settings_url(**kwargs)
+      proxy_url(__method__, **kwargs)
+    end
+
+    def unsubscribe_newsletters_url(**kwargs)
+      proxy_url(__method__, **kwargs)
+    end
+
+    def organization_official_url
+      return "#" unless sent?
+
+      organization.official_url || proxy_url(:root_url)
+    end
+
     private
 
     def author_belongs_to_organization
       return if !author || !organization
 
       errors.add(:author, :invalid) unless author.organization == organization
     end
+
+    def proxy_url(method, **kwargs)
+      return "#" unless sent?
+
+      router.public_send(method, host: organization.host, **kwargs)
+    end
+
+    def router
+      @router ||= EngineRouter.new("decidim", {})
+    end
   end
 end

decidim-core/app/packs/stylesheets/decidim/email.scss

@@ -25,6 +25,13 @@ body{
   -moz-box-sizing: border-box;
   -webkit-box-sizing: border-box;
   box-sizing: border-box;
+
+  &.preview{
+    .see-on-website,
+    .unsubscribe{
+      cursor: not-allowed;
+    }
+  }
 }
 
 .ExternalClass{

decidim-core/app/views/decidim/newsletter_mailer/newsletter.html.erb

@@ -1,16 +1,16 @@
 <%= decidim_sanitize_newsletter cell.to_s %>
 
 <% content_for :note do %>
-  <%== t ".note", organization_name: h(@organization.name), link: decidim.notifications_settings_url(host: @organization.host) %>
+  <%== t ".note", organization_name: h(@organization.name), link: @newsletter.notifications_settings_url %>
 <% end %>
 
 <% content_for :unsubscribe do %>
-  <%== t ".unsubscribe", link: decidim.unsubscribe_newsletters_url(host: @organization.host, u: @encrypted_token ) %>
+  <%== t ".unsubscribe", link: @newsletter.unsubscribe_newsletters_url(u: @encrypted_token) %>
 <% end %>
 
 <% content_for :see_on_website do %>
   <center style="display: none">
     <%== CGI.unescapeHTML truncate(cell.body, max_length: 50) %>
   </center>
-  <%== t ".see_on_website", link: decidim.newsletter_url(host: @organization.host, id: @newsletter) %>
+  <%== t ".see_on_website", link: @newsletter.url %>
 <% end %>

decidim-core/app/views/decidim/newsletters/show.html.erb

@@ -5,7 +5,7 @@
   newsletter: newsletter,
   recipient_user: @user
 ) %>
-<%= decidim_sanitize_editor @cell.to_s %>
+<%= decidim_sanitize_newsletter @cell.to_s %>
 
 <% content_for :note do %>
   <%== t "note", scope: "decidim.newsletter_mailer.newsletter", organization_name: h(@organization.name), link: decidim.notifications_settings_url(host: @organization.host) %>

decidim-core/app/views/layouts/decidim/_mailer_logo.html.erb

@@ -1,6 +1,6 @@
 <% if organization %>
-  <% if @custom_url_for_mail_root.present? %>
-    <% url = @custom_url_for_mail_root %>
+  <% if defined?(custom_url_for_mail_root) && custom_url_for_mail_root.present? %>
+    <% url = custom_url_for_mail_root %>
   <% else %>
     <% url = decidim.root_url(host: organization.host) %>
   <% end %>

decidim-core/app/views/layouts/decidim/newsletter_base.html.erb

@@ -6,7 +6,7 @@
     <%= stylesheet_pack_tag "decidim_email" %>
   </head>
 
-  <body>
+  <%= content_tag :body, class: @preview ? "preview" : nil do %>
     <!-- <style> -->
     <table class="body">
       <% if content_for?(:see_on_website) %>
@@ -42,5 +42,5 @@
         </td>
       </tr>
     </table>
-  </body>
+  <% end %>
 </html>

decidim-core/config/locales/en.yml

@@ -1114,7 +1114,7 @@ en:
     newsletter_mailer:
       newsletter:
         note: You received this email because you're subscribed to newsletters on %{organization_name}. You can change your settings on your <a href="%{link}">notifications page</a>.
-        see_on_website: Can’t see this email correctly? View it on the <a href="%{link}" target="_blank">website</a>.
+        see_on_website: Can’t see this email correctly? View it on the <a href="%{link}" target="_blank" class="see-on-website">website</a>.
         unsubscribe: To opt out of receiving this type of email, <a href="%{link}" target="_blank" class="unsubscribe">Unsubscribe</a>.
     newsletter_templates:
       basic_only_text:

decidim-core/spec/cells/decidim/newsletter_templates/basic_only_text_cell_spec.rb

@@ -22,13 +22,13 @@
 
   controller Decidim::PagesController
 
-  context "when the organization has no logo setted" do
+  context "when the organization has no logo set" do
     it "shows the custom body" do
       expect(subject).to have_text(body)
     end
   end
 
-  context "when the organization has a logo setted" do
+  context "when the organization has a logo set" do
     let(:logo) do
       Rack::Test::UploadedFile.new(
         Decidim::Dev.test_file("city.jpeg", "image/jpeg"),
@@ -44,5 +44,26 @@
     it "renders the organization logo" do
       expect(subject.to_s).to include(logo_url)
     end
+
+    it "renders an anchor URL with the logo before the newsletter is sent" do
+      expect(subject).to have_css(".decidim-bar a[href='#'] img[class='float-right']")
+    end
+
+    context "when the newsletter is sent" do
+      let(:newsletter) { create :newsletter, :sent, organization: organization }
+
+      it "renders the organization's official URL" do
+        expect(subject).to have_css(".decidim-bar a[href='#{organization.official_url}'] img[class='float-right']")
+      end
+
+      context "when the organization does not have an official URL" do
+        let(:organization) { create(:organization, official_url: nil) }
+        let(:newsletter) { create :newsletter, :sent, organization: organization }
+
+        it "renders the URL to Decidim instead" do
+          expect(subject).to have_css(".decidim-bar a[href='http://#{organization.host}:#{Capybara.server_port}/'] img[class='float-right']")
+        end
+      end
+    end
   end
 end

decidim-core/spec/helpers/decidim/newsletters_helper_spec.rb

@@ -36,13 +36,13 @@ module Decidim
 
         let(:newsletter) { create(:newsletter) }
 
-        it { is_expected.to eq(decidim.root_url(host: organization.host) + utm_codes(organization.host, newsletter.id.to_s)) }
+        it { is_expected.to eq(decidim.root_url(host: organization.host, port: Capybara.server_port) + utm_codes(organization.host, newsletter.id.to_s)) }
       end
 
       describe "when newsletter not present" do
         subject { helper.custom_url_for_mail_root(organization) }
 
-        it { is_expected.to eq(decidim.root_url(host: organization.host)) }
+        it { is_expected.to eq(decidim.root_url(host: organization.host, port: Capybara.server_port)) }
       end
     end
   end