Merge pull request #1373 from MITx/feature/zoldak/django-comment-client-tests

Add unit tests for django comment client

lms/djangoapps/django_comment_client/helpers.py

@@ -12,6 +12,9 @@ import pystache_custom as pystache
 import urllib
 import os
 
+# This method is used to pluralize the words "discussion" and "comment"
+# when referring to how many discussion threads or comments the user
+# has contributed to.
 def pluralize(singular_term, count):
     if int(count) >= 2 or int(count) == 0:
         return singular_term + 's'

lms/djangoapps/django_comment_client/models.py

@@ -46,11 +46,13 @@ class Role(models.Model):
     def add_permission(self, permission):
         self.permissions.add(Permission.objects.get_or_create(name=permission)[0])
 
+
     def has_permission(self, permission):
         course = get_course_by_id(self.course_id)
-        if self.name == FORUM_ROLE_STUDENT and \
-           (permission.startswith('edit') or permission.startswith('update') or permission.startswith('create')) and \
-           (not course.forum_posts_allowed):
+        changing_comments = permission.startswith('edit') or \
+            permission.startswith('update') or permission.startswith('create')
+        in_blackout_period = not course.forum_posts_allowed
+        if (self.name == FORUM_ROLE_STUDENT) and in_blackout_period and changing_comments:
             return False
         
         return self.permissions.filter(name=permission).exists()

lms/djangoapps/django_comment_client/mustache_helpers.py

@@ -5,6 +5,8 @@ import urllib
 import sys
 import inspect
 
+# This method is used to pluralize the words "discussion" and "comment"
+# which is why you need to tack on an "s" for the case of 0 or two or more.
 def pluralize(content, text):
     num, word = text.split(' ')
     num = int(num or '0')

lms/djangoapps/django_comment_client/permissions.py

@@ -29,6 +29,7 @@ def has_permission(user, permission, course_id=None):
 
 
 CONDITIONS = ['is_open', 'is_author']
+# data may be a json file
 def check_condition(user, condition, course_id, data):
     def check_open(user, condition, course_id, data):
         try:
@@ -61,8 +62,10 @@ def check_conditions_permissions(user, permissions, course_id, **kwargs):
     def test(user, per, operator="or"):
         if isinstance(per, basestring):
             if per in CONDITIONS:
-                return check_condition(user, per, course_id, kwargs)
+                return check_condition(user, per, course_id, kwargs['data'])
             return cached_has_permission(user, per, course_id=course_id)
+        # TODO: refactor this to be more clear.
+        # e.g. the "and operator in" bit on the next line is not needed?
         elif isinstance(per, list) and operator in ["and", "or"]:
             results = [test(user, x, operator="and") for x in per]
             if operator == "or":
@@ -102,4 +105,4 @@ def check_permissions_by_view(user, course_id, content, name):
         p = VIEW_PERMISSIONS[name]
     except KeyError:
         logging.warning("Permission for view named %s does not exist in permissions.py" % name)
-    return check_conditions_permissions(user, p, course_id, content=content)
+    return check_conditions_permissions(user, p, course_id, data=content)

lms/djangoapps/django_comment_client/tests.py

