Merge pull request #1239 from MITx/bugfix/dave/forum_missing_vertical

Fix parent lookups when the same named tag exists in multiple courses

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -345,9 +345,9 @@ class ModuleStore(object):
         '''
         raise NotImplementedError
 
-    def get_parent_locations(self, location):
-        '''Find all locations that are the parents of this location.  Needed
-        for path_to_location().
+    def get_parent_locations(self, location, course_id):
+        '''Find all locations that are the parents of this location in this 
+        course.  Needed for path_to_location().
 
         returns an iterable of things that can be passed to Location.
         '''

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

@@ -309,9 +309,9 @@ class MongoModuleStore(ModuleStoreBase):
 
         self._update_single_item(location, {'metadata': metadata})
 
-    def get_parent_locations(self, location):
-        '''Find all locations that are the parents of this location.  Needed
-        for path_to_location().
+    def get_parent_locations(self, location, course_id):
+        '''Find all locations that are the parents of this location in this 
+        course.  Needed for path_to_location().
 
         If there is no data at location in this modulestore, raise
             ItemNotFoundError.

common/lib/xmodule/xmodule/modulestore/search.py

@@ -64,7 +64,7 @@ def path_to_location(modulestore, course_id, location):
             # isn't found so we don't have to do it explicitly.  Call this
             # first to make sure the location is there (even if it's a course, and
             # we would otherwise immediately exit).
-            parents = modulestore.get_parent_locations(loc)
+            parents = modulestore.get_parent_locations(loc, course_id)
 
             # print 'Processing loc={0}, path={1}'.format(loc, path)
             if loc.category == "course":

common/lib/xmodule/xmodule/modulestore/tests/test_modulestore.py

@@ -23,12 +23,3 @@ def check_path_to_location(modulestore):
     for location in not_found:
         assert_raises(ItemNotFoundError, path_to_location, modulestore, course_id, location)
 
-    # Since our test files are valid, there shouldn't be any
-    # elements with no path to them.  But we can look for them in
-    # another course.
-    no_path = (
-        "i4x://edX/simple/video/Lost_Video",
-        )
-    for location in no_path:
-        assert_raises(NoPathToItem, path_to_location, modulestore, course_id, location)
-

common/lib/xmodule/xmodule/modulestore/xml.py

@@ -275,14 +275,16 @@ class XMLModuleStore(ModuleStoreBase):
             class_ = getattr(import_module(module_path), class_name)
             self.default_class = class_
 
-        self.parent_tracker = ParentTracker()
+        self.parent_trackers = defaultdict(ParentTracker)
 
         # If we are specifically asked for missing courses, that should
         # be an error.  If we are asked for "all" courses, find the ones
-        # that have a course.xml
+        # that have a course.xml. We sort the dirs in alpha order so we always
+        # read things in the same order (OS differences in load order have
+        # bitten us in the past.)
         if course_dirs is None:
-            course_dirs = [d for d in os.listdir(self.data_dir) if
-                           os.path.exists(self.data_dir / d / "course.xml")]
+            course_dirs = sorted([d for d in os.listdir(self.data_dir) if
+                                  os.path.exists(self.data_dir / d / "course.xml")])
 
         for course_dir in course_dirs:
             self.try_load_course(course_dir)
@@ -307,7 +309,7 @@ class XMLModuleStore(ModuleStoreBase):
         if course_descriptor is not None:
             self.courses[course_dir] = course_descriptor
             self._location_errors[course_descriptor.location] = errorlog
-            self.parent_tracker.make_known(course_descriptor.location)
+            self.parent_trackers[course_descriptor.id].make_known(course_descriptor.location)
         else:
             # Didn't load course.  Instead, save the errors elsewhere.
             self.errored_courses[course_dir] = errorlog
@@ -432,7 +434,7 @@ class XMLModuleStore(ModuleStoreBase):
                 course_dir,
                 policy,
                 tracker,
-                self.parent_tracker,
+                self.parent_trackers[course_id],
                 self.load_error_modules,
             )
 
@@ -541,9 +543,9 @@ class XMLModuleStore(ModuleStoreBase):
         """
         raise NotImplementedError("XMLModuleStores are read-only")
 
-    def get_parent_locations(self, location):
-        '''Find all locations that are the parents of this location.  Needed
-        for path_to_location().
+    def get_parent_locations(self, location, course_id):
+        '''Find all locations that are the parents of this location in this 
+        course.  Needed for path_to_location().
 
         If there is no data at location in this modulestore, raise
             ItemNotFoundError.
@@ -552,7 +554,7 @@ class XMLModuleStore(ModuleStoreBase):
         be empty if there are no parents.
         '''
         location = Location.ensure_fully_specified(location)
-        if not self.parent_tracker.is_known(location):
-            raise ItemNotFoundError(location)
+        if not self.parent_trackers[course_id].is_known(location):
+            raise ItemNotFoundError("{0} not in {1}".format(location, course_id))
 
-        return self.parent_tracker.parents(location)
+        return self.parent_trackers[course_id].parents(location)