From ac3c84b8641a98e8d09b7a87d8faa06e32214a83 Mon Sep 17 00:00:00 2001 From: James Meakin <12661555+jmsmkn@users.noreply.github.com> Date: Tue, 14 Jan 2025 08:19:37 +0100 Subject: [PATCH] Preview emails using the full page editor and standard email rendering (#3778) Adds the full page markdown editor for emails along with iframed previews using the standard email rendering. As there are differences in email clients and browsers a test email still needs to be sent out, but at least the previews are isolated from the GC CSS/JS environment. Screenshot 2025-01-09 at 11 37 36 Screenshot 2025-01-09 at 11 37 56 Closes #3682 --- app/grandchallenge/emails/forms.py | 24 ++- .../emails/migrations/0001_initial.py | 7 +- app/grandchallenge/emails/models.py | 25 +++- .../js/emails/email_markdown_preview.mjs | 11 ++ .../templates/emails/email_body_update.html | 17 +++ .../emails/templates/emails/email_detail.html | 26 ++-- .../emails/templates/emails/email_form.html | 13 +- .../email_full_page_markdown_widget.html | 15 ++ .../emails/templates/emails/email_list.html | 3 +- .../emails/email_rendered_detail.html | 1 + app/grandchallenge/emails/urls.py | 18 ++- app/grandchallenge/emails/views.py | 67 +++++++-- app/grandchallenge/emails/widgets.py | 23 +++ app/tests/emails_tests/test_views.py | 139 ++++++++++++++++-- 14 files changed, 333 insertions(+), 56 deletions(-) create mode 100644 app/grandchallenge/emails/static/js/emails/email_markdown_preview.mjs create mode 100644 app/grandchallenge/emails/templates/emails/email_body_update.html create mode 100644 app/grandchallenge/emails/templates/emails/email_full_page_markdown_widget.html create mode 100644 app/grandchallenge/emails/templates/emails/email_rendered_detail.html create mode 100644 app/grandchallenge/emails/widgets.py diff --git a/app/grandchallenge/emails/forms.py b/app/grandchallenge/emails/forms.py index 9cfead375d..258d4d8e7d 100644 --- a/app/grandchallenge/emails/forms.py +++ b/app/grandchallenge/emails/forms.py @@ -1,15 +1,27 @@ from django import forms from grandchallenge.core.forms import SaveFormInitMixin -from grandchallenge.core.widgets import MarkdownEditorInlineWidget from grandchallenge.emails.models import Email +from grandchallenge.emails.widgets import MarkdownEditorEmailFullPageWidget +from grandchallenge.subdomains.utils import reverse -class EmailForm(SaveFormInitMixin, forms.ModelForm): +class EmailMetadataForm(SaveFormInitMixin, forms.ModelForm): class Meta: model = Email - fields = ( - "subject", - "body", + fields = ("subject",) + + +class EmailBodyForm(SaveFormInitMixin, forms.ModelForm): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + self.fields["body"].widget = MarkdownEditorEmailFullPageWidget( + preview_url=reverse( + "emails:rendered-detail", kwargs={"pk": self.instance.pk} + ) ) - widgets = {"body": MarkdownEditorInlineWidget} + + class Meta: + model = Email + fields = ("body",) diff --git a/app/grandchallenge/emails/migrations/0001_initial.py b/app/grandchallenge/emails/migrations/0001_initial.py index 4a794e02fe..6642156694 100644 --- a/app/grandchallenge/emails/migrations/0001_initial.py +++ b/app/grandchallenge/emails/migrations/0001_initial.py @@ -23,12 +23,7 @@ class Migration(migrations.Migration): ), ), ("subject", models.CharField(max_length=1024)), - ( - "body", - models.TextField( - help_text="Email body will be prepended with 'Dear [username],' and will end with 'Kind regards, Grand Challenge team' and a link to unsubscribe from the mailing list." - ), - ), + ("body", models.TextField()), ("sent", models.BooleanField(default=False)), ("sent_at", models.DateTimeField(blank=True, null=True)), ( diff --git a/app/grandchallenge/emails/models.py b/app/grandchallenge/emails/models.py index ad46dab767..b4de8b9311 100644 --- a/app/grandchallenge/emails/models.py +++ b/app/grandchallenge/emails/models.py @@ -1,15 +1,17 @@ +from django.contrib.sites.models import Site from django.db import models from django.urls import reverse +from guardian.utils import get_anonymous_user from grandchallenge.core.models import UUIDModel +from grandchallenge.emails.emails import create_email_object +from grandchallenge.profiles.models import EmailSubscriptionTypes class Email(models.Model): subject = models.CharField(max_length=1024) - body = models.TextField( - help_text="Email body will be prepended with 'Dear [username],' and will end with 'Kind regards, Grand Challenge team' and a link to unsubscribe from the mailing list." - ) + body = models.TextField() sent = models.BooleanField(default=False) sent_at = models.DateTimeField(blank=True, null=True) status_report = models.JSONField( @@ -25,6 +27,23 @@ class Meta: def __str__(self): return self.subject + @property + def rendered_body(self): + email = create_email_object( + recipient=get_anonymous_user(), + site=Site.objects.get_current(), + subject=self.subject, + markdown_message=self.body, + subscription_type=EmailSubscriptionTypes.SYSTEM, + connection=None, + ) + alternatives = [ + alternative + for alternative in email.alternatives + if alternative[1] == "text/html" + ] + return alternatives[0][0] + def get_absolute_url(self): return reverse("emails:detail", kwargs={"pk": self.pk}) diff --git a/app/grandchallenge/emails/static/js/emails/email_markdown_preview.mjs b/app/grandchallenge/emails/static/js/emails/email_markdown_preview.mjs new file mode 100644 index 0000000000..e3ad51072a --- /dev/null +++ b/app/grandchallenge/emails/static/js/emails/email_markdown_preview.mjs @@ -0,0 +1,11 @@ +document.addEventListener("DOMContentLoaded", event => { + const iframes = document.querySelectorAll(".markdownx-preview"); + for (const iframe of iframes) { + const observer = new MutationObserver(() => { + iframe.srcdoc = iframe.innerHTML; + }); + observer.observe(iframe, { + childList: true, + }); + } +}); diff --git a/app/grandchallenge/emails/templates/emails/email_body_update.html b/app/grandchallenge/emails/templates/emails/email_body_update.html new file mode 100644 index 0000000000..53039e6d7f --- /dev/null +++ b/app/grandchallenge/emails/templates/emails/email_body_update.html @@ -0,0 +1,17 @@ +{% extends "emails/email_form.html" %} +{% load crispy_forms_tags %} + +{% block container %}container-fluid{% endblock %} + +{% block content %} + +

Update Email

+ + + + {% crispy form %} + +{% endblock %} diff --git a/app/grandchallenge/emails/templates/emails/email_detail.html b/app/grandchallenge/emails/templates/emails/email_detail.html index 2832c9b352..6de1392f97 100644 --- a/app/grandchallenge/emails/templates/emails/email_detail.html +++ b/app/grandchallenge/emails/templates/emails/email_detail.html @@ -1,6 +1,7 @@ {% extends "base.html" %} {% load url %} {% load bleach %} +{% load static %} {% block title %} {{ object.subject }} - Emails - {{ block.super }} @@ -19,27 +20,28 @@

{{ object.subject }}

Sent on {{ object.sent_at|date:"j N Y" }}

{% endif %}
+ + +
-
- {# The html email body is set to width:600px #} -
-

Dear user,

- {{ object.body|md2email_html }} -

— Your Grand Challenge Team

-
+ +
+
+
{% if not object.sent and "emails.change_email" in perms %} {% endif %}
diff --git a/app/grandchallenge/emails/templates/emails/email_form.html b/app/grandchallenge/emails/templates/emails/email_form.html index 957fe6106f..ab3c1d74a3 100644 --- a/app/grandchallenge/emails/templates/emails/email_form.html +++ b/app/grandchallenge/emails/templates/emails/email_form.html @@ -1,5 +1,6 @@ {% extends "base.html" %} {% load crispy_forms_tags %} +{% load static %} {% block title %} {{ object|yesno:"Update,Create Email" }} {% if object %} - {{ object }} {% else %} - Emails {% endif %} - {{ block.super }} @@ -16,10 +17,14 @@ {% endblock %} {% block content %} +

{% if object %}Update{% else %}Create{% endif %} Email

- + {% crispy form %} + +{% endblock %} + +{% block script %} + {{ block.super }} + {% endblock %} diff --git a/app/grandchallenge/emails/templates/emails/email_full_page_markdown_widget.html b/app/grandchallenge/emails/templates/emails/email_full_page_markdown_widget.html new file mode 100644 index 0000000000..a5ab292615 --- /dev/null +++ b/app/grandchallenge/emails/templates/emails/email_full_page_markdown_widget.html @@ -0,0 +1,15 @@ +
+ {% include "markdownx/partials/toolbar.html" %} +
+
+
+ {% include "markdownx/partials/textarea.html" %} +
+
+
+
+ +
+
+
+
diff --git a/app/grandchallenge/emails/templates/emails/email_list.html b/app/grandchallenge/emails/templates/emails/email_list.html index b5bf665cfd..62222696a6 100644 --- a/app/grandchallenge/emails/templates/emails/email_list.html +++ b/app/grandchallenge/emails/templates/emails/email_list.html @@ -36,7 +36,8 @@

Emails

{% if object.sent %} Sent on {{ object.sent_at|date:"j N Y" }} {% else %} - Edit + Edit Body + Edit Metadata {% endif %} diff --git a/app/grandchallenge/emails/templates/emails/email_rendered_detail.html b/app/grandchallenge/emails/templates/emails/email_rendered_detail.html new file mode 100644 index 0000000000..cd111347cb --- /dev/null +++ b/app/grandchallenge/emails/templates/emails/email_rendered_detail.html @@ -0,0 +1 @@ +{{ object.rendered_body }} diff --git a/app/grandchallenge/emails/urls.py b/app/grandchallenge/emails/urls.py index 873cd9aa4c..5fc5ed0eff 100644 --- a/app/grandchallenge/emails/urls.py +++ b/app/grandchallenge/emails/urls.py @@ -1,10 +1,12 @@ from django.urls import path from grandchallenge.emails.views import ( + EmailBodyUpdate, EmailCreate, EmailDetail, EmailList, - EmailUpdate, + EmailMetadataUpdate, + RenderedEmailDetail, ) app_name = "emails" @@ -13,5 +15,17 @@ path("", EmailList.as_view(), name="list"), path("create/", EmailCreate.as_view(), name="create"), path("/", EmailDetail.as_view(), name="detail"), - path("/update/", EmailUpdate.as_view(), name="update"), + path( + "/rendered/", + RenderedEmailDetail.as_view(), + name="rendered-detail", + ), + path( + "/metadata-update/", + EmailMetadataUpdate.as_view(), + name="metadata-update", + ), + path( + "/body-update/", EmailBodyUpdate.as_view(), name="body-update" + ), ] diff --git a/app/grandchallenge/emails/views.py b/app/grandchallenge/emails/views.py index dfa511ae9d..1847a74feb 100644 --- a/app/grandchallenge/emails/views.py +++ b/app/grandchallenge/emails/views.py @@ -1,10 +1,13 @@ from django.contrib.auth.mixins import PermissionRequiredMixin from django.core.exceptions import PermissionDenied +from django.utils.decorators import method_decorator +from django.views.decorators.clickjacking import xframe_options_sameorigin from django.views.generic import CreateView, DetailView, ListView, UpdateView from guardian.mixins import LoginRequiredMixin -from grandchallenge.emails.forms import EmailForm +from grandchallenge.emails.forms import EmailBodyForm, EmailMetadataForm from grandchallenge.emails.models import Email +from grandchallenge.subdomains.utils import reverse class EmailCreate( @@ -13,26 +16,53 @@ class EmailCreate( CreateView, ): model = Email - form_class = EmailForm + form_class = EmailMetadataForm permission_required = "emails.add_email" raise_exception = True + def get_success_url(self): + """On successful creation, go to content update.""" + return reverse( + "emails:body-update", + kwargs={ + "pk": self.object.pk, + }, + ) -class EmailUpdate( + +class UnsentEmailRequiredMixin: + def get_object(self, *args, **kwargs): + obj = super().get_object(*args, **kwargs) + + if obj.sent: + raise PermissionDenied + else: + return obj + + +class EmailMetadataUpdate( LoginRequiredMixin, PermissionRequiredMixin, + UnsentEmailRequiredMixin, UpdateView, ): model = Email - form_class = EmailForm + form_class = EmailMetadataForm permission_required = "emails.change_email" raise_exception = True - def has_permission(self): - if self.get_object().sent: - raise PermissionDenied - else: - return super().has_permission() + +class EmailBodyUpdate( + LoginRequiredMixin, + PermissionRequiredMixin, + UnsentEmailRequiredMixin, + UpdateView, +): + model = Email + form_class = EmailBodyForm + template_name_suffix = "_body_update" + permission_required = "emails.change_email" + raise_exception = True class EmailDetail(LoginRequiredMixin, PermissionRequiredMixin, DetailView): @@ -41,6 +71,25 @@ class EmailDetail(LoginRequiredMixin, PermissionRequiredMixin, DetailView): raise_exception = True +@method_decorator(xframe_options_sameorigin, name="dispatch") +class RenderedEmailDetail( + LoginRequiredMixin, PermissionRequiredMixin, DetailView +): + model = Email + template_name_suffix = "_rendered_detail" + permission_required = "emails.view_email" + raise_exception = True + + def post(self, request, *args, **kwargs): + """Generate a preview of the email with the new content""" + self.object = self.get_object() + + self.object.body = request.POST["content"] + + context = self.get_context_data(object=self.object) + return self.render_to_response(context) + + class EmailList(LoginRequiredMixin, PermissionRequiredMixin, ListView): model = Email permission_required = "emails.view_email" diff --git a/app/grandchallenge/emails/widgets.py b/app/grandchallenge/emails/widgets.py new file mode 100644 index 0000000000..b1fbbec9e8 --- /dev/null +++ b/app/grandchallenge/emails/widgets.py @@ -0,0 +1,23 @@ +from grandchallenge.core.widgets import MarkdownEditorFullPageWidget + + +class MarkdownEditorEmailFullPageWidget(MarkdownEditorFullPageWidget): + template_name = "emails/email_full_page_markdown_widget.html" + + def __init__(self, *args, preview_url, **kwargs): + super().__init__(*args, **kwargs) + self.preview_url = preview_url + + def add_markdownx_attrs(self, *args, **kwargs): + attrs = super().add_markdownx_attrs(*args, **kwargs) + attrs.update( + { + "data-markdownx-urls-path": self.preview_url, + } + ) + return attrs + + class Media: + js = [ + "js/emails/email_markdown_preview.mjs", + ] diff --git a/app/tests/emails_tests/test_views.py b/app/tests/emails_tests/test_views.py index 6ef1bad2ae..97af8a6732 100644 --- a/app/tests/emails_tests/test_views.py +++ b/app/tests/emails_tests/test_views.py @@ -26,24 +26,27 @@ def test_email_create(client): viewname="emails:create", client=client, method=client.post, - data={"subject": "Test email", "body": "Some dummy content"}, + data={"subject": "Test email"}, user=user, ) assert response.status_code == 302 assert Email.objects.count() == 1 - assert Email.objects.get().subject == "Test email" - assert Email.objects.get().body == "Some dummy content" + + email = Email.objects.get() + + assert response.url == f"https://testserver/emails/{email.pk}/body-update/" + assert email.subject == "Test email" @pytest.mark.django_db -def test_email_update(client): +def test_email_metadata_update(client): user = UserFactory() email = EmailFactory(subject="Test email", body="Test content") response = get_view_for_user( - viewname="emails:update", + viewname="emails:metadata-update", client=client, method=client.post, - data={"subject": "Updated subject", "body": "Updated content"}, + data={"subject": "Updated subject"}, reverse_kwargs={"pk": email.pk}, user=user, ) @@ -52,33 +55,79 @@ def test_email_update(client): # only users with permission can create emails assign_perm("emails.change_email", user) response = get_view_for_user( - viewname="emails:update", + viewname="emails:metadata-update", client=client, method=client.post, - data={"subject": "Updated subject", "body": "Updated content"}, + data={"subject": "Updated subject"}, reverse_kwargs={"pk": email.pk}, user=user, ) assert response.status_code == 302 email.refresh_from_db() assert email.subject == "Updated subject" + assert email.body == "Test content" + + # but not when the email has been sent + email.sent = True + email.save() + response = get_view_for_user( + viewname="emails:metadata-update", + client=client, + method=client.post, + data={"subject": "Updated again"}, + reverse_kwargs={"pk": email.pk}, + user=user, + ) + assert response.status_code == 403 + email.refresh_from_db() + assert email.subject == "Updated subject" + assert email.body == "Test content" + + +@pytest.mark.django_db +def test_email_body_update(client): + user = UserFactory() + email = EmailFactory(subject="Test email", body="Test content") + response = get_view_for_user( + viewname="emails:body-update", + client=client, + method=client.post, + data={"body": "Updated content"}, + reverse_kwargs={"pk": email.pk}, + user=user, + ) + assert response.status_code == 403 + + # only users with permission can create emails + assign_perm("emails.change_email", user) + response = get_view_for_user( + viewname="emails:body-update", + client=client, + method=client.post, + data={"body": "Updated content"}, + reverse_kwargs={"pk": email.pk}, + user=user, + ) + assert response.status_code == 302 + email.refresh_from_db() + assert email.subject == "Test email" assert email.body == "Updated content" # but not when the email has been sent email.sent = True email.save() response = get_view_for_user( - viewname="emails:update", + viewname="emails:body-update", client=client, method=client.post, - data={"subject": "Updated again", "body": "New content"}, + data={"body": "New content"}, reverse_kwargs={"pk": email.pk}, user=user, ) assert response.status_code == 403 email.refresh_from_db() - assert email.subject != "Updated again" - assert email.body != "New content" + assert email.subject == "Test email" + assert email.body == "Updated content" @pytest.mark.django_db @@ -93,7 +142,7 @@ def test_email_detail_permission(client): ) assert response.status_code == 403 - # only users with permission can create emails + # only users with permission can view emails assign_perm("emails.view_email", user) response = get_view_for_user( viewname="emails:detail", @@ -104,6 +153,70 @@ def test_email_detail_permission(client): assert response.status_code == 200 +@pytest.mark.django_db +def test_email_rendered_detail_get_permission(client): + user = UserFactory() + email = EmailFactory(subject="Test email", body="Test content") + response = get_view_for_user( + viewname="emails:rendered-detail", + client=client, + reverse_kwargs={"pk": email.pk}, + user=user, + ) + assert response.status_code == 403 + + # only users with permission can view emails + assign_perm("emails.view_email", user) + response = get_view_for_user( + viewname="emails:rendered-detail", + client=client, + reverse_kwargs={"pk": email.pk}, + user=user, + ) + assert response.status_code == 200 + assert "Dear AnonymousUser," in response.rendered_content + assert "Test content" in response.rendered_content + + # Should use a system email without an unsubscribe link + # as the generated email is for the anonymous user, not the request.user + assert ( + "This is an automated service email from" in response.rendered_content + ) + + +@pytest.mark.django_db +def test_email_rendered_detail_get_post_permission(client): + user = UserFactory() + email = EmailFactory(subject="Test email", body="Test content") + response = get_view_for_user( + viewname="emails:rendered-detail", + client=client, + reverse_kwargs={"pk": email.pk}, + user=user, + method=client.post, + data={"content": "New content"}, + ) + assert response.status_code == 403 + + # only users with permission can view emails + assign_perm("emails.view_email", user) + response = get_view_for_user( + viewname="emails:rendered-detail", + client=client, + reverse_kwargs={"pk": email.pk}, + user=user, + method=client.post, + data={"content": "New content"}, + ) + assert response.status_code == 200 + assert "Dear AnonymousUser," in response.rendered_content + assert "New content" in response.rendered_content + + # The email shouldn't be updated as the post is only a preview + email.refresh_from_db() + assert email.body == "Test content" + + @pytest.mark.django_db def test_email_list_permission(client): user = UserFactory()