PLAT-2028 - Create mailing list retirement API endpoint
- Removes "email-optin" UserOrgTags for the user - Creates and uses a new "UserRetireMailingsSignal" signal - Creates and uses a new "CanRetireUser" permission - Creates and uses a new setting "RETIREMENT_SERVICE_WORKER_USERNAME" - Creates a signal handler to globally opt-out the user from Sailthru
This commit is contained in:
25
lms/djangoapps/bulk_email/signals.py
Normal file
25
lms/djangoapps/bulk_email/signals.py
Normal file
@@ -0,0 +1,25 @@
|
||||
"""
|
||||
Signal handlers for the bulk_email app
|
||||
"""
|
||||
|
||||
from django.dispatch import receiver
|
||||
|
||||
from student.models import CourseEnrollment
|
||||
from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_MAILINGS
|
||||
|
||||
from .models import Optout
|
||||
|
||||
|
||||
@receiver(USER_RETIRE_MAILINGS)
|
||||
def force_optout_all(sender, **kwargs): # pylint: disable=unused-argument
|
||||
"""
|
||||
When a user is retired from all mailings this method will create an Optout
|
||||
row for any courses they may be enrolled in.
|
||||
"""
|
||||
user = kwargs.get('user', None)
|
||||
|
||||
if not user:
|
||||
raise TypeError('Expected a User type, but received None.')
|
||||
|
||||
for enrollment in CourseEnrollment.objects.filter(user=user):
|
||||
Optout.objects.get_or_create(user=user, course_id=enrollment.course.id)
|
||||
87
lms/djangoapps/bulk_email/tests/test_signals.py
Normal file
87
lms/djangoapps/bulk_email/tests/test_signals.py
Normal file
@@ -0,0 +1,87 @@
|
||||
"""
|
||||
Unit tests for student optouts from course email
|
||||
"""
|
||||
import json
|
||||
|
||||
from django.core import mail
|
||||
from django.core.management import call_command
|
||||
from django.core.urlresolvers import reverse
|
||||
from mock import Mock, patch
|
||||
from nose.plugins.attrib import attr
|
||||
from six import text_type
|
||||
|
||||
from bulk_email.models import BulkEmailFlag, Optout
|
||||
from bulk_email.signals import force_optout_all
|
||||
from student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
|
||||
from xmodule.modulestore.tests.factories import CourseFactory
|
||||
|
||||
|
||||
@attr(shard=1)
|
||||
@patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message', autospec=True))
|
||||
class TestOptoutCourseEmailsBySignal(ModuleStoreTestCase):
|
||||
"""
|
||||
Tests that the force_optout_all signal receiver opts the user out of course emails
|
||||
"""
|
||||
def setUp(self):
|
||||
super(TestOptoutCourseEmailsBySignal, self).setUp()
|
||||
self.course = CourseFactory.create(run='testcourse1', display_name="Test Course Title")
|
||||
self.instructor = AdminFactory.create()
|
||||
self.student = UserFactory.create()
|
||||
self.enrollment = CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id)
|
||||
|
||||
# load initial content (since we don't run migrations as part of tests):
|
||||
call_command("loaddata", "course_email_template.json")
|
||||
|
||||
self.client.login(username=self.student.username, password="test")
|
||||
|
||||
self.send_mail_url = reverse('send_email', kwargs={'course_id': text_type(self.course.id)})
|
||||
self.success_content = {
|
||||
'course_id': text_type(self.course.id),
|
||||
'success': True,
|
||||
}
|
||||
BulkEmailFlag.objects.create(enabled=True, require_course_email_auth=False)
|
||||
|
||||
def test_optout_row_created_on_signal(self):
|
||||
"""
|
||||
Make sure the correct row is created for a user enrolled in a course
|
||||
"""
|
||||
force_optout_all(sender=self.__class__, user=self.student)
|
||||
self.assertEqual(Optout.objects.filter(user=self.student, course_id=self.course.id).count(), 1)
|
||||
|
||||
def send_test_email(self):
|
||||
"""
|
||||
Navigate to the instructor dash's email view to send bulk email
|
||||
"""
|
||||
# Pull up email view on instructor dashboard
|
||||
url = reverse('instructor_dashboard', kwargs={'course_id': text_type(self.course.id)})
|
||||
response = self.client.get(url)
|
||||
email_section = '<div class="vert-left send-email" id="section-send-email">'
|
||||
|
||||
# If this fails, it is likely because BulkEmailFlag.is_enabled() is set to False
|
||||
self.assertIn(email_section, response.content)
|
||||
|
||||
test_email = {
|
||||
'action': 'Send email',
|
||||
'send_to': '["myself", "staff", "learners"]',
|
||||
'subject': 'test subject for all',
|
||||
'message': 'test message for all'
|
||||
}
|
||||
response = self.client.post(self.send_mail_url, test_email)
|
||||
self.assertEquals(json.loads(response.content), self.success_content)
|
||||
|
||||
def test_optout_course(self):
|
||||
"""
|
||||
Make sure student does not receive course email after being opted out
|
||||
"""
|
||||
# Use the signal receiver to for the opt-out
|
||||
force_optout_all(sender=self.__class__, user=self.student)
|
||||
|
||||
# Try to send a bulk course email
|
||||
self.client.login(username=self.instructor.username, password="test")
|
||||
self.send_test_email()
|
||||
|
||||
# Assert that self.student.email not in mail.to, outbox should only contain "myself" target
|
||||
self.assertEqual(len(mail.outbox), 1)
|
||||
self.assertEqual(len(mail.outbox[0].to), 1)
|
||||
self.assertEqual(mail.outbox[0].to[0], self.instructor.email)
|
||||
@@ -6,14 +6,17 @@ import logging
|
||||
from random import randint
|
||||
|
||||
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 celery.exceptions import TimeoutError
|
||||
from six import text_type
|
||||
|
||||
import third_party_auth
|
||||
from course_modes.models import CourseMode
|
||||
from email_marketing.models import EmailMarketingConfiguration
|
||||
from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_MAILINGS
|
||||
from openedx.core.djangoapps.waffle_utils import WaffleSwitchNamespace
|
||||
from lms.djangoapps.email_marketing.tasks import update_user, update_user_email, get_email_cookies_via_sailthru
|
||||
from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY
|
||||
@@ -94,10 +97,10 @@ def add_email_marketing_cookies(sender, response=None, user=None,
|
||||
_log_sailthru_api_call_time(time_before_call)
|
||||
|
||||
except TimeoutError as exc:
|
||||
log.error("Timeout error while attempting to obtain cookie from Sailthru: %s", unicode(exc))
|
||||
log.error("Timeout error while attempting to obtain cookie from Sailthru: %s", text_type(exc))
|
||||
return response
|
||||
except SailthruClientError as exc:
|
||||
log.error("Exception attempting to obtain cookie from Sailthru: %s", unicode(exc))
|
||||
log.error("Exception attempting to obtain cookie from Sailthru: %s", text_type(exc))
|
||||
return response
|
||||
except Exception:
|
||||
log.error("Exception Connecting to celery task for %s", user.email)
|
||||
@@ -226,7 +229,7 @@ def _create_sailthru_user_vars(user, profile, registration=None):
|
||||
|
||||
if profile.year_of_birth:
|
||||
sailthru_vars['year_of_birth'] = profile.year_of_birth
|
||||
sailthru_vars['country'] = unicode(profile.country.code)
|
||||
sailthru_vars['country'] = text_type(profile.country.code)
|
||||
|
||||
if registration:
|
||||
sailthru_vars['activation_key'] = registration.activation_key
|
||||
@@ -258,3 +261,43 @@ 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_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:
|
||||
user(User): Django model of type returned from get_user_model()
|
||||
Returns:
|
||||
None
|
||||
"""
|
||||
user = kwargs.get('user', None)
|
||||
|
||||
if not user:
|
||||
raise TypeError('Expected a User type, but received None.')
|
||||
|
||||
email_config = EmailMarketingConfiguration.current()
|
||||
if not email_config.enabled:
|
||||
return
|
||||
|
||||
sailthru_parms = {"id": user.email, "keys": {"optout_email": "all"}}
|
||||
|
||||
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 %s from Sailthru - %s" % (user.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 %s from Sailthru - %s" % (user.email, error.get_message())
|
||||
log.error(error_msg)
|
||||
raise Exception(error_msg)
|
||||
|
||||
@@ -20,6 +20,7 @@ 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 (
|
||||
@@ -578,6 +579,34 @@ 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__, user=self.user)
|
||||
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__, user=self.user)
|
||||
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__, user=self.user)
|
||||
self.assertTrue(mock_sailthru.called)
|
||||
self.assertTrue(mock_log_error.called)
|
||||
|
||||
|
||||
class MockSailthruResponse(object):
|
||||
"""
|
||||
|
||||
7
openedx/core/djangoapps/user_api/accounts/signals.py
Normal file
7
openedx/core/djangoapps/user_api/accounts/signals.py
Normal file
@@ -0,0 +1,7 @@
|
||||
"""
|
||||
Django Signal related functionality for user_api accounts
|
||||
"""
|
||||
|
||||
from django.dispatch import Signal
|
||||
|
||||
USER_RETIRE_MAILINGS = Signal(providing_args=["user"])
|
||||
@@ -15,18 +15,19 @@ from django.core.urlresolvers import reverse
|
||||
from django.test import TestCase
|
||||
from django.test.testcases import TransactionTestCase
|
||||
from django.test.utils import override_settings
|
||||
from mock import patch
|
||||
from mock import MagicMock, patch
|
||||
from nose.plugins.attrib import attr
|
||||
from pytz import UTC
|
||||
from rest_framework import status
|
||||
from rest_framework.test import APIClient, APITestCase
|
||||
|
||||
from openedx.core.djangoapps.user_api.accounts import ACCOUNT_VISIBILITY_PREF_KEY
|
||||
from openedx.core.djangoapps.user_api.models import UserPreference
|
||||
from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_MAILINGS
|
||||
from openedx.core.djangoapps.user_api.models import UserPreference, UserOrgTag
|
||||
from openedx.core.djangoapps.user_api.preferences.api import set_user_preference
|
||||
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms
|
||||
from openedx.core.lib.token_utils import JwtBuilder
|
||||
from student.models import LanguageProficiency, PendingEmailChange, UserProfile
|
||||
from student.models import PendingEmailChange, UserProfile, get_retired_username_by_username
|
||||
from student.tests.factories import (
|
||||
TEST_PASSWORD,
|
||||
ContentTypeFactory,
|
||||
@@ -918,3 +919,96 @@ class TestAccountDeactivation(TestCase):
|
||||
expected_status=status.HTTP_401_UNAUTHORIZED,
|
||||
expected_activation_status=True
|
||||
)
|
||||
|
||||
|
||||
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS')
|
||||
class TestAccountRetireMailings(TestCase):
|
||||
"""
|
||||
Tests the account retire mailings endpoint.
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
super(TestAccountRetireMailings, self).setUp()
|
||||
self.test_user = UserFactory()
|
||||
self.test_superuser = SuperuserFactory()
|
||||
self.test_service_user = UserFactory()
|
||||
|
||||
UserOrgTag.objects.create(user=self.test_user, key='email-optin', org="foo", value="True")
|
||||
UserOrgTag.objects.create(user=self.test_user, key='email-optin', org="bar", value="True")
|
||||
|
||||
self.url = reverse('accounts_retire_mailings', kwargs={'username': self.test_user.username})
|
||||
|
||||
def build_jwt_headers(self, user):
|
||||
"""
|
||||
Helper function for creating headers for the JWT authentication.
|
||||
"""
|
||||
token = JwtBuilder(user).build_token([])
|
||||
headers = {
|
||||
'HTTP_AUTHORIZATION': 'JWT ' + token
|
||||
}
|
||||
return headers
|
||||
|
||||
def build_post(self, user):
|
||||
retired_username = get_retired_username_by_username(user.username)
|
||||
return {'retired_username': retired_username}
|
||||
|
||||
def assert_status_and_tag_count(self, headers, expected_status=status.HTTP_204_NO_CONTENT, expected_tag_count=2,
|
||||
expected_tag_value="False", 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)
|
||||
|
||||
# Check that the expected number of tags with the correct value exist
|
||||
tag_count = UserOrgTag.objects.filter(user=self.test_user, value=expected_tag_value).count()
|
||||
self.assertEqual(tag_count, expected_tag_count)
|
||||
|
||||
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 = self.build_jwt_headers(self.test_superuser)
|
||||
self.assert_status_and_tag_count(headers)
|
||||
|
||||
def test_superuser_retires_user_subscriptions_no_orgtags(self):
|
||||
"""
|
||||
Verify the call succeeds when the user doesn't have any org tags.
|
||||
"""
|
||||
UserOrgTag.objects.all().delete()
|
||||
headers = self.build_jwt_headers(self.test_superuser)
|
||||
self.assert_status_and_tag_count(headers, expected_tag_count=0)
|
||||
|
||||
def test_unauthorized_rejection(self):
|
||||
"""
|
||||
Verify unauthorized users cannot retire subscriptions.
|
||||
"""
|
||||
headers = self.build_jwt_headers(self.test_user)
|
||||
|
||||
# User should still have 2 "True" subscriptions.
|
||||
self.assert_status_and_tag_count(headers, expected_status=status.HTTP_403_FORBIDDEN, expected_tag_value="True")
|
||||
|
||||
def test_signal_failure(self):
|
||||
"""
|
||||
Verify that if a signal fails the transaction is rolled back and a proper error message is returned.
|
||||
"""
|
||||
headers = self.build_jwt_headers(self.test_superuser)
|
||||
|
||||
mock_handler = MagicMock()
|
||||
mock_handler.side_effect = Exception("Tango")
|
||||
|
||||
try:
|
||||
USER_RETIRE_MAILINGS.connect(mock_handler)
|
||||
|
||||
# User should still have 2 "True" subscriptions.
|
||||
self.assert_status_and_tag_count(
|
||||
headers,
|
||||
expected_status=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
expected_tag_value="True",
|
||||
expected_content="Tango"
|
||||
)
|
||||
finally:
|
||||
USER_RETIRE_MAILINGS.disconnect(mock_handler)
|
||||
|
||||
@@ -6,22 +6,28 @@ https://openedx.atlassian.net/wiki/display/TNL/User+API
|
||||
"""
|
||||
|
||||
from django.db import transaction
|
||||
from django.contrib.auth import get_user_model
|
||||
from edx_rest_framework_extensions.authentication import JwtAuthentication
|
||||
from rest_framework import permissions
|
||||
from rest_framework import status
|
||||
from rest_framework.response import Response
|
||||
from rest_framework.views import APIView
|
||||
from rest_framework.viewsets import ViewSet
|
||||
from six import text_type
|
||||
|
||||
from .api import get_account_settings, update_account_settings
|
||||
from .permissions import CanDeactivateUser
|
||||
from ..errors import UserNotFound, UserNotAuthorized, AccountUpdateError, AccountValidationError
|
||||
from openedx.core.djangoapps.user_api.preferences.api import update_email_opt_in
|
||||
from openedx.core.lib.api.authentication import (
|
||||
SessionAuthenticationAllowInactiveUser,
|
||||
OAuth2AuthenticationAllowInactiveUser,
|
||||
)
|
||||
from openedx.core.lib.api.parsers import MergePatchParser
|
||||
from student.models import User
|
||||
from student.models import User, get_potentially_retired_user_by_username_and_hash
|
||||
|
||||
from .api import get_account_settings, update_account_settings
|
||||
from .permissions import CanDeactivateUser, CanRetireUser
|
||||
from .signals import USER_RETIRE_MAILINGS
|
||||
from ..errors import UserNotFound, UserNotAuthorized, AccountUpdateError, AccountValidationError
|
||||
from ..models import UserOrgTag
|
||||
|
||||
|
||||
class AccountViewSet(ViewSet):
|
||||
@@ -249,3 +255,42 @@ class AccountDeactivationView(APIView):
|
||||
user.save()
|
||||
account_settings = get_account_settings(request, [username])[0]
|
||||
return Response(account_settings)
|
||||
|
||||
|
||||
class AccountRetireMailingsView(APIView):
|
||||
"""
|
||||
Part of the retirement API, accepts POSTs to unsubscribe a user
|
||||
from all email lists.
|
||||
"""
|
||||
authentication_classes = (JwtAuthentication, )
|
||||
permission_classes = (permissions.IsAuthenticated, CanRetireUser)
|
||||
|
||||
def post(self, request, username):
|
||||
"""
|
||||
POST /api/user/v1/accounts/{username}/retire_mailings/
|
||||
|
||||
Allows an administrative user to take the following actions
|
||||
on behalf of an LMS user:
|
||||
- Update UserOrgTags to opt the user out of org emails
|
||||
- Call Sailthru API to force opt-out the user from all email lists
|
||||
"""
|
||||
user_model = get_user_model()
|
||||
retired_username = request.data['retired_username']
|
||||
|
||||
try:
|
||||
user = get_potentially_retired_user_by_username_and_hash(username, retired_username)
|
||||
|
||||
with transaction.atomic():
|
||||
# Take care of org emails first, using the existing API for consistency
|
||||
for preference in UserOrgTag.objects.filter(user=user, key='email-optin'):
|
||||
update_email_opt_in(user, preference.org, False)
|
||||
|
||||
# This signal allows lms' email_marketing and other 3rd party email
|
||||
# providers to unsubscribe the user as well
|
||||
USER_RETIRE_MAILINGS.send(sender=self.__class__, user=user)
|
||||
except user_model.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)
|
||||
|
||||
return Response(status=status.HTTP_204_NO_CONTENT)
|
||||
|
||||
@@ -6,7 +6,7 @@ from django.conf import settings
|
||||
from django.conf.urls import url
|
||||
|
||||
from ..profile_images.views import ProfileImageView
|
||||
from .accounts.views import AccountDeactivationView, AccountViewSet
|
||||
from .accounts.views import AccountDeactivationView, AccountRetireMailingsView, AccountViewSet
|
||||
from .preferences.views import PreferencesDetailView, PreferencesView
|
||||
from .verification_api.views import PhotoVerificationStatusView
|
||||
from .validation.views import RegistrationValidationView
|
||||
@@ -50,6 +50,11 @@ urlpatterns = [
|
||||
AccountDeactivationView.as_view(),
|
||||
name='accounts_deactivation'
|
||||
),
|
||||
url(
|
||||
r'^v1/accounts/{}/retire_mailings/$'.format(settings.USERNAME_PATTERN),
|
||||
AccountRetireMailingsView.as_view(),
|
||||
name='accounts_retire_mailings'
|
||||
),
|
||||
url(
|
||||
r'^v1/accounts/{}/verification_status/$'.format(settings.USERNAME_PATTERN),
|
||||
PhotoVerificationStatusView.as_view(),
|
||||
|
||||
Reference in New Issue
Block a user