@@ -1,111 +0,0 @@
-from django.contrib.auth.models import User, Group
-from django.core.urlresolvers import reverse
-from django.test import TestCase
-from django.test.client import RequestFactory
-from django.conf import settings
-
-from mock import Mock
-
-from override_settings import override_settings
-
-import xmodule.modulestore.django
-
-from student.models import CourseEnrollment
-
-from django.db.models.signals import m2m_changed, pre_delete, pre_save, post_delete, post_save
-from django.dispatch.dispatcher import _make_id
-import string
-import random
-from .permissions import has_permission
-from .models import Role, Permission
-
-from xmodule.modulestore.django import modulestore
-from xmodule.modulestore import Location
-from xmodule.modulestore.xml_importer import import_from_xml
-from xmodule.modulestore.xml import XMLModuleStore
-
-import comment_client
-
-from courseware.tests.tests import PageLoader, TEST_DATA_XML_MODULESTORE
-
-#@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
-#class TestCohorting(PageLoader):
-#    """Check that cohorting works properly"""
-#
-#    def setUp(self):
-#        xmodule.modulestore.django._MODULESTORES = {}
-#
-#        # Assume courses are there
-#        self.toy = modulestore().get_course("edX/toy/2012_Fall")
-#
-#        # Create two accounts
-#        self.student = 'view@test.com'
-#        self.student2 = 'view2@test.com'
-#        self.password = 'foo'
-#        self.create_account('u1', self.student, self.password)
-#        self.create_account('u2', self.student2, self.password)
-#        self.activate_user(self.student)
-#        self.activate_user(self.student2)
-#
-#    def test_create_thread(self):
-#        my_save = Mock()
-#        comment_client.perform_request = my_save
-#
-#        resp = self.client.post(
-#            reverse('django_comment_client.base.views.create_thread',
-#                    kwargs={'course_id': 'edX/toy/2012_Fall',
-#                            'commentable_id': 'General'}),
-#                                        {'some': "some",
-#                                         'data': 'data'})
-#        self.assertTrue(my_save.called)
-#
-#        #self.assertEqual(resp.status_code, 200)
-#        #self.assertEqual(my_save.something, "expected", "complaint if not true")
-#
-#        self.toy.metadata["cohort_config"] = {"cohorted": True}
-#
-#        # call the view again ...
-#
-#       # assert that different things happened
-
-
-
-class PermissionsTestCase(TestCase):
-    def random_str(self, length=15, chars=string.ascii_uppercase + string.digits):
-        return ''.join(random.choice(chars) for x in range(length))
-
-    def setUp(self):
-
-        self.course_id = "edX/toy/2012_Fall"
-
-        self.moderator_role = Role.objects.get_or_create(name="Moderator", course_id=self.course_id)[0]
-        self.student_role = Role.objects.get_or_create(name="Student", course_id=self.course_id)[0]
-
-        self.student = User.objects.create(username=self.random_str(),
-                            password="123456", email="john@yahoo.com")
-        self.moderator = User.objects.create(username=self.random_str(),
-                            password="123456", email="staff@edx.org")
-        self.moderator.is_staff = True
-        self.moderator.save()
-        self.student_enrollment = CourseEnrollment.objects.create(user=self.student, course_id=self.course_id)
-        self.moderator_enrollment = CourseEnrollment.objects.create(user=self.moderator, course_id=self.course_id)
-
-    def tearDown(self):
-        self.student_enrollment.delete()
-        self.moderator_enrollment.delete()
-
-# Do we need to have this? We shouldn't be deleting students, ever
-#        self.student.delete()
-#        self.moderator.delete()
-
-    def testDefaultRoles(self):
-        self.assertTrue(self.student_role in self.student.roles.all())
-        self.assertTrue(self.moderator_role in self.moderator.roles.all())
-
-    def testPermission(self):
-        name = self.random_str()
-        self.moderator_role.add_permission(name)
-        self.assertTrue(has_permission(self.moderator, name, self.course_id))
-
-        self.student_role.add_permission(name)
-        self.assertTrue(has_permission(self.student, name, self.course_id))

lms/djangoapps/django_comment_client/tests/test_helpers.py

@@ -0,0 +1,15 @@
+import string
+import random
+import collections
+
+from django.test import TestCase                         
+
+from django_comment_client.helpers import pluralize
+
+class PluralizeTestCase(TestCase):
+
+    def testPluralize(self):
+        self.term = "cat"
+        self.assertEqual(pluralize(self.term, 0), "cats")
+        self.assertEqual(pluralize(self.term, 1), "cat")
+        self.assertEqual(pluralize(self.term, 2), "cats")

lms/djangoapps/django_comment_client/tests/test_middleware.py

@@ -0,0 +1,28 @@
+import string
+import random
+import collections
+
+from django.test import TestCase                         
+
+import comment_client
+import django.http
+import django_comment_client.middleware as middleware
+
+class AjaxExceptionTestCase(TestCase):
+
+# TODO: check whether the correct error message is produced. 
+# The error message should be the same as the argument to CommentClientError
+	def setUp(self):
+		self.a = middleware.AjaxExceptionMiddleware()
+		self.request1 = django.http.HttpRequest()
+		self.request0 = django.http.HttpRequest()
+		self.exception1 = comment_client.CommentClientError('{}')
+		self.exception0 = ValueError()
+		self.request1.META['HTTP_X_REQUESTED_WITH'] = "XMLHttpRequest"
+		self.request0.META['HTTP_X_REQUESTED_WITH'] = "SHADOWFAX"
+		
+	def test_process_exception(self):
+		self.assertIsInstance(self.a.process_exception(self.request1, self.exception1), middleware.JsonError)
+		self.assertIsNone(self.a.process_exception(self.request1, self.exception0))
+		self.assertIsNone(self.a.process_exception(self.request0, self.exception1))
+		self.assertIsNone(self.a.process_exception(self.request0, self.exception0))

