neon-jungle / wagtail-birdsong

Create, send, preview, edit and test email campaigns from within Wagtail
BSD 3-Clause "New" or "Revised" License
106 stars 27 forks source link

relative Urls in Email Links #8

Open glanzel opened 3 years ago

glanzel commented 3 years ago

As far as I can see if you use the StreamlineField in the Email Body as described in the documentation: class SaleEmail(Campaign): body = StreamField(DefaultBlocks())

All Page and Document Links inserted with the Editors Link Buttons have relative Urls.

seb-b commented 3 years ago

Unfortunately this is a problem with Wagtail, if you have a single Site, all links inserted using richtext edit handler are relative instead of absolute. You can get around this by adding another dummy Site via the admin that isn't active.

A better solution would be to use a hook to alter the richtext link handlers so they are always absolute or create a richtext field specific for emails that handles this?

glanzel commented 3 years ago

my solution was to add custom link handlers to wagtail_hooks and make sure that my newsletter app gets called after birdsong in the settings. That seems to work. But it might not be the best/shortest code possible. I you want to i could include it in a pull request.

# Link Handlers for absolute Urls

from django.utils.html import escape
from wagtail.core import hooks
from wagtail.core.rich_text import LinkHandler
from wagtail.core.rich_text.pages import PageLinkHandler 
from wagtail.documents.rich_text import DocumentLinkHandler
from wagtail.core.models import Site
from django.core.exceptions import ObjectDoesNotExist

class AbsoluteLink(object):
    @classmethod
    def getAbsoluteLink(cls, page):
        try:
            root_path = Site.get_site_root_paths()
            # that is probably not ideal but worked for me
            root_url = root_path[0].root_url 
            page_url = page.url
            full_url = f"{root_url}{page_url}"
            return f'<a href="{escape(full_url)}">'
        except (ObjectDoesNotExist, KeyError):
            return "<a>"

class PageAbsoluteLinkHandler(PageLinkHandler):
    identifier = "page"
    @classmethod
    def expand_db_attributes(cls, attrs):
        try:
            page = cls.get_instance(attrs)
            return AbsoluteLink.getAbsoluteLink(page)
        except (ObjectDoesNotExist, KeyError):
            return "<a>"

class DocumentAbsoluteLinkHandler(DocumentLinkHandler):
    identifier = "document"
    @classmethod
    def expand_db_attributes(cls, attrs):
        try:
            document = cls.get_instance(attrs)
            return AbsoluteLink.getAbsoluteLink(document)
        except (ObjectDoesNotExist, KeyError):
            return "<a>"

@hooks.register('register_rich_text_features')
def register_link_handler(features):
    features.register_link_type(DocumentAbsoluteLinkHandler)
    features.register_link_type(PageAbsoluteLinkHandler)
seb-b commented 3 years ago

I'm not sure about making this the default approach for the plugin, this will affect all richtext fields, event outside of emails.

This implementations also assumes we want the first site root path which isn't always correct, you could call page.full_url for pages, and it's probably safe to assume that Site.objects.get(is_default_site=True) is correct for documents. Maybe putting it behind a flag is the best approach and documenting how to switch it on/off

glanzel commented 3 years ago

good advice. i changed it as you suggested. instead of @hooks.register('register_rich_text_features') i could use @hooks.register_temporarily('register_rich_text_features') so it only affects the rich text fields in the birdsong campaing.

but then i have to move it to editor.py. I already tested it and it seems to work.

should i provide that code to you ? and do you think a flag is still necesarry?

danhayden commented 3 years ago

I was having an issue with relative URLs in emails also.

While I'm sure there is a better way, my solution was to update the content variable in SMTPEmailBackend, this enabled me to replace relative link, document and image URLs with absolute URLs in the email content without messing with anything else.

# birdsong/backends/smtp.py

from wagtail.core.models import Site  # new

class SMTPEmailBackend(BaseEmailBackend):
    def send_campaign(self, request, campaign, contacts, test_send=False):
        # ...
        root_url = Site.objects.get(is_default_site=True).root_url   # new
        for contact in contacts:
            content = (
                render_to_string(
                    campaign.get_template(request),
                    campaign.get_context(request, contact),
                )
                # replace relative link and document urls with absolute urls
                .replace('href="/', f'href="{root_url}/')  # new
                # replace relative image urls with absolute urls
                .replace('src="/', f'src="{root_url}/')  # new
            )
        # ...

Perhaps there is a better way to do this outside of birdsong/backends/smtp.py so that it wouldn't require duplication in other backends if/when added.

mjepronk commented 3 years ago

I did it in the template:

from django import template

from bs4 import BeautifulSoup

register = template.Library()

@register.filter
def absolutize_urls(value, request):
    """
    Absolutize URL's in the given content using `request` from context.
    """
    soup = BeautifulSoup(value, 'html.parser')

    for tag in soup.find_all('a', href=True):
        tag['href'] = request.build_absolute_uri(tag['href'])

    for tag in soup.find_all('img', src=True):
        tag['src'] = request.build_absolute_uri(tag['src'])

    return str(soup)
{% filter absolutize_urls:request %}
<body>
  <p>
    <a href="{% url 'some-view' %}">this URL will be made absolute using request.build_absolute_uri</a>
   ...
</body>
{% endfilter %}