From cccb5cd7b673d7c9af701aadbd2e998304176e51 Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Fri, 11 Jul 2014 11:30:31 -0400 Subject: [PATCH] Defensively checks that children exist in has_changes --- .../lib/xmodule/xmodule/modulestore/mongo/draft.py | 6 +++++- .../xmodule/modulestore/tests/test_mongo.py | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index d4a5b197f7..13a8eb7a9d 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -591,7 +591,11 @@ class DraftModuleStore(MongoModuleStore): :return: True if the draft and published versions differ """ - item = self.get_item(location) + try: + item = self.get_item(location) + # defensively check that the parent's child actually exists + except ItemNotFoundError: + return False # don't check children if this block has changes (is not public) if self.compute_publish_state(item) != PublishState.public: diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index e490b404a2..b95e58e339 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -534,6 +534,20 @@ class TestMongoModuleStore(unittest.TestCase): self.draft_store.publish(location, self.dummy_user) self.assertFalse(self.draft_store.has_changes(location)) + def test_has_changes_missing_child(self): + """ + Tests that has_changes() returns False when a published parent points to a child that doesn't exist. + """ + location = Location('edX', 'missing', '2012_Fall', 'sequential', 'parent') + + # Create the parent and point it to a fake child + parent = self.draft_store.create_and_save_xmodule(location, user_id=self.dummy_user) + parent.children += [Location('edX', 'missing', '2012_Fall', 'vertical', 'does_not_exist')] + self.draft_store.update_item(parent, self.dummy_user) + + # Check the parent for changes should return False and not throw an exception + self.assertFalse(self.draft_store.has_changes(location)) + def _create_test_tree(self, name, user_id=None): """ Creates and returns a tree with the following structure: