Merge branch 'hotfix/23.08.3' into develop

osf/models/user.py

@@ -972,14 +972,7 @@ def deactivate_account(self):
                 user_id=self._id,
                 username=self.username
             )
-        except mailchimp_utils.mailchimp.ListNotSubscribedError:
-            pass
-        except mailchimp_utils.mailchimp.InvalidApiKeyError:
-            if not website_settings.ENABLE_EMAIL_SUBSCRIPTIONS:
-                pass
-            else:
-                raise
-        except mailchimp_utils.mailchimp.EmailNotExistsError:
+        except mailchimp_utils.OSFError:
             pass
         # Call to `unsubscribe` above saves, and can lead to stale data
         self.reload()

osf_tests/test_user.py

@@ -23,7 +23,6 @@
 from framework.celery_tasks import handlers
 from website import settings
 from website import filters
-from website import mailchimp_utils
 from website.project.signals import contributor_added
 from website.project.views.contributor import notify_added_contributor
 from website.views import find_bookmark_collection
@@ -1509,7 +1508,7 @@ def test_send_user_merged_signal(self, mock_get_mailchimp_api, dupe, merge_dupe)
     @mock.patch('website.mailchimp_utils.get_mailchimp_api')
     @mock.patch('website.mailchimp_utils.unsubscribe_mailchimp_async')
     def test_merged_user_unsubscribed_from_mailing_lists(self, mock_mailchimp_api, mock_unsubscribe, dupe, merge_dupe, email_subscriptions_enabled):
-        list_name = 'foo'
+        list_name = settings.MAILCHIMP_GENERAL_LIST
         dupe.mailchimp_mailing_lists[list_name] = True
         dupe.save()
         merge_dupe()
@@ -1694,10 +1693,6 @@ def test_deactivate_account_and_remove_sessions(self, mock_mail):
         assert not SessionStore().exists(session_key=session1.session_key)
         assert not SessionStore().exists(session_key=session2.session_key)
 
-    def test_deactivate_account_api(self):
-        settings.ENABLE_EMAIL_SUBSCRIPTIONS = True
-        with pytest.raises(mailchimp_utils.mailchimp.InvalidApiKeyError):
-            self.user.deactivate_account()
 
 # Copied from tests/modes/test_user.py
 @pytest.mark.enable_bookmark_creation
