From 9b3abd5598f4a7abc3760e4b724b2ce23506788f Mon Sep 17 00:00:00 2001 From: bmedx Date: Tue, 27 Nov 2018 10:22:05 -0500 Subject: [PATCH] DEPR-8 - Remove retire_mailings endpoint and supporting code --- lms/djangoapps/email_marketing/signals.py | 57 --------------- .../email_marketing/tests/test_signals.py | 48 ------------- .../djangoapps/user_api/accounts/signals.py | 3 - .../accounts/tests/test_retirement_views.py | 70 ------------------- .../djangoapps/user_api/accounts/views.py | 40 ----------- openedx/core/djangoapps/user_api/urls.py | 6 -- 6 files changed, 224 deletions(-) diff --git a/lms/djangoapps/email_marketing/signals.py b/lms/djangoapps/email_marketing/signals.py index 512e122cb9..6c66f33e9e 100644 --- a/lms/djangoapps/email_marketing/signals.py +++ b/lms/djangoapps/email_marketing/signals.py @@ -9,7 +9,6 @@ import crum from celery.exceptions import TimeoutError from django.conf import settings from django.dispatch import receiver -from sailthru.sailthru_client import SailthruClient from sailthru.sailthru_error import SailthruClientError from six import text_type @@ -19,7 +18,6 @@ from email_marketing.models import EmailMarketingConfiguration from lms.djangoapps.email_marketing.tasks import get_email_cookies_via_sailthru, update_user, update_user_email from openedx.core.djangoapps.user_authn.cookies import CREATE_LOGON_COOKIE from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY -from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_THIRD_PARTY_MAILINGS from openedx.core.djangoapps.waffle_utils import WaffleSwitchNamespace from student.signals import SAILTHRU_AUDIT_PURCHASE from student.views import REGISTER_USER @@ -261,58 +259,3 @@ def _log_sailthru_api_call_time(time_before_call): time_before_call.isoformat(' '), time_after_call.isoformat(' '), delta_sailthru_api_call_time.microseconds / 1000) - - -@receiver(USER_RETIRE_THIRD_PARTY_MAILINGS) -def force_unsubscribe_all(sender, **kwargs): # pylint: disable=unused-argument - """ - Synchronously(!) unsubscribes the given user from all Sailthru email lists. - - In the future this could be moved to a Celery task, however this is currently - only used as part of user retirement, where we need a very reliable indication - of success or failure. - - Args: - email: Email address to unsubscribe - new_email (optional): Email address to change 3rd party services to for this user (used in retirement to clear - personal information from the service) - Returns: - None - """ - email = kwargs.get('email', None) - new_email = kwargs.get('new_email', None) - - if not email: - raise TypeError('Expected an email address to unsubscribe, but received None.') - - email_config = EmailMarketingConfiguration.current() - if not email_config.enabled: - return - - sailthru_parms = { - "id": email, - "optout_email": "all", - "fields": {"optout_email": 1} - } - - # If we have a new email address to change to, do that as well - if new_email: - sailthru_parms["keys"] = { - "email": new_email - } - sailthru_parms["fields"]["keys"] = 1 - sailthru_parms["keysconflict"] = "merge" - - try: - sailthru_client = SailthruClient(email_config.sailthru_key, email_config.sailthru_secret) - sailthru_response = sailthru_client.api_post("user", sailthru_parms) - except SailthruClientError as exc: - error_msg = "Exception attempting to opt-out user {} from Sailthru - {}".format(email, text_type(exc)) - log.error(error_msg) - raise Exception(error_msg) - - if not sailthru_response.is_ok(): - error = sailthru_response.get_error() - error_msg = "Error attempting to opt-out user {} from Sailthru - {}".format(email, error.get_message()) - log.error(error_msg) - raise Exception(error_msg) diff --git a/lms/djangoapps/email_marketing/tests/test_signals.py b/lms/djangoapps/email_marketing/tests/test_signals.py index f82009362a..d15b1f6cec 100644 --- a/lms/djangoapps/email_marketing/tests/test_signals.py +++ b/lms/djangoapps/email_marketing/tests/test_signals.py @@ -20,7 +20,6 @@ from email_marketing.signals import ( add_email_marketing_cookies, email_marketing_register_user, email_marketing_user_field_changed, - force_unsubscribe_all, update_sailthru ) from email_marketing.tasks import ( @@ -555,53 +554,6 @@ class EmailMarketingTests(TestCase): email_marketing_user_field_changed(None, self.user, table='auth_user', setting='email', old_value='new@a.com') self.assertFalse(mock_update_user.called) - @patch('email_marketing.tasks.log.error') - @patch('email_marketing.tasks.SailthruClient.api_post') - def test_sailthru_on_success(self, mock_sailthru, mock_log_error): - """ - Ensure that a successful unsubscribe does not trigger an error log - """ - mock_sailthru.return_value = SailthruResponse(JsonResponse({'ok': True})) - force_unsubscribe_all(sender=self.__class__, email=self.user.email) - self.assertTrue(mock_sailthru.called) - self.assertFalse(mock_log_error.called) - - @patch('email_marketing.tasks.log.error') - @patch('email_marketing.tasks.SailthruClient.api_post') - def test_sailthru_on_connection_error(self, mock_sailthru, mock_log_error): - mock_sailthru.side_effect = SailthruClientError - with self.assertRaises(Exception): - force_unsubscribe_all(sender=self.__class__, email=self.user.email) - self.assertTrue(mock_log_error.called) - - @patch('email_marketing.tasks.log.error') - @patch('email_marketing.tasks.SailthruClient.api_post') - def test_sailthru_on_bad_response(self, mock_sailthru, mock_log_error): - mock_sailthru.return_value = SailthruResponse(JsonResponse({'error': 100, 'errormsg': 'Got an error'})) - with self.assertRaises(Exception): - force_unsubscribe_all(sender=self.__class__, email=self.user.email) - self.assertTrue(mock_sailthru.called) - self.assertTrue(mock_log_error.called) - - @patch('email_marketing.tasks.log.error') - @patch('email_marketing.tasks.SailthruClient.api_post') - def test_sailthru_with_email_changed(self, mock_sailthru, mock_log_error): - mock_sailthru.return_value = SailthruResponse(JsonResponse({'ok': True})) - force_unsubscribe_all(sender=self.__class__, email=self.user.email, new_email='email@somewhere.invalid') - mock_sailthru.assert_called_with("user", { - "id": self.user.email, - "optout_email": "all", - "keys": { - "email": "email@somewhere.invalid" - }, - "fields": { - "optout_email": 1, - "keys": 1 - }, - "keysconflict": "merge" - }) - self.assertFalse(mock_log_error.called) - class MockSailthruResponse(object): """ diff --git a/openedx/core/djangoapps/user_api/accounts/signals.py b/openedx/core/djangoapps/user_api/accounts/signals.py index 107ccdf064..752f3dc00b 100644 --- a/openedx/core/djangoapps/user_api/accounts/signals.py +++ b/openedx/core/djangoapps/user_api/accounts/signals.py @@ -4,9 +4,6 @@ Django Signal related functionality for user_api accounts from django.dispatch import Signal -# Signal to retire a user from third party mailing services, such as Sailthru. -USER_RETIRE_THIRD_PARTY_MAILINGS = Signal(providing_args=["user"]) - # Signal to retire a user from LMS-initiated mailings (course mailings, etc) USER_RETIRE_MAILINGS = Signal(providing_args=["user"]) diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py index bb971fc92f..ae0c1822a8 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py @@ -46,7 +46,6 @@ from openedx.core.djangoapps.credit.models import ( from openedx.core.djangoapps.course_groups.models import CourseUserGroup, UnregisteredLearnerCohortAssignments from openedx.core.djangoapps.oauth_dispatch.jwt import create_jwt_for_user from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory -from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_THIRD_PARTY_MAILINGS from openedx.core.djangoapps.user_api.models import ( RetirementState, UserRetirementStatus, @@ -248,75 +247,6 @@ class TestDeactivateLogout(RetirementTestCase): self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) -@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') -class TestAccountRetireMailings(RetirementTestCase): - """ - Tests the account retire mailings endpoint. - """ - def setUp(self): - super(TestAccountRetireMailings, self).setUp() - self.test_superuser = SuperuserFactory() - self.test_service_user = UserFactory() - - # Should be created in parent setUpClass - retiring_email_lists = RetirementState.objects.get(state_name='RETIRING_EMAIL_LISTS') - self.retirement = create_retirement_status(UserFactory(), state=retiring_email_lists) - self.test_user = self.retirement.user - - self.url = reverse('accounts_retire_mailings') - - def build_post(self, user): - return {'username': user.username} - - def assert_status(self, headers, expected_status=status.HTTP_204_NO_CONTENT, expected_content=None): - """ - Helper function for making a request to the retire subscriptions endpoint, and asserting the status. - """ - response = self.client.post(self.url, self.build_post(self.test_user), **headers) - - self.assertEqual(response.status_code, expected_status) - - if expected_content: - self.assertEqual(response.content.strip('"'), expected_content) - - def test_superuser_retires_user_subscriptions(self): - """ - Verify a user's subscriptions are retired when a superuser posts to the retire subscriptions endpoint. - """ - headers = build_jwt_headers(self.test_superuser) - self.assert_status(headers) - - def test_unauthorized_rejection(self): - """ - Verify unauthorized users cannot retire subscriptions. - """ - headers = build_jwt_headers(self.test_user) - - # User should still have 2 "True" subscriptions. - self.assert_status(headers, expected_status=status.HTTP_403_FORBIDDEN) - - def test_signal_failure(self): - """ - Verify that if a signal fails the transaction is rolled back and a proper error message is returned. - """ - headers = build_jwt_headers(self.test_superuser) - - mock_handler = mock.MagicMock() - mock_handler.side_effect = Exception("Tango") - - try: - USER_RETIRE_THIRD_PARTY_MAILINGS.connect(mock_handler) - - # User should still have 2 "True" subscriptions. - self.assert_status( - headers, - expected_status=status.HTTP_500_INTERNAL_SERVER_ERROR, - expected_content="Tango" - ) - finally: - USER_RETIRE_THIRD_PARTY_MAILINGS.disconnect(mock_handler) - - @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') class TestPartnerReportingCleanup(ModuleStoreTestCase): """ diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index e5d35b955e..1ed32c376e 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -75,7 +75,6 @@ from .signals import ( USER_RETIRE_LMS_CRITICAL, USER_RETIRE_LMS_MISC, USER_RETIRE_MAILINGS, - USER_RETIRE_THIRD_PARTY_MAILINGS ) from ..message_types import DeletionNotificationMessage @@ -335,45 +334,6 @@ class AccountDeactivationView(APIView): return Response(get_account_settings(request, [username])[0]) -class AccountRetireMailingsView(APIView): - """ - Part of the retirement API, accepts POSTs to unsubscribe a user - from all EXTERNAL email lists (ex: Sailthru). LMS email subscriptions - are handled in the LMS retirement endpoints. - """ - authentication_classes = (JwtAuthentication, ) - permission_classes = (permissions.IsAuthenticated, CanRetireUser) - - def post(self, request): - """ - POST /api/user/v1/accounts/{username}/retire_mailings/ - - Fires the USER_RETIRE_THIRD_PARTY_MAILINGS signal, currently the - only receiver is email_marketing to force opt-out the user from - externally managed email lists. - """ - username = request.data['username'] - - try: - retirement = UserRetirementStatus.get_retirement_for_retirement_action(username) - - with transaction.atomic(): - # This signal allows lms' email_marketing and other 3rd party email - # providers to unsubscribe the user - USER_RETIRE_THIRD_PARTY_MAILINGS.send( - sender=self.__class__, - email=retirement.original_email, - new_email=retirement.retired_email, - user=retirement.user - ) - - return Response(status=status.HTTP_204_NO_CONTENT) - except UserRetirementStatus.DoesNotExist: - return Response(status=status.HTTP_404_NOT_FOUND) - except Exception as exc: # pylint: disable=broad-except - return Response(text_type(exc), status=status.HTTP_500_INTERNAL_SERVER_ERROR) - - class DeactivateLogoutView(APIView): """ POST /api/user/v1/accounts/deactivate_logout/ diff --git a/openedx/core/djangoapps/user_api/urls.py b/openedx/core/djangoapps/user_api/urls.py index 4eba47566d..bdea5668ca 100644 --- a/openedx/core/djangoapps/user_api/urls.py +++ b/openedx/core/djangoapps/user_api/urls.py @@ -8,7 +8,6 @@ from django.conf.urls import url from ..profile_images.views import ProfileImageView from .accounts.views import ( AccountDeactivationView, - AccountRetireMailingsView, AccountRetirementPartnerReportView, AccountRetirementStatusView, AccountRetirementView, @@ -96,11 +95,6 @@ urlpatterns = [ AccountDeactivationView.as_view(), name='accounts_deactivation' ), - url( - r'^v1/accounts/retire_mailings/$', - AccountRetireMailingsView.as_view(), - name='accounts_retire_mailings' - ), url( r'^v1/accounts/deactivate_logout/$', DeactivateLogoutView.as_view(),