From 00653433a5370378589ce3a4da4e0cd6825247dc Mon Sep 17 00:00:00 2001 From: Taranjeet Singh Date: Thu, 13 Sep 2018 12:49:47 +0530 Subject: [PATCH] Adds optional "unsubscribe" link and api support to let users opt out of email updates. Scheduled emails show "unsubscribe" link if waffle switch `schedules.course_update_show_unsubscribe` is enabled, and settings.ACE_ENABLED_POLICIES respects `bulk_email_optout`. API endpoint allows GET/POST requests, which: * GET asks for confirmation of opt-out * POST accepts "unsubscribe" or "cancel", where "unsubscribe" creates the Optout entry, and "cancel" does nothing. Fixes flaky tests: * The resolvers handle users in "bins", which are groups that depend on the user ID. * The test user ID varies depending on the test order. * This change ensures that the bin requested matches the user for the test. --- lms/djangoapps/bulk_email/tests/test_views.py | 82 ++++++++++++++++ lms/djangoapps/bulk_email/urls.py | 18 ++++ lms/djangoapps/bulk_email/views.py | 72 ++++++++++++++ lms/templates/bulk_email/unsubscribe.html | 48 +++++++++ lms/urls.py | 4 + .../ace_common/edx_ace/common/base_body.html | 7 ++ openedx/core/djangoapps/schedules/config.py | 8 +- .../core/djangoapps/schedules/resolvers.py | 20 +++- .../schedules/tests/test_resolvers.py | 97 +++++++++++++++++-- openedx/core/djangolib/testing/utils.py | 31 +++--- 10 files changed, 363 insertions(+), 24 deletions(-) create mode 100644 lms/djangoapps/bulk_email/tests/test_views.py create mode 100644 lms/djangoapps/bulk_email/urls.py create mode 100644 lms/djangoapps/bulk_email/views.py create mode 100644 lms/templates/bulk_email/unsubscribe.html diff --git a/lms/djangoapps/bulk_email/tests/test_views.py b/lms/djangoapps/bulk_email/tests/test_views.py new file mode 100644 index 0000000000..54151b8b98 --- /dev/null +++ b/lms/djangoapps/bulk_email/tests/test_views.py @@ -0,0 +1,82 @@ +# -*- coding: utf-8 -*- +""" +Test the bulk email opt out view. +""" +from six import text_type + +import ddt +from django.http import Http404 +from django.test.client import RequestFactory +from django.test.utils import override_settings +from django.urls import reverse + +from bulk_email.models import Optout +from bulk_email.views import opt_out_email_updates +from lms.djangoapps.discussion.notification_prefs.views import UsernameCipher +from openedx.core.lib.tests import attr +from student.tests.factories import UserFactory +from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase + + +@attr(shard=1) +@ddt.ddt +@override_settings(SECRET_KEY="test secret key") +class OptOutEmailUpdatesViewTest(ModuleStoreTestCase): + """ + Check the opt out email functionality. + """ + def setUp(self): + super(OptOutEmailUpdatesViewTest, self).setUp() + self.user = UserFactory.create(username="testuser1") + self.token = UsernameCipher.encrypt('testuser1') + self.request_factory = RequestFactory() + self.course = CourseFactory.create(run='testcourse1', display_name='Test Course Title') + self.url = reverse('bulk_email_opt_out', args=[self.token, text_type(self.course.id)]) + + # Ensure we start with no opt-out records + self.assertEqual(Optout.objects.count(), 0) + + def test_opt_out_email_confirm(self): + """ + Ensure that the default GET view asks for confirmation. + """ + response = self.client.get(self.url) + self.assertContains(response, "Do you want to unsubscribe from emails for Test Course Title?") + self.assertEqual(Optout.objects.count(), 0) + + def test_opt_out_email_unsubscribe(self): + """ + Ensure that the POSTing "confirm" creates the opt-out record. + """ + response = self.client.post(self.url, {'submit': 'confirm'}) + self.assertContains(response, "You have been unsubscribed from emails for Test Course Title.") + self.assertEqual(Optout.objects.count(), 1) + + def test_opt_out_email_cancel(self): + """ + Ensure that the POSTing "cancel" does not create the opt-out record + """ + response = self.client.post(self.url, {'submit': 'cancel'}) + self.assertContains(response, "You have not been unsubscribed from emails for Test Course Title.") + self.assertEqual(Optout.objects.count(), 0) + + @ddt.data( + ("ZOMG INVALID BASE64 CHARS!!!", "base64url", False), + ("Non-ASCII\xff", "base64url", False), + ("D6L8Q01ztywqnr3coMOlq0C3DG05686lXX_1ArEd0ok", "base64url", False), + ("AAAAAAAAAAA=", "initialization_vector", False), + ("nMXVK7PdSlKPOovci-M7iqS09Ux8VoCNDJixLBmj", "aes", False), + ("AAAAAAAAAAAAAAAAAAAAAMoazRI7ePLjEWXN1N7keLw=", "padding", False), + ("AAAAAAAAAAAAAAAAAAAAACpyUxTGIrUjnpuUsNi7mAY=", "username", False), + ("_KHGdCAUIToc4iaRGy7K57mNZiiXxO61qfKT08ExlY8=", "course", 'course-v1:testcourse'), + ) + @ddt.unpack + def test_unsubscribe_invalid_token(self, token, message, course): + """ + Make sure that view returns 404 in case token is not valid + """ + request = self.request_factory.get("dummy") + with self.assertRaises(Http404) as err: + opt_out_email_updates(request, token, course) + self.assertIn(message, err) diff --git a/lms/djangoapps/bulk_email/urls.py b/lms/djangoapps/bulk_email/urls.py new file mode 100644 index 0000000000..9beea793e1 --- /dev/null +++ b/lms/djangoapps/bulk_email/urls.py @@ -0,0 +1,18 @@ +""" +URLs for bulk_email app +""" + +from django.conf import settings +from django.conf.urls import url + +from bulk_email import views + +urlpatterns = [ + url( + r'^email/optout/(?P[a-zA-Z0-9-_=]+)/{}/$'.format( + settings.COURSE_ID_PATTERN, + ), + views.opt_out_email_updates, + name='bulk_email_opt_out', + ), +] diff --git a/lms/djangoapps/bulk_email/views.py b/lms/djangoapps/bulk_email/views.py new file mode 100644 index 0000000000..5e59d63c7a --- /dev/null +++ b/lms/djangoapps/bulk_email/views.py @@ -0,0 +1,72 @@ +""" +Views to support bulk email functionalities like opt-out. +""" + +from __future__ import division + +import logging + +from six import text_type + +from django.contrib.auth.models import User +from django.http import Http404 + +from bulk_email.models import Optout +from courseware.courses import get_course_by_id +from edxmako.shortcuts import render_to_response +from lms.djangoapps.discussion.notification_prefs.views import ( + UsernameCipher, + UsernameDecryptionException, +) + +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey + + +log = logging.getLogger(__name__) + + +def opt_out_email_updates(request, token, course_id): + """ + A view that let users opt out of any email updates. + + This meant is meant to be the target of an opt-out link or button. + The `token` parameter must decrypt to a valid username. + The `course_id` is the string course key of any course. + + Raises a 404 if there are any errors parsing the input. + """ + try: + username = UsernameCipher().decrypt(token.encode()) + user = User.objects.get(username=username) + course_key = CourseKey.from_string(course_id) + course = get_course_by_id(course_key, depth=0) + except UnicodeDecodeError: + raise Http404("base64url") + except UsernameDecryptionException as exn: + raise Http404(text_type(exn)) + except User.DoesNotExist: + raise Http404("username") + except InvalidKeyError: + raise Http404("course") + + context = { + 'course': course, + 'cancelled': False, + 'confirmed': False, + } + + if request.method == 'POST': + if request.POST.get('submit') == 'confirm': + Optout.objects.get_or_create(user=user, course_id=course.id) + log.info( + u"User %s (%s) opted out of receiving emails from course %s", + user.username, + user.email, + course_id, + ) + context['confirmed'] = True + else: + context['cancelled'] = True + + return render_to_response('bulk_email/unsubscribe.html', context) diff --git a/lms/templates/bulk_email/unsubscribe.html b/lms/templates/bulk_email/unsubscribe.html new file mode 100644 index 0000000000..270f8dc604 --- /dev/null +++ b/lms/templates/bulk_email/unsubscribe.html @@ -0,0 +1,48 @@ +<%page expression_filter="h" /> +<%inherit file="../main.html" /> +<%! + from openedx.core.djangolib.markup import Text + from django.utils.translation import ugettext as _ +%> +<%def name="header()"> +%if confirmed: + ${Text(_("Unsubscribe Successful"))} +%elif cancelled: + ${Text(_("Unsubscribe Cancelled"))} +%else: + ${Text(_("Confirm Unsubscribe"))} +%endif + + +<%block name="pagetitle">${header()} +
+ +
+

