diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
index 3874d8d872..155a12d223 100644
--- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
+++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
@@ -1061,13 +1061,15 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
raise ItemNotFoundError(locator)
course = self._lookup_course(locator.course_key)
- parent_id = self._get_parent_from_structure(BlockKey.from_usage_key(locator), course.structure)
- if parent_id is None:
+ parent_ids = self._get_parents_from_structure(BlockKey.from_usage_key(locator), course.structure)
+ if len(parent_ids) == 0:
return None
+ # find alphabetically least
+ parent_ids.sort(key=lambda parent: (parent.type, parent.id))
return BlockUsageLocator.make_relative(
locator,
- block_type=parent_id.type,
- block_id=parent_id.id,
+ block_type=parent_ids[0].type,
+ block_id=parent_ids[0].id,
)
def get_orphans(self, course_key, **kwargs):
@@ -2041,8 +2043,8 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
for subtree_root in subtree_list:
if BlockKey.from_usage_key(subtree_root) != source_structure['root']:
# find the parents and put root in the right sequence
- parent = self._get_parent_from_structure(BlockKey.from_usage_key(subtree_root), source_structure)
- if parent is not None: # may be a detached category xblock
+ parents = self._get_parents_from_structure(BlockKey.from_usage_key(subtree_root), source_structure)
+ for parent in parents:
if parent not in destination_blocks:
raise ItemNotFoundError(parent)
orphans.update(
@@ -2101,8 +2103,8 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
new_structure = self.version_structure(usage_locator.course_key, original_structure, user_id)
new_blocks = new_structure['blocks']
new_id = new_structure['_id']
- parent_block_key = self._get_parent_from_structure(block_key, original_structure)
- if parent_block_key:
+ parent_block_keys = self._get_parents_from_structure(block_key, original_structure)
+ for parent_block_key in parent_block_keys:
parent_block = new_blocks[parent_block_key]
parent_block['fields']['children'].remove(block_key)
parent_block['edit_info']['edited_on'] = datetime.datetime.now(UTC)
@@ -2626,15 +2628,16 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
}
@contract(block_key=BlockKey)
- def _get_parent_from_structure(self, block_key, structure):
+ def _get_parents_from_structure(self, block_key, structure):
"""
Given a structure, find block_key's parent in that structure. Note returns
the encoded format for parent
"""
- for parent_block_key, value in structure['blocks'].iteritems():
- if block_key in value['fields'].get('children', []):
- return parent_block_key
- return None
+ return [
+ parent_block_key
+ for parent_block_key, value in structure['blocks'].iteritems()
+ if block_key in value['fields'].get('children', [])
+ ]
def _sync_children(self, source_parent, destination_parent, new_child):
"""
@@ -2733,7 +2736,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
"""
Delete the orphan and any of its descendants which no longer have parents.
"""
- if self._get_parent_from_structure(orphan, structure) is None:
+ if len(self._get_parents_from_structure(orphan, structure)) == 0:
for child in structure['blocks'][orphan]['fields'].get('children', []):
self._delete_if_true_orphan(BlockKey(*child), structure)
del structure['blocks'][orphan]
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
index d055c8a602..a8712f4d66 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
@@ -1962,6 +1962,46 @@ class TestMixedModuleStore(CourseComparisonTest):
with self.store.branch_setting(ModuleStoreEnum.Branch.published_only, course_id):
self.assertTrue(self.store.has_item(vertical_loc))
+ @ddt.data(ModuleStoreEnum.Type.split) # Need to fix and add ModuleStoreEnum.Type.mongo,
+ def test_delete_dag(self, default):
+ """
+ Test that deleting an element with more than one parent fully removes it from the course.
+ """
+ # set the default modulestore
+ with MongoContentstoreBuilder().build() as contentstore:
+ self.store = MixedModuleStore(
+ contentstore=contentstore,
+ create_modulestore_instance=create_modulestore_instance,
+ mappings={},
+ **self.OPTIONS
+ )
+ self.addCleanup(self.store.close_all_connections)
+ with self.store.default_store(default):
+ dest_course_key = self.store.make_course_key('a', 'course', 'course')
+ courses = import_from_xml(
+ self.store, self.user_id, DATA_DIR, ['xml_dag'], load_error_modules=False,
+ static_content_store=contentstore,
+ target_course_id=dest_course_key,
+ create_course_if_not_present=True,
+ )
+ course_id = courses[0].id
+ # ensure both parents point to the dag item
+ dag_item = course_id.make_usage_key('html', 'toyhtml')
+ one_parent = course_id.make_usage_key('vertical', 'vertical_test')
+ other_parent = course_id.make_usage_key('vertical', 'zeta')
+ with self.store.bulk_operations(course_id):
+ # actually should test get_parent but it's not alphabetized yet
+ self.assertEqual(self.store.get_parent_location(dag_item), one_parent)
+ for parent_loc in [one_parent, other_parent]:
+ parent = self.store.get_item(parent_loc)
+ self.assertIn(dag_item, parent.children)
+ # just testing draft branch assuming it doesn't matter which branch
+ with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, course_id):
+ self.store.delete_item(dag_item, self.user_id)
+ for parent_loc in [one_parent, other_parent]:
+ parent = self.store.get_item(parent_loc)
+ self.assertNotIn(dag_item, parent.children)
+
@ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
def test_import_edit_import(self, default):
"""
diff --git a/common/test/data/xml_dag/README.md b/common/test/data/xml_dag/README.md
new file mode 100644
index 0000000000..1c9f2d3b27
--- /dev/null
+++ b/common/test/data/xml_dag/README.md
@@ -0,0 +1,2 @@
+Minimal course for testing the handling of non-tree courses. The same html component occurs under more than one
+vertical.
\ No newline at end of file
diff --git a/common/test/data/xml_dag/course.xml b/common/test/data/xml_dag/course.xml
new file mode 100644
index 0000000000..ebe7e5f887
--- /dev/null
+++ b/common/test/data/xml_dag/course.xml
@@ -0,0 +1 @@
+
diff --git a/common/test/data/xml_dag/course/2012_Fall.xml b/common/test/data/xml_dag/course/2012_Fall.xml
new file mode 100644
index 0000000000..40193e8b87
--- /dev/null
+++ b/common/test/data/xml_dag/course/2012_Fall.xml
@@ -0,0 +1,13 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/common/test/data/xml_dag/html/toyhtml.html b/common/test/data/xml_dag/html/toyhtml.html
new file mode 100644
index 0000000000..08f0504f36
--- /dev/null
+++ b/common/test/data/xml_dag/html/toyhtml.html
@@ -0,0 +1 @@
+Sample
diff --git a/common/test/data/xml_dag/html/toyhtml.xml b/common/test/data/xml_dag/html/toyhtml.xml
new file mode 100644
index 0000000000..4e84f53549
--- /dev/null
+++ b/common/test/data/xml_dag/html/toyhtml.xml
@@ -0,0 +1 @@
+
diff --git a/common/test/data/xml_dag/policies/2012_Fall.json b/common/test/data/xml_dag/policies/2012_Fall.json
new file mode 100644
index 0000000000..82bfa20787
--- /dev/null
+++ b/common/test/data/xml_dag/policies/2012_Fall.json
@@ -0,0 +1,8 @@
+{
+ "course/2012_Fall": {
+ "graceperiod": "2 days 5 hours 59 minutes 59 seconds",
+ "start": "2015-07-17T12:00",
+ "display_name": "Toy Course",
+ "graded": "true",
+ }
+}
diff --git a/common/test/data/xml_dag/static/just_a_test.jpg b/common/test/data/xml_dag/static/just_a_test.jpg
new file mode 100644
index 0000000000..6bb7f377a0
Binary files /dev/null and b/common/test/data/xml_dag/static/just_a_test.jpg differ
diff --git a/common/test/data/xml_dag/vertical/vertical_test.xml b/common/test/data/xml_dag/vertical/vertical_test.xml
new file mode 100644
index 0000000000..dbb00bed9d
--- /dev/null
+++ b/common/test/data/xml_dag/vertical/vertical_test.xml
@@ -0,0 +1,4 @@
+