lms/djangoapps/django_comment_client/tests/test_models.py

@@ -0,0 +1,54 @@
+import django_comment_client.models as models
+import django_comment_client.permissions as permissions
+from django.test import TestCase
+from nose.plugins.skip import SkipTest
+from courseware.courses import get_course_by_id
+
+class RoleClassTestCase(TestCase):
+    def setUp(self):
+        self.course_id = "edX/toy/2012_Fall"
+        self.student_role = models.Role.objects.create(name="Student",
+                                                       course_id=self.course_id)
+
+    def test_unicode(self):
+        self.assertEqual(str(self.student_role), "Student for edX/toy/2012_Fall")
+
+        self.admin_for_all = models.Role.objects.create(name="Administrator")
+        self.assertEqual(str(self.admin_for_all), "Administrator for all courses")
+
+    def test_has_permission(self):
+        self.student_role.add_permission("delete_thread")
+        self.TA_role = models.Role.objects.create(name="Community TA",
+                                                  course_id=self.course_id)
+        self.assertTrue(self.student_role.has_permission("delete_thread"))
+        self.assertFalse(self.TA_role.has_permission("delete_thread"))
+
+    # Toy course does not have a blackout period defined.
+    def test_students_can_create_if_not_during_blackout(self):
+        self.student_role.add_permission("create_comment")
+        self.assertTrue(self.student_role.has_permission("create_comment"))
+
+    def test_students_cannot_create_during_blackout(self):
+        # Not sure how to set up these conditions
+        raise SkipTest()
+
+    def test_inherit_permissions(self):
+        self.student_role.add_permission("delete_thread")
+        self.TA_role = models.Role.objects.create(name="Community TA",
+                                                  course_id=self.course_id)
+        self.TA_role.inherit_permissions(self.student_role)
+        self.assertTrue(self.TA_role.has_permission("delete_thread"))
+
+    # TODO: You should not be able to inherit permissions across courses?
+    def test_inherit_permissions_across_courses(self):
+        raise SkipTest()
+        self.student_role.add_permission("delete_thread")
+        self.course_id_2 = "MITx/6.002x/2012_Fall"
+        self.admin_role = models.Role.objects.create(name="Administrator",
+                                                     course_id=self.course_id_2)
+        self.admin_role.inherit_permissions(self.student_role)
+
+class PermissionClassTestCase(TestCase):
+    def test_unicode(self):
+        self.permission = permissions.Permission.objects.create(name="test")
+        self.assertEqual(str(self.permission), "test")

lms/djangoapps/django_comment_client/tests/test_mustache_helpers.py

@@ -0,0 +1,26 @@
+import string
+import random
+import collections
+
+from django.test import TestCase                         
+
+import django_comment_client.mustache_helpers as mustache_helpers
+
+class PluralizeTestCase(TestCase):
+
+	def test_pluralize(self):
+		self.text1 = '0 goat'
+		self.text2 = '1 goat'
+		self.text3 = '7 goat'
+		self.content = 'unused argument'
+		self.assertEqual(mustache_helpers.pluralize(self.content, self.text1), 'goats')
+		self.assertEqual(mustache_helpers.pluralize(self.content, self.text2), 'goat')
+		self.assertEqual(mustache_helpers.pluralize(self.content, self.text3), 'goats')
+
+class CloseThreadTextTestCase(TestCase):
+	
+	def test_close_thread_text(self):
+		self.contentClosed = {'closed': True}
+		self.contentOpen = {'closed': False}
+		self.assertEqual(mustache_helpers.close_thread_text(self.contentClosed), 'Re-open thread')
+		self.assertEqual(mustache_helpers.close_thread_text(self.contentOpen), 'Close thread')

lms/djangoapps/django_comment_client/tests/test_permissions.py

