From c2f8d646c41df46897c11a5ec292a5a5fa843bc7 Mon Sep 17 00:00:00 2001 From: John Tordoff <> Date: Mon, 18 Mar 2024 11:41:58 -0400 Subject: [PATCH] move mailchimp signals --- framework/auth/signals.py | 1 + osf/models/user.py | 9 +++++++++ osf_tests/test_notable_domains.py | 10 ---------- website/mailchimp_utils.py | 6 +++--- website/profile/views.py | 27 +++++---------------------- 5 files changed, 18 insertions(+), 35 deletions(-) diff --git a/framework/auth/signals.py b/framework/auth/signals.py index 15f279520e5c..4d3dfffff560 100644 --- a/framework/auth/signals.py +++ b/framework/auth/signals.py @@ -11,3 +11,4 @@ user_account_reactivated = signals.signal('user-account-reactivated') unconfirmed_user_created = signals.signal('unconfirmed-user-created') +user_update_mailchimp_subscription = signals.signal('user-update-mailchimp-subscription') diff --git a/osf/models/user.py b/osf/models/user.py index 4969654b28dc..c77968e197ca 100644 --- a/osf/models/user.py +++ b/osf/models/user.py @@ -746,6 +746,15 @@ def merge_user(self, user): notifications_configured = user.notifications_configured.copy() notifications_configured.update(self.notifications_configured) self.notifications_configured = notifications_configured + if not website_settings.RUNNING_MIGRATION: + for key, value in user.mailchimp_mailing_lists.items(): + # subscribe to each list if either user was subscribed + subscription = value or self.mailchimp_mailing_lists.get(key) + signals.user_update_mailchimp_subscription.send(self, list_name=key, subscription=subscription) + + # clear subscriptions for merged user + signals.user_update_mailchimp_subscription.send(user, list_name=key, subscription=False) + for target_id, timestamp in user.comments_viewed_timestamp.items(): if not self.comments_viewed_timestamp.get(target_id): self.comments_viewed_timestamp[target_id] = timestamp diff --git a/osf_tests/test_notable_domains.py b/osf_tests/test_notable_domains.py index 205f0c190370..b89e97c3c58c 100644 --- a/osf_tests/test_notable_domains.py +++ b/osf_tests/test_notable_domains.py @@ -320,7 +320,6 @@ def ignored_notable_domain(self): note=NotableDomain.Note.IGNORED, ) - @pytest.mark.enable_enqueue_task @pytest.mark.parametrize('factory', [NodeFactory, CommentFactory, PreprintFactory, RegistrationFactory, UserFactory]) def test_from_spam_to_unknown_one_spam_domain(self, factory, spam_notable_domain_one, spam_notable_domain_two, unknown_notable_domain, ignored_notable_domain): obj_one = factory() @@ -339,7 +338,6 @@ def test_from_spam_to_unknown_one_spam_domain(self, factory, spam_notable_domain assert obj_one.spam_status == SpamStatus.UNKNOWN assert len(obj_one.spam_data['domains']) == 0 - @pytest.mark.enable_enqueue_task @pytest.mark.parametrize('factory', [NodeFactory, CommentFactory, PreprintFactory, RegistrationFactory, UserFactory]) def test_from_spam_to_unknown_two_spam_domains(self, factory, spam_notable_domain_one, spam_notable_domain_two, unknown_notable_domain, ignored_notable_domain): obj_two = factory() @@ -358,7 +356,6 @@ def test_from_spam_to_unknown_two_spam_domains(self, factory, spam_notable_domai assert obj_two.spam_status == SpamStatus.SPAM assert set(obj_two.spam_data['domains']) == set([self.spam_domain_two.netloc]) - @pytest.mark.enable_enqueue_task @pytest.mark.parametrize('factory', [NodeFactory, CommentFactory, PreprintFactory, RegistrationFactory, UserFactory]) def test_from_spam_to_unknown_marked_by_external(self, factory, spam_notable_domain_one, spam_notable_domain_two, unknown_notable_domain, ignored_notable_domain): obj_three = factory() @@ -379,7 +376,6 @@ def test_from_spam_to_unknown_marked_by_external(self, factory, spam_notable_dom assert obj_three.spam_status == SpamStatus.SPAM assert len(obj_three.spam_data['domains']) == 0 - @pytest.mark.enable_enqueue_task @pytest.mark.parametrize('factory', [NodeFactory, CommentFactory, PreprintFactory, RegistrationFactory, UserFactory]) def test_from_spam_to_ignored_one_spam_domain(self, factory, spam_notable_domain_one, spam_notable_domain_two, unknown_notable_domain, ignored_notable_domain): obj_one = factory() @@ -398,7 +394,6 @@ def test_from_spam_to_ignored_one_spam_domain(self, factory, spam_notable_domain assert obj_one.spam_status == SpamStatus.UNKNOWN assert len(obj_one.spam_data['domains']) == 0 - @pytest.mark.enable_enqueue_task @pytest.mark.parametrize('factory', [NodeFactory, CommentFactory, PreprintFactory, RegistrationFactory, UserFactory]) def test_from_spam_to_ignored_two_spam_domains(self, factory, spam_notable_domain_one, spam_notable_domain_two, unknown_notable_domain, ignored_notable_domain): obj_two = factory() @@ -417,7 +412,6 @@ def test_from_spam_to_ignored_two_spam_domains(self, factory, spam_notable_domai assert obj_two.spam_status == SpamStatus.SPAM assert set(obj_two.spam_data['domains']) == set([self.spam_domain_two.netloc]) - @pytest.mark.enable_enqueue_task @pytest.mark.parametrize('factory', [NodeFactory, CommentFactory, PreprintFactory, RegistrationFactory, UserFactory]) def test_from_spam_to_ignored_makred_by_external(self, factory, spam_notable_domain_one, spam_notable_domain_two, unknown_notable_domain, ignored_notable_domain): obj_three = factory() @@ -438,7 +432,6 @@ def test_from_spam_to_ignored_makred_by_external(self, factory, spam_notable_dom assert obj_three.spam_status == SpamStatus.SPAM assert len(obj_three.spam_data['domains']) == 0 - @pytest.mark.enable_enqueue_task @pytest.mark.parametrize('factory', [NodeFactory, CommentFactory, PreprintFactory, RegistrationFactory, UserFactory]) def test_from_unknown_to_spam_unknown_plus_ignored(self, factory, unknown_notable_domain, ignored_notable_domain): obj_one = factory() @@ -457,7 +450,6 @@ def test_from_unknown_to_spam_unknown_plus_ignored(self, factory, unknown_notabl assert obj_one.spam_status == SpamStatus.SPAM assert set(obj_one.spam_data['domains']) == set([self.unknown_domain.netloc]) - @pytest.mark.enable_enqueue_task @pytest.mark.parametrize('factory', [NodeFactory, CommentFactory, PreprintFactory, RegistrationFactory, UserFactory]) def test_from_unknown_to_spam_unknown_only(self, factory, unknown_notable_domain, ignored_notable_domain): obj_two = factory() @@ -476,7 +468,6 @@ def test_from_unknown_to_spam_unknown_only(self, factory, unknown_notable_domain assert obj_two.spam_status == SpamStatus.SPAM assert set(obj_two.spam_data['domains']) == set([self.unknown_domain.netloc]) - @pytest.mark.enable_enqueue_task @pytest.mark.parametrize('factory', [NodeFactory, CommentFactory, PreprintFactory, RegistrationFactory, UserFactory]) def test_from_ignored_to_spam_unknown_plus_ignored(self, factory, unknown_notable_domain, ignored_notable_domain): obj_one = factory() @@ -495,7 +486,6 @@ def test_from_ignored_to_spam_unknown_plus_ignored(self, factory, unknown_notabl assert obj_one.spam_status == SpamStatus.SPAM assert set(obj_one.spam_data['domains']) == set([self.ignored_domain.netloc]) - @pytest.mark.enable_enqueue_task @pytest.mark.parametrize('factory', [NodeFactory, CommentFactory, PreprintFactory, RegistrationFactory, UserFactory]) def test_from_ignored_to_spam_ignored_only(self, factory, unknown_notable_domain, ignored_notable_domain): obj_two = factory() diff --git a/website/mailchimp_utils.py b/website/mailchimp_utils.py index 2a9aa32ec547..58cad24e70d7 100644 --- a/website/mailchimp_utils.py +++ b/website/mailchimp_utils.py @@ -74,7 +74,7 @@ def subscribe_mailchimp(list_name, user_id): user.save() -def unsubscribe_mailchimp(list_name, user_id, username=None, send_goodbye=True): +def unsubscribe_mailchimp(list_name, user_id, username=None): """Unsubscribe a user from a mailchimp mailing list given its name. :param str list_name: mailchimp mailing list name @@ -113,10 +113,10 @@ def unsubscribe_mailchimp(list_name, user_id, username=None, send_goodbye=True): @queued_task @app.task @transaction.atomic -def unsubscribe_mailchimp_async(list_name, user_id, username=None, send_goodbye=True): +def unsubscribe_mailchimp_async(list_name, user_id, username=None): """ Same args as unsubscribe_mailchimp, used to have the task be run asynchronously """ - unsubscribe_mailchimp(list_name=list_name, user_id=user_id, username=username, send_goodbye=send_goodbye) + unsubscribe_mailchimp(list_name=list_name, user_id=user_id, username=username) @user_confirmed.connect def subscribe_on_confirm(user): diff --git a/website/profile/views.py b/website/profile/views.py index bee7e2699c2f..892cf4cbaddc 100644 --- a/website/profile/views.py +++ b/website/profile/views.py @@ -19,7 +19,8 @@ from framework.auth.signals import ( user_account_merged, user_account_deactivated, - user_account_reactivated + user_account_reactivated, + user_update_mailchimp_subscription ) from framework.exceptions import HTTPError, PermissionsError from framework.flask import redirect # VOL-aware redirect @@ -46,7 +47,6 @@ publish_merged_user ) - from api.waffle.utils import storage_i18n_flag_active logger = logging.getLogger(__name__) @@ -515,8 +515,8 @@ def user_choose_mailing_lists(auth, **kwargs): all_mailing_lists.update(user.osf_mailing_lists) return {'message': 'Successfully updated mailing lists', 'result': all_mailing_lists}, 200 - -def update_mailchimp_subscription(user, list_name, subscription, send_goodbye=True): +@user_update_mailchimp_subscription.connect +def update_mailchimp_subscription(user, list_name, subscription): """ Update mailing list subscription in mailchimp. :param obj user: current user @@ -530,7 +530,7 @@ def update_mailchimp_subscription(user, list_name, subscription, send_goodbye=Tr pass else: try: - mailchimp_utils.unsubscribe_mailchimp_async(list_name, user._id, username=user.username, send_goodbye=send_goodbye) + mailchimp_utils.unsubscribe_mailchimp_async(list_name, user._id, username=user.username) except (MailChimpError, OSFError): # User has already unsubscribed, so nothing to do pass @@ -542,23 +542,6 @@ def send_account_merged_message(user): publish_merged_user(user) -@user_account_deactivated.connect -def send_mailchimp_unsubscribe(user): - """ Tells mailchimp a to unsubscribe a merged user """ - if not settings.RUNNING_MIGRATION: - for key, value in user.mailchimp_mailing_lists.items(): - update_mailchimp_subscription(user, list_name=key, subscription=False, send_goodbye=False) - - -@user_account_merged.connect -def send_mailchimp_merge(user): - """ Tells mailchimp a to merge a users subscriptions """ - if not settings.RUNNING_MIGRATION: - for key, value in user.merged_by.mailchimp_mailing_lists.items(): - subscription = value or user.merged_by.mailchimp_mailing_lists.get(key) - update_mailchimp_subscription(user, list_name=key, subscription=subscription, send_goodbye=False) - - @user_account_deactivated.connect def send_account_deactivation_message(user): """ Sends a message using Celery messaging to alert other services that an osf.io user has been deactivated."""