Add unit tests

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()
 

lms/djangoapps/django_comment_client/utils.py

-import time
 from collections import defaultdict
-from importlib import import_module
-
-from courseware.models import StudentModuleCache
-from courseware.module_render import get_module
-from xmodule.modulestore import Location
-from xmodule.modulestore.django import modulestore
-from xmodule.modulestore.search import path_to_location
+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.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 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
 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]
 

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
-
-# need to make sure the roles actually exist.  They don't seem to yet....        
-   # <p><font color="red">Error: unknown rolename "Administrator"</font></p>
-
-#        print response
-#        context = response.context
-#        self.assertTrue(context.contains("Added %s to %s forum role = %s" % ('u1', course.id, 'Administrator')))
-        
+        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)
+
+    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)
+
+    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('<th>%s</th>' % header)>0)
+            self.assertTrue(response.content.find('<td>%s</td>' % 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('<td>%s</td>' % roles)>=0)

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('<','&lt;')
 
@@ -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 ='<font color="red">Error: user %s does not have rolename "%s", cannot remove</font>' % (uname, rolename)
+            msg ='<font color="red">Error: user "%s" does not have rolename "%s", cannot remove</font>' % (uname, rolename)
         else: 
             user.roles.remove(role)
-            msg = '<font color="green">Removed %s from %s forum role = %s</font>' % (user, course_id, rolename)
+            msg = '<font color="green">Removed "%s" from "%s" forum role = "%s"</font>' % (user, course_id, rolename)
     else:
         if (alreadyexists):
-            msg = '<font color="red">Error: user %s already has rolename "%s", cannot add</font>' % (uname, rolename)
+            msg = '<font color="red">Error: user "%s" already has rolename "%s", cannot add</font>' % (uname, rolename)
         else: 
             user.roles.add(role)
-            msg = '<font color="green">Added %s to %s forum role = %s</font>' % (user, course_id, rolename)
+            msg = '<font color="green">Added "%s" to "%s" forum role = "%s"</font>' % (user, course_id, rolename)
 
     return msg
     

lms/templates/courseware/instructor_dashboard.html

@@ -140,7 +140,7 @@ function goto( mode)
   %if instructor_access:
     <hr width="40%" style="align:left">
     <p>
-    <input type="submit" name="action" value="List course forum administrators">
+    <input type="submit" name="action" value="List course forum admins">
     <p>
     <input type="text" name="forumadmin"> <input type="submit" name="action" value="Remove forum admin">
     <input type="submit" name="action" value="Add forum admin">