@@ -0,0 +1,128 @@
+import string
+import random
+import collections
+
+import factory
+from django.test import TestCase                         
+
+from django.contrib.auth.models import User
+from student.models import UserProfile, CourseEnrollment
+from django_comment_client.models import Role, Permission
+import django_comment_client.permissions as p
+
+class UserFactory(factory.Factory):
+    FACTORY_FOR = User
+    username = 'robot'
+    password = '123456'
+    email = 'robot@edx.org'
+    is_active = True
+    is_staff = False
+
+class CourseEnrollmentFactory(factory.Factory):
+    FACTORY_FOR = CourseEnrollment  
+    user = factory.SubFactory(UserFactory)
+    course_id = 'edX/toy/2012_Fall'
+
+class RoleFactory(factory.Factory):
+    FACTORY_FOR = Role
+    name = 'Student'
+    course_id = 'edX/toy/2012_Fall'
+
+class PermissionFactory(factory.Factory):
+    FACTORY_FOR = Permission  
+    name = 'create_comment'
+
+
+class PermissionsTestCase(TestCase):
+    def setUp(self):
+        self.course_id = "edX/toy/2012_Fall"
+
+        self.student_role = RoleFactory(name='Student')
+        self.moderator_role = RoleFactory(name='Moderator')
+        self.student = UserFactory(username='student', email='student@edx.org')
+        self.moderator = UserFactory(username='moderator', email='staff@edx.org', is_staff=True)
+        self.update_thread_permission = PermissionFactory(name='update_thread')
+        self.update_thread_permission.roles.add(self.student_role)
+        self.update_thread_permission.roles.add(self.moderator_role)
+        self.manage_moderator_permission = PermissionFactory(name='manage_moderator')
+        self.manage_moderator_permission.roles.add(self.moderator_role)
+        self.student_enrollment = CourseEnrollmentFactory(user=self.student)
+        self.moderator_enrollment = CourseEnrollmentFactory(user=self.moderator)
+
+        self.student_open_thread = {'content': {
+                                    'closed': False,
+                                    'user_id': str(self.student.id)}
+                                    }
+        self.student_closed_thread = {'content': {
+                                    'closed': True,
+                                    'user_id': str(self.student.id)}
+                                    }
+
+    def test_user_has_permission(self):
+        s_ut = p.has_permission(self.student, 'update_thread', self.course_id)
+        m_ut = p.has_permission(self.moderator, 'update_thread', self.course_id)
+        s_mm = p.has_permission(self.student, 'manage_moderator', self.course_id)
+        m_mm = p.has_permission(self.moderator, 'manage_moderator', self.course_id)
+        self.assertTrue(s_ut)
+        self.assertTrue(m_ut)
+        self.assertFalse(s_mm)
+        self.assertTrue(m_mm)
+
+    def test_check_conditions(self):
+        # Checks whether the discussion thread is open, or whether the author is user
+        s_o = p.check_condition(self.student, 'is_open', self.course_id, self.student_open_thread)
+        s_a = p.check_condition(self.student, 'is_author', self.course_id, self.student_open_thread)
+        m_c = p.check_condition(self.moderator, 'is_open', self.course_id, self.student_closed_thread)
+        m_a = p.check_condition(self.moderator,'is_author', self.course_id, self.student_open_thread)
+        self.assertTrue(s_o)
+        self.assertTrue(s_a)
+        self.assertFalse(m_c)
+        self.assertFalse(m_a)
+
+    def test_check_conditions_and_permissions(self):
+        # Check conditions
+        ret = p.check_conditions_permissions(self.student,
+                                            'is_open',
+                                            self.course_id,
+                                            data=self.student_open_thread)
+        self.assertTrue(ret)
+
+        # Check permissions
+        ret = p.check_conditions_permissions(self.student, 
+                                            'update_thread',
+                                            self.course_id,
+                                            data=self.student_open_thread)
+        self.assertTrue(ret)
+
+        # Check that a list of permissions/conditions will be OR'd
+        ret = p.check_conditions_permissions(self.moderator, 
+                                            ['is_open','manage_moderator'],
+                                            self.course_id,
+                                            data=self.student_open_thread)
+        self.assertTrue(ret)
+
+        # Check that a list of permissions will be OR'd
+        ret = p.check_conditions_permissions(self.student, 
+                                            ['update_thread','manage_moderator'],
+                                            self.course_id,
+                                            data=self.student_open_thread)
+        self.assertTrue(ret)
+
+        # Check that a list of list of permissions will be AND'd
+        ret = p.check_conditions_permissions(self.student, 
+                                            [['update_thread','manage_moderator']],
+                                            self.course_id,
+                                            data=self.student_open_thread)
+        self.assertFalse(ret)
+
+    def test_check_permissions_by_view(self):
+        ret = p.check_permissions_by_view(self.student, self.course_id,
+                                    self.student_open_thread, 'openclose_thread')
+        self.assertFalse(ret)
+
+        # Check a view permission that includes both a condition and a permission
+        self.vote_permission = PermissionFactory(name='vote')
+        self.vote_permission.roles.add(self.student_role)
+        ret = p.check_permissions_by_view(self.student, self.course_id,
+                                    self.student_open_thread, 'vote_for_comment')
+        self.assertTrue(ret)

