From f488aa059b8622e54fdc485019cd50197d7a79d4 Mon Sep 17 00:00:00 2001 From: Miles Steele Date: Wed, 10 Jul 2013 14:28:06 -0400 Subject: [PATCH] rewrite test_access, add stricter argument to access --- lms/djangoapps/instructor/access.py | 14 +- .../instructor/tests/test_access.py | 240 +++++++++--------- 2 files changed, 128 insertions(+), 126 deletions(-) diff --git a/lms/djangoapps/instructor/access.py b/lms/djangoapps/instructor/access.py index c8d9abbb3d..b6f21a0fb8 100644 --- a/lms/djangoapps/instructor/access.py +++ b/lms/djangoapps/instructor/access.py @@ -23,7 +23,7 @@ def list_with_level(course, level): There could be other levels specific to the course. If there is no Group for that course-level, returns an empty list """ - if level in ['beta']: + if level is 'beta': grpname = course_beta_test_group_name(course.location) else: grpname = get_access_group_name(course, level) @@ -60,10 +60,12 @@ def _change_access(course, user, level, mode): mode is one of ['allow', 'revoke'] """ - if level in ['beta']: + if level is 'beta': grpname = course_beta_test_group_name(course.location) - else: + elif level in ['instructor', 'staff']: grpname = get_access_group_name(course, level) + else: + raise ValueError("unrecognized level '{}'".format(level)) group, _ = Group.objects.get_or_create(name=grpname) if mode == 'allow': @@ -78,9 +80,11 @@ def update_forum_role_membership(course_id, user, rolename, mode): """ Change forum access of user. - rolename is one of [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA] + `rolename` is one of [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA] + `mode` is one of ['allow', 'revoke'] - mode is one of ['allow', 'revoke'] + if `mode` is bad, raises ValueError + if `rolename` does not exist, raises Role.DoesNotExist """ role = Role.objects.get(course_id=course_id, name=rolename) diff --git a/lms/djangoapps/instructor/tests/test_access.py b/lms/djangoapps/instructor/tests/test_access.py index 5895504ee1..74e7d48e79 100644 --- a/lms/djangoapps/instructor/tests/test_access.py +++ b/lms/djangoapps/instructor/tests/test_access.py @@ -11,183 +11,181 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from django.test.utils import override_settings from courseware.tests.modulestore_config import TEST_DATA_MONGO_MODULESTORE -from courseware.access import get_access_group_name +from courseware.access import (get_access_group_name, + course_beta_test_group_name) from django_comment_common.models import (Role, - # FORUM_ROLE_ADMINISTRATOR, - FORUM_ROLE_MODERATOR, - FORUM_ROLE_COMMUNITY_TA) -from instructor.access import allow_access, revoke_access, list_with_level, update_forum_role_membership + FORUM_ROLE_MODERATOR) +from instructor.access import (allow_access, + revoke_access, + list_with_level, + update_forum_role_membership) @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) -class TestInstructorAccessControlDB(ModuleStoreTestCase): - """ Test instructor access administration against database effects """ +class TestInstructorAccessList(ModuleStoreTestCase): + """ Test access listings. """ + def setUp(self): + self.course = CourseFactory.create() + self.instructors = [UserFactory.create() for _ in xrange(4)] + for user in self.instructors: + allow_access(self.course, user, 'instructor') + self.beta_testers = [UserFactory.create() for _ in xrange(4)] + for user in self.beta_testers: + allow_access(self.course, user, 'beta') + + def test_list_instructors(self): + instructors = list_with_level(self.course, 'instructor') + self.assertEqual(set(instructors), set(self.instructors)) + + def test_list_beta(self): + beta_testers = list_with_level(self.course, 'beta') + self.assertEqual(set(beta_testers), set(self.beta_testers)) + + +@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) +class TestInstructorAccessAllow(ModuleStoreTestCase): + """ Test access allow. """ def setUp(self): self.course = CourseFactory.create() def test_allow(self): user = UserFactory() - level = 'staff' - - allow_access(self.course, user, level) - - self.assertIn(user, Group.objects.get(name=get_access_group_name(self.course, 'staff')).user_set.all()) + allow_access(self.course, user, 'staff') + group = Group.objects.get( + name=get_access_group_name(self.course, 'staff') + ) + self.assertIn(user, group.user_set.all()) def test_allow_twice(self): user = UserFactory() - level = 'staff' + allow_access(self.course, user, 'staff') + allow_access(self.course, user, 'staff') + group = Group.objects.get( + name=get_access_group_name(self.course, 'staff') + ) + self.assertIn(user, group.user_set.all()) - allow_access(self.course, user, level) - self.assertIn(user, Group.objects.get(name=get_access_group_name(self.course, 'staff')).user_set.all()) - allow_access(self.course, user, level) - self.assertIn(user, Group.objects.get(name=get_access_group_name(self.course, 'staff')).user_set.all()) - - def test_allow_revoke(self): + def test_allow_beta(self): + """ Test allow beta against list beta. """ user = UserFactory() - level = 'staff' + allow_access(self.course, user, 'beta') + self.assertIn(user, list_with_level(self.course, 'beta')) - allow_access(self.course, user, level) - self.assertIn(user, Group.objects.get(name=get_access_group_name(self.course, 'staff')).user_set.all()) - revoke_access(self.course, user, level) - self.assertNotIn(user, Group.objects.get(name=get_access_group_name(self.course, 'staff')).user_set.all()) - allow_access(self.course, user, level) - self.assertIn(user, Group.objects.get(name=get_access_group_name(self.course, 'staff')).user_set.all()) - revoke_access(self.course, user, level) - self.assertNotIn(user, Group.objects.get(name=get_access_group_name(self.course, 'staff')).user_set.all()) - - def test_revoke_without_group(self): + @raises(ValueError) + def test_allow_badlevel(self): user = UserFactory() - level = 'staff' - - revoke_access(self.course, user, level) - self.assertNotIn(user, Group.objects.get(name=get_access_group_name(self.course, 'staff')).user_set.all()) - - def test_revoke_with_group(self): - user = UserFactory() - level = 'staff' - - Group(name=get_access_group_name(self.course, level)) - revoke_access(self.course, user, level) - self.assertNotIn(user, Group.objects.get(name=get_access_group_name(self.course, 'staff')).user_set.all()) - - def test_allow_disallow_multiuser(self): - users = [UserFactory() for _ in xrange(3)] - levels = ['staff', 'instructor', 'staff'] - antilevels = ['instructor', 'staff', 'instructor'] - - allow_access(self.course, users[0], levels[0]) - allow_access(self.course, users[1], levels[1]) - allow_access(self.course, users[2], levels[2]) - self.assertIn(users[0], Group.objects.get(name=get_access_group_name(self.course, levels[0])).user_set.all()) - self.assertIn(users[1], Group.objects.get(name=get_access_group_name(self.course, levels[1])).user_set.all()) - self.assertIn(users[2], Group.objects.get(name=get_access_group_name(self.course, levels[2])).user_set.all()) - - revoke_access(self.course, users[0], levels[0]) - revoke_access(self.course, users[0], antilevels[0]) - self.assertNotIn(users[0], Group.objects.get(name=get_access_group_name(self.course, levels[0])).user_set.all()) - self.assertIn(users[1], Group.objects.get(name=get_access_group_name(self.course, levels[1])).user_set.all()) - self.assertIn(users[2], Group.objects.get(name=get_access_group_name(self.course, levels[2])).user_set.all()) - - revoke_access(self.course, users[1], levels[1]) - allow_access(self.course, users[0], antilevels[0]) - self.assertNotIn(users[0], Group.objects.get(name=get_access_group_name(self.course, levels[0])).user_set.all()) - self.assertIn(users[0], Group.objects.get(name=get_access_group_name(self.course, antilevels[0])).user_set.all()) - self.assertNotIn(users[1], Group.objects.get(name=get_access_group_name(self.course, levels[1])).user_set.all()) - self.assertIn(users[2], Group.objects.get(name=get_access_group_name(self.course, levels[2])).user_set.all()) + allow_access(self.course, user, 'robot-not-a-level') + group = Group.objects.get(name=get_access_group_name(self.course, 'robot-not-a-level')) + self.assertIn(user, group.user_set.all()) + @raises(Exception) + def test_allow_noneuser(self): + user = None + allow_access(self.course, user, 'staff') + group = Group.objects.get(name=get_access_group_name(self.course, 'staff')) + self.assertIn(user, group.user_set.all()) @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) -class TestInstructorAccessControlPrefilledDB(ModuleStoreTestCase): - """ - Test access with existing users. - """ +class TestInstructorAccessRevoke(ModuleStoreTestCase): + """ Test access revoke. """ def setUp(self): self.course = CourseFactory.create() - # setup instructors - self.instructors = set([UserFactory.create(), UserFactory.create()]) - for user in self.instructors: - allow_access(self.course, user, 'instructor') - - def test_list_with_level(self): - instructors = set(list_with_level(self.course, 'instructor')) - self.assertEqual(instructors, self.instructors) - - def test_list_with_level_not_yet_group(self): - instructors = set(list_with_level(self.course, 'staff')) - self.assertEqual(instructors, set()) - - def test_list_with_level_bad_group(self): - self.assertEqual(set(list_with_level(self.course, 'robot-definitely-not-a-group')), set()) - - def test_list_with_level_beta(self): - beta_testers_result = set(list_with_level(self.course, 'beta')) - self.assertEqual(set(), beta_testers_result) - - beta_testers = set([UserFactory.create(), UserFactory.create()]) - for user in beta_testers: + self.staff = [UserFactory.create() for _ in xrange(4)] + for user in self.staff: + allow_access(self.course, user, 'staff') + self.beta_testers = [UserFactory.create() for _ in xrange(4)] + for user in self.beta_testers: allow_access(self.course, user, 'beta') - beta_testers_result = set(list_with_level(self.course, 'beta')) - self.assertEqual(beta_testers, beta_testers_result) + + def test_revoke(self): + user = self.staff[0] + revoke_access(self.course, user, 'staff') + group = Group.objects.get( + name=get_access_group_name(self.course, 'staff') + ) + self.assertNotIn(user, group.user_set.all()) + + def test_revoke_twice(self): + user = self.staff[0] + revoke_access(self.course, user, 'staff') + group = Group.objects.get( + name=get_access_group_name(self.course, 'staff') + ) + self.assertNotIn(user, group.user_set.all()) + + def test_revoke_beta(self): + user = self.beta_testers[0] + revoke_access(self.course, user, 'beta') + self.assertNotIn(user, list_with_level(self.course, 'beta')) + + @raises(ValueError) + def test_revoke_badrolename(self): + user = UserFactory() + revoke_access(self.course, user, 'robot-not-a-level') + group = Group.objects.get( + name=get_access_group_name(self.course, 'robot-not-a-level') + ) + self.assertNotIn(user, group.user_set.all()) @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) -class TestInstructorAccessForumDB(ModuleStoreTestCase): +class TestInstructorAccessForum(ModuleStoreTestCase): """ Test forum access control. """ def setUp(self): self.course = CourseFactory.create() - self.moderators = set([UserFactory.create() for _ in xrange(4)]) - self.mod_role = Role.objects.create(course_id=self.course.id, name=FORUM_ROLE_MODERATOR) + self.mod_role = Role.objects.create( + course_id=self.course.id, + name=FORUM_ROLE_MODERATOR + ) + self.moderators = [UserFactory.create() for _ in xrange(4)] for user in self.moderators: self.mod_role.users.add(user) - def test_update_forum_membership_allow_existing_role(self): + def test_allow(self): user = UserFactory.create() update_forum_role_membership(self.course.id, user, FORUM_ROLE_MODERATOR, 'allow') self.assertIn(user, self.mod_role.users.all()) - def test_update_forum_membership_allow_existing_role_allowed_user(self): + def test_allow_twice(self): user = UserFactory.create() update_forum_role_membership(self.course.id, user, FORUM_ROLE_MODERATOR, 'allow') + self.assertIn(user, self.mod_role.users.all()) update_forum_role_membership(self.course.id, user, FORUM_ROLE_MODERATOR, 'allow') self.assertIn(user, self.mod_role.users.all()) @raises(Role.DoesNotExist) - def test_update_forum_membership_allow_not_existing_role(self): + def test_allow_badrole(self): user = UserFactory.create() - update_forum_role_membership(self.course.id, user, FORUM_ROLE_COMMUNITY_TA, 'allow') + update_forum_role_membership(self.course.id, user, 'robot-not-a-real-role', 'allow') - def test_update_forum_membership_revoke_existing_role(self): - user = iter(self.moderators).next() + def test_revoke(self): + user = self.moderators[0] update_forum_role_membership(self.course.id, user, FORUM_ROLE_MODERATOR, 'revoke') self.assertNotIn(user, self.mod_role.users.all()) - def test_update_forum_membership_existing_role_revoked_user(self): - user = iter(self.moderators).next() + def test_revoke_twice(self): + user = self.moderators[0] update_forum_role_membership(self.course.id, user, FORUM_ROLE_MODERATOR, 'revoke') + self.assertNotIn(user, self.mod_role.users.all()) + update_forum_role_membership(self.course.id, user, FORUM_ROLE_MODERATOR, 'revoke') + self.assertNotIn(user, self.mod_role.users.all()) + + def test_revoke_notallowed(self): + user = UserFactory() update_forum_role_membership(self.course.id, user, FORUM_ROLE_MODERATOR, 'revoke') self.assertNotIn(user, self.mod_role.users.all()) @raises(Role.DoesNotExist) - def test_update_forum_membership_revoke_not_existing_role(self): - user = iter(self.moderators).next() - update_forum_role_membership(self.course.id, user, FORUM_ROLE_COMMUNITY_TA, 'revoke') - - @raises(Role.DoesNotExist) - def test_update_forum_membership_bad_role_allow(self): - user = UserFactory.create() - update_forum_role_membership(self.course.id, user, 'robot-definitely-not-a-forum-role', 'allow') - - @raises(Role.DoesNotExist) - def test_update_forum_membership_bad_role_revoke(self): - user = UserFactory.create() - update_forum_role_membership(self.course.id, user, 'robot-definitely-not-a-forum-role', 'revoke') + def test_revoke_badrole(self): + user = self.moderators[0] + update_forum_role_membership(self.course.id, user, 'robot-not-a-real-role', 'allow') @raises(ValueError) - def test_update_forum_membership_bad_mode(self): - user = iter(self.moderators).next() + def test_bad_mode(self): + user = UserFactory() update_forum_role_membership(self.course.id, user, FORUM_ROLE_MODERATOR, 'robot-not-a-mode')