From dfacbbdd6941bdddbe20f03abc8b0e7fe56dbaa1 Mon Sep 17 00:00:00 2001 From: Jason Bau Date: Tue, 21 Jan 2014 19:18:31 -0800 Subject: [PATCH] Notifier re-subcribe via link * fix assertPrefValid in tests to decrypt instead of compare * rename unsubscribe -> set_subscribe --- lms/djangoapps/notification_prefs/tests.py | 27 +++++++++++----------- lms/djangoapps/notification_prefs/views.py | 13 ++++++----- lms/urls.py | 5 ++-- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/lms/djangoapps/notification_prefs/tests.py b/lms/djangoapps/notification_prefs/tests.py index b7422dacb2..f464527df3 100644 --- a/lms/djangoapps/notification_prefs/tests.py +++ b/lms/djangoapps/notification_prefs/tests.py @@ -9,7 +9,7 @@ from django.test.utils import override_settings from mock import Mock, patch from notification_prefs import NOTIFICATION_PREF_KEY -from notification_prefs.views import ajax_enable, ajax_disable, ajax_status, unsubscribe +from notification_prefs.views import ajax_enable, ajax_disable, ajax_status, set_subscription, UsernameCipher from student.tests.factories import UserFactory from user_api.models import UserPreference from util.testing import UrlResetMixin @@ -51,10 +51,12 @@ class NotificationPrefViewTest(UrlResetMixin, TestCase): def assertPrefValid(self, user): """Ensure that the correct preference for the user is persisted""" - self.assertEqual( - UserPreference.objects.get(user=user, key=NOTIFICATION_PREF_KEY).value, - self.tokens[user] - ) + pref = UserPreference.objects.get(user=user, key=NOTIFICATION_PREF_KEY) + self.assertTrue(pref) # check exists and only 1 (.get) + # now coerce username to utf-8 encoded str, since we test with non-ascii unicdoe above and + # the unittest framework has hard time coercing to unicode. + # decrypt also can't take a unicode input, so coerce its input to str + self.assertEqual(str(user.username.encode('utf-8')), UsernameCipher().decrypt(str(pref.value))) def assertNotPrefExists(self, user): """Ensure that the user does not have a persisted preference""" @@ -177,13 +179,13 @@ class NotificationPrefViewTest(UrlResetMixin, TestCase): def test_unsubscribe_post(self): request = self.request_factory.post("dummy") - response = unsubscribe(request, "dummy") + response = set_subscription(request, "dummy", subscribe=False) self.assertEqual(response.status_code, 405) def test_unsubscribe_invalid_token(self): def test_invalid_token(token, message): request = self.request_factory.get("dummy") - self.assertRaisesRegexp(Http404, "^{}$".format(message), unsubscribe, request, token) + self.assertRaisesRegexp(Http404, "^{}$".format(message), set_subscription, request, token, False) # Invalid base64 encoding test_invalid_token("ZOMG INVALID BASE64 CHARS!!!", "base64url") @@ -218,7 +220,7 @@ class NotificationPrefViewTest(UrlResetMixin, TestCase): def test_user(user): request = self.request_factory.get("dummy") request.user = AnonymousUser() - response = unsubscribe(request, self.tokens[user]) + response = set_subscription(request, self.tokens[user], subscribe=False) self.assertEqual(response.status_code, 200) self.assertNotPrefExists(user) @@ -229,8 +231,8 @@ class NotificationPrefViewTest(UrlResetMixin, TestCase): self.create_prefs() request = self.request_factory.get("dummy") request.user = AnonymousUser() - unsubscribe(request, self.tokens[self.user]) - response = unsubscribe(request, self.tokens[self.user]) + set_subscription(request, self.tokens[self.user], False) + response = set_subscription(request, self.tokens[self.user], subscribe=False) self.assertEqual(response.status_code, 200) self.assertNotPrefExists(self.user) @@ -240,10 +242,9 @@ class NotificationPrefViewTest(UrlResetMixin, TestCase): self.assertFalse(UserPreference.objects.filter(user=user, key=NOTIFICATION_PREF_KEY)) request = self.request_factory.get("dummy") request.user = AnonymousUser() - response = unsubscribe(request, self.tokens[user], resubscribe=True) + response = set_subscription(request, self.tokens[user], subscribe=True) self.assertEqual(response.status_code, 200) - # this new DB entry will have a new value, so can't use assertPrefValid. Just check existence - self.assertTrue(UserPreference.objects.filter(user=user, key=NOTIFICATION_PREF_KEY)) + self.assertPrefValid(user) for user in self.tokens.keys(): test_user(user) diff --git a/lms/djangoapps/notification_prefs/views.py b/lms/djangoapps/notification_prefs/views.py index f6a52144de..e1d64cb84f 100644 --- a/lms/djangoapps/notification_prefs/views.py +++ b/lms/djangoapps/notification_prefs/views.py @@ -153,13 +153,14 @@ def ajax_status(request): @require_GET -def unsubscribe(request, token, resubscribe=False): # pylint: disable=unused-argument +def set_subscription(request, token, subscribe): # pylint: disable=unused-argument """ A view that disables or re-enables notifications for a user who may not be authenticated This view is meant to be the target of an unsubscribe link. The request must be a GET, and the `token` parameter must decrypt to a valid username. - The resubscribe feature allows "undo" of accidentally clicking on unsubscribe + The subscribe flag feature controls whether the view subscribes or unsubscribes the user, with subscribe=True + used to "undo" accidentally clicking on the unsubscribe link A 405 will be returned if the request method is not GET. A 404 will be returned if the token parameter does not decrypt to a valid username. On @@ -175,13 +176,13 @@ def unsubscribe(request, token, resubscribe=False): # pylint: disable=unused-ar except User.DoesNotExist: raise Http404("username") - if not resubscribe: - UserPreference.objects.filter(user=user, key=NOTIFICATION_PREF_KEY).delete() - return render_to_response("unsubscribe.html", {'token': token}) - else: + if subscribe: UserPreference.objects.get_or_create(user=user, key=NOTIFICATION_PREF_KEY, defaults={ "value": UsernameCipher.encrypt(user.username) }) return render_to_response("resubscribe.html", {'token': token}) + else: + UserPreference.objects.filter(user=user, key=NOTIFICATION_PREF_KEY).delete() + return render_to_response("unsubscribe.html", {'token': token}) diff --git a/lms/urls.py b/lms/urls.py index 198f4b751c..758d28aa92 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -338,9 +338,10 @@ if settings.COURSEWARE_ENABLED: url(r'^notification_prefs/enable/', 'notification_prefs.views.ajax_enable'), url(r'^notification_prefs/disable/', 'notification_prefs.views.ajax_disable'), url(r'^notification_prefs/status/', 'notification_prefs.views.ajax_status'), - url(r'^notification_prefs/unsubscribe/(?P[a-zA-Z0-9-_=]+)/', 'notification_prefs.views.unsubscribe'), + url(r'^notification_prefs/unsubscribe/(?P[a-zA-Z0-9-_=]+)/', + 'notification_prefs.views.set_subscription', {'subscribe': False}, name="unsubscribe_forum_update"), url(r'^notification_prefs/resubscribe/(?P[a-zA-Z0-9-_=]+)/', - 'notification_prefs.views.unsubscribe', {'resubscribe': True}, name="resubscribe_forum_update"), + 'notification_prefs.views.set_subscription', {'subscribe': True}, name="resubscribe_forum_update"), ) urlpatterns += ( # This MUST be the last view in the courseware--it's a catch-all for custom tabs.