Merge pull request #2315 from django-oscar/refactor-alerts-dispatcher

Refactor product alerts to use Communication Events

docs/source/ref/apps/customer.rst

@@ -39,3 +39,5 @@ as affected stock records are updated. Alternatively, the management command
 The context for the alert email body contains a ``hurry`` variable that is set
 to ``True`` if the number of active alerts for a product is greater than the
 quantity of the product available in stock.
+
+Alerts are sent using the Communication Event framework.

docs/source/releases/v1.5.rst

@@ -53,7 +53,7 @@ These methods/modules have been removed:
 
 Minor changes
 ~~~~~~~~~~~~~
- - Add billing address to user's address book during checkout (:issue:`1532`).
+ - Added billing address to user's address book during checkout (:issue:`1532`).
    Number of usages for billing and shipping addresses tracked separately:
    billing address in ``UserAddress.num_orders_as_billing_address`` field and
    shipping address in ``UserAddress.num_order_as_shipping_address``
@@ -63,10 +63,10 @@ Minor changes
    set if the number of alerts for a product exceeds the quantity in stock,
    rather than the other way around.
 
- - Update search to use the haystack CharField instead of the EdgeNgramField
+ - Updated search to use the haystack CharField instead of the EdgeNgramField
    to fix irrelevant results (:issue:`2128`)
 
- - Update ``customer.utils.Dispatcher`` to improve the ability to customise
+ - Updated ``customer.utils.Dispatcher`` to improve the ability to customise
    the implementation (:issue:`2303`)
 
  - Fixed logic for determining "hurry mode" on stock alerts. Hurry mode is now
@@ -80,6 +80,12 @@ Minor changes
    database with new columns and copies into it original data -
    https://docs.djangoproject.com/en/1.11/topics/migrations/#sqlite
 
+ - Allowed the communication event dispatcher to accept an optional
+   ``mail_connection`` to use when sending email messages.
+
+ - Changed product alerts to be sent using the Communication Event framework.
+   The old email templates will continue to be supported until Oscar 2.0.
+
 .. _incompatible_in_1.5:
 
 Backwards incompatible changes in Oscar 1.5
@@ -122,11 +128,11 @@ Backwards incompatible changes in Oscar 1.5
    ``OSCAR_GOOGLE_ANALYTICS_ID`` and ``OSCAR_USE_LESS`` respectively in order
    to keep all Oscar settings under common namespace.
 
- - Remove ``display_version`` and ``version`` variables from templates and
+ - Removed ``display_version`` and ``version`` variables from templates and
    template context.
 
-- Offer ``Applicator`` is now loaded from the ``offer.applicator`` module, instead of ``offer.utils``.
-  Old path is deprecated and won't be supported in the next Oscar versions.
+ - Offer ``Applicator`` is now loaded from the ``offer.applicator`` module, instead of ``offer.utils``.
+   Old path is deprecated and won't be supported in the next Oscar versions.
 
 
 Dependency changes

src/oscar/apps/customer/alerts/utils.py

@@ -1,14 +1,18 @@
 import logging
+import warnings
 
 from django.conf import settings
 from django.contrib.sites.models import Site
 from django.core import mail
 from django.db.models import Max
-from django.template import loader
+from django.template import loader, TemplateDoesNotExist
 
 from oscar.apps.customer.notifications import services
+from oscar.apps.customer.utils import Dispatcher
 from oscar.core.loading import get_class, get_model
+from oscar.utils.deprecation import RemovedInOscar20Warning
 
+CommunicationEventType = get_model('customer', 'CommunicationEventType')
 ProductAlert = get_model('customer', 'ProductAlert')
 Product = get_model('catalogue', 'Product')
 Selector = get_class('partner.strategy', 'Selector')
@@ -36,16 +40,38 @@ def send_alert_confirmation(alert):
         'alert': alert,
         'site': Site.objects.get_current(),
     }