@@ -1963,14 +1958,9 @@ def is_mrm_field(value):
         other_user.external_accounts.add(ExternalAccountFactory())
 
         self.user.mailchimp_mailing_lists = {
-            'user': True,
-            'shared_gt': True,
-            'shared_lt': False,
         }
         other_user.mailchimp_mailing_lists = {
-            'other': True,
-            'shared_gt': False,
-            'shared_lt': True,
+            settings.MAILCHIMP_GENERAL_LIST: True
         }
 
         self.user.security_messages = {
@@ -2044,10 +2034,7 @@ def is_mrm_field(value):
             ]),
             'recently_added': set(),
             'mailchimp_mailing_lists': {
-                'user': True,
-                'other': True,
-                'shared_gt': True,
-                'shared_lt': True,
+                settings.MAILCHIMP_GENERAL_LIST: True
             },
             'osf_mailing_lists': {
                 'Open Science Framework Help': True
@@ -2076,7 +2063,6 @@ def is_mrm_field(value):
         # mock mailchimp
         mock_client = mock.MagicMock()
         mock_get_mailchimp_api.return_value = mock_client
-        mock_client.lists.list.return_value = {'data': [{'id': x, 'list_name': list_name} for x, list_name in enumerate(self.user.mailchimp_mailing_lists)]}
 
         with run_celery_tasks():
             # perform the merge

requirements.txt

@@ -21,7 +21,7 @@ git+https://github.com/cos-forks/[email protected]+cos0
 kombu==4.2.0
 itsdangerous==1.1.0
 lxml==4.6.5
-mailchimp==2.0.9
+mailchimp3==3.0.18
 nameparser==0.5.3
 bcrypt==3.1.4
 python-dateutil==2.8.1

tests/test_mailchimp.py

@@ -1,13 +1,15 @@
 # -*- coding: utf-8 -*-
+from hashlib import md5
 import mock
 import pytest
-from website import mailchimp_utils
-from tests.base import OsfTestCase
+from mailchimp3.mailchimpclient import MailChimpError
 from nose.tools import *  # noqa; PEP8 asserts
-from osf_tests.factories import UserFactory
-import mailchimp
 
 from framework.celery_tasks import handlers
+from osf_tests.factories import UserFactory
+from tests.base import OsfTestCase
+from website import mailchimp_utils
+from website.settings import MAILCHIMP_GENERAL_LIST, MAILCHIMP_LIST_MAP
 
 
 @pytest.mark.enable_enqueue_task
@@ -18,69 +20,50 @@ def setUp(self, *args, **kwargs):
         with self.context:
             handlers.celery_before_request()
 
-    @mock.patch('website.mailchimp_utils.get_mailchimp_api')
-    def test_get_list_id_from_name(self, mock_get_mailchimp_api):
-        list_name = 'foo'
-        mock_client = mock.MagicMock()
-        mock_get_mailchimp_api.return_value = mock_client
-        mock_client.lists.list.return_value = {'data': [{'id': 1, 'list_name': list_name}]}
-        list_id = mailchimp_utils.get_list_id_from_name(list_name)
-        mock_client.lists.list.assert_called_with(filters={'list_name': list_name})
-        assert_equal(list_id, 1)
-
     @mock.patch('website.mailchimp_utils.get_mailchimp_api')
     def test_get_list_name_from_id(self, mock_get_mailchimp_api):
         list_id = '12345'
         mock_client = mock.MagicMock()
         mock_get_mailchimp_api.return_value = mock_client
-        mock_client.lists.list.return_value = {'data': [{'id': list_id, 'name': 'foo'}]}
+        mock_client.lists.get.return_value = {'id': list_id, 'name': 'foo'}
         list_name = mailchimp_utils.get_list_name_from_id(list_id)
-        mock_client.lists.list.assert_called_with(filters={'list_id': list_id})
+        mock_client.lists.get.assert_called_with(list_id=list_id)
         assert_equal(list_name, 'foo')
 
     @mock.patch('website.mailchimp_utils.get_mailchimp_api')
-    def test_subscribe_called_with_correct_arguments(self, mock_get_mailchimp_api):
-        list_name = 'foo'
+    def test_subscribe_called(self, mock_get_mailchimp_api):
+        list_name = MAILCHIMP_GENERAL_LIST
         user = UserFactory()
         mock_client = mock.MagicMock()
         mock_get_mailchimp_api.return_value = mock_client
-        mock_client.lists.list.return_value = {'data': [{'id': 1, 'list_name': list_name}]}
+        mock_client.lists.get.return_value = {'id': 1, 'list_name': list_name}
         list_id = mailchimp_utils.get_list_id_from_name(list_name)
         mailchimp_utils.subscribe_mailchimp(list_name, user._id)
         handlers.celery_teardown_request()
-        mock_client.lists.subscribe.assert_called_with(
-            id=list_id,
-            email={'email': user.username},
-            merge_vars={
-                'fname': user.given_name,
-                'lname': user.family_name,
-            },
-            double_optin=False,
-            update_existing=True,
-        )
+        mock_client.lists.members.create_or_update.assert_called()
 
     @mock.patch('website.mailchimp_utils.get_mailchimp_api')
     def test_subscribe_fake_email_does_not_throw_validation_error(self, mock_get_mailchimp_api):
-        list_name = 'foo'
+        list_name = MAILCHIMP_GENERAL_LIST
         user = UserFactory(username='[email protected]')
+        assert list_name not in user.mailchimp_mailing_lists
         mock_client = mock.MagicMock()
         mock_get_mailchimp_api.return_value = mock_client
-        mock_client.lists.list.return_value = {'data': [{'id': 1, 'list_name': list_name}]}
-        mock_client.lists.subscribe.side_effect = mailchimp.ValidationError
+        mock_client.lists.members.create_or_update.side_effect = MailChimpError
         mailchimp_utils.subscribe_mailchimp(list_name, user._id)
         handlers.celery_teardown_request()
         user.reload()
         assert_false(user.mailchimp_mailing_lists[list_name])
 
     @mock.patch('website.mailchimp_utils.get_mailchimp_api')
     def test_unsubscribe_called_with_correct_arguments(self, mock_get_mailchimp_api):
-        list_name = 'foo'
+        list_name = MAILCHIMP_GENERAL_LIST
+        list_id = MAILCHIMP_LIST_MAP[MAILCHIMP_GENERAL_LIST]
         user = UserFactory()
+        user_hash = md5(user.username.lower().encode()).hexdigest()
         mock_client = mock.MagicMock()
         mock_get_mailchimp_api.return_value = mock_client
-        mock_client.lists.list.return_value = {'data': [{'id': 2, 'list_name': list_name}]}
-        list_id = mailchimp_utils.get_list_id_from_name(list_name)
         mailchimp_utils.unsubscribe_mailchimp_async(list_name, user._id)
         handlers.celery_teardown_request()
-        mock_client.lists.unsubscribe.assert_called_with(id=list_id, email={'email': user.username}, send_goodbye=True)
+        mock_client.lists.members.delete.assert_called_with(list_id=list_id, subscriber_hash=user_hash)
 

tests/test_views.py

@@ -3,6 +3,7 @@
 """Views tests for the OSF."""
 
 from __future__ import absolute_import
+from hashlib import md5
 
 import datetime as dt
 from rest_framework import status as http_status
@@ -58,7 +59,7 @@
     send_claim_email,
     send_claim_registered_email,
 )
