Make template context and notification metadata separate entities

keystone_api/apps/allocations/templates/expiration_email.html → keystone_api/apps/allocations/jinja2/expiration_email.html

@@ -4,8 +4,8 @@
   <p>Dear {{ user.username }},</p>
 {% endif %}
 <p>
-  This notification is to remind you your HPC compute allocation <strong>"{{ request_title }}"</strong>
-  is set to expire in {{ days_to_expire }} days on {{ request_expire }}.
+  This notification is to remind you your HPC compute allocation <strong>"{{ request.title }}"</strong>
+  is set to expire in {{ days_to_expire }} days on {{ request.expire.isoformat() }}.
 </p>
 <p>Sincerely,</p>
 <p>The keystone HPC utility</p>

keystone_api/apps/allocations/tasks.py

@@ -160,21 +160,24 @@ def send_expiry_notification_for_request(user: User, request: AllocationRequest)
 
     if notification_sent:
         log.debug(f'Existing notification found.')
+        return
 
-    else:
-        log.debug(f'Sending new notification for request #{request.id} to user {user.username}.')
-        send_notification_template(
-            user=user,
-            subject=f'Allocation Expires on {request.expire}',
-            template='expiration_email.html',
-            notification_type=Notification.NotificationType.request_status,
-            notification_metadata={
-                'request_id': request.id,
-                'request_title': request.title,
-                'request_expire': request.expire.isoformat(),
-                'days_to_expire': days_until_expire
-            }
-        )
+    log.debug(f'Sending new notification for request #{request.id} to user {user.username}.')
+    send_notification_template(
+        user=user,
+        subject=f'Allocation Expires on {request.expire}',
+        template='expiration_email.html',
+        context={
+            'user': user,
+            'request': request,
+            'days_to_expire': days_until_expire
+        },
+        notification_type=Notification.NotificationType.request_status,
+        notification_metadata={
+            'request_id': request.id,
+            'days_to_expire': days_until_expire
+        }
+    )
 
 
 @shared_task()

keystone_api/apps/notifications/shortcuts.py

@@ -18,8 +18,8 @@ def send_notification(
     subject: str,
     plain_text: str,
     html_text: str,
-    notification_type: Notification.NotificationType = None,
-    notification_metadata: dict = None
+    notification_type: Notification.NotificationType,
+    notification_metadata: dict | None = None
 ) -> None:
     """Send a notification email to a specified user with both plain text and HTML content.
 
@@ -52,23 +52,25 @@ def send_notification_template(
     user: User,
     subject: str,
     template: str,
-    notification_type: Notification.NotificationType = None,
-    notification_metadata: dict = None
+    context: dict,
+    notification_type: Notification.NotificationType,
+    notification_metadata: dict | None = None
 ) -> None:
     """Render an email template and send it to a specified user.
 
     Args:
         user: The user object to whom the email will be sent.
         subject: The subject line of the email.
         template: The name of the template file to render.
+        context: Variable definitions used to populate the template.
         notification_type: Optionally categorize the notification type.
         notification_metadata: Metadata to store alongside the notification.
-    """
 
-    context = {'user': user}
-    context.update(notification_metadata)
+    Raises:
+        UndefinedError: When template variables are not defined in the notification metadata
+    """
 
-    html_content = render_to_string(template, context)
+    html_content = render_to_string(template, context, using='jinja2')
     text_content = strip_tags(html_content)
 
     send_notification(
@@ -95,5 +97,5 @@ def send_general_notification(user: User, subject: str, message: str) -> None:
         subject=subject,
         template='general.html',
         notification_type=Notification.NotificationType.general_message,
-        notification_metadata={'message': message}
+        context={'user': user, 'message': message}
     )

keystone_api/apps/notifications/tests/shortcuts/test_send_notification.py

@@ -44,16 +44,16 @@ def test_email_content(self) -> None:
         self.assertEqual(len(mail.outbox), 1)
 
         email = mail.outbox[0]
-        self.assertEqual(email.subject, self.subject)
-        self.assertEqual(email.body, self.plain_text)
-        self.assertEqual(email.from_email, settings.EMAIL_FROM_ADDRESS)
-        self.assertEqual(email.to, [self.user.email])
-        self.assertEqual(email.alternatives, [(self.html_text, 'text/html')])
+        self.assertEqual(self.subject, email.subject)
+        self.assertEqual(self.plain_text, email.body)
+        self.assertEqual(settings.EMAIL_FROM_ADDRESS, email.from_email)
+        self.assertEqual([self.user.email], email.to)
+        self.assertEqual([(self.html_text, 'text/html')], email.alternatives)
 
     def test_database_is_updated(self) -> None:
         """Test a record of the email is stored in the database"""
 
         notification = Notification.objects.get(user=self.user)
