From 3eb9058dd23ef74ccd054d3a84222c3dec777de9 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. --- lms/djangoapps/bulk_email/tests/test_views.py | 80 +++++++++++++++ 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 | 11 ++- .../core/djangoapps/schedules/resolvers.py | 11 +++ .../schedules/tests/test_resolvers.py | 97 +++++++++++++++++-- 9 files changed, 341 insertions(+), 7 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..c062eff69d --- /dev/null +++ b/lms/djangoapps/bulk_email/tests/test_views.py @@ -0,0 +1,80 @@ +# -*- 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 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") + self.assertRaisesRegexp(Http404, "^{}$".format(message), opt_out_email_updates, request, token, course) 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..299047c176 --- /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 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 ba2be98a95..5f4c4cd777 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -746,6 +746,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 2c5d3efc24..918c321405 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 @@ -175,6 +175,13 @@ {{ contact_mailing_address }} + {% if unsubscribe_url %} + + + {% trans "Unsubscribe from these emails." %} + + + {% endif %} diff --git a/openedx/core/djangoapps/schedules/config.py b/openedx/core/djangoapps/schedules/config.py index 0ae579e396..1e831175e8 100644 --- a/openedx/core/djangoapps/schedules/config.py +++ b/openedx/core/djangoapps/schedules/config.py @@ -1,7 +1,14 @@ -from openedx.core.djangoapps.waffle_utils import WaffleFlagNamespace, CourseWaffleFlag, WaffleFlag +""" +Contains waffle flags and switches for use with the Schedules app. +""" +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, @@ -16,3 +23,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 1fad58c654..56af9cf388 100644 --- a/openedx/core/djangoapps/schedules/resolvers.py +++ b/openedx/core/djangoapps/schedules/resolvers.py @@ -13,6 +13,8 @@ from edx_ace.recipient import Recipient from edx_django_utils.monitoring import function_trace, set_custom_metric from courseware.date_summary import verified_upgrade_deadline_link, verified_upgrade_link_is_valid +from lms.djangoapps.notification_prefs.views import UsernameCipher +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 @@ -358,6 +360,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), @@ -367,6 +377,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 6cca8dfd71..2b4057b5de 100644 --- a/openedx/core/djangoapps/schedules/tests/test_resolvers.py +++ b/openedx/core/djangoapps/schedules/tests/test_resolvers.py @@ -1,27 +1,51 @@ +""" +Tests for the Schedules app resolvers. +""" import datetime from unittest import skipUnless import ddt from django.conf import settings -from mock import Mock +from freezegun import freeze_time +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 SiteFactory, SiteConfigurationFactory +from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, 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(CacheIsolationTestCase): + """ + 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, CacheIsolationTestCase): + """ + 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, @@ -67,3 +91,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) + + +@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") +@override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True) +@freeze_time('2017-08-01 01:00:00', tz_offset=0, tick=False) +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=1, + ) + + def test_schedule_context(self): + resolver = self.create_resolver() + schedules = list(resolver.schedules_for_bin()) + expected_context = { + 'course_name': self.course.display_name, + 'course_url': '/courses/{}/course/'.format(self.course.id), + 'week_num': 1, + 'week_highlights': ['good stuff'], + 'course_ids': [str(self.course.id)], + 'platform_name': u'\xe9dX', + 'mobile_store_urls': {'google': '#', 'apple': '#'}, + 'homepage_url': '/', + 'template_revision': 'unknown', + 'contact_email': 'info@example.com', + 'social_media_urls': {}, + 'dashboard_url': '/dashboard', + 'contact_mailing_address': '', + 'show_upsell': False, + 'unsubscribe_url': None, + } + self.assertEqual(schedules, [(self.user, None, expected_context)]) + + @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'])