Merge pull request #1441 from MITx/hotfix/dave/revert_comment_client_changes
Revert "Fix permissions bug and add test cases for django comment client...
This commit is contained in:
Victor Shnayder
@@ -46,13 +46,11 @@ 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)
|
||||
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:
|
||||
if self.name == FORUM_ROLE_STUDENT and \
|
||||
(permission.startswith('edit') or permission.startswith('update') or permission.startswith('create')) and \
|
||||
(not course.forum_posts_allowed):
|
||||
return False
|
||||
|
||||
return self.permissions.filter(name=permission).exists()
|
||||
|
||||
@@ -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)
|
||||
|
||||
111
lms/djangoapps/django_comment_client/tests.py
Normal file
111
lms/djangoapps/django_comment_client/tests.py
Normal file
@@ -0,0 +1,111 @@
|
||||
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))
|
||||
@@ -1,54 +0,0 @@
|
||||
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")
|
||||
@@ -1,128 +0,0 @@
|
||||
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)
|
||||
Reference in New Issue
Block a user