Check if a user exists before adding them to the mailing list (Fixes #591) (#592)

* Check if a user exists before adding them to the mailing list (Fixes #591)

* Fix function call

* Apply suggestion from @MelissaAutumn

Author: MelissaAutumn

src/thunderbird_accounts/subscription/tasks.py

@@ -1,9 +1,11 @@
+import sentry_sdk
 from requests.exceptions import JSONDecodeError
 import base64
 from django.conf import settings
 import datetime
 import logging
 import requests
+import hashlib
 
 from celery import shared_task
 from django.core.signing import Signer, BadSignature
@@ -412,6 +414,30 @@ def update_thundermail_quota(self, plan_uuid):
 def add_subscriber_to_mailchimp_list(self, user_uuid):
     """Adds a user's thundermail address to the primary tbpro mailing list.
     This mailing list contains automations to trigger welcome emails and such."""
+    try:
+        basic_auth = base64.b64encode(
+            f'this_does_not_support_bearer_auth:{settings.MAILCHIMP_API_KEY}'.encode()
+        ).decode()
+    except (UnicodeEncodeError, UnicodeDecodeError) as ex:
+        logging.error(f'Could not send request to mailchimp due to error: {ex}')
+        return {
+            'user_uuid': user_uuid,
+            'task_status': 'failed',
+            'reason': 'unicode error',
+        }
+
+    def mailchimp_api_query(method: str, api_endpoint: str, _json: dict | None = None) -> requests.Response:
+        """Small method to query mailchimp's api"""
+        api_url = f'https://{settings.MAILCHIMP_DC}.api.mailchimp.com/3.0/lists/{settings.MAILCHIMP_LIST_ID}'
+        response: requests.Response = requests.request(
+            method=method.upper(),
+            url=f'{api_url}{api_endpoint}',
+            headers={'Authorization': f'Basic {basic_auth}'},
+            json=_json,
+        )
+        response.raise_for_status()
+        return response
+
     try:
         user: User = User.objects.get(pk=user_uuid)
     except User.DoesNotExist:
@@ -430,26 +456,52 @@ def add_subscriber_to_mailchimp_list(self, user_uuid):
             'reason': 'user is not subscribed',
         }
 
-    try:
-        basic_auth = base64.b64encode(
-            f'this_does_not_support_bearer_auth:{settings.MAILCHIMP_API_KEY}'.encode()
-        ).decode()
-    except (UnicodeEncodeError, UnicodeDecodeError) as ex:
-        logging.error(f'Could not send request to mailchimp due to error: {ex}')
-        return {
-            'user_uuid': user_uuid,
-            'task_status': 'failed',
-            'reason': 'unicode error',
-        }
-
+    # Loop through the thundermail and recovery email and attempt to update or add a new tag to the mailchimp entry.
+    # It's a bit of a bit long and most of that is error catching.
     for val in [(user.stalwart_primary_email, 'new_user'), (user.email, 'welcome')]:
+        email, tag = val
+        md5_hasher = hashlib.new('md5')
+        md5_hasher.update(email.lower().encode())
+        hashed_email = md5_hasher.hexdigest()
+
+        # Check if the user is on the list, and if they are then update their tags array with our new one.
         try:
-            email, tag = val
+            response = mailchimp_api_query('get', f'/members/{hashed_email}')
+            response.raise_for_status()
+
+            data = response.json() or {}
+            tags = {t.get('name'): True for t in data.get('tags', [])}
+
+            # They're already in the mailing list with the assigned tag, so don't do anything.
+            if tags.get(tag):
+                continue
+
+            response = mailchimp_api_query(
+                'post', f'/members/{hashed_email}/tags', _json={'tags': [{'name': tag, 'status': 'active'}]}
+            )
 
-            response: requests.Response = requests.post(
-                f'https://{settings.MAILCHIMP_DC}.api.mailchimp.com/3.0/lists/{settings.MAILCHIMP_LIST_ID}/members',
-                headers={'Authorization': f'Basic {basic_auth}'},
-                json={
+            # Update tag has no response, so if we haven't ran into an exception continue along to the next email/tag.
+            continue
+
+        except requests.exceptions.RequestException as ex:
+            # Something error error'd out, we won't capture this just yet but we should add the context
+            # in case the next request fails.
+
+            # Error details reference: https://mailchimp.com/developer/marketing/docs/errors/#error-glossary
+            try:
+                error_details = ex.response.json()
+            except (JSONDecodeError, AttributeError):
+                error_details = {}
+
+            # Send some extra information to sentry
+            sentry_sdk.set_extra('tag_error_details', error_details)
+
+        # Try to create the user with the tag we want
+        try:
+            response = mailchimp_api_query(
+                'post',
+                '/members',
+                _json={
                     'email_address': email,
                     'status': 'subscribed',
                     'email_type': 'html',
@@ -458,21 +510,23 @@ def add_subscriber_to_mailchimp_list(self, user_uuid):
                     'tags': [tag],  # Tagged for automation purposes
                 },
             )
-            response.raise_for_status()
         except requests.exceptions.RequestException as ex:
-            logging.error(f'Could not send request to mailchimp due to error: {ex}')
-
+            # Error details reference: https://mailchimp.com/developer/marketing/docs/errors/#error-glossary
             try:
-                title = ex.response.json().get('title')
-            except JSONDecodeError:
-                title = 'N/A'
+                error_details = ex.response.json()
+            except (JSONDecodeError, AttributeError):
+                error_details = {}
+
+            # Send some extra information to sentry
+            sentry_sdk.set_extra('error_details', error_details)
+            sentry_sdk.capture_exception(ex)
 
             return {
                 'user_uuid': user_uuid,
                 'task_status': 'failed',
                 'reason': 'mailchimp error',
-                'error_msg_title': title,
-                'error_status_code': ex.response.status_code,
+                'error_msg_title': error_details.get('title', 'N/A'),
+                'error_status_code': ex.response.status_code if ex.response else None,
             }
     return {
         'user_uuid': user_uuid,

src/thunderbird_accounts/subscription/tests/test_tasks.py

@@ -1,6 +1,7 @@
 import requests
 import json
 import datetime
+import hashlib
 from pathlib import Path
 from unittest.mock import patch, Mock, MagicMock
 
@@ -1022,14 +1023,25 @@ def setUp(self):
     def tearDown(self):
         super().tearDown()
 
-    @patch('requests.post')
-    def test_success(self, requests_post_mock: MagicMock):
+    @patch('requests.request')
+    def test_success_create(self, request_mock: MagicMock):
         """Ensure that given a subscribed user with a valid stalwart email that
         they will successfully be added to the mailchimp list"""
-        fake_response = requests.Response()
-        fake_response.status_code = 200
+        get_member_response = requests.Response()
+        get_member_response.status_code = 404
+
+        create_member_response = requests.Response()
+        create_member_response.status_code = 200
+
+        request_mock.side_effect = [
+            # Thundermail
+            get_member_response,
+            create_member_response,
+            # Recovery Email
+            get_member_response,
+            create_member_response,
+        ]
 
-        requests_post_mock.return_value = fake_response
         task_results: dict | None = tasks.add_subscriber_to_mailchimp_list.delay(str(self.test_user.uuid)).get(
             timeout=10
         )
@@ -1038,53 +1050,116 @@ def test_success(self, requests_post_mock: MagicMock):
         self.assertEqual(task_results.get('task_status'), 'success')
         self.assertEqual(task_results.get('user_uuid'), str(self.test_user.uuid))
 
-        # We call it twice
-        self.assertEqual(requests_post_mock.call_count, 2)
-        thundermail_request, recovery_email_request = requests_post_mock.call_args_list
+        # We call it 4 times (2 for both emails)
+        self.assertEqual(request_mock.call_count, 4)
+
+        # Reverse order because of .pop()
+        email_addresses = [self.test_user.email, self.test_user.stalwart_primary_email]
+        for i in range(0, 4, 2):
+            get_req = request_mock.call_args_list[i]
+            update_tag_req = request_mock.call_args_list[i + 1]
+            address_to_test = email_addresses.pop()
+
+            md5_hasher = hashlib.new('md5')
+            md5_hasher.update(address_to_test.lower().encode())
+            hashed_email = md5_hasher.hexdigest()
+
+            # Test the GET request and the POST (update tag) request
+            self.assertIn(hashed_email, get_req[1]['url'])
+            self.assertNotIn('/tags', update_tag_req[1]['url'])
+            self.assertEqual(address_to_test, update_tag_req[1]['json']['email_address'])
+
+    @patch('requests.request')
+    def test_success_update(self, request_mock: MagicMock):
+        """Ensure that given a subscribed user with a valid stalwart email that
+        they will successfully have the proper tag added to their entry."""
+        fake_response = requests.Response()
+        fake_response.status_code = 200
+
+        get_member_response = requests.Response()
+        get_member_response.status_code = 200
+        get_member_response._content = b'{}'
 
-        # Ensure the call args were correct
-        self.assertEqual(thundermail_request[1]['json']['email_address'], self.test_user.stalwart_primary_email)
-        self.assertEqual(recovery_email_request[1]['json']['email_address'], self.test_user.email)
+        update_member_tag = requests.Response()
+        update_member_tag.status_code = 200
+
+        request_mock.side_effect = [
+            # Thundermail
+            get_member_response,
+            update_member_tag,
+            # Recovery Email
+            get_member_response,
+            update_member_tag,
+        ]
+
+        task_results: dict | None = tasks.add_subscriber_to_mailchimp_list.delay(str(self.test_user.uuid)).get(
+            timeout=10
+        )
+
+        self.assertIsNotNone(task_results)
+        self.assertEqual(task_results.get('task_status'), 'success')
+        self.assertEqual(task_results.get('user_uuid'), str(self.test_user.uuid))
 
-    @patch('requests.post')
-    def test_bad_user(self, requests_post_mock: MagicMock):
+        # We call it 4 times (2 for both emails)
+        self.assertEqual(request_mock.call_count, 4)
+
+        # Reverse order because of .pop()
+        email_addresses = [self.test_user.email, self.test_user.stalwart_primary_email]
+        tags = ['welcome', 'new_user']
+        for i in range(0, 4, 2):
+            get_req = request_mock.call_args_list[i]
+            update_tag_req = request_mock.call_args_list[i + 1]
+            address_to_test = email_addresses.pop()
+            tag_to_test = tags.pop()
+
+            md5_hasher = hashlib.new('md5')
+            md5_hasher.update(address_to_test.lower().encode())
+            hashed_email = md5_hasher.hexdigest()
+
+            # Test the GET request and the POST (update tag) request
+            self.assertIn(hashed_email, get_req[1]['url'])
+            self.assertIn('/tags', update_tag_req[1]['url'])
+            self.assertEqual([{'name': tag_to_test, 'status': 'active'}], update_tag_req[1]['json']['tags'])
+
+    @patch('requests.request')
+    def test_bad_user(self, request_mock: MagicMock):
         """Ensure that we catch users that do not exist."""
         fake_response = requests.Response()
         fake_response.status_code = 500
 
         bad_user_uuid = '4b6c15c9b2c94c9389de992f05d1441b'
 
-        requests_post_mock.return_value = fake_response
+        request_mock.return_value = fake_response
         task_results: dict | None = tasks.add_subscriber_to_mailchimp_list.delay(bad_user_uuid).get(timeout=10)
 
         self.assertIsNotNone(task_results)
         self.assertEqual(task_results.get('task_status'), 'failed')
         self.assertEqual(task_results.get('user_uuid'), bad_user_uuid)
         self.assertEqual(task_results.get('reason'), 'user does not exist')
 
-    @patch('requests.post')
-    def test_user_not_subscribed(self, requests_post_mock: MagicMock):
+    @patch('requests.request')
+    def test_user_not_subscribed(self, request_mock: MagicMock):
         """Ensure that we catch users who are not subscribed"""
         fake_response = requests.Response()
         fake_response.status_code = 500
 
         test_user = User.objects.create_user('test2@example.com', 'test2@example.org', '1234')
 
-        requests_post_mock.return_value = fake_response
+        request_mock.return_value = fake_response
         task_results: dict | None = tasks.add_subscriber_to_mailchimp_list.delay(str(test_user.uuid)).get(timeout=10)
 
         self.assertIsNotNone(task_results)
         self.assertEqual(task_results.get('task_status'), 'failed')
         self.assertEqual(task_results.get('user_uuid'), str(test_user.uuid))
         self.assertEqual(task_results.get('reason'), 'user is not subscribed')
 
-    @patch('requests.post')
-    def test_bad_request(self, requests_post_mock: MagicMock):
+    @patch('requests.request')
+    def test_bad_request(self, request_mock: MagicMock):
         """Ensure we catch bad requests from mailchimp"""
         fake_response = requests.Response()
         fake_response.status_code = 404
 
-        requests_post_mock.return_value = fake_response
+        request_mock.return_value = fake_response
         task_results: dict | None = tasks.add_subscriber_to_mailchimp_list.delay(str(self.test_user.uuid)).get(
             timeout=10
         )