Merge pull request #8680 from edx/hotfix/2015-06-25

fix the source_version xblock after discard changes

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', []):

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.