Notifier re-subcribe via link

* fix assertPrefValid in tests to decrypt instead of compare
* rename unsubscribe -> set_subscribe

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)

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})

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<token>[a-zA-Z0-9-_=]+)/', 'notification_prefs.views.unsubscribe'),
+            url(r'^notification_prefs/unsubscribe/(?P<token>[a-zA-Z0-9-_=]+)/',
+                'notification_prefs.views.set_subscription', {'subscribe': False}, name="unsubscribe_forum_update"),
             url(r'^notification_prefs/resubscribe/(?P<token>[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.