-        self.assertEqual(notification.message, self.plain_text)
-        self.assertEqual(notification.notification_type, self.notification_type)
-        self.assertEqual(notification.metadata, self.notification_metadata)
+        self.assertEqual(self.plain_text, notification.message)
+        self.assertEqual(self.notification_type, notification.notification_type)
+        self.assertEqual(self.notification_metadata, notification.metadata)

keystone_api/apps/notifications/tests/shortcuts/test_send_notification_template.py

@@ -1,5 +1,6 @@
-"""Tests for the `send_notification_template` function."""
+"""Unit tests for the `send_notification_template` function."""
 
+import jinja2
 from django.core import mail
 from django.template.exceptions import TemplateDoesNotExist
 from django.test import override_settings, TestCase
@@ -25,51 +26,65 @@ def setUp(self) -> None:
             password='foobar123'
         )
 
-        self.subject = 'Test Subject'
-        self.notification_type = Notification.NotificationType.general_message
-        self.notification_metadata = {'key': 'value'}
-
     def test_email_content(self) -> None:
         """Test an email notification is sent with the correct content."""
 
+        subject = 'Test subject'
+
         send_notification_template(
             self.user,
-            self.subject,
-            'general.html',
-            self.notification_type,
-            self.notification_metadata
+            subject,
+            template='general.html',
+            context={"user": self.user, "message": "this is a message"},
+            notification_type=Notification.NotificationType.general_message
         )
 
         self.assertEqual(len(mail.outbox), 1)
         email = mail.outbox[0]
 
-        self.assertEqual(email.subject, self.subject)
-        self.assertEqual(email.from_email, settings.EMAIL_FROM_ADDRESS)
-        self.assertEqual(email.to, [self.user.email])
+        self.assertEqual(subject, email.subject)
+        self.assertEqual(settings.EMAIL_FROM_ADDRESS, email.from_email)
+        self.assertEqual([self.user.email], email.to)
 
     def test_database_is_updated(self) -> None:
         """Test a record of the email is stored in the database."""
 
+        notification_type = Notification.NotificationType.general_message
+        notification_metadata = {'key': 'value'}
+
         send_notification_template(
             self.user,
-            self.subject,
-            'general.html',
-            self.notification_type,
-            self.notification_metadata
+            "Test subject",
+            template='general.html',
+            context={"user": self.user, "message": "this is a message"},
+            notification_type=notification_type,
+            notification_metadata=notification_metadata
         )
 
         notification = Notification.objects.get(user=self.user)
-        self.assertEqual(notification.notification_type, self.notification_type)
-        self.assertEqual(notification.metadata, self.notification_metadata)
+        self.assertEqual(notification_type, notification.notification_type)
+        self.assertEqual(notification_metadata, notification.metadata)
 
     def test_missing_template(self) -> None:
         """Test an error is raised when a template is not found."""
 
         with self.assertRaises(TemplateDoesNotExist):
             send_notification_template(
                 self.user,
-                self.subject,
-                'this_template_does_not_exist',
-                self.notification_type,
-                self.notification_metadata
+                "Test subject",
+                template='this_template_does_not_exist',
+                context=dict(),
+                notification_type=Notification.NotificationType.general_message
+            )
+
+    def test_incomplete_rendering(self) -> None:
+        """Test an error is raise when a template isn't completely rendered."""
+
+        with self.assertRaises(jinja2.UndefinedError):
+            send_notification_template(
+                self.user,
+                "Test subject",
+                template='general.html',
+                context=dict(),
+                notification_type=Notification.NotificationType.general_message
             )

keystone_api/main/settings.py

@@ -8,6 +8,7 @@
 
 import environ
 from django.core.management.utils import get_random_secret_key
+from jinja2 import StrictUndefined
 
 BASE_DIR = Path(__file__).resolve().parent.parent
 sys.path.insert(0, str(BASE_DIR))
@@ -95,7 +96,7 @@
 ]
 
 TEMPLATES = [
-    {
+    {  # The default backend rquired by Django builtins (e.g., the admin)
         'BACKEND': 'django.template.backends.django.DjangoTemplates',
         'APP_DIRS': True,
         'OPTIONS': {
@@ -107,6 +108,13 @@
             ],
         },
     },
+    {  # Jinja2 backend used when rendering user notifications
+        "BACKEND": "django.template.backends.jinja2.Jinja2",
+        'APP_DIRS': True,
+        "OPTIONS": {
+            "undefined": StrictUndefined,
+        },
+    },
 ]
 
 # Base styling for the Admin UI