-from website.settings import EXTERNAL_EMBER_APPS
+from website.settings import EXTERNAL_EMBER_APPS, MAILCHIMP_GENERAL_LIST
 from website.project.views.node import _should_show_wiki_widget, abbrev_authors
 from website.util import api_url_for, web_url_for
 from website.util import rubeus
@@ -1463,14 +1464,16 @@ def test_resend_confirmation_return_emails(self, send_mail):
     @mock.patch('website.mailchimp_utils.get_mailchimp_api')
     def test_update_user_mailing_lists(self, mock_get_mailchimp_api, send_mail):
         email = fake_email()
+        email_hash = md5(email.lower().encode()).hexdigest()
         self.user.emails.create(address=email)
-        list_name = 'foo'
+        list_name = MAILCHIMP_GENERAL_LIST
         self.user.mailchimp_mailing_lists[list_name] = True
         self.user.save()
+        user_hash = md5(self.user.username.lower().encode()).hexdigest()
 
         mock_client = mock.MagicMock()
         mock_get_mailchimp_api.return_value = mock_client
-        mock_client.lists.list.return_value = {'data': [{'id': 1, 'list_name': list_name}]}
+        mock_client.lists.get.return_value = {'id': 1, 'list_name': list_name}
         list_id = mailchimp_utils.get_list_id_from_name(list_name)
 
         url = api_url_for('update_user', uid=self.user._id)
@@ -1482,21 +1485,23 @@ def test_update_user_mailing_lists(self, mock_get_mailchimp_api, send_mail):
         # the test app doesn't have celery handlers attached, so we need to call this manually.
         handlers.celery_teardown_request()
 