+ <%block name="pageheader">${header()} +

+

+ <%block name="pagecontent"> + %if confirmed: + ${Text(_("You have been unsubscribed from emails for {course}.")).format( + course=course.display_name_with_default + )} + %elif cancelled: + ${Text(_("You have not been unsubscribed from emails for {course}.")).format( + course=course.display_name_with_default + )} + %else: + ${Text(_("Do you want to unsubscribe from emails for {course}?")).format( + course=course.display_name_with_default + )} +

+

+ + + +
+ %endif + +

+
+
diff --git a/lms/urls.py b/lms/urls.py index 840956af5b..bbf60f4565 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -722,6 +722,10 @@ if settings.FEATURES.get('ENABLE_DISCUSSION_SERVICE'): ), ] +urlpatterns += [ + url(r'^bulk_email/', include('bulk_email.urls')), +] + urlpatterns += [ url( r'^courses/{}/tab/(?P[^/]+)/$'.format( diff --git a/openedx/core/djangoapps/ace_common/templates/ace_common/edx_ace/common/base_body.html b/openedx/core/djangoapps/ace_common/templates/ace_common/edx_ace/common/base_body.html index 044514b40d..7130554dcf 100644 --- a/openedx/core/djangoapps/ace_common/templates/ace_common/edx_ace/common/base_body.html +++ b/openedx/core/djangoapps/ace_common/templates/ace_common/edx_ace/common/base_body.html @@ -177,6 +177,13 @@ {{ contact_mailing_address }} + {% if unsubscribe_url %} + + + {% trans "Unsubscribe from these emails." as tmsg %}{{ tmsg | force_escape }} + + + {% endif %} diff --git a/openedx/core/djangoapps/schedules/config.py b/openedx/core/djangoapps/schedules/config.py index ae110465d8..b9e4e73a32 100644 --- a/openedx/core/djangoapps/schedules/config.py +++ b/openedx/core/djangoapps/schedules/config.py @@ -3,9 +3,13 @@ Contains configuration for schedules app """ from __future__ import absolute_import -from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag, WaffleFlag, WaffleFlagNamespace +from openedx.core.djangoapps.waffle_utils import ( + WaffleFlagNamespace, CourseWaffleFlag, WaffleFlag, + WaffleSwitch, WaffleSwitchNamespace, +) WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace(name=u'schedules') +WAFFLE_SWITCH_NAMESPACE = WaffleSwitchNamespace(name=u'schedules') CREATE_SCHEDULE_WAFFLE_FLAG = CourseWaffleFlag( waffle_namespace=WAFFLE_FLAG_NAMESPACE, @@ -20,3 +24,5 @@ COURSE_UPDATE_WAFFLE_FLAG = CourseWaffleFlag( ) DEBUG_MESSAGE_WAFFLE_FLAG = WaffleFlag(WAFFLE_FLAG_NAMESPACE, u'enable_debugging') + +COURSE_UPDATE_SHOW_UNSUBSCRIBE_WAFFLE_SWITCH = WaffleSwitch(WAFFLE_SWITCH_NAMESPACE, u'course_update_show_unsubscribe') diff --git a/openedx/core/djangoapps/schedules/resolvers.py b/openedx/core/djangoapps/schedules/resolvers.py index 9011382635..51d2a139de 100644 --- a/openedx/core/djangoapps/schedules/resolvers.py +++ b/openedx/core/djangoapps/schedules/resolvers.py @@ -15,7 +15,9 @@ from edx_ace.recipient_resolver import RecipientResolver from edx_django_utils.monitoring import function_trace, set_custom_metric from lms.djangoapps.courseware.date_summary import verified_upgrade_deadline_link, verified_upgrade_link_is_valid +from lms.djangoapps.discussion.notification_prefs.views import UsernameCipher from openedx.core.djangoapps.ace_common.template_context import get_base_template_context +from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_SHOW_UNSUBSCRIBE_WAFFLE_SWITCH from openedx.core.djangoapps.schedules.content_highlights import get_week_highlights from openedx.core.djangoapps.schedules.exceptions import CourseUpdateDoesNotExist from openedx.core.djangoapps.schedules.models import Schedule, ScheduleExperience @@ -91,6 +93,13 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): with function_trace('enqueue_send_task'): self.async_send_task.apply_async((self.site.id, str(msg)), retry=False) + @classmethod + def bin_num_for_user_id(cls, user_id): + """ + Returns the bin number used for the given (numeric) user ID. + """ + return user_id % cls.num_bins + def get_schedules_with_target_date_by_bin_and_orgs( self, order_by='enrollment__user__id' ): @@ -109,7 +118,7 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): courseenrollment__is_active=True, **schedule_day_equals_target_day_filter ).annotate( - id_mod=F('id') % self.num_bins + id_mod=self.bin_num_for_user_id(F('id')) ).filter( id_mod=self.bin_num ) @@ -363,6 +372,14 @@ class CourseUpdateResolver(BinnedSchedulesBaseResolver): ) # continue to the next schedule, don't yield an email for this one else: + unsubscribe_url = None + if (COURSE_UPDATE_SHOW_UNSUBSCRIBE_WAFFLE_SWITCH.is_enabled() and + 'bulk_email_optout' in settings.ACE_ENABLED_POLICIES): + unsubscribe_url = reverse('bulk_email_opt_out', kwargs={ + 'token': UsernameCipher.encrypt(user.username), + 'course_id': str(enrollment.course_id), + }) + template_context.update({ 'course_name': schedule.enrollment.course.display_name, 'course_url': _get_trackable_course_home_url(enrollment.course_id), @@ -372,6 +389,7 @@ class CourseUpdateResolver(BinnedSchedulesBaseResolver): # This is used by the bulk email optout policy 'course_ids': [str(enrollment.course_id)], + 'unsubscribe_url': unsubscribe_url, }) template_context.update(_get_upsell_information_for_schedule(user, schedule)) diff --git a/openedx/core/djangoapps/schedules/tests/test_resolvers.py b/openedx/core/djangoapps/schedules/tests/test_resolvers.py index ed0687347f..89f9a0b7b4 100644 --- a/openedx/core/djangoapps/schedules/tests/test_resolvers.py +++ b/openedx/core/djangoapps/schedules/tests/test_resolvers.py @@ -8,25 +8,47 @@ from unittest import skipUnless import ddt from django.conf import settings -from mock import Mock +from django.test import TestCase +from django.test.utils import override_settings +from mock import Mock, patch +from waffle.testutils import override_switch -from openedx.core.djangoapps.schedules.resolvers import BinnedSchedulesBaseResolver +from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_WAFFLE_FLAG +from openedx.core.djangoapps.schedules.resolvers import ( + BinnedSchedulesBaseResolver, + CourseUpdateResolver, +) from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory -from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms +from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag +from openedx.core.djangolib.testing.utils import CacheIsolationMixin, skip_unless_lms +from student.tests.factories import CourseEnrollmentFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory + + +class SchedulesResolverTestMixin(CacheIsolationMixin): + """ + Base class for the resolver tests. + """ + def setUp(self): + super(SchedulesResolverTestMixin, self).setUp() + self.site = SiteFactory.create() + self.site_config = SiteConfigurationFactory(site=self.site) + self.schedule_config = ScheduleConfigFactory.create(site=self.site) @ddt.ddt @skip_unless_lms @skipUnless('openedx.core.djangoapps.schedules.apps.SchedulesConfig' in settings.INSTALLED_APPS, "Can't test schedules if the app isn't installed") -class TestBinnedSchedulesBaseResolver(CacheIsolationTestCase): +class TestBinnedSchedulesBaseResolver(SchedulesResolverTestMixin, TestCase): + """ + Tests the BinnedSchedulesBaseResolver. + """ def setUp(self): super(TestBinnedSchedulesBaseResolver, self).setUp() - self.site = SiteFactory.create() - self.site_config = SiteConfigurationFactory(site=self.site) - self.schedule_config = ScheduleConfigFactory.create(site=self.site) self.resolver = BinnedSchedulesBaseResolver( async_send_task=Mock(name='async_send_task'), site=self.site, @@ -72,3 +94,64 @@ class TestBinnedSchedulesBaseResolver(CacheIsolationTestCase): result = self.resolver.filter_by_org(mock_query) mock_query.exclude.assert_called_once_with(enrollment__course__org__in=expected_org_list) self.assertEqual(result, mock_query.exclude.return_value) + + +@skip_unless_lms +@skipUnless('openedx.core.djangoapps.schedules.apps.SchedulesConfig' in settings.INSTALLED_APPS, + "Can't test schedules if the app isn't installed") +class TestCourseUpdateResolver(SchedulesResolverTestMixin, ModuleStoreTestCase): + """ + Tests the CourseUpdateResolver. + """ + def setUp(self): + super(TestCourseUpdateResolver, self).setUp() + self.course = CourseFactory(highlights_enabled_for_messaging=True, self_paced=True) + with self.store.bulk_operations(self.course.id): + ItemFactory.create(parent=self.course, category='chapter', highlights=[u'good stuff']) + + def create_resolver(self): + """ + Creates a CourseUpdateResolver with an enrollment to schedule. + """ + with patch('openedx.core.djangoapps.schedules.signals.get_current_site') as mock_get_current_site: + mock_get_current_site.return_value = self.site_config.site + enrollment = CourseEnrollmentFactory(course_id=self.course.id, user=self.user, mode=u'audit') + + return CourseUpdateResolver( + async_send_task=Mock(name='async_send_task'), + site=self.site_config.site, + target_datetime=enrollment.schedule.start, + day_offset=-7, + bin_num=CourseUpdateResolver.bin_num_for_user_id(self.user.id), + ) + + @override_settings(CONTACT_MAILING_ADDRESS='123 Sesame Street') + @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True) + def test_schedule_context(self): + resolver = self.create_resolver() + schedules = list(resolver.schedules_for_bin()) + expected_context = { + 'contact_email': 'info@example.com', + 'contact_mailing_address': '123 Sesame Street', + 'course_ids': [str(self.course.id)], + 'course_name': self.course.display_name, + 'course_url': '/courses/{}/course/'.format(self.course.id), + 'dashboard_url': '/dashboard', + 'homepage_url': '/', + 'mobile_store_urls': {}, + 'platform_name': u'\xe9dX', + 'show_upsell': False, + 'social_media_urls': {}, + 'template_revision': 'release', + 'unsubscribe_url': None, + 'week_highlights': ['good stuff'], + 'week_num': 1, + } + self.assertEqual(schedules, [(self.user, None, expected_context)]) + + @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True) + @override_switch('schedules.course_update_show_unsubscribe', True) + def test_schedule_context_show_unsubscribe(self): + resolver = self.create_resolver() + schedules = list(resolver.schedules_for_bin()) + self.assertIn('optout', schedules[0][2]['unsubscribe_url']) diff --git a/openedx/core/djangolib/testing/utils.py b/openedx/core/djangolib/testing/utils.py index 81d38ad7a9..e194768225 100644 --- a/openedx/core/djangolib/testing/utils.py +++ b/openedx/core/djangolib/testing/utils.py @@ -48,6 +48,22 @@ class CacheIsolationMixin(object): __settings_overrides = [] __old_settings = [] + @classmethod + def setUpClass(cls): + super(CacheIsolationMixin, cls).setUpClass() + cls.start_cache_isolation() + + @classmethod + def tearDownClass(cls): + cls.end_cache_isolation() + super(CacheIsolationMixin, cls).tearDownClass() + + def setUp(self): + super(CacheIsolationMixin, self).setUp() + + self.clear_caches() + self.addCleanup(self.clear_caches) + @classmethod def start_cache_isolation(cls): """ @@ -131,21 +147,6 @@ class CacheIsolationTestCase(CacheIsolationMixin, TestCase): :py:class:`CacheIsolationMixin`) at class setup, and flushes the cache between every test. """ - @classmethod - def setUpClass(cls): - super(CacheIsolationTestCase, cls).setUpClass() - cls.start_cache_isolation() - - @classmethod - def tearDownClass(cls): - cls.end_cache_isolation() - super(CacheIsolationTestCase, cls).tearDownClass() - - def setUp(self): - super(CacheIsolationTestCase, self).setUp() - - self.clear_caches() - self.addCleanup(self.clear_caches) class _AssertNumQueriesContext(CaptureQueriesContext):