Merge pull request #4302 from edx/benmcmorran/fix-has-changes

Fixes has_changes to search all the way to leaf nodes

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

@@ -498,15 +498,16 @@ class DraftModuleStore(MongoModuleStore):
         :return: True if the draft and published versions differ
         """
 
-        # a non direct only xblock has changes if it is in a non public state
-        if location.category not in DIRECT_ONLY_CATEGORIES:
-            return self.compute_publish_state(self.get_item(location)) != PublishState.public
+        item = self.get_item(location)
+
+        # don't check children if this block has changes (is not public)
+        if self.compute_publish_state(item) != PublishState.public:
+            return True
+        # if this block doesn't have changes, then check its children
+        elif item.has_children:
+            return any([self.has_changes(child) for child in item.children])
+        # otherwise there are no changes
         else:
-            item = self.get_item(location)
-            if item.has_children:
-                for child in item.children:
-                    if self.has_changes(child):
-                        return True
             return False
 
     def publish(self, location, user_id):

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

@@ -661,6 +661,32 @@ class TestMongoModuleStore(unittest.TestCase):
         self.assertFalse(self.draft_store.has_changes(locations['grandparent']))
         self.assertFalse(self.draft_store.has_changes(locations['parent']))
 
+    def test_has_changes_non_direct_only_children(self):
+        """
+        Tests that has_changes() returns true after editing the child of a vertical (both not direct only categories).
+        """
+        dummy_user = 123
+        parent_location = Location('edX', 'test', 'non_direct_only_children', 'vertical', 'parent')
+        child_location = Location('edX', 'test', 'non_direct_only_children', 'html', 'child')
+
+        parent = self.draft_store.create_and_save_xmodule(parent_location, user_id=dummy_user)
+        child = self.draft_store.create_and_save_xmodule(child_location, user_id=dummy_user)
+        parent.children += [child_location]
+        self.draft_store.update_item(parent, user_id=dummy_user)
+        self.draft_store.publish(parent_location, dummy_user)
+
+        # Verify that there are no changes
+        self.assertFalse(self.draft_store.has_changes(parent_location))
+        self.assertFalse(self.draft_store.has_changes(child_location))
+
+        # Change the child
+        child.display_name = 'Changed Display Name'
+        self.draft_store.update_item(child, user_id=123)
+
+        # Verify that both parent and child have changes
+        self.assertTrue(self.draft_store.has_changes(parent_location))
+        self.assertTrue(self.draft_store.has_changes(child_location))
+
     def test_update_edit_info_ancestors(self):
         """
         Tests that edited_on, edited_by, subtree_edited_on, and subtree_edited_by are set correctly during update