diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index 76ce0a6db4..93a5eac922 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -624,6 +624,9 @@ class DraftModuleStore(MongoModuleStore): return True # if this block doesn't have changes, then check its children elif xblock.has_children: + # fix a bug where dangling pointers should imply a change + if len(xblock.children) > len(xblock.get_children()): + return True return any([self.has_changes(child) for child in xblock.get_children()]) # otherwise there are no changes else: diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py index bb59da2a7d..11b23295a4 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -268,8 +268,10 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli def has_changes_subtree(block_key): draft_block = get_block(draft_course, block_key) + if draft_block is None: # temporary fix for bad pointers TNL-1141 + return True published_block = get_block(published_course, block_key) - if not published_block: + if published_block is None: return True # check if the draft has changed since the published was created 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 2b70cb7584..d055c8a602 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -663,25 +663,30 @@ class TestMixedModuleStore(CourseComparisonTest): self.assertTrue(self._has_changes(parent.location)) self.assertTrue(self._has_changes(child.location)) - @ddt.data('draft', 'split') - def test_has_changes_missing_child(self, default_ms): + @ddt.data(*itertools.product( + ('draft', 'split'), + (ModuleStoreEnum.Branch.draft_preferred, ModuleStoreEnum.Branch.published_only) + )) + @ddt.unpack + def test_has_changes_missing_child(self, default_ms, default_branch): """ Tests that has_changes() does not throw an exception when a child doesn't exist. """ self.initdb(default_ms) - # Create the parent and point it to a fake child - parent = self.store.create_item( - self.user_id, - self.course.id, - 'vertical', - block_id='parent', - ) - parent.children += [self.course.id.make_usage_key('vertical', 'does_not_exist')] - parent = self.store.update_item(parent, self.user_id) + with self.store.branch_setting(default_branch, self.course.id): + # Create the parent and point it to a fake child + parent = self.store.create_item( + self.user_id, + self.course.id, + 'vertical', + block_id='parent', + ) + parent.children += [self.course.id.make_usage_key('vertical', 'does_not_exist')] + parent = self.store.update_item(parent, self.user_id) - # Check the parent for changes should return True and not throw an exception - self.assertTrue(self.store.has_changes(parent)) + # Check the parent for changes should return True and not throw an exception + self.assertTrue(self.store.has_changes(parent)) # Draft # Find: find parents (definition.children query), get parent, get course (fill in run?),