Revert "Fix permissions bug and add test cases for django comment client permissions."

This reverts commit e7450874.

lms/djangoapps/django_comment_client/permissions.py

@@ -29,7 +29,6 @@ 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:
@@ -62,10 +61,8 @@ 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['data'])
+                return check_condition(user, per, course_id, kwargs)
             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":
@@ -105,4 +102,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, data=content)
+    return check_conditions_permissions(user, p, course_id, content=content)

lms/djangoapps/django_comment_client/tests.py

+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_permissions.py

-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)
\ No newline at end of file