From aa46fadb485f8507ea8fc6fbe87a617da64d49d8 Mon Sep 17 00:00:00 2001 From: medmunds Date: Wed, 2 Dec 2015 15:58:23 -0800 Subject: [PATCH] Clean up global MANDRILL_SETTINGS * Clean up Djrill backend __init__ * Fold MANDRILL_SUBACCOUNT into global_settings logic * Add some missing override tests * Update docs --- djrill/mail/backends/djrill.py | 41 +++++---- djrill/tests/test_mandrill_send.py | 102 ++++++++++++---------- djrill/tests/test_mandrill_subaccounts.py | 48 +++++----- docs/history.rst | 3 + docs/installation.rst | 43 ++++++--- docs/usage/sending_mail.rst | 25 ++---- 6 files changed, 141 insertions(+), 121 deletions(-) diff --git a/djrill/mail/backends/djrill.py b/djrill/mail/backends/djrill.py index 233ef60..a0942b5 100644 --- a/djrill/mail/backends/djrill.py +++ b/djrill/mail/backends/djrill.py @@ -26,29 +26,33 @@ class DjrillBackend(BaseEmailBackend): """ def __init__(self, **kwargs): - """ - Set the API key, API url and set the action url. - """ + """Init options from Django settings""" super(DjrillBackend, self).__init__(**kwargs) - self.api_key = getattr(settings, "MANDRILL_API_KEY", None) + + try: + self.api_key = settings.MANDRILL_API_KEY + except AttributeError: + raise ImproperlyConfigured("Set MANDRILL_API_KEY in settings.py to use Djrill") + self.api_url = getattr(settings, "MANDRILL_API_URL", "https://mandrillapp.com/api/1.0") if not self.api_url.endswith("/"): self.api_url += "/" - self.session = None self.global_settings = {} - for setting_key in getattr(settings, "MANDRILL_SETTINGS", {}): - if not isinstance(settings.MANDRILL_SETTINGS, dict): - raise ImproperlyConfigured("MANDRILL_SETTINGS must be a dict " - "in the settings.py file.") - self.global_settings[setting_key] = settings.MANDRILL_SETTINGS[setting_key] + try: + self.global_settings.update(settings.MANDRILL_SETTINGS) + except AttributeError: + pass # no MANDRILL_SETTINGS setting + except (TypeError, ValueError): # e.g., not enumerable + raise ImproperlyConfigured("MANDRILL_SETTINGS must be a dict or mapping") + + try: + self.global_settings["subaccount"] = settings.MANDRILL_SUBACCOUNT + except AttributeError: + pass # no MANDRILL_SUBACCOUNT setting - self.subaccount = getattr(settings, "MANDRILL_SUBACCOUNT", None) self.ignore_recipient_status = getattr(settings, "MANDRILL_IGNORE_RECIPIENT_STATUS", False) - - if not self.api_key: - raise ImproperlyConfigured("You have not set your mandrill api key " - "in the settings.py file.") + self.session = None def open(self): """ @@ -298,6 +302,7 @@ class DjrillBackend(BaseEmailBackend): # Mandrill attributes that require conversion: if hasattr(message, 'send_at'): api_params['send_at'] = self.encode_date_for_mandrill(message.send_at) + # setting send_at in global_settings wouldn't make much sense def _make_mandrill_to_list(self, message, recipients, recipient_type="to"): """Create a Mandrill 'to' field from a list of emails. @@ -324,9 +329,6 @@ class DjrillBackend(BaseEmailBackend): 'google_analytics_domains', 'google_analytics_campaign', 'metadata'] - if self.subaccount: - msg_dict['subaccount'] = self.subaccount - for attr in mandrill_attrs: if attr in self.global_settings: msg_dict[attr] = self.global_settings[attr] @@ -336,7 +338,8 @@ class DjrillBackend(BaseEmailBackend): # Allow simple python dicts in place of Mandrill # [{name:name, value:value},...] arrays... - # Allow merge of global and per message global_merge_var, the former taking precedent + # Merge global and per message global_merge_vars + # (in conflicts, per-message vars win) global_merge_vars = {} if 'global_merge_vars' in self.global_settings: global_merge_vars.update(self.global_settings['global_merge_vars']) diff --git a/djrill/tests/test_mandrill_send.py b/djrill/tests/test_mandrill_send.py index a767ad8..c6a7a5a 100644 --- a/djrill/tests/test_mandrill_send.py +++ b/djrill/tests/test_mandrill_send.py @@ -617,6 +617,7 @@ class DjrillRecipientsRefusedTests(DjrillBackendMockAPITestCase): 'tags': ['djrill'], 'preserve_recipients': True, 'view_content_link': True, + 'subaccount': 'example-subaccount', 'tracking_domain': 'example.com', 'signing_domain': 'example.com', 'return_path_domain': 'example.com', @@ -625,7 +626,7 @@ class DjrillRecipientsRefusedTests(DjrillBackendMockAPITestCase): 'metadata': ['djrill'], 'merge_language': 'mailchimp', 'global_merge_vars': {'TEST': 'djrill'}, - 'async': False, + 'async': True, 'ip_pool': 'Pool1', 'invalid': 'invalid', }) @@ -644,16 +645,17 @@ class DjrillMandrillGlobalFeatureTests(DjrillBackendMockAPITestCase): self.assert_mandrill_called("/messages/send.json") data = self.get_api_call_data() self.assertEqual(data['message']['from_name'], 'Djrill Test') - self.assertTrue(data['message']['important'], True) - self.assertTrue(data['message']['track_opens'], True) - self.assertTrue(data['message']['track_clicks'], True) - self.assertTrue(data['message']['auto_text'], True) - self.assertTrue(data['message']['auto_html'], True) - self.assertTrue(data['message']['inline_css'], True) - self.assertTrue(data['message']['url_strip_qs'], True) + self.assertTrue(data['message']['important']) + self.assertTrue(data['message']['track_opens']) + self.assertTrue(data['message']['track_clicks']) + self.assertTrue(data['message']['auto_text']) + self.assertTrue(data['message']['auto_html']) + self.assertTrue(data['message']['inline_css']) + self.assertTrue(data['message']['url_strip_qs']) self.assertEqual(data['message']['tags'], ['djrill']) - self.assertTrue(data['message']['preserve_recipients'], True) - self.assertTrue(data['message']['view_content_link'], True) + self.assertTrue(data['message']['preserve_recipients']) + self.assertTrue(data['message']['view_content_link']) + self.assertEqual(data['message']['subaccount'], 'example-subaccount') self.assertEqual(data['message']['tracking_domain'], 'example.com') self.assertEqual(data['message']['signing_domain'], 'example.com') self.assertEqual(data['message']['return_path_domain'], 'example.com') @@ -666,8 +668,7 @@ class DjrillMandrillGlobalFeatureTests(DjrillBackendMockAPITestCase): self.assertFalse('merge_vars' in data['message']) self.assertFalse('recipient_metadata' in data['message']) # Options at top level of api params (not in message dict): - self.assertFalse('send_at' in data) - self.assertEqual(data['async'], False) + self.assertTrue(data['async']) self.assertEqual(data['ip_pool'], 'Pool1') # Option that shouldn't be added self.assertFalse('invalid' in data['message']) @@ -675,44 +676,53 @@ class DjrillMandrillGlobalFeatureTests(DjrillBackendMockAPITestCase): def test_global_options_override(self): """Test that manually settings options overrides global settings """ - self.message.important = True - self.message.auto_text = True - self.message.auto_html = True - self.message.inline_css = True - self.message.preserve_recipients = True + self.message.from_name = "override" + self.message.important = False + self.message.track_opens = False + self.message.track_clicks = False + self.message.auto_text = False + self.message.auto_html = False + self.message.inline_css = False + self.message.url_strip_qs = False + self.message.tags = ['override'] + self.message.preserve_recipients = False self.message.view_content_link = False - self.message.tracking_domain = "click.example.com" - self.message.signing_domain = "example.com" - self.message.return_path_domain = "support.example.com" - self.message.subaccount = "marketing-dept" - self.message.async = True + self.message.subaccount = "override" + self.message.tracking_domain = "override.example.com" + self.message.signing_domain = "override.example.com" + self.message.return_path_domain = "override.example.com" + self.message.google_analytics_domains = ['override.example.com'] + self.message.google_analytics_campaign = ['UA-99999999-1'] + self.message.metadata = ['override'] + self.message.merge_language = 'handlebars' + self.message.async = False self.message.ip_pool = "Bulk Pool" self.message.send() data = self.get_api_call_data() - self.assertEqual(data['message']['important'], True) - self.assertEqual(data['message']['auto_text'], True) - self.assertEqual(data['message']['auto_html'], True) - self.assertEqual(data['message']['inline_css'], True) - self.assertEqual(data['message']['preserve_recipients'], True) - self.assertEqual(data['message']['view_content_link'], False) - self.assertEqual(data['message']['tracking_domain'], "click.example.com") - self.assertEqual(data['message']['signing_domain'], "example.com") - self.assertEqual(data['message']['return_path_domain'], "support.example.com") - self.assertEqual(data['message']['subaccount'], "marketing-dept") - self.assertEqual(data['async'], True) - self.assertEqual(data['ip_pool'], "Bulk Pool") - - def test_global_options_override_tracking(self): - """Test that manually settings options overrides global settings - """ - self.message.track_opens = False - self.message.track_clicks = False - self.message.url_strip_qs = False - self.message.send() - data = self.get_api_call_data() - self.assertEqual(data['message']['track_opens'], False) - self.assertEqual(data['message']['track_clicks'], False) - self.assertEqual(data['message']['url_strip_qs'], False) + self.assertEqual(data['message']['from_name'], 'override') + self.assertFalse(data['message']['important']) + self.assertFalse(data['message']['track_opens']) + self.assertFalse(data['message']['track_clicks']) + self.assertFalse(data['message']['auto_text']) + self.assertFalse(data['message']['auto_html']) + self.assertFalse(data['message']['inline_css']) + self.assertFalse(data['message']['url_strip_qs']) + self.assertEqual(data['message']['tags'], ['override']) + self.assertFalse(data['message']['preserve_recipients']) + self.assertFalse(data['message']['view_content_link']) + self.assertEqual(data['message']['subaccount'], 'override') + self.assertEqual(data['message']['tracking_domain'], 'override.example.com') + self.assertEqual(data['message']['signing_domain'], 'override.example.com') + self.assertEqual(data['message']['return_path_domain'], 'override.example.com') + self.assertEqual(data['message']['google_analytics_domains'], ['override.example.com']) + self.assertEqual(data['message']['google_analytics_campaign'], ['UA-99999999-1']) + self.assertEqual(data['message']['metadata'], ['override']) + self.assertEqual(data['message']['merge_language'], 'handlebars') + self.assertEqual(data['message']['global_merge_vars'], + [{'name': 'TEST', 'content': 'djrill'}]) + # Options at top level of api params (not in message dict): + self.assertFalse(data['async']) + self.assertEqual(data['ip_pool'], 'Bulk Pool') def test_global_merge(self): # Test that global settings merge in diff --git a/djrill/tests/test_mandrill_subaccounts.py b/djrill/tests/test_mandrill_subaccounts.py index 09f1e97..b6e7430 100644 --- a/djrill/tests/test_mandrill_subaccounts.py +++ b/djrill/tests/test_mandrill_subaccounts.py @@ -7,40 +7,36 @@ from .mock_backend import DjrillBackendMockAPITestCase class DjrillMandrillSubaccountTests(DjrillBackendMockAPITestCase): """Test Djrill backend support for Mandrill subaccounts""" - def test_send_basic(self): - mail.send_mail('Subject here', 'Here is the message.', - 'from@example.com', ['to@example.com'], fail_silently=False) - self.assert_mandrill_called("/messages/send.json") + def test_no_subaccount_by_default(self): + mail.send_mail('Subject', 'Body', 'from@example.com', ['to@example.com']) data = self.get_api_call_data() - self.assertEqual(data['message']['subject'], "Subject here") - self.assertEqual(data['message']['text'], "Here is the message.") - self.assertFalse('from_name' in data['message']) - self.assertEqual(data['message']['from_email'], "from@example.com") - self.assertEqual(len(data['message']['to']), 1) - self.assertEqual(data['message']['to'][0]['email'], "to@example.com") self.assertFalse('subaccount' in data['message']) - @override_settings(MANDRILL_SUBACCOUNT="test_subaccount") - def test_send_from_subaccount(self): - mail.send_mail('Subject here', 'Here is the message.', - 'from@example.com', ['to@example.com'], fail_silently=False) - self.assert_mandrill_called("/messages/send.json") + @override_settings(MANDRILL_SETTINGS={'subaccount': 'test_subaccount'}) + def test_subaccount_setting(self): + mail.send_mail('Subject', 'Body', 'from@example.com', ['to@example.com']) data = self.get_api_call_data() - self.assertEqual(data['message']['subject'], "Subject here") - self.assertEqual(data['message']['text'], "Here is the message.") - self.assertFalse('from_name' in data['message']) - self.assertEqual(data['message']['from_email'], "from@example.com") - self.assertEqual(len(data['message']['to']), 1) - self.assertEqual(data['message']['to'][0]['email'], "to@example.com") self.assertEqual(data['message']['subaccount'], "test_subaccount") - @override_settings(MANDRILL_SUBACCOUNT="global_setting_subaccount") + @override_settings(MANDRILL_SETTINGS={'subaccount': 'global_setting_subaccount'}) def test_subaccount_message_overrides_setting(self): - message = mail.EmailMessage( - 'Subject here', 'Here is the message', - 'from@example.com', ['to@example.com']) + message = mail.EmailMessage('Subject', 'Body', 'from@example.com', ['to@example.com']) message.subaccount = "individual_message_subaccount" # should override global setting message.send() - self.assert_mandrill_called("/messages/send.json") + data = self.get_api_call_data() + self.assertEqual(data['message']['subaccount'], "individual_message_subaccount") + + # Djrill 1.x offered dedicated MANDRILL_SUBACCOUNT setting. + # In Djrill 2.x, you should use the MANDRILL_SETTINGS dict as in the earlier tests. + # But we still support the old setting for compatibility: + @override_settings(MANDRILL_SUBACCOUNT="legacy_setting_subaccount") + def test_subaccount_legacy_setting(self): + mail.send_mail('Subject', 'Body', 'from@example.com', ['to@example.com']) + data = self.get_api_call_data() + self.assertEqual(data['message']['subaccount'], "legacy_setting_subaccount") + + message = mail.EmailMessage('Subject', 'Body', 'from@example.com', ['to@example.com']) + message.subaccount = "individual_message_subaccount" # should override legacy setting + message.send() data = self.get_api_call_data() self.assertEqual(data['message']['subaccount'], "individual_message_subaccount") diff --git a/docs/history.rst b/docs/history.rst index 9f58ff0..f07f0b0 100644 --- a/docs/history.rst +++ b/docs/history.rst @@ -96,6 +96,9 @@ Other Djrill 2.0 Changes (You can also directly manage your own long-lived Djrill connection across multiple sends, by calling open and close on :ref:`Django's email backend `.) +* Add global :setting:`MANDRILL_SETTINGS` dict that can provide defaults + for most Djrill message options. + * Add :exc:`djrill.NotSerializableForMandrillError` diff --git a/docs/installation.rst b/docs/installation.rst index 5aaf15b..e639f72 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -61,21 +61,42 @@ on invalid or rejected recipients. (Default ``False``.) .. versionadded:: 2.0 +.. setting:: MANDRILL_SETTINGS + +MANDRILL_SETTINGS +~~~~~~~~~~~~~~~~~ + +You can supply global default options to apply to all messages sent through Djrill. +Set :setting:`!MANDRILL_SETTINGS` to a dict of these options. Example:: + + MANDRILL_SETTINGS = { + 'subaccount': 'client-347', + 'tracking_domain': 'example.com', + 'track_opens': True, + } + +See :ref:`mandrill-send-support` for a list of available options. (Everything +*except* :attr:`merge_vars`, :attr:`recipient_metadata`, and :attr:`send_at` +can be used with :setting:`!MANDRILL_SETTINGS`.) + +Attributes set on individual EmailMessage objects will override the global +:setting:`!MANDRILL_SETTINGS` for that message. :attr:`global_merge_vars` +on an EmailMessage will be merged with any ``global_merge_vars`` in +:setting:`!MANDRILL_SETTINGS` (with the ones on the EmailMessage taking +precedence if there are conflicting var names). + +.. versionadded:: 2.0 + + .. setting:: MANDRILL_SUBACCOUNT MANDRILL_SUBACCOUNT ~~~~~~~~~~~~~~~~~~~ -If you are using Mandrill's `subaccounts`_ feature, you can globally set the -subaccount for all messages sent through Djrill:: - - MANDRILL_SUBACCOUNT = "client-347" - -(You can also set or override the :attr:`subaccount` on each individual message, -with :ref:`Mandrill-specific sending options `.) - -.. versionadded:: 1.0 - MANDRILL_SUBACCOUNT global setting - +Prior to Djrill 2.0, the :setting:`!MANDRILL_SUBACCOUNT` setting could +be used to globally set the `Mandrill subaccount `_. +Although this is still supported for compatibility with existing code, +new code should set a global subaccount in :setting:`MANDRILL_SETTINGS` +as shown above. .. _subaccounts: http://help.mandrill.com/entries/25523278-What-are-subaccounts- diff --git a/docs/usage/sending_mail.rst b/docs/usage/sending_mail.rst index 18cf66a..264f237 100644 --- a/docs/usage/sending_mail.rst +++ b/docs/usage/sending_mail.rst @@ -103,24 +103,17 @@ Some notes and limitations: Mandrill-Specific Options ------------------------- -.. setting:: MANDRILL_SETTINGS - Most of the options from the Mandrill `messages/send API `_ `message` struct can be set directly on an :class:`~django.core.mail.EmailMessage` -(or subclass) object: - -Most of these options can be globally set in your project's :file:`settings.py` -using :setting:`MANDRILL_SETTINGS`. For Example:: - - MANDRILL_SETTINGS = { - 'tracking_domain': 'example.com', - 'track_opens': True, - } +(or subclass) object. .. note:: - ``merge_vars`` and ``recipient_metadata`` cannot be set globally. ``global_merge_vars`` is merged - (see :attribute:`global_merge_vars`) + + You can set global defaults for common options with the + :setting:`MANDRILL_SETTINGS` setting, to avoid having to + set them on every message. + .. These attributes are in the same order as they appear in the Mandrill API docs... @@ -215,10 +208,6 @@ using :setting:`MANDRILL_SETTINGS`. For Example:: Merge data must be strings or other JSON-serializable types. (See :ref:`formatting-merge-data` for details.) - .. note:: - - If using :setting:`MANDRILL_SETTINGS` then the message ``dict`` will be merged and overwrite any duplicates. - .. attribute:: merge_vars ``dict``: per-recipient merge variables (most useful with :ref:`mandrill-templates`). The keys @@ -244,8 +233,6 @@ using :setting:`MANDRILL_SETTINGS`. For Example:: .. attribute:: subaccount ``str``: the ID of one of your subaccounts to use for sending this message. - (The subaccount on an individual message will override any global - :setting:`MANDRILL_SUBACCOUNT` setting.) .. versionadded:: 0.7