-    subject_tpl = loader.get_template('customer/alerts/emails/'
-                                      'confirmation_subject.txt')
-    body_tpl = loader.get_template('customer/alerts/emails/'
-                                   'confirmation_body.txt')
-    mail.send_mail(
-        subject_tpl.render(ctx).strip(),
-        body_tpl.render(ctx),
-        settings.OSCAR_FROM_EMAIL,
-        [alert.email],
-    )
+
+    # For backwards compability, we check if the old (non-communication-event)
+    # templates exist, and use them if they do.
+    # This will be removed in Oscar 2.0
+    try:
+        subject_tpl = loader.get_template('customer/alerts/emails/'
+                                          'confirmation_subject.txt')
+        body_tpl = loader.get_template('customer/alerts/emails/'
+                                       'confirmation_body.txt')
+        warnings.warn(
+            "Product alert notifications now use the CommunicationEvent. "
+            "Move '{}' to '{}', and '{}' to '{}'".format(
+                'customer/alerts/emails/confirmation_subject.txt',
+                'customer/emails/commtype_product_alert_confirmation_subject.txt',
+                'customer/alerts/emails/confirmation_body.txt',
+                'customer/emails/commtype_product_alert_confirmation_body.txt',
+            ),
+            category=RemovedInOscar20Warning, stacklevel=2
+        )
+
+        messages = {
+            'subject': subject_tpl.render(ctx).strip(),
+            'body': body_tpl.render(ctx),
+            'html': '',
+            'sms': '',
+        }
+    except TemplateDoesNotExist:
+        code = 'PRODUCT_ALERT_CONFIRMATION'
+        messages = CommunicationEventType.objects.get_and_render(code, ctx)
+
+    if messages and messages['body']:
+        Dispatcher().dispatch_direct_messages(alert.email, messages)
 
 
 def send_product_alerts(product):
@@ -75,14 +101,37 @@ def send_product_alerts(product):
     # hurry_mode is false if num_in_stock is None
     hurry_mode = num_in_stock is not None and alerts.count() > num_in_stock
 
-    # Load templates
-    message_tpl = loader.get_template('customer/alerts/message.html')
-    email_subject_tpl = loader.get_template('customer/alerts/emails/'
-                                            'alert_subject.txt')
-    email_body_tpl = loader.get_template('customer/alerts/emails/'
-                                         'alert_body.txt')
+    # For backwards compability, we check if the old (non-communication-event)
+    # templates exist, and use them if they do.
+    # This will be removed in Oscar 2.0
+    try:
+        email_subject_tpl = loader.get_template('customer/alerts/emails/'
+                                                'alert_subject.txt')
+        email_body_tpl = loader.get_template('customer/alerts/emails/'
+                                             'alert_body.txt')
+
+        use_deprecated_templates = True
+        warnings.warn(
+            "Product alert notifications now use the CommunicationEvent. "
+            "Move '{}' to '{}', and '{}' to '{}'".format(
+                'customer/alerts/emails/alert_subject.txt',
+                'customer/emails/commtype_product_alert_subject.txt',
+                'customer/alerts/emails/alert_body.txt',
+                'customer/emails/commtype_product_alert_body.txt',
+            ),
+            category=RemovedInOscar20Warning, stacklevel=2
+        )
+
+    except TemplateDoesNotExist:
+        code = 'PRODUCT_ALERT'
+        try:
+            event_type = CommunicationEventType.objects.get(code=code)
+        except CommunicationEventType.DoesNotExist:
+            event_type = CommunicationEventType.objects.model(code=code)
+        use_deprecated_templates = False
 
-    emails = []
+    messages_to_send = []
+    user_messages_to_send = []
     num_notifications = 0
     selector = Selector()
     for alert in alerts:
@@ -100,26 +149,41 @@ def send_product_alerts(product):
         if alert.user:
             # Send a site notification
             num_notifications += 1
+            message_tpl = loader.get_template('customer/alerts/message.html')
             services.notify_user(alert.user, message_tpl.render(ctx))
 
