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

Backport 'Fix newsletters unwanted CSS and 404 page on preview' to v0.27 #10354

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
Expand Up @@ -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
Expand Down
Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions decidim-admin/app/models/decidim/admin/fake_newsletter.rb
Expand Up @@ -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
Expand Down
27 changes: 26 additions & 1 deletion decidim-admin/spec/models/decidim/admin/fake_newsletter_spec.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Up @@ -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
Expand Up @@ -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
Expand Up @@ -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>
Expand All @@ -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 %>
Expand Down Expand Up @@ -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 %>
Expand Down
Expand Up @@ -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>
Expand All @@ -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 %>
Expand Down Expand Up @@ -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 %>
Expand Down
1 change: 1 addition & 0 deletions decidim-core/app/helpers/decidim/newsletters_helper.rb
Expand Up @@ -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
Expand Down
13 changes: 10 additions & 3 deletions decidim-core/app/mailers/decidim/newsletter_mailer.rb
Expand Up @@ -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
Expand All @@ -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
}
Expand Down
28 changes: 28 additions & 0 deletions decidim-core/app/models/decidim/newsletter.rb
Expand Up @@ -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
7 changes: 7 additions & 0 deletions decidim-core/app/packs/stylesheets/decidim/email.scss
Expand Up @@ -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{
Expand Down
@@ -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 %>
2 changes: 1 addition & 1 deletion decidim-core/app/views/decidim/newsletters/show.html.erb
Expand Up @@ -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) %>
Expand Down
4 changes: 2 additions & 2 deletions 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 %>
Expand Down
Expand Up @@ -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) %>
Expand Down Expand Up @@ -42,5 +42,5 @@
</td>
</tr>
</table>
</body>
<% end %>
</html>
2 changes: 1 addition & 1 deletion decidim-core/config/locales/en.yml
Expand Up @@ -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:
Expand Down
Expand Up @@ -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"),
Expand All @@ -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
4 changes: 2 additions & 2 deletions decidim-core/spec/helpers/decidim/newsletters_helper_spec.rb
Expand Up @@ -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
Expand Down