From ce71a940d943daf89a6bdc8084d19c8e430fe935 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Tue, 23 Dec 2014 15:36:33 -0500 Subject: [PATCH] Test for get_parent across branches --- .../xmodule/xmodule/modulestore/mongo/base.py | 9 ++-- .../tests/test_mixed_modulestore.py | 50 +++++++++++-------- 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 15022f6bec..85564417b1 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -627,11 +627,9 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo def _compute_metadata_inheritance_tree(self, course_id): ''' - TODO (cdodge) This method can be deleted when the 'split module store' work has been completed + Find all inheritable fields from all xblocks in the course which may define inheritable data ''' # get all collections in the course, this query should not return any leaf nodes - # note this is a bit ugly as when we add new categories of containers, we have to add it here - course_id = self.fill_in_run(course_id) query = SON([ ('_id.tag', 'i4x'), @@ -639,6 +637,9 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo ('_id.course', course_id.course), ('_id.category', {'$in': BLOCK_TYPES_WITH_CHILDREN}) ]) + # if we're only dealing in the published branch, then only get published containers + if self.get_branch_setting() == ModuleStoreEnum.Branch.published_only: + query['_id.revision'] = None # we just want the Location, children, and inheritable metadata record_filter = {'_id': 1, 'definition.children': 1} @@ -663,6 +664,8 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo location_url = unicode(location) if location_url in results_by_url: # found either draft or live to complement the other revision + # FIXME this is wrong. If the child was moved in draft from one parent to the other, it will + # show up under both in this logic: https://openedx.atlassian.net/browse/TNL-1075 existing_children = results_by_url[location_url].get('definition', {}).get('children', []) additional_children = result.get('definition', {}).get('children', []) total_children = existing_children + additional_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 5c0e23ea46..b0ec1dc447 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -5,7 +5,6 @@ Unit tests for the Mixed Modulestore, with DDT for the various stores (Split, Dr from collections import namedtuple import datetime import ddt -from importlib import import_module import itertools import mimetypes from uuid import uuid4 @@ -33,7 +32,7 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator from xmodule.exceptions import InvalidVersionError from xmodule.modulestore import ModuleStoreEnum -from xmodule.modulestore.draft_and_published import UnsupportedRevisionError, ModuleStoreDraftAndPublished +from xmodule.modulestore.draft_and_published import UnsupportedRevisionError from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError, ReferentialIntegrityError, NoPathToItem from xmodule.modulestore.mixed import MixedModuleStore from xmodule.modulestore.search import path_to_location @@ -920,28 +919,39 @@ class TestMixedModuleStore(CourseComparisonTest): # publish the course self.course = self.store.publish(self.course.location, self.user_id) - # make drafts of verticals - self.store.convert_to_draft(self.vertical_x1a, self.user_id) - self.store.convert_to_draft(self.vertical_y1a, self.user_id) + with self.store.bulk_operations(self.course.id): + # make drafts of verticals + self.store.convert_to_draft(self.vertical_x1a, self.user_id) + self.store.convert_to_draft(self.vertical_y1a, self.user_id) - # move child problem_x1a_1 to vertical_y1a - child_to_move_location = self.problem_x1a_1 - new_parent_location = self.vertical_y1a - old_parent_location = self.vertical_x1a + # move child problem_x1a_1 to vertical_y1a + child_to_move_location = self.problem_x1a_1 + new_parent_location = self.vertical_y1a + old_parent_location = self.vertical_x1a - old_parent = self.store.get_item(old_parent_location) - old_parent.children.remove(child_to_move_location.replace(version_guid=old_parent.location.version_guid)) - self.store.update_item(old_parent, self.user_id) + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + old_parent = self.store.get_item(child_to_move_location).get_parent() - new_parent = self.store.get_item(new_parent_location) - new_parent.children.append(child_to_move_location.replace(version_guid=new_parent.location.version_guid)) - self.store.update_item(new_parent, self.user_id) + self.assertEqual(old_parent_location, old_parent.location) - self.verify_get_parent_locations_results([ - (child_to_move_location, new_parent_location, None), - (child_to_move_location, new_parent_location, ModuleStoreEnum.RevisionOption.draft_preferred), - (child_to_move_location, old_parent_location.for_branch(ModuleStoreEnum.BranchName.published), ModuleStoreEnum.RevisionOption.published_only), - ]) + child_to_move_contextualized = child_to_move_location.map_into_course(old_parent.location.course_key) + old_parent.children.remove(child_to_move_contextualized) + self.store.update_item(old_parent, self.user_id) + + new_parent = self.store.get_item(new_parent_location) + new_parent.children.append(child_to_move_location) + self.store.update_item(new_parent, self.user_id) + + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + self.assertEqual(new_parent_location, self.store.get_item(child_to_move_location).get_parent().location) + with self.store.branch_setting(ModuleStoreEnum.Branch.published_only): + self.assertEqual(old_parent_location, self.store.get_item(child_to_move_location).get_parent().location) + + self.verify_get_parent_locations_results([ + (child_to_move_location, new_parent_location, None), + (child_to_move_location, new_parent_location, ModuleStoreEnum.RevisionOption.draft_preferred), + (child_to_move_location, old_parent_location.for_branch(ModuleStoreEnum.BranchName.published), ModuleStoreEnum.RevisionOption.published_only), + ]) # publish the course again self.store.publish(self.course.location, self.user_id)