Made fill_in_run public in mongo and added fill_in_run to mixed which funnels to mongo

cms/djangoapps/contentstore/views/component.py

@@ -409,6 +409,9 @@ def _get_item_in_course(request, usage_key):
 
     Verifies that the caller has permission to access this item.
     """
+    # usage_key's course_key may have an empty run property
+    usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key))
+
     course_key = usage_key.course_key
 
     if not has_course_access(request.user, course_key):

cms/djangoapps/contentstore/views/item.py

@@ -101,6 +101,9 @@ def xblock_handler(request, usage_key_string):
     """
     if usage_key_string:
         usage_key = UsageKey.from_string(usage_key_string)
+        # usage_key's course_key may have an empty run property
+        usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key))
+
         if not has_course_access(request.user, usage_key.course_key):
             raise PermissionDenied()
 
@@ -135,7 +138,15 @@ def xblock_handler(request, usage_key_string):
     elif request.method in ('PUT', 'POST'):
         if 'duplicate_source_locator' in request.json:
             parent_usage_key = UsageKey.from_string(request.json['parent_locator'])
+            # usage_key's course_key may have an empty run property
+            parent_usage_key = parent_usage_key.replace(
+                course_key=modulestore().fill_in_run(parent_usage_key.course_key)
+            )
             duplicate_source_usage_key = UsageKey.from_string(request.json['duplicate_source_locator'])
+            # usage_key's course_key may have an empty run property
+            duplicate_source_usage_key = duplicate_source_usage_key.replace(
+                course_key=modulestore().fill_in_run(duplicate_source_usage_key.course_key)
+            )
 
             dest_usage_key = _duplicate_item(
                 parent_usage_key,
@@ -167,6 +178,8 @@ def xblock_view_handler(request, usage_key_string, view_name):
             the second is the resource description
     """
     usage_key = UsageKey.from_string(usage_key_string)
+    # usage_key's course_key may have an empty run property
+    usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key))
     if not has_course_access(request.user, usage_key.course_key):
         raise PermissionDenied()
 
@@ -376,6 +389,8 @@ def _save_item(user, usage_key, data=None, children=None, metadata=None, nullout
 def _create_item(request):
     """View for create items."""
     usage_key = UsageKey.from_string(request.json['parent_locator'])
+    # usage_key's course_key may have an empty run property
+    usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key))
     category = request.json['category']
 
     display_name = request.json.get('display_name')

cms/djangoapps/contentstore/views/transcripts_ajax.py

@@ -533,6 +533,9 @@ def _get_item(request, data):
     """
     usage_key = UsageKey.from_string(data.get('locator'))
 
+    # usage_key's course_key may have an empty run property
+    usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key))
+
     # This is placed before has_course_access() to validate the location,
     # because has_course_access() raises  r if location is invalid.
     item = modulestore().get_item(usage_key)

common/lib/xmodule/xmodule/modulestore/mixed.py

@@ -118,6 +118,17 @@ class MixedModuleStore(ModuleStoreWriteBase):
                 return store
         return None
 
+    def fill_in_run(self, course_key):
+        """
+        Some course_keys are used without runs. This function calls the corresponding
+        fill_in_run function on the appropriate modulestore.
+        """
+        store = self._get_modulestore_for_courseid(course_key)
+        if not hasattr(store, 'fill_in_run'):
+            return course_key
+        return store.fill_in_run(course_key)
+
+
     def has_item(self, usage_key, **kwargs):
         """
         Does the course include the xblock who's id is reference?

common/lib/xmodule/xmodule/modulestore/mongo/base.py

@@ -263,7 +263,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
         Convert a single serialized UsageKey string in a ReferenceField into a UsageKey.
         """
         key = Location.from_deprecated_string(ref_string)
-        return key.replace(run=self.modulestore._fill_in_run(key.course_key).run)
+        return key.replace(run=self.modulestore.fill_in_run(key.course_key).run)
 
     def __setattr__(self, name, value):
         return super(CachingDescriptorSystem, self).__setattr__(name, value)
