Merge pull request #19320 from edx/bmedx/remove_3rd_part_mailings_retirement
DEPR-8 - Remove retire_mailings endpoint and supporting code
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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):
|
||||
"""
|
||||
|
||||
@@ -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"])
|
||||
|
||||
|
||||
@@ -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):
|
||||
"""
|
||||
|
||||
@@ -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/
|
||||
|
||||
@@ -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(),
|
||||
|
||||
Reference in New Issue
Block a user