From 32e0021260d89e0bbf7372ac1f306b001a26ccb3 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Tue, 21 Jan 2014 13:10:07 -0500 Subject: [PATCH 1/2] Create mapping for previously unmapped locations in order to get course_id STUD-1212 --- common/djangoapps/student/roles.py | 2 +- common/djangoapps/student/tests/test_roles.py | 41 ++++++++++++++++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 9da258e19b..01252f742f 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -168,7 +168,7 @@ class CourseRole(GroupBasedRole): else: groupnames.append('{0}_{1}'.format(role, course_context)) try: - locator = loc_mapper().translate_location(course_context, self.location, False, False) + locator = loc_mapper().translate_location(course_context, self.location, True, True) groupnames.append('{0}_{1}'.format(role, locator.package_id)) except (InvalidLocationError, ItemNotFoundError): # if it's never been mapped, the auth won't be via the Locator syntax diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py index d7ec715426..cd20d16cf6 100644 --- a/common/djangoapps/student/tests/test_roles.py +++ b/common/djangoapps/student/tests/test_roles.py @@ -8,7 +8,9 @@ from xmodule.modulestore import Location from courseware.tests.factories import UserFactory, StaffFactory, InstructorFactory from student.tests.factories import AnonymousUserFactory -from student.roles import GlobalStaff, CourseRole +from student.roles import GlobalStaff, CourseRole, CourseStaffRole +from xmodule.modulestore.django import loc_mapper +from xmodule.modulestore.locator import BlockUsageLocator class RolesTestCase(TestCase): @@ -45,3 +47,40 @@ class RolesTestCase(TestCase): self.assertTrue(CourseRole("role", lowercase_loc).has_user(uppercase_user)) self.assertTrue(CourseRole("role", uppercase_loc).has_user(uppercase_user)) + def test_course_role(self): + """ + Test that giving a user a course role enables access appropriately + """ + course_locator = loc_mapper().translate_location( + self.course.course_id, self.course, add_entry_if_missing=True + ) + self.assertFalse( + CourseStaffRole(course_locator).has_user(self.student), + "Student has premature access to {}".format(unicode(course_locator)) + ) + self.assertFalse( + CourseStaffRole(self.course).has_user(self.student), + "Student has premature access to {}".format(self.course.url()) + ) + CourseStaffRole(course_locator).add_users(self.student) + self.assertTrue( + CourseStaffRole(course_locator).has_user(self.student), + "Student doesn't have access to {}".format(unicode(course_locator)) + ) + self.assertTrue( + CourseStaffRole(self.course).has_user(self.student), + "Student doesn't have access to {}".format(unicode(self.course.url())) + ) + # now try accessing something internal to the course + vertical_locator = BlockUsageLocator( + package_id=course_locator.package_id, branch='published', block_id='madeup' + ) + vertical_location = self.course.replace(category='vertical', name='madeuptoo') + self.assertTrue( + CourseStaffRole(vertical_locator).has_user(self.student), + "Student doesn't have access to {}".format(unicode(vertical_locator)) + ) + self.assertTrue( + CourseStaffRole(vertical_location, course_context=self.course.course_id).has_user(self.student), + "Student doesn't have access to {}".format(unicode(vertical_location.url())) + ) From 9d6ed66e24c8295d213c614f0fd5d6560f38d813 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Wed, 22 Jan 2014 10:46:27 -0500 Subject: [PATCH 2/2] Add loc mapper fn from locn to CourseLocator --- common/djangoapps/student/roles.py | 2 +- .../xmodule/modulestore/loc_mapper_store.py | 33 ++++++++++++++++++- .../modulestore/tests/test_location_mapper.py | 8 +++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 01252f742f..13b9b38a15 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -168,7 +168,7 @@ class CourseRole(GroupBasedRole): else: groupnames.append('{0}_{1}'.format(role, course_context)) try: - locator = loc_mapper().translate_location(course_context, self.location, True, True) + locator = loc_mapper().translate_location_to_course_locator(course_context, self.location) groupnames.append('{0}_{1}'.format(role, locator.package_id)) except (InvalidLocationError, ItemNotFoundError): # if it's never been mapped, the auth won't be via the Locator syntax diff --git a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py index 5a37c260f7..d05810b473 100644 --- a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py +++ b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py @@ -7,7 +7,7 @@ import pymongo import bson.son from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError -from xmodule.modulestore.locator import BlockUsageLocator +from xmodule.modulestore.locator import BlockUsageLocator, CourseLocator from xmodule.modulestore import Location import urllib @@ -249,6 +249,37 @@ class LocMapperStore(object): return result return None + def translate_location_to_course_locator(self, old_style_course_id, location, published=True): + """ + Used when you only need the CourseLocator and not a full BlockUsageLocator. Probably only + useful for get_items which wildcards name or category. + + :param course_id: old style course id + """ + # doesn't use caching as cache requires full location w/o wildcards + location_id = self._interpret_location_course_id(old_style_course_id, location) + if old_style_course_id is None: + old_style_course_id = self._generate_location_course_id(location_id) + + maps = self.location_map.find(location_id) + maps = list(maps) + if len(maps) == 0: + raise ItemNotFoundError() + elif len(maps) == 1: + entry = maps[0] + else: + # find entry w/o name, if any; otherwise, pick arbitrary + entry = maps[0] + for item in maps: + if 'name' not in item['_id']: + entry = item + break + if published: + branch = entry['prod_branch'] + else: + branch = entry['draft_branch'] + return CourseLocator(package_id=entry['course_id'], branch=branch) + def _add_to_block_map(self, location, location_id, block_map): '''add the given location to the block_map and persist it''' if self._block_id_is_guid(location.name): diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py index dee30ba491..73fa287b4c 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py @@ -89,6 +89,14 @@ class TestLocationMapper(unittest.TestCase): self.assertEqual(prob_locator.block_id, block_id) self.assertEqual(prob_locator.branch, branch) + course_locator = loc_mapper().translate_location_to_course_locator( + old_style_course_id, + location, + published=(branch == 'published'), + ) + self.assertEqual(course_locator.package_id, new_style_package_id) + self.assertEqual(course_locator.branch, branch) + def test_translate_location_read_only(self): """ Test the variants of translate_location which don't create entries, just decode