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

relative Urls in Email Links #8

Open
glanzel opened this issue Feb 2, 2021 · 6 comments
Open

relative Urls in Email Links #8

glanzel opened this issue Feb 2, 2021 · 6 comments

Comments

@glanzel
Copy link

glanzel commented Feb 2, 2021

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
Copy link
Member

seb-b commented Feb 8, 2021

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
Copy link
Author

glanzel commented Feb 8, 2021

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
Copy link
Member

seb-b commented Feb 9, 2021

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
Copy link
Author

glanzel commented Feb 10, 2021

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
Copy link
Contributor

danhayden commented May 11, 2021

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
Copy link

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 %}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants