fix mailchimp async return value problem

Author: Johnetordoff

api/users/serializers.py

@@ -38,7 +38,8 @@
 from osf.models.user_message import MessageTypes
 from osf.models.provider import AbstractProviderGroupObjectPermission
 from osf.utils.requests import string_type_request_headers
-from website.profile.views import update_osf_help_mails_subscription, update_mailchimp_subscription
+from website import settings
+from website.profile.views import update_mailchimp_subscription
 from website.settings import MAILCHIMP_GENERAL_LIST, OSF_HELP_LIST, CONFIRM_REGISTRATIONS_BY_EMAIL
 from website.util import api_v2_url
 
@@ -543,10 +544,14 @@ class UserSettingsUpdateSerializer(UserSettingsSerializer):
 
     def update_email_preferences(self, instance, attr, value):
         if self.MAP_MAIL[attr] == OSF_HELP_LIST:
-            update_osf_help_mails_subscription(user=instance, subscribe=value)
-        else:
+            instance.osf_mailing_lists[settings.OSF_HELP_LIST] = value
+        elif self.MAP_MAIL[attr] == MAILCHIMP_GENERAL_LIST:
             update_mailchimp_subscription(instance, self.MAP_MAIL[attr], value)
+        else:
+            raise exceptions.ValidationError(detail='Invalid email preference.')
+
         instance.save()
+        return instance
 
     def update_two_factor(self, instance, value, two_factor_addon):
         if value:
@@ -581,6 +586,15 @@ def to_representation(self, instance):
         Overriding to_representation allows using different serializers for the request and response.
         """
         context = self.context
+
+        # special case to show that we sent a request to subscribe, but don't
+        # change actual value until the task executes in case it doesn't and they need to try again
+        if getattr(instance, 'assume_mailchimp_completed_request', False):
+            instance.mailchimp_mailing_lists[MAILCHIMP_GENERAL_LIST] = True
+            instance.save = lambda _: NotImplementedError(
+                'Cannot save read comments in UserSettingsUpdateSerializer.to_representation',
+            )
+
         return UserSettingsSerializer(instance=instance, context=context).data
 
     def update(self, instance, validated_data):
@@ -597,6 +611,11 @@ def update(self, instance, validated_data):
             elif attr in self.MAP_MAIL.keys():
                 self.update_email_preferences(instance, attr, value)
 
+                # special case to show that we sent a request to subscribe, but don't
+                # change actual value until the task executes in case it doesn't and they need to try again
+                if value and self.MAP_MAIL[attr] == MAILCHIMP_GENERAL_LIST:
+                    instance.assume_mailchimp_completed_request = True
+
         return instance
 
 

api_tests/users/views/test_user_settings_detail.py

@@ -206,8 +206,10 @@ def test_authorized_patch_200(self, mailchimp_mock, app, user_one, payload, url)
         assert res.status_code == 200
 
         user_one.refresh_from_db()
+        assert res.json['data']['attributes']['subscribe_osf_help_email'] is False
         assert user_one.osf_mailing_lists[OSF_HELP_LIST] is False
         mailchimp_mock.assert_called_with(user_one, MAILCHIMP_GENERAL_LIST, True)
+        assert res.json['data']['attributes']['subscribe_osf_general_email'] is True
 
     def test_bad_payload_patch_400(self, app, user_one, bad_payload, url):
         res = app.patch_json_api(url, bad_payload, auth=user_one.auth, expect_errors=True)

website/profile/views.py

@@ -496,7 +496,7 @@ def user_choose_mailing_lists(auth, **kwargs):
         for list_name, subscribe in json_data.items():
             # TO DO: change this to take in any potential non-mailchimp, something like try: update_subscription(), except IndexNotFound: update_mailchimp_subscription()
             if list_name == settings.OSF_HELP_LIST:
-                update_osf_help_mails_subscription(user=user, subscribe=subscribe)
+                user.osf_mailing_lists[settings.OSF_HELP_LIST] = subscribe
             else:
                 update_mailchimp_subscription(user, list_name, subscribe)
     else:
@@ -602,10 +602,6 @@ def impute_names(**kwargs):
     return auth_utils.impute_names(name)
 
 
-def update_osf_help_mails_subscription(user, subscribe):
-    user.osf_mailing_lists[settings.OSF_HELP_LIST] = subscribe
-    user.save()
-
 @must_be_logged_in
 def serialize_names(**kwargs):
     user = kwargs['auth'].user