Merge pull request #2244 from edx/dhm/role_package_id

Create mapping for previously unmapped locations in order to get

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_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

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()))
+        )

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):

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