lms/djangoapps/django_comment_client/tests/test_utils.py

@@ -0,0 +1,89 @@
+import string
+import random
+import collections
+
+from django.test import TestCase                         
+
+import factory
+from django.contrib.auth.models import User
+from student.models import UserProfile, CourseEnrollment
+from django_comment_client.models import Role, Permission
+
+import django_comment_client.models as models
+import django_comment_client.utils as utils
+
+import xmodule.modulestore.django as django
+
+class UserFactory(factory.Factory):
+    FACTORY_FOR = User
+    username = 'robot'
+    password = '123456'
+    email = 'robot@edx.org'
+    is_active = True
+    is_staff = False
+
+class CourseEnrollmentFactory(factory.Factory):
+    FACTORY_FOR = CourseEnrollment  
+    user = factory.SubFactory(UserFactory)
+    course_id = 'edX/toy/2012_Fall'
+
+class RoleFactory(factory.Factory):
+    FACTORY_FOR = Role
+    name = 'Student'
+    course_id = 'edX/toy/2012_Fall'
+
+class PermissionFactory(factory.Factory):
+    FACTORY_FOR = Permission  
+    name = 'create_comment'
+
+class DictionaryTestCase(TestCase):
+    def test_extract(self):
+        d = {'cats': 'meow', 'dogs': 'woof'}
+        k = ['cats', 'dogs', 'hamsters']
+        expected = {'cats': 'meow', 'dogs': 'woof', 'hamsters': None}
+        self.assertEqual(utils.extract(d, k), expected)
+
+    def test_strip_none(self):
+        d = {'cats': 'meow', 'dogs': 'woof', 'hamsters': None}
+        expected = {'cats': 'meow', 'dogs': 'woof'}
+        self.assertEqual(utils.strip_none(d), expected)
+
+    def test_strip_blank(self):
+        d = {'cats': 'meow', 'dogs': 'woof', 'hamsters': ' ', 'yetis': ''}
+        expected = {'cats': 'meow', 'dogs': 'woof'}
+        self.assertEqual(utils.strip_blank(d), expected)
+
+    def test_merge_dict(self):
+        d1 ={'cats': 'meow', 'dogs': 'woof'}
+        d2 ={'lions': 'roar','ducks': 'quack'}
+        expected ={'cats': 'meow', 'dogs': 'woof','lions': 'roar','ducks': 'quack'}
+        self.assertEqual(utils.merge_dict(d1, d2), expected)
+
+class AccessUtilsTestCase(TestCase):
+    def setUp(self):
+        self.course_id = 'edX/toy/2012_Fall'
+        self.student_role = RoleFactory(name='Student', course_id=self.course_id)
+        self.moderator_role = RoleFactory(name='Moderator', course_id=self.course_id)
+        self.student1 = UserFactory(username='student', email='student@edx.org')
+        self.student1_enrollment = CourseEnrollmentFactory(user=self.student1)
+        self.student_role.users.add(self.student1)
+        self.student2 = UserFactory(username='student2', email='student2@edx.org')
+        self.student2_enrollment = CourseEnrollmentFactory(user=self.student2)
+        self.moderator = UserFactory(username='moderator', email='staff@edx.org', is_staff=True)
+        self.moderator_enrollment = CourseEnrollmentFactory(user=self.moderator)
+        self.moderator_role.users.add(self.moderator)
+
+    def test_get_role_ids(self):
+        ret = utils.get_role_ids(self.course_id)
+        expected = {u'Moderator': [3], u'Student': [1, 2], 'Staff': [3]}
+        self.assertEqual(ret, expected)
+
+    def test_has_forum_access(self):
+        ret = utils.has_forum_access('student', self.course_id, 'Student')
+        self.assertTrue(ret)
+
+        ret = utils.has_forum_access('not_a_student', self.course_id, 'Student')
+        self.assertFalse(ret)
+
+        ret = utils.has_forum_access('student', self.course_id, 'NotARole')
+        self.assertFalse(ret)

lms/djangoapps/django_comment_client/utils.py

@@ -35,6 +35,7 @@ def strip_blank(dic):
         return isinstance(v, str) and len(v.strip()) == 0
     return dict([(k, v) for k, v in dic.iteritems() if not _is_blank(v)])
 
+# TODO should we be checking if d1 and d2 have the same keys with different values?
 def merge_dict(dic1, dic2):
     return dict(dic1.items() + dic2.items())