diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 1dedc2f394..b8e425eb27 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -1875,6 +1875,11 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): block_data.fields = settings new_id = new_structure['_id'] + + # source_version records which revision a block was copied from. In this method, we're updating + # the block, so it's no longer a direct copy, and we can remove the source_version reference. + block_data.edit_info.source_version = None + self.version_block(block_data, user_id, new_id) self.update_structure(course_key, new_structure) # update the index entry if appropriate @@ -2969,8 +2974,11 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): if getattr(destination_block.edit_info, key) is None: setattr(destination_block.edit_info, key, val) - # introduce new edit info field for tracing where copied/published blocks came - destination_block.edit_info.source_version = new_block.edit_info.update_version + # If the block we are copying from was itself a copy, then just + # reference the original source, rather than the copy. + destination_block.edit_info.source_version = ( + new_block.edit_info.source_version or new_block.edit_info.update_version + ) if blacklist != EXCLUDE_ALL: for child in destination_block.fields.get('children', []): 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 e83c9be516..c490b0f614 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -523,6 +523,75 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): component = self.store.publish(component.location, self.user_id) self.assertFalse(self.store.has_changes(component)) + @ddt.data('draft', 'split') + def test_unit_stuck_in_draft_mode(self, default_ms): + """ + After revert_to_published() the has_changes() should return false if draft has no changes + """ + self.initdb(default_ms) + + test_course = self.store.create_course('testx', 'GreekHero', 'test_run', self.user_id) + + # Create a dummy component to test against + xblock = self.store.create_item( + self.user_id, + test_course.id, + 'vertical', + block_id='test_vertical' + ) + + # Not yet published, so changes are present + self.assertTrue(self.store.has_changes(xblock)) + + # Publish and verify that there are no unpublished changes + component = self.store.publish(xblock.location, self.user_id) + self.assertFalse(self.store.has_changes(component)) + + self.store.revert_to_published(component.location, self.user_id) + component = self.store.get_item(component.location) + self.assertFalse(self.store.has_changes(component)) + + # Publish and verify again + component = self.store.publish(component.location, self.user_id) + self.assertFalse(self.store.has_changes(component)) + + @ddt.data('draft', 'split') + def test_unit_stuck_in_published_mode(self, default_ms): + """ + After revert_to_published() the has_changes() should return true if draft has changes + """ + self.initdb(default_ms) + + test_course = self.store.create_course('testx', 'GreekHero', 'test_run', self.user_id) + + # Create a dummy component to test against + xblock = self.store.create_item( + self.user_id, + test_course.id, + 'vertical', + block_id='test_vertical' + ) + + # Not yet published, so changes are present + self.assertTrue(self.store.has_changes(xblock)) + + # Publish and verify that there are no unpublished changes + component = self.store.publish(xblock.location, self.user_id) + self.assertFalse(self.store.has_changes(component)) + + # Discard changes and verify that there are no changes + self.store.revert_to_published(component.location, self.user_id) + component = self.store.get_item(component.location) + self.assertFalse(self.store.has_changes(component)) + + # Change the component, then check that there now are changes + component = self.store.get_item(component.location) + component.display_name = 'Changed Display Name' + self.store.update_item(component, self.user_id) + + # Verify that changes are present + self.assertTrue(self.store.has_changes(component)) + def setup_has_changes(self, default_ms): """ Common set up for has_changes tests below.