From f9e9f21627598390ecdf1c6d90e94471e83cae1b Mon Sep 17 00:00:00 2001 From: Bart van der Schoor Date: Fri, 16 Feb 2024 12:29:42 +0100 Subject: [PATCH] Fixed issue with creating missing templates through the admin Also expanded admin tests, added WebTest and fixed some Django settings not being accessed through the settings wrapper --- mail_editor/admin.py | 2 +- mail_editor/forms.py | 4 +- mail_editor/helpers.py | 9 ++- mail_editor/mail_template.py | 2 +- mail_editor/models.py | 6 +- mail_editor/settings.py | 5 +- mail_editor/views.py | 6 +- setup.py | 1 + tests/test_admin.py | 109 +++++++++++++++++++++++++---------- 9 files changed, 99 insertions(+), 45 deletions(-) diff --git a/mail_editor/admin.py b/mail_editor/admin.py index c34097c..89161f2 100644 --- a/mail_editor/admin.py +++ b/mail_editor/admin.py @@ -74,7 +74,7 @@ def get_preview_link(self, obj=None): get_preview_link.short_description = _("Preview") def get_preview_url(self, obj=None): - if obj: + if obj and obj.pk: return reverse("admin:mailtemplate_preview", kwargs={"pk": obj.id}) def get_urls(self): diff --git a/mail_editor/forms.py b/mail_editor/forms.py index f4dc6be..0ff0328 100644 --- a/mail_editor/forms.py +++ b/mail_editor/forms.py @@ -15,12 +15,14 @@ class Meta: fields = ('template_type', 'remarks', 'subject', 'body') widgets = { 'body': CKEditorWidget(config_name='mail_editor'), - 'template_type': forms.Select(choices=get_choices()) + 'template_type': forms.Select(choices=[]) } def __init__(self, *args, **kwargs): super(MailTemplateForm, self).__init__(*args, **kwargs) + self.fields["template_type"].widget.choices = get_choices() + template = loader.get_template('mail/_outer_table.html') # TODO: This only works when sites-framework is installed. try: diff --git a/mail_editor/helpers.py b/mail_editor/helpers.py index 1a6bab0..835b06d 100644 --- a/mail_editor/helpers.py +++ b/mail_editor/helpers.py @@ -1,11 +1,10 @@ import logging -from django.conf import settings from django.contrib.sites.shortcuts import get_current_site from django.template import loader from django.utils.safestring import mark_safe from django.utils.translation import gettext_lazy as _ - +from .settings import settings from .models import MailTemplate try: @@ -40,7 +39,7 @@ def find_template(template_name, language=None): def get_subject(template_name): - config = settings.MAIL_EDITOR_CONF + config = settings.TEMPLATES template_config = config.get(template_name) if template_config: @@ -52,7 +51,7 @@ def get_subject(template_name): def get_body(template_name): - config = settings.MAIL_EDITOR_CONF + config = settings.TEMPLATES template_config = config.get(template_name) default = _('Your content here...') @@ -67,7 +66,7 @@ def get_body(template_name): def get_base_template_path(template_name): - config = settings.MAIL_EDITOR_CONF + config = settings.TEMPLATES template_config = config.get(template_name) if template_config: diff --git a/mail_editor/mail_template.py b/mail_editor/mail_template.py index 9393896..0a8bfd3 100644 --- a/mail_editor/mail_template.py +++ b/mail_editor/mail_template.py @@ -20,7 +20,7 @@ def __init__(self, template): self.config = template.config def validate(self, field): - if self.config is None: # can't validate + if not self.config: # can't validate return template = self.check_syntax_errors(getattr(self.template, field)) self.check_variables(template, field) diff --git a/mail_editor/models.py b/mail_editor/models.py index e29dcfa..2af50cc 100644 --- a/mail_editor/models.py +++ b/mail_editor/models.py @@ -63,7 +63,7 @@ class Meta: def __init__(self, *args, **kwargs): super(MailTemplate, self).__init__(*args, **kwargs) - self.config = get_config()[self.template_type] + self.config = get_config().get(self.template_type) or dict() def __str__(self): if self.internal_name: @@ -76,7 +76,7 @@ def __str__(self): def clean(self): validate_template(self) - if getattr(django_settings, 'MAIL_EDITOR_UNIQUE_LANGUAGE_TEMPLATES', True): + if settings.UNIQUE_LANGUAGE_TEMPLATES: queryset = ( self.__class__.objects.filter( language=self.language, template_type=self.template_type @@ -96,7 +96,7 @@ def reload_template(self): self.base_template_path = get_base_template_path(self.template_type) def get_base_context(self): - base_context = getattr(django_settings, 'MAIL_EDITOR_BASE_CONTEXT', {}).copy() + base_context = copy.deepcopy(settings.BASE_CONTEXT) dynamic = settings.DYNAMIC_CONTEXT if dynamic: base_context.update(dynamic()) diff --git a/mail_editor/settings.py b/mail_editor/settings.py index ce15896..fa4401c 100644 --- a/mail_editor/settings.py +++ b/mail_editor/settings.py @@ -24,6 +24,9 @@ def BASE_CONTEXT(self): @property def DYNAMIC_CONTEXT(self): + """ + callable to return optional extra context + """ dynamic = getattr(django_settings, 'MAIL_EDITOR_DYNAMIC_CONTEXT', None) if dynamic: return import_string(dynamic) @@ -52,7 +55,7 @@ def UNIQUE_LANGUAGE_TEMPLATES(self): def get_choices(): choices = [] for key, values in settings.TEMPLATES.items(): - choices += [(key, values.get('name'))] + choices += [(key, values.get('name', key.title()))] return choices diff --git a/mail_editor/views.py b/mail_editor/views.py index 30d8478..873a63c 100644 --- a/mail_editor/views.py +++ b/mail_editor/views.py @@ -1,5 +1,4 @@ from django import forms -from django.conf import settings from django.contrib import admin from django.contrib import messages from django.http import HttpResponse @@ -7,11 +6,12 @@ from django.utils.translation import gettext_lazy as _ from django.views.generic import View from django.views.generic.detail import DetailView, SingleObjectMixin -from django.views.generic.edit import FormMixin, FormView +from django.views.generic.edit import FormView from .models import MailTemplate from .process import process_html from .utils import variable_help_text +from .settings import settings class TemplateVariableView(View): @@ -28,7 +28,7 @@ def get(self, request, *args, **kwargs): subject_ctx, body_ctx = template.get_preview_contexts() _subject, body = template.render(body_ctx, subject_ctx) - body, _attachments = process_html(body, settings.MAIL_EDITOR_BASE_HOST, extract_attachments=False) + body, _attachments = process_html(body, settings.BASE_HOST, extract_attachments=False) return HttpResponse(body, content_type="text/html") diff --git a/setup.py b/setup.py index b7e21c5..1d58515 100644 --- a/setup.py +++ b/setup.py @@ -34,6 +34,7 @@ 'pytest-pep8', 'pytest-pylint', 'pytest-pythonpath', + 'django-webtest', ], # metadata diff --git a/tests/test_admin.py b/tests/test_admin.py index 7dcde2e..4ee3044 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -3,9 +3,10 @@ from django.test import TestCase, override_settings from django.urls import reverse from django.utils.translation import gettext_lazy as _ +from django_webtest import WebTest from mail_editor.helpers import find_template - +from mail_editor.models import MailTemplate try: from unittest.mock import patch @@ -26,7 +27,7 @@ @override_settings(MAIL_EDITOR_CONF=CONFIG) -class AdminPreviewTestCase(TestCase): +class AdminTestCase(WebTest): def setUp(self): site_patch = patch("mail_editor.helpers.get_current_site") current_site_mock = site_patch.start() @@ -41,11 +42,20 @@ def tearDown(self): def test_changelist_view(self): template = find_template("test_template") - url = reverse('admin:{}_{}_changelist'.format(template._meta.app_label, template._meta.model_name)) + url = reverse('admin:mail_editor_mailtemplate_changelist') + + response = self.app.get(url, user=self.super_user) + + # test search isn't broken + form = response.forms["changelist-search"] + form["q"] = "test" + response = form.submit() + self.assertEqual([template], list(response.context["cl"].queryset.all())) - self.client.force_login(self.super_user) - response = self.client.get(url) - self.assertEqual(response.status_code, 200) + form = response.forms["changelist-search"] + form["q"] = "not_found" + response = form.submit() + self.assertEqual([], list(response.context["cl"].queryset.all())) def test_changelist_view__reset_template_action(self): template = find_template("test_template") @@ -53,15 +63,13 @@ def test_changelist_view__reset_template_action(self): template.subject = "something else" template.save() - url = reverse('admin:{}_{}_changelist'.format(template._meta.app_label, template._meta.model_name)) + url = reverse('admin:mail_editor_mailtemplate_changelist') - self.client.force_login(self.super_user) - data = { - 'action': 'reload_templates', - '_selected_action': [str(template.pk)], - } - response = self.client.post(url, data) - self.assertEqual(response.status_code, 302) + response = self.app.get(url, user=self.super_user) + form = response.forms["changelist-form"] + form['action'] = 'reload_templates' + form['_selected_action'] = [str(template.pk)] + response = form.submit().follow() template.refresh_from_db() self.assertIn(str(_("Test mail sent from testcase with {{ id }}")), template.body, ) @@ -70,34 +78,77 @@ def test_changelist_view__reset_template_action(self): def test_change_view(self): template = find_template("test_template") - url = reverse('admin:{}_{}_change'.format(template._meta.app_label, template._meta.model_name), args=[template.id]) + url = reverse('admin:mail_editor_mailtemplate_change', args=[template.id]) + + response = self.app.get(url, user=self.super_user) + form = response.forms["mailtemplate_form"] + form["subject"] = "mail subject" + form.submit() + template.refresh_from_db() + self.assertEqual(template.subject, "mail subject") + + def test_add_view(self): + url = reverse('admin:mail_editor_mailtemplate_add') + + response = self.app.get(url, user=self.super_user) + form = response.forms["mailtemplate_form"] + form["template_type"] = "test_template" + form["subject"] = "mail subject" + form["body"] = "mail body" + response = form.submit().follow() + + template = MailTemplate.objects.get() + self.assertEqual(template.template_type, "test_template") + self.assertEqual(template.subject, "mail subject") + self.assertEqual(template.body, "mail body") + + def test_add_view__handle_duplicates(self): + template = find_template("test_template") - self.client.force_login(self.super_user) - response = self.client.get(url) - self.assertEqual(response.status_code, 200) + url = reverse('admin:mail_editor_mailtemplate_add') + + with self.subTest("unique language templates"): + with override_settings(MAIL_EDITOR_UNIQUE_LANGUAGE_TEMPLATES=True): + response = self.app.get(url, user=self.super_user) + form = response.forms["mailtemplate_form"] + form["template_type"] = "test_template" + form["subject"] = "mail subject" + form["body"] = "mail body" + response = form.submit(status=200) + self.assertEqual(str(response.context["errors"][0][0]), _('Mail template with this type and language already exists')) + + self.assertEqual(1, MailTemplate.objects.filter(template_type="test_template").count()) + + with self.subTest("not-unique language templates"): + with override_settings(MAIL_EDITOR_UNIQUE_LANGUAGE_TEMPLATES=False): + response = self.app.get(url, user=self.super_user) + form = response.forms["mailtemplate_form"] + form["template_type"] = "test_template" + form["subject"] = "mail subject" + form["body"] = "mail body" + response = form.submit().follow() + self.assertEqual(2, MailTemplate.objects.filter(template_type="test_template").count()) def test_variable_view(self): template = find_template("test_template") url = reverse('admin:mailtemplate_variables', args=[template.template_type]) - self.client.force_login(self.super_user) - response = self.client.get(url) - self.assertEqual(response.status_code, 200) + response = self.app.get(url, user=self.super_user) def test_preview_view(self): template = find_template("test_template") url = reverse('admin:mailtemplate_preview', args=[template.id]) - self.client.force_login(self.super_user) - response = self.client.get(url) - self.assertEqual(response.status_code, 200) + response = self.app.get(url, user=self.super_user) self.assertContains(response, _("Important message for --id--")) - # test sending the email - self.client.post(url, {"recipient": "test@example.com"}) + # send the mail + form = response.forms['mailtemplate_form'] + form["recipient"] = "test@example.com" + response = form.submit() self.assertEqual(len(mail.outbox), 1) message = mail.outbox[0] @@ -109,8 +160,6 @@ def test_render_view(self): url = reverse('admin:mailtemplate_render', args=[template.id]) - self.client.force_login(self.super_user) - response = self.client.get(url) - self.assertEqual(response.status_code, 200) + response = self.app.get(url, user=self.super_user) - self.assertContains(response, _("Test mail sent from testcase with --id--")) + self.assertIn(str(_("Test mail sent from testcase with --id--")), response.text)