-        assert mock_client.lists.unsubscribe.called
-        mock_client.lists.unsubscribe.assert_called_with(
-            id=list_id,
-            email={'email': self.user.username},
-            send_goodbye=True
+        assert mock_client.lists.members.delete.called
+        mock_client.lists.members.delete.assert_called_with(
+            list_id=list_id,
+            subscriber_hash=user_hash
         )
-        mock_client.lists.subscribe.assert_called_with(
-            id=list_id,
-            email={'email': email},
-            merge_vars={
-                'fname': self.user.given_name,
-                'lname': self.user.family_name,
-            },
-            double_optin=False,
-            update_existing=True
+        mock_client.lists.members.create_or_update.assert_called_with(
+            list_id=list_id,
+            subscriber_hash=email_hash,
+            data={
+                'status': 'subscribed',
+                'status_if_new': 'subscribed',
+                'email_address': email,
+                'merge_fields': {
+                    'FNAME': self.user.given_name,
+                    'LNAME': self.user.family_name
+                }
+            }
         )
         handlers.celery_teardown_request()
 
@@ -1505,13 +1510,13 @@ def test_update_user_mailing_lists(self, mock_get_mailchimp_api, send_mail):
     def test_unsubscribe_mailchimp_not_called_if_user_not_subscribed(self, mock_get_mailchimp_api, send_mail):
         email = fake_email()
         self.user.emails.create(address=email)
-        list_name = 'foo'
+        list_name = MAILCHIMP_GENERAL_LIST
         self.user.mailchimp_mailing_lists[list_name] = False
         self.user.save()
 
         mock_client = mock.MagicMock()
         mock_get_mailchimp_api.return_value = mock_client
-        mock_client.lists.list.return_value = {'data': [{'id': 1, 'list_name': list_name}]}
+        mock_client.lists.get.return_value = {'id': 1, 'list_name': list_name}
 
         url = api_url_for('update_user', uid=self.user._id)
         emails = [
@@ -1520,8 +1525,8 @@ def test_unsubscribe_mailchimp_not_called_if_user_not_subscribed(self, mock_get_
         payload = {'locale': '', 'id': self.user._id, 'emails': emails}
         self.app.put_json(url, payload, auth=self.user.auth)
 
-        assert_equal(mock_client.lists.unsubscribe.call_count, 0)
-        assert_equal(mock_client.lists.subscribe.call_count, 0)
+        assert_equal(mock_client.lists.members.delete.call_count, 0)
+        assert_equal(mock_client.lists.members.create_or_update.call_count, 0)
         handlers.celery_teardown_request()
 
     def test_user_update_region(self):
@@ -4269,10 +4274,10 @@ def test_osf_help_mails_unsubscribe(self):
     @mock.patch('website.mailchimp_utils.get_mailchimp_api')
     def test_user_choose_mailing_lists_updates_user_dict(self, mock_get_mailchimp_api):
         user = AuthUserFactory()
-        list_name = 'OSF General'
+        list_name = MAILCHIMP_GENERAL_LIST
         mock_client = mock.MagicMock()
         mock_get_mailchimp_api.return_value = mock_client
-        mock_client.lists.list.return_value = {'data': [{'id': 1, 'list_name': list_name}]}
+        mock_client.lists.get.return_value = {'id': 1, 'list_name': list_name}
         list_id = mailchimp_utils.get_list_id_from_name(list_name)
 
         payload = {settings.MAILCHIMP_GENERAL_LIST: True}
@@ -4290,14 +4295,7 @@ def test_user_choose_mailing_lists_updates_user_dict(self, mock_get_mailchimp_ap
         )
 
         # check that user is subscribed
-        mock_client.lists.subscribe.assert_called_with(id=list_id,
-                                                       email={'email': user.username},
-                                                       merge_vars={
-                                                           'fname': user.given_name,
-                                                           'lname': user.family_name,
-                                                       },
-                                                       double_optin=False,
-                                                       update_existing=True)
+        mock_client.lists.members.create_or_update.assert_called()
 
     def test_get_mailchimp_get_endpoint_returns_200(self):
         url = api_url_for('mailchimp_get_endpoint')
@@ -4310,14 +4308,14 @@ def test_mailchimp_webhook_subscribe_action_does_not_change_user(self, mock_get_
             webhooks update the OSF database.
         """
         list_id = '12345'
-        list_name = 'OSF General'
+        list_name = MAILCHIMP_GENERAL_LIST
         mock_client = mock.MagicMock()
         mock_get_mailchimp_api.return_value = mock_client
-        mock_client.lists.list.return_value = {'data': [{'id': list_id, 'name': list_name}]}
+        mock_client.lists.get.return_value = {'id': list_id, 'name': list_name}
 
         # user is not subscribed to a list
         user = AuthUserFactory()
-        user.mailchimp_mailing_lists = {'OSF General': False}
+        user.mailchimp_mailing_lists = {MAILCHIMP_GENERAL_LIST: False}
         user.save()
 
         # user subscribes and webhook sends request to OSF
@@ -4375,7 +4373,7 @@ def test_sync_data_from_mailchimp_unsubscribes_user(self, mock_get_mailchimp_api
         list_name = 'OSF General'
         mock_client = mock.MagicMock()
         mock_get_mailchimp_api.return_value = mock_client
-        mock_client.lists.list.return_value = {'data': [{'id': list_id, 'name': list_name}]}
+        mock_client.lists.get.return_value = {'id': list_id, 'name': list_name}
 
         # user is subscribed to a list
         user = AuthUserFactory()