-        # Build email and add to list
-        emails.append(
-            mail.EmailMessage(
-                email_subject_tpl.render(ctx).strip(),
-                email_body_tpl.render(ctx),
-                settings.OSCAR_FROM_EMAIL,
-                [alert.get_email_address()],
-            )
-        )
+        # Build message and add to list
+        if use_deprecated_templates:
+            messages = {
+                'subject': email_subject_tpl.render(ctx).strip(),
+                'body': email_body_tpl.render(ctx),
+                'html': '',
+                'sms': '',
+            }
+        else:
+            messages = event_type.get_messages(ctx)
+
+        if messages and messages['body']:
+            if alert.user:
+                user_messages_to_send.append(
+                    (alert.user, messages)
+                )
+            else:
+                messages_to_send.append(
+                    (alert.get_email_address(), messages)
+                )
         alert.close()
 
-    # Send all emails in one go to prevent multiple SMTP
-    # connections to be opened
-    if emails:
+    # Send all messages using one SMTP connection to avoid opening lots of them
+    if messages_to_send or user_messages_to_send:
         connection = mail.get_connection()
         connection.open()
-        connection.send_messages(emails)
+        disp = Dispatcher(mail_connection=connection)
+        for message in messages_to_send:
+            disp.dispatch_direct_messages(*message)
+        for message in user_messages_to_send:
+            disp.dispatch_user_messages(*message)
         connection.close()
 
-    logger.info("Sent %d notifications and %d emails", num_notifications,
-                len(emails))
+    logger.info("Sent %d notifications and %d messages", num_notifications,
+                len(messages_to_send) + len(user_messages_to_send))

src/oscar/apps/customer/utils.py

@@ -15,10 +15,13 @@ Email = get_model('customer', 'Email')
 
 
 class Dispatcher(object):
-    def __init__(self, logger=None):
+    def __init__(self, logger=None, mail_connection=None):
         if not logger:
             logger = logging.getLogger(__name__)
         self.logger = logger
+        # Supply a mail_connection if you want the dispatcher to use that
+        # instead of opening a new one.
+        self.mail_connection = mail_connection
 
     # Public API methods
 
@@ -113,7 +116,11 @@ class Dispatcher(object):
                                  from_email=from_email,
                                  to=[recipient])
         self.logger.info("Sending email to %s" % recipient)
-        email.send()
+
+        if self.mail_connection:
+            self.mail_connection.send_messages([email])
+        else:
+            email.send()
 
         return email
 

tests/functional/customer/test_alert.py

@@ -1,9 +1,15 @@
+import mock
+import os
+import warnings
+
 from django_webtest import WebTest
 from django.core.urlresolvers import reverse
 from django.core import mail
 from django.test import TestCase
+from oscar.utils.deprecation import RemovedInOscar20Warning
 
-from oscar.apps.customer.alerts.utils import send_product_alerts
+from oscar.apps.customer.alerts.utils import (send_alert_confirmation,
+    send_product_alerts)
 from oscar.apps.customer.models import ProductAlert
 from oscar.test.factories import create_product, create_stockrecord
 from oscar.test.factories import UserFactory
@@ -133,3 +139,96 @@ class TestHurryMode(TestCase):
 
         self.assertIn('Beware that the amount of items in stock is limited',
             mail.outbox[0].body)
