diff --git a/lms/djangoapps/django_comment_client/models.py b/lms/djangoapps/django_comment_client/models.py index b1b8f985c6..ebc9f38e96 100644 --- a/lms/djangoapps/django_comment_client/models.py +++ b/lms/djangoapps/django_comment_client/models.py @@ -4,6 +4,11 @@ import logging from courseware.courses import get_course_by_id +FORUM_ROLE_ADMINISTRATOR = 'Administrator' +FORUM_ROLE_MODERATOR = 'Moderator' +FORUM_ROLE_COMMUNITY_TA = 'Community TA' +FORUM_ROLE_STUDENT = 'Student' + class Role(models.Model): name = models.CharField(max_length=30, null=False, blank=False) users = models.ManyToManyField(User, related_name="roles") @@ -28,7 +33,7 @@ class Role(models.Model): if self.name == "Student" and \ (permission.startswith('edit') or permission.startswith('update') or permission.startswith('create')) and \ (not course.forum_posts_allowed): - return False + return False return self.permissions.filter(name=permission).exists() diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index b3a1626d22..6bd05f1edd 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -1,27 +1,21 @@ -import time from collections import defaultdict -from importlib import import_module - -from courseware.models import StudentModuleCache -from courseware.module_render import get_module +from django.contrib.auth.models import User +from django.core.urlresolvers import reverse +from django.db import connection +from django.http import HttpResponse +from django.utils import simplejson +from django_comment_client.models import Role +from django_comment_client.permissions import check_permissions_by_view +from mitxmako import middleware from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore from xmodule.modulestore.search import path_to_location -from django.http import HttpResponse -from django.utils import simplejson -from django.db import connection -from django.conf import settings -from django.core.urlresolvers import reverse -from django.contrib.auth.models import User -from django_comment_client.permissions import check_permissions_by_view -from django_comment_client.models import Role -from mitxmako import middleware - import logging -import operator -import itertools -import urllib import pystache_custom as pystache +import time +import urllib + + # TODO these should be cached via django's caching rather than in-memory globals @@ -47,9 +41,16 @@ def get_role_ids(course_id): staff = list(User.objects.filter(is_staff=True).values_list('id', flat=True)) roles_with_ids = {'Staff': staff} for role in roles: - roles_with_ids[role.name] = list(role.users.values_list('id', flat=True)) + roles_with_ids[role.name] = list(role.users.values_list('id', flat=True)) return roles_with_ids +def has_forum_access(uname, course_id, rolename): + try: + role = Role.objects.get(name=rolename, course_id=course_id) + except Role.DoesNotExist: + return False + return role.users.filter(username=uname).exists() + def get_full_modules(): global _FULLMODULES if not _FULLMODULES: @@ -132,7 +133,7 @@ def initialize_discussion_info(course): return course_id = course.id - url_course_id = course_id.replace('/', '_').replace('.', '_') + # url_course_id = course_id.replace('/', '_').replace('.', '_') all_modules = get_full_modules()[course_id] diff --git a/lms/djangoapps/instructor/tests.py b/lms/djangoapps/instructor/tests.py index fbe00c4b35..07da4d2895 100644 --- a/lms/djangoapps/instructor/tests.py +++ b/lms/djangoapps/instructor/tests.py @@ -8,21 +8,20 @@ Notes for running by hand: django-admin.py test --settings=lms.envs.test --pythonpath=. lms/djangoapps/instructor """ -import courseware.tests.tests as ct - -from nose import SkipTest -from mock import patch, Mock -from override_settings import override_settings - -# Need access to internal func to put users in the right group from courseware.access import _course_staff_group_name -from django.contrib.auth.models import User, Group -from django.conf import settings +from django.contrib.auth.models import \ + Group # Need access to internal func to put users in the right group from django.core.urlresolvers import reverse - +from django_comment_client.models import Role, FORUM_ROLE_ADMINISTRATOR, \ + FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA +from django_comment_client.utils import has_forum_access +from override_settings import override_settings +from xmodule.modulestore.django import modulestore +import courseware.tests.tests as ct import xmodule.modulestore.django -from xmodule.modulestore.django import modulestore + + @override_settings(MODULESTORE=ct.TEST_DATA_XML_MODULESTORE) @@ -83,13 +82,22 @@ class TestInstructorDashboardGradeDownloadCSV(ct.PageLoader): ''' self.assertEqual(body, expected_body, msg) +FORUM_ROLES = [ FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA ] +FORUM_ADMIN_ACTION_SUFFIX = { FORUM_ROLE_ADMINISTRATOR : 'admin', FORUM_ROLE_MODERATOR : 'moderator', FORUM_ROLE_COMMUNITY_TA : 'community TA'} +FORUM_ADMIN_USER = { FORUM_ROLE_ADMINISTRATOR : 'forumadmin', FORUM_ROLE_MODERATOR : 'forummoderator', FORUM_ROLE_COMMUNITY_TA : 'forummoderator'} + +def action_name(operation, rolename): + if operation == 'List': + return '%s course forum %ss' % (operation, FORUM_ADMIN_ACTION_SUFFIX[rolename]) + else: + return '%s forum %s' % (operation, FORUM_ADMIN_ACTION_SUFFIX[rolename]) @override_settings(MODULESTORE=ct.TEST_DATA_XML_MODULESTORE) class TestInstructorDashboardForumAdmin(ct.PageLoader): ''' Check for change in forum admin role memberships ''' - + def setUp(self): xmodule.modulestore.django._MODULESTORES = {} courses = modulestore().get_courses() @@ -118,28 +126,84 @@ class TestInstructorDashboardForumAdmin(ct.PageLoader): self.login(self.instructor, self.password) self.enroll(self.toy) - def test_add_forum_admin(self): - print "running test_add_forum_admin" + def initialize_roles(self, course_id): + self.admin_role = Role.objects.get_or_create(name=FORUM_ROLE_ADMINISTRATOR, course_id=course_id)[0] + self.moderator_role = Role.objects.get_or_create(name=FORUM_ROLE_MODERATOR, course_id=course_id)[0] + self.community_ta_role = Role.objects.get_or_create(name=FORUM_ROLE_COMMUNITY_TA, course_id=course_id)[0] + + def test_add_forum_admin_users_for_unknown_user(self): + print "running test_add_forum_admin_users_for_unknown_user" course = self.toy url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) -# msg = "url = %s\n" % url - response = self.client.post(url, {'action': 'Add forum admin', - 'forumadmin': 'u1'}) -# msg += "instructor dashboard download csv grades: response = '%s'\n" % response -# -# self.assertEqual(response['Content-Type'],'text/csv',msg) -# -# cdisp = response['Content-Disposition'].replace('TT_2012','2012') # jenkins course_id is TT_2012_Fall instead of 2012_Fall? -# msg += "cdisp = '%s'\n" % cdisp -# self.assertEqual(cdisp,'attachment; filename=grades_edX/toy/2012_Fall.csv',msg) -# -# body = response.content.replace('\r','') -# msg += "body = '%s'\n" % body + username = 'unknown' + for action in ['Add', 'Remove']: + for rolename in FORUM_ROLES: + response = self.client.post(url, {'action': action_name(action, rolename), FORUM_ADMIN_USER[rolename]: username}) + self.assertTrue(response.content.find('Error: unknown username "%s"' % username)>=0) -# need to make sure the roles actually exist. They don't seem to yet.... - #

