Add tests for changing table status without staff permissions.

cms/djangoapps/course_creators/models.py

@@ -54,20 +54,23 @@ def post_init_callback(sender, **kwargs):
 @receiver(post_save, sender=CourseCreator)
 def post_save_callback(sender, **kwargs):
     """
-    Extend to update state_changed time and modify the course creator group in authz.py.
+    Extend to update state_changed time and fire event to update course creator group, if appropriate.
     """
     instance = kwargs['instance']
     # We only wish to modify the state_changed time if the state has been modified. We don't wish to
     # modify it for changes to the notes field.
     if instance.state != instance.orig_state:
-        if hasattr(instance, 'admin'):
+        # If either old or new state is 'granted', we must manipulate the course creator
+        # group maintained by authz. That requires staff permissions (stored admin).
+        if instance.state == CourseCreator.GRANTED or instance.orig_state == CourseCreator.GRANTED:
+            assert hasattr(instance, 'admin'), 'Must have stored staff user to change course creator group'
             update_creator_state.send(
                 sender=sender,
                 caller=instance.admin,
                 user=instance.user,
                 add=instance.state == CourseCreator.GRANTED
             )
-        # TODO: Else must be sure that state change does not switch to or from granted
+
         instance.state_changed = timezone.now()
         instance.orig_state = instance.state
         instance.save()

cms/djangoapps/course_creators/tests/test_views.py

@@ -7,7 +7,7 @@ from django.contrib.auth.models import User
 from django.core.exceptions import PermissionDenied
 
 from course_creators.views import add_user_with_status_unrequested, add_user_with_status_granted
-from course_creators.views import get_course_creator_status, update_course_creator_group
+from course_creators.views import get_course_creator_status, update_course_creator_group, user_requested_access
 from course_creators.models import CourseCreator
 from auth.authz import is_user_in_creator_group
 import mock
@@ -26,14 +26,11 @@ class CourseCreatorView(TestCase):
 
     def test_staff_permission_required(self):
         """
-        Tests that add methods and course creator group method must be called with staff permissions.
+        Tests that any method changing the course creator authz group must be called with staff permissions.
         """
         with self.assertRaises(PermissionDenied):
             add_user_with_status_granted(self.user, self.user)
 
-        with self.assertRaises(PermissionDenied):
-            add_user_with_status_unrequested(self.user, self.user)
-
         with self.assertRaises(PermissionDenied):
             update_course_creator_group(self.user, self.user, True)
 
@@ -41,7 +38,7 @@ class CourseCreatorView(TestCase):
         self.assertIsNone(get_course_creator_status(self.user))
 
     def test_add_unrequested(self):
-        add_user_with_status_unrequested(self.admin, self.user)
+        add_user_with_status_unrequested(self.user)
         self.assertEqual('unrequested', get_course_creator_status(self.user))
 
         # Calling add again will be a no-op (even if state is different).
@@ -57,7 +54,7 @@ class CourseCreatorView(TestCase):
             self.assertEqual('granted', get_course_creator_status(self.user))
 
             # Calling add again will be a no-op (even if state is different).
-            add_user_with_status_unrequested(self.admin, self.user)
+            add_user_with_status_unrequested(self.user)
             self.assertEqual('granted', get_course_creator_status(self.user))
 
             self.assertTrue(is_user_in_creator_group(self.user))
@@ -69,3 +66,17 @@ class CourseCreatorView(TestCase):
             self.assertTrue(is_user_in_creator_group(self.user))
             update_course_creator_group(self.admin, self.user, False)
             self.assertFalse(is_user_in_creator_group(self.user))
+
+    def test_user_requested_access(self):
+        add_user_with_status_unrequested(self.user)
+        self.assertEqual('unrequested', get_course_creator_status(self.user))
+        user_requested_access(self.user)
+        self.assertEqual('pending', get_course_creator_status(self.user))
+
+    def test_user_requested_already_granted(self):
+        add_user_with_status_granted(self.admin, self.user)
+        self.assertEqual('granted', get_course_creator_status(self.user))
+        # Will not "downgrade" to pending because that would require removing the
+        # user from the authz course creator group (and that can only be done by an admin).
+        user_requested_access(self.user)
+        self.assertEqual('granted', get_course_creator_status(self.user))

cms/djangoapps/course_creators/views.py

@@ -20,10 +20,11 @@ def add_user_with_status_granted(caller, user):
     """
     Adds a user to the course creator table with status 'granted'.
 
-    If the user is already in the table, this method is a no-op
-    (state will not be changed). Caller must have staff permissions.
-
     This method also adds the user to the course creator group maintained by authz.py.
+    Caller must have staff permissions.
+
+    If the user is already in the table, this method is a no-op
+    (state will not be changed).
     """
     _add_user(user, CourseCreator.GRANTED)
     update_course_creator_group(caller, user, True)
@@ -64,11 +65,13 @@ def user_requested_access(user):
     """
     User has requested course creator access.
 
-    This changes the user state to CourseCreator.PENDING.
+    This changes the user state to CourseCreator.PENDING, unless the user
+    state is already CourseCreator.GRANTED, in which case this method is a no-op.
     """
     user = CourseCreator.objects.get(user=user)
-    user.state = CourseCreator.PENDING
-    user.save()
+    if user.state != CourseCreator.GRANTED:
+        user.state = CourseCreator.PENDING
+        user.save()
 
 
 def _add_user(user, state):