diff --git a/cms/djangoapps/contentstore/tests/test_users.py b/cms/djangoapps/contentstore/tests/test_users.py index bba4d42f4d..701dd246d8 100644 --- a/cms/djangoapps/contentstore/tests/test_users.py +++ b/cms/djangoapps/contentstore/tests/test_users.py @@ -47,6 +47,9 @@ class IndexCourseCreatorTests(CourseTestCase): self.enable_creator_group = {"ENABLE_CREATOR_GROUP": True} + self.admin = User.objects.create_user('Mark', 'mark+courses@edx.org', 'foo') + self.admin.is_staff = True + def test_get_course_creator_status_disable_creation(self): # DISABLE_COURSE_CREATION is True (this is the case on edx, where we have a marketing site). # Only edx staff can create courses. @@ -80,17 +83,14 @@ class IndexCourseCreatorTests(CourseTestCase): # ENABLE_CREATOR_GROUP is True. This is the case on edge. # Check return value for a non-staff user who has been granted access. with mock.patch.dict('django.conf.settings.MITX_FEATURES', self.enable_creator_group): - # self.user has staff permissions, can call this method. - add_user_with_status_granted(self.user, self.user) - # now make self.user non-staff self._set_user_non_staff() + add_user_with_status_granted(self.admin, self.user) self.assertEquals('granted', _get_course_creator_status(self.user)) def test_get_course_creator_status_creator_group_denied(self): # ENABLE_CREATOR_GROUP is True. This is the case on edge. # Check return value for a non-staff user who has been denied access. with mock.patch.dict('django.conf.settings.MITX_FEATURES', self.enable_creator_group): - # make self.user non-staff self._set_user_non_staff() self._set_user_denied() self.assertEquals('denied', _get_course_creator_status(self.user)) @@ -137,10 +137,8 @@ class IndexCourseCreatorTests(CourseTestCase): def test_course_creator_group_granted(self): # Test index page content with ENABLE_CREATOR_GROUP True, non-staff member with access granted. with mock.patch.dict('django.conf.settings.MITX_FEATURES', self.enable_creator_group): - # self.user has staff permissions, can call this method. - add_user_with_status_granted(self.user, self.user) - # now make self.user non-staff self._set_user_non_staff() + add_user_with_status_granted(self.admin, self.user) self._assert_can_create() def test_course_creator_group_denied(self): @@ -188,9 +186,6 @@ class IndexCourseCreatorTests(CourseTestCase): self.table_entry = CourseCreator(user=self.user) self.table_entry.save() - self.admin = User.objects.create_user('Mark', 'admin+courses@edx.org', 'foo') - self.admin.is_staff = True - self.deny_request = HttpRequest() self.deny_request.user = self.admin @@ -198,4 +193,3 @@ class IndexCourseCreatorTests(CourseTestCase): self.table_entry.state = CourseCreator.DENIED self.creator_admin.save_model(self.deny_request, self.table_entry, None, True) - diff --git a/cms/djangoapps/course_creators/tests/test_views.py b/cms/djangoapps/course_creators/tests/test_views.py index 943120f3c0..95c50ffb76 100644 --- a/cms/djangoapps/course_creators/tests/test_views.py +++ b/cms/djangoapps/course_creators/tests/test_views.py @@ -80,3 +80,13 @@ class CourseCreatorView(TestCase): # 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)) + + def test_add_user_unrequested_staff(self): + # Users marked as is_staff will not be added to the course creator table. + add_user_with_status_unrequested(self.admin) + self.assertIsNone(get_course_creator_status(self.admin)) + + def test_add_user_granted_staff(self): + # Users marked as is_staff will not be added to the course creator table. + add_user_with_status_granted(self.admin, self.admin) + self.assertIsNone(get_course_creator_status(self.admin)) diff --git a/cms/djangoapps/course_creators/views.py b/cms/djangoapps/course_creators/views.py index 6b2d91acb2..e9b38ed169 100644 --- a/cms/djangoapps/course_creators/views.py +++ b/cms/djangoapps/course_creators/views.py @@ -12,6 +12,9 @@ def add_user_with_status_unrequested(user): If the user is already in the table, this method is a no-op (state will not be changed). + + If the user is marked as is_staff, this method is a no-op (user + will not be added to table). """ _add_user(user, CourseCreator.UNREQUESTED) @@ -20,14 +23,17 @@ def add_user_with_status_granted(caller, user): """ Adds a user to the course creator table with status 'granted'. - This method also adds the user to the course creator group maintained by authz.py. + If appropriate, 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). + + If the user is marked as is_staff, this method is a no-op (user + will not be added to table, nor added to authz.py group). """ - _add_user(user, CourseCreator.GRANTED) - update_course_creator_group(caller, user, True) + if _add_user(user, CourseCreator.GRANTED): + update_course_creator_group(caller, user, True) def update_course_creator_group(caller, user, add): @@ -78,9 +84,16 @@ def _add_user(user, state): """ Adds a user to the course creator table with the specified state. + Returns True if user was added to table, else False. + If the user is already in the table, this method is a no-op - (state will not be changed). + (state will not be changed, method will return False). + + If the user is marked as is_staff, this method is a no-op (False will be returned). """ - if CourseCreator.objects.filter(user=user).count() == 0: + if not user.is_staff and CourseCreator.objects.filter(user=user).count() == 0: entry = CourseCreator(user=user, state=state) entry.save() + return True + + return False