From d0d225d8a92086dc0fb3df8c387498a37c2518b9 Mon Sep 17 00:00:00 2001 From: Waqas Khalid Date: Wed, 24 Sep 2014 15:20:40 +0500 Subject: [PATCH] Course admin should be able to de-admin dicussion admins As a global staff or course staff once you added yourself in the discussion admins you cannot revoke it back unless you remove yourself as course staff. Any of the course staff member should be able to remove others from discussion admins whether they are course staff or not. TNL-315 --- lms/djangoapps/instructor/tests/test_api.py | 29 ++++++--------------- lms/djangoapps/instructor/views/api.py | 4 --- 2 files changed, 8 insertions(+), 25 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 7ecf8378fd..1f5d8da0cf 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -1299,35 +1299,22 @@ class TestInstructorAPILevelsAccess(ModuleStoreTestCase, LoginEnrollmentTestCase # Seed forum roles for course. seed_permissions_roles(self.course.id) - # Test add discussion admin with email. - self.assert_update_forum_role_membership(self.other_user.email, "Administrator", "allow") + for user in [self.instructor, self.other_user]: + for identifier_attr in [user.email, user.username]: + for rolename in ["Administrator", "Moderator", "Community TA"]: + for action in ["allow", "revoke"]: + self.assert_update_forum_role_membership(user, identifier_attr, rolename, action) - # Test revoke discussion admin with email. - self.assert_update_forum_role_membership(self.other_user.email, "Administrator", "revoke") - - # Test add discussion moderator with username. - self.assert_update_forum_role_membership(self.other_user.username, "Moderator", "allow") - - # Test revoke discussion moderator with username. - self.assert_update_forum_role_membership(self.other_user.username, "Moderator", "revoke") - - # Test add discussion community TA with email. - self.assert_update_forum_role_membership(self.other_user.email, "Community TA", "allow") - - # Test revoke discussion community TA with username. - self.assert_update_forum_role_membership(self.other_user.username, "Community TA", "revoke") - - def assert_update_forum_role_membership(self, unique_student_identifier, rolename, action): + def assert_update_forum_role_membership(self, current_user, identifier, rolename, action): """ Test update forum role membership. Get unique_student_identifier, rolename and action and update forum role. """ - url = reverse('update_forum_role_membership', kwargs={'course_id': self.course.id.to_deprecated_string()}) response = self.client.get( url, { - 'unique_student_identifier': unique_student_identifier, + 'unique_student_identifier': identifier, 'rolename': rolename, 'action': action, } @@ -1336,7 +1323,7 @@ class TestInstructorAPILevelsAccess(ModuleStoreTestCase, LoginEnrollmentTestCase # Status code should be 200. self.assertEqual(response.status_code, 200) - user_roles = self.other_user.roles.filter(course_id=self.course.id).values_list("name", flat=True) + user_roles = current_user.roles.filter(course_id=self.course.id).values_list("name", flat=True) if action == 'allow': self.assertIn(rolename, user_roles) elif action == 'revoke': diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 0e31886073..f15133be19 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -1495,10 +1495,6 @@ def update_forum_role_membership(request, course_id): )) user = get_student_from_identifier(unique_student_identifier) - target_is_instructor = has_access(user, 'instructor', course) - # cannot revoke instructor - if target_is_instructor and action == 'revoke' and rolename == FORUM_ROLE_ADMINISTRATOR: - return HttpResponseBadRequest("Cannot revoke instructor forum admin privileges.") try: update_forum_role(course_id, user, rolename, action)