diff --git a/lms/djangoapps/instructor/tests.py b/lms/djangoapps/instructor/tests.py index 07da4d2895..2c498ab6f4 100644 --- a/lms/djangoapps/instructor/tests.py +++ b/lms/djangoapps/instructor/tests.py @@ -167,7 +167,7 @@ class TestInstructorDashboardForumAdmin(ct.PageLoader): course = self.toy self.initialize_roles(course.id) url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) - username = 'u1' + username = 'u2' for rolename in FORUM_ROLES: response = self.client.post(url, {'action': action_name('Add', rolename), FORUM_ADMIN_USER[rolename]: username}) self.assertTrue(response.content.find('Added "%s" to "%s" forum role = "%s"' % (username, course.id, rolename))>=0) @@ -181,7 +181,7 @@ class TestInstructorDashboardForumAdmin(ct.PageLoader): course = self.toy self.initialize_roles(course.id) url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) - username = 'u1' + username = 'u2' for rolename in FORUM_ROLES: # perform an add, and follow with a second identical add: self.client.post(url, {'action': action_name('Add', rolename), FORUM_ADMIN_USER[rolename]: username}) @@ -189,15 +189,27 @@ class TestInstructorDashboardForumAdmin(ct.PageLoader): self.assertTrue(response.content.find('Error: user "%s" already has rolename "%s", cannot add' % (username, rolename))>=0) self.assertTrue(has_forum_access(username, course.id, rolename)) + def test_add_nonstaff_forum_admin_users(self): + print "test_add_and_readd_forum_admin_users" + course = self.toy + self.initialize_roles(course.id) + url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) + username = 'u1' + rolename = FORUM_ROLE_ADMINISTRATOR + response = self.client.post(url, {'action': action_name('Add', rolename), FORUM_ADMIN_USER[rolename]: username}) + self.assertTrue(response.content.find('Error: user "%s" should first be added as staff' % username)>=0) + def test_list_forum_admin_users(self): print "test_list_forum_admin_users" course = self.toy self.initialize_roles(course.id) url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) - username = 'u1' - added_roles = [] + username = 'u2' + added_roles = ['Student'] # u2 is already added as a student to the discussion forums + self.assertTrue(has_forum_access(username, course.id, 'Student')) for rolename in FORUM_ROLES: response = self.client.post(url, {'action': action_name('Add', rolename), FORUM_ADMIN_USER[rolename]: username}) + self.assertTrue(has_forum_access(username, course.id, rolename)) response = self.client.post(url, {'action': action_name('List', rolename), FORUM_ADMIN_USER[rolename]: username}) for header in ['Username', 'Full name', 'Roles']: self.assertTrue(response.content.find('%s' % header)>0) @@ -206,4 +218,4 @@ class TestInstructorDashboardForumAdmin(ct.PageLoader): added_roles.append(rolename) added_roles.sort() roles = ', '.join(added_roles) - self.assertTrue(response.content.find('%s' % roles)>=0) + self.assertTrue(response.content.find('%s' % roles)>=0, 'not finding roles "%s"' % roles) diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 60cc634cde..2ae383e25b 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -213,13 +213,13 @@ def instructor_dashboard(request, course_id): elif action == 'Remove forum admin': uname = request.POST['forumadmin'] - msg += _update_forum_role_membership(uname, course_id, FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_REMOVE) + msg += _update_forum_role_membership(uname, course, FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_REMOVE) track.views.server_track(request, '%s %s as %s for %s' % (FORUM_ROLE_REMOVE, uname, FORUM_ROLE_ADMINISTRATOR, course_id), {}, page='idashboard') elif action == 'Add forum admin': uname = request.POST['forumadmin'] - msg += _update_forum_role_membership(uname, course_id, FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_ADD) + msg += _update_forum_role_membership(uname, course, FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_ADD) track.views.server_track(request, '%s %s as %s for %s' % (FORUM_ROLE_ADD, uname, FORUM_ROLE_ADMINISTRATOR, course_id), {}, page='idashboard') @@ -231,13 +231,13 @@ def instructor_dashboard(request, course_id): elif action == 'Remove forum moderator': uname = request.POST['forummoderator'] - msg += _update_forum_role_membership(uname, course_id, FORUM_ROLE_MODERATOR, FORUM_ROLE_REMOVE) + msg += _update_forum_role_membership(uname, course, FORUM_ROLE_MODERATOR, FORUM_ROLE_REMOVE) track.views.server_track(request, '%s %s as %s for %s' % (FORUM_ROLE_REMOVE, uname, FORUM_ROLE_MODERATOR, course_id), {}, page='idashboard') elif action == 'Add forum moderator': uname = request.POST['forummoderator'] - msg += _update_forum_role_membership(uname, course_id, FORUM_ROLE_MODERATOR, FORUM_ROLE_ADD) + msg += _update_forum_role_membership(uname, course, FORUM_ROLE_MODERATOR, FORUM_ROLE_ADD) track.views.server_track(request, '%s %s as %s for %s' % (FORUM_ROLE_ADD, uname, FORUM_ROLE_MODERATOR, course_id), {}, page='idashboard') @@ -249,13 +249,13 @@ def instructor_dashboard(request, course_id): elif action == 'Remove forum community TA': uname = request.POST['forummoderator'] - msg += _update_forum_role_membership(uname, course_id, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_REMOVE) + msg += _update_forum_role_membership(uname, course, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_REMOVE) track.views.server_track(request, '%s %s as %s for %s' % (FORUM_ROLE_REMOVE, uname, FORUM_ROLE_COMMUNITY_TA, course_id), {}, page='idashboard') elif action == 'Add forum community TA': uname = request.POST['forummoderator'] - msg += _update_forum_role_membership(uname, course_id, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_ADD) + msg += _update_forum_role_membership(uname, course, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_ADD) track.views.server_track(request, '%s %s as %s for %s' % (FORUM_ROLE_ADD, uname, FORUM_ROLE_COMMUNITY_TA, course_id), {}, page='idashboard') @@ -316,12 +316,12 @@ def _list_course_forum_members(course_id, rolename, datatable): return msg -def _update_forum_role_membership(uname, course_id, rolename, add_or_remove): +def _update_forum_role_membership(uname, course, rolename, add_or_remove): ''' Supports adding a user to a course's forum role uname = username string for user - course_ID = course's ID string + course = course object rolename = one of "Administrator", "Moderator", "Community TA" add_or_remove = one of "add" or "remove" @@ -334,7 +334,7 @@ def _update_forum_role_membership(uname, course_id, rolename, add_or_remove): except User.DoesNotExist: return 'Error: unknown username "%s"' % uname try: - role = Role.objects.get(name=rolename, course_id=course_id) + role = Role.objects.get(name=rolename, course_id=course.id) except Role.DoesNotExist: return 'Error: unknown rolename "%s"' % rolename @@ -347,13 +347,16 @@ def _update_forum_role_membership(uname, course_id, rolename, add_or_remove): msg ='Error: user "%s" does not have rolename "%s", cannot remove' % (uname, rolename) else: user.roles.remove(role) - msg = 'Removed "%s" from "%s" forum role = "%s"' % (user, course_id, rolename) + msg = 'Removed "%s" from "%s" forum role = "%s"' % (user, course.id, rolename) else: if (alreadyexists): msg = 'Error: user "%s" already has rolename "%s", cannot add' % (uname, rolename) else: - user.roles.add(role) - msg = 'Added "%s" to "%s" forum role = "%s"' % (user, course_id, rolename) + if (rolename == FORUM_ROLE_ADMINISTRATOR and not has_access(user, course, 'staff')): + msg = 'Error: user "%s" should first be added as staff before adding as a forum administrator, cannot add' % uname + else: + user.roles.add(role) + msg = 'Added "%s" to "%s" forum role = "%s"' % (user, course.id, rolename) return msg