+
+
+class TestAlertMessageSending(TestCase):
+
+    def setUp(self):
+        self.user = UserFactory()
+        self.product = create_product()
+        create_stockrecord(self.product, num_in_stock=1)
+
+    @mock.patch('oscar.apps.customer.utils.Dispatcher.dispatch_direct_messages')
+    def test_alert_confirmation_uses_dispatcher(self, mock_dispatch):
+        alert = ProductAlert.objects.create(
+            email='test@example.com',
+            key='dummykey',
+            status=ProductAlert.UNCONFIRMED,
+            product=self.product
+        )
+        send_alert_confirmation(alert)
+        self.assertEqual(mock_dispatch.call_count, 1)
+        self.assertEqual(mock_dispatch.call_args[0][0], 'test@example.com')
+
+    @mock.patch('oscar.apps.customer.utils.Dispatcher.dispatch_user_messages')
+    def test_alert_uses_dispatcher(self, mock_dispatch):
+        ProductAlert.objects.create(user=self.user, product=self.product)
+        send_product_alerts(self.product)
+        self.assertEqual(mock_dispatch.call_count, 1)
+        self.assertEqual(mock_dispatch.call_args[0][0], self.user)
+
+    def test_alert_creates_email_obj(self):
+        ProductAlert.objects.create(user=self.user, product=self.product)
+        send_product_alerts(self.product)
+        self.assertEqual(self.user.emails.count(), 1)
+
+    @staticmethod
+    def _load_deprecated_template(path):
+        from django.template.loader import select_template
+        # Replace the old template paths with new ones, thereby mocking
+        # the presence of the deprecated templates.
+        if path.startswith('customer/alerts/emails/'):
+            old_path = path.replace('customer/alerts/emails/', '')
+            path_map = {
+                'confirmation_body.txt': 'confirmation_body.txt',
+                'confirmation_subject.txt': 'confirmation_subject.txt',
+                'alert_body.txt': 'body.txt',
+                'alert_subject.txt': 'subject.txt',
+            }
+            new_path = 'customer/emails/commtype_product_alert_{}'.format(
+                path_map[old_path]
+            )
+            # We use 'select_template' because 'load_template' is mocked...
+            return select_template([new_path])
+        return select_template([path])
+
+    @mock.patch('oscar.apps.customer.alerts.utils.loader.get_template')
+    def test_deprecated_notification_templates_work(self, mock_loader):
+        """
+        This test can be removed when support for the deprecated templates is
+        dropped.
+        """
+        mock_loader.side_effect = self._load_deprecated_template
+        with warnings.catch_warnings(record=True) as warning_list:
+            warnings.simplefilter("always")
+
+            alert = ProductAlert.objects.create(
+                email='test@example.com',
+                key='dummykey',
+                status=ProductAlert.UNCONFIRMED,
+                product=self.product
+            )
+            send_alert_confirmation(alert)
+            # Check that warnings were raised
+            self.assertTrue(any(item.category == RemovedInOscar20Warning
+                                                    for item in warning_list))
+            # Check that alerts were still sent
+            self.assertEqual(len(mail.outbox), 1)
+
+    @mock.patch('oscar.apps.customer.alerts.utils.loader.get_template')
+    def test_deprecated_alert_templates_work(self, mock_loader):
+        """
+        This test can be removed when support for the deprecated templates is
+        dropped.
+        """
+        mock_loader.side_effect = self._load_deprecated_template
+        with warnings.catch_warnings(record=True) as warning_list:
+            warnings.simplefilter("always")
+
+            ProductAlert.objects.create(user=self.user, product=self.product)
+            send_product_alerts(self.product)
+            # Check that warnings were raised
+            self.assertTrue(any(item.category == RemovedInOscar20Warning
+                                                    for item in warning_list))
+            # Check that alerts were still sent
+            self.assertEqual(len(mail.outbox), 1)

tests/integration/customer/test_dispatcher.py

@@ -88,3 +88,13 @@ class TestDispatcher(TestCase):
         ctx = {'user': user,
                'site': SiteFactory()}
         self._dispatch_user_messages(user, 'REGISTRATION', ctx, 'Thank you for registering.')
+
+    def test_dispatcher_uses_email_connection(self):
+        connection = mail.get_connection()
+        disp = Dispatcher(mail_connection=connection)
+        disp.dispatch_direct_messages('test@example.com', {
+            'subject': 'Test',
+            'body': 'This is a test.',
+            'html': '',
+        })
+        self.assertEqual(len(mail.outbox), 1)