@@ -409,7 +409,11 @@ class MongoModuleStore(ModuleStoreWriteBase):
             self.ignore_write_events_on_courses.remove(course_id)
             self.refresh_cached_metadata_inheritance_tree(course_id)
 
-    def _fill_in_run(self, course_key):
+    def fill_in_run(self, course_key):
+        """
+        In mongo some course_keys are used without runs. This helper function returns
+        a course_key with the run filled in, if the course does actually exist.
+        """
         if course_key.run is not None:
             return course_key
 
@@ -437,7 +441,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
         # get all collections in the course, this query should not return any leaf nodes
         # note this is a bit ugly as when we add new categories of containers, we have to add it here
 
-        course_id = self._fill_in_run(course_id)
+        course_id = self.fill_in_run(course_id)
         block_types_with_children = set(
             name for name, class_ in XBlock.load_classes() if getattr(class_, 'has_children', False)
         )
@@ -513,7 +517,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
         '''
         tree = {}
 
-        course_id = self._fill_in_run(course_id)
+        course_id = self.fill_in_run(course_id)
         if not force_refresh:
             # see if we are first in the request cache (if present)
             if self.request_cache is not None and course_id in self.request_cache.data.get('metadata_inheritance', {}):
@@ -592,7 +596,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
 
         data = {}
         to_process = list(items)
-        course_key = self._fill_in_run(course_key)
+        course_key = self.fill_in_run(course_key)
         while to_process and depth is None or depth >= 0:
             children = []
             for item in to_process:
@@ -620,7 +624,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
         """
         Load an XModuleDescriptor from item, using the children stored in data_cache
         """
-        course_key = self._fill_in_run(course_key)
+        course_key = self.fill_in_run(course_key)
         location = Location._from_deprecated_son(item['location'], course_key.run)
         data_dir = getattr(item, 'data_dir', location.course)
         root = self.fs_root / data_dir
@@ -657,7 +661,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
         Load a list of xmodules from the data in items, with children cached up
         to specified depth
         """
-        course_key = self._fill_in_run(course_key)
+        course_key = self.fill_in_run(course_key)
         data_cache = self._cache_children(course_key, items, depth)
 
         # if we are loading a course object, if we're not prefetching children (depth != 0) then don't
@@ -710,7 +714,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
         Get the course with the given courseid (org/course/run)
         """
         assert(isinstance(course_key, CourseKey))
-        course_key = self._fill_in_run(course_key)
+        course_key = self.fill_in_run(course_key)
         location = course_key.make_usage_key('course', course_key.run)
         try:
             return self.get_item(location, depth=depth)
@@ -727,7 +731,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
         otherwise, do a case sensitive search
         """
         assert(isinstance(course_key, CourseKey))
-        course_key = self._fill_in_run(course_key)
+        course_key = self.fill_in_run(course_key)
         location = course_key.make_usage_key('course', course_key.run)
         if ignore_case:
             course_query = location.to_deprecated_son('_id.')
@@ -910,7 +914,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
         :param runtime: if you already have an xblock from the course, the xblock.runtime value
         :param fields: a dictionary of field names and values for the new xmodule
         """
-        location = location.replace(run=self._fill_in_run(location.course_key).run)
+        location = location.replace(run=self.fill_in_run(location.course_key).run)
         # differs from split mongo in that I believe most of this logic should be above the persistence
         # layer but added it here to enable quick conversion. I'll need to reconcile these.
         if metadata is None:
@@ -1143,7 +1147,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
         """
         Return an array of all of the locations (deprecated string format) for orphans in the course.
         """
-        course_key = self._fill_in_run(course_key)
+        course_key = self.fill_in_run(course_key)
         detached_categories = [name for name, __ in XBlock.load_tagged_classes("detached")]
         query = self._course_key_to_son(course_key)
         query['_id.category'] = {'$nin': detached_categories}

common/lib/xmodule/xmodule/tests/test_import.py

@@ -444,9 +444,6 @@ class ImportTestCase(BaseCourseTestCase):
             for msg, err
             in modulestore.get_course_errors(course.id)
         ]
-        for msg, err in errors:
-            print("msg: " + str(msg))
-            print("err: " + str(err))
 
         self.assertTrue(any(
             expect in msg or expect in err