Error: unknown rolename "Administrator"

+ def test_add_forum_admin_users_for_missing_roles(self): + print "test_add_forum_admin_users_for_missing_roles" + course = self.toy + url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) + username = 'u1' + for action in ['Add', 'Remove']: + for rolename in FORUM_ROLES: + response = self.client.post(url, {'action': action_name(action, rolename), FORUM_ADMIN_USER[rolename]: username}) + self.assertTrue(response.content.find('Error: unknown rolename "%s"' % rolename)>=0) -# print response -# context = response.context -# self.assertTrue(context.contains("Added %s to %s forum role = %s" % ('u1', course.id, 'Administrator'))) - + def test_remove_forum_admin_users_for_missing_users(self): + print "test_remove_forum_admin_users_for_missing_users" + course = self.toy + self.initialize_roles(course.id) + url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) + username = 'u1' + action = 'Remove' + for rolename in FORUM_ROLES: + response = self.client.post(url, {'action': action_name(action, rolename), FORUM_ADMIN_USER[rolename]: username}) + self.assertTrue(response.content.find('Error: user "%s" does not have rolename "%s"' % (username, rolename))>=0) + + def test_add_and_remove_forum_admin_users(self): + print "test_add_and_remove_forum_admin_users" + course = self.toy + self.initialize_roles(course.id) + url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) + username = 'u1' + 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) + self.assertTrue(has_forum_access(username, course.id, rolename)) + response = self.client.post(url, {'action': action_name('Remove', rolename), FORUM_ADMIN_USER[rolename]: username}) + self.assertTrue(response.content.find('Removed "%s" from "%s" forum role = "%s"' % (username, course.id, rolename))>=0) + self.assertFalse(has_forum_access(username, course.id, rolename)) + + def test_add_and_readd_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' + 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}) + response = self.client.post(url, {'action': action_name('Add', rolename), FORUM_ADMIN_USER[rolename]: username}) + 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_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 = [] + for rolename in FORUM_ROLES: + response = self.client.post(url, {'action': action_name('Add', rolename), FORUM_ADMIN_USER[rolename]: username}) + 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) + self.assertTrue(response.content.find('%s' % username)>=0) + # concatenate all roles for user, in sorted order: + added_roles.append(rolename) + added_roles.sort() + roles = ', '.join(added_roles) + self.assertTrue(response.content.find('%s' % roles)>=0) diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 11cab79c57..60cc634cde 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -27,27 +27,17 @@ from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError, NoPathToItem from xmodule.modulestore.search import path_to_location -from django_comment_client.models import Role +from django_comment_client.models import Role, FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA +from django_comment_client.utils import has_forum_access log = logging.getLogger("mitx.courseware") template_imports = {'urllib': urllib} -# TODO: move these to views for forum role -FORUM_ROLE_ADMINISTRATOR = 'Administrator' -FORUM_ROLE_MODERATOR = 'Moderator' -FORUM_ROLE_COMMUNITY_TA = 'Community TA' +# internal commands for managing forum roles: FORUM_ROLE_ADD = 'add' FORUM_ROLE_REMOVE = 'remove' -def has_forum_access(uname, course_id, rolename): - try: - role = Role.objects.get(name=rolename, course_id=course_id) - except Role.DoesNotExist: - return False - return role.users.filter(username=uname).exists() - - @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) @@ -56,7 +46,9 @@ def instructor_dashboard(request, course_id): course = get_course_with_access(request.user, course_id, 'staff') instructor_access = has_access(request.user, course, 'instructor') # an instructor can manage staff lists + forum_admin_access = has_forum_access(request.user, course_id, FORUM_ROLE_ADMINISTRATOR) + msg = '' #msg += ('POST=%s' % dict(request.POST)).replace('<','<') @@ -212,7 +204,7 @@ def instructor_dashboard(request, course_id): #---------------------------------------- # forum administration - elif action == 'List course forum administrators': + elif action == 'List course forum admins': rolename = FORUM_ROLE_ADMINISTRATOR datatable = {} msg += _list_course_forum_members(course_id, rolename, datatable) @@ -352,16 +344,16 @@ def _update_forum_role_membership(uname, course_id, rolename, add_or_remove): log.debug('rolename=%s' % rolename) if (add_or_remove == FORUM_ROLE_REMOVE): if (not alreadyexists): - msg ='Error: user %s does not have rolename "%s", cannot remove' % (uname, rolename) + 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) + 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) + msg = 'Added "%s" to "%s" forum role = "%s"' % (user, course_id, rolename) return msg diff --git a/lms/templates/courseware/instructor_dashboard.html b/lms/templates/courseware/instructor_dashboard.html index 69bfabf64d..74bc25fcbe 100644 --- a/lms/templates/courseware/instructor_dashboard.html +++ b/lms/templates/courseware/instructor_dashboard.html @@ -140,7 +140,7 @@ function goto( mode) %if instructor_access:

- +