From 0bede12821f5cf4a7254d27dcf3f036fadbd2450 Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Mon, 13 Mar 2017 19:00:17 +0500 Subject: [PATCH] Fix multiple copies of components on discard changing a move operation - TNL-6670 --- cms/djangoapps/contentstore/views/item.py | 18 +- .../contentstore/views/tests/test_item.py | 87 +++- .../modulestore/draft_and_published.py | 59 +++ .../xmodule/modulestore/mongo/draft.py | 31 ++ .../modulestore/split_mongo/split_draft.py | 20 + .../tests/test_mixed_modulestore.py | 372 ++++++++++++++++++ 6 files changed, 577 insertions(+), 10 deletions(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 5e6eeac1ac..3b32c99085 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -733,8 +733,8 @@ def _move_item(source_usage_key, target_parent_usage_key, user, target_index=Non source_type=source_type, target_parent_type=target_parent_type, ) - elif source_parent.location == target_parent.location: - error = _('You can not move an item into the same parent.') + elif source_parent.location == target_parent.location or source_item.location in target_parent.children: + error = _('Item is already present in target location.') elif source_item.location == target_parent.location: error = _('You can not move an item into itself.') elif is_source_item_in_target_parents(source_item, target_parent): @@ -761,16 +761,16 @@ def _move_item(source_usage_key, target_parent_usage_key, user, target_index=Non if error: return JsonResponse({'error': error}, status=400) - # Remove reference from old parent. - source_parent.children.remove(source_item.location) - store.update_item(source_parent, user.id) - # When target_index is provided, insert xblock at target_index position, otherwise insert at the end. insert_at = target_index if target_index is not None else len(target_parent.children) - # Add to new parent at particular location. - target_parent.children.insert(insert_at, source_item.location) - store.update_item(target_parent, user.id) + store.update_item_parent( + item_location=source_item.location, + new_parent_location=target_parent.location, + old_parent_location=source_parent.location, + insert_at=insert_at, + user_id=user.id + ) log.info( 'MOVE: %s moved from %s to %s at %d index', diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 1bf35d1d73..d50c5c19d8 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -31,6 +31,7 @@ from xblock_django.models import XBlockConfiguration, XBlockStudioConfiguration, from xmodule.capa_module import CapaDescriptor from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore +from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_SPLIT_MODULESTORE from xmodule.modulestore.tests.factories import ItemFactory, LibraryFactory, check_mongo_calls, CourseFactory from xmodule.x_module import STUDIO_VIEW, STUDENT_VIEW @@ -878,10 +879,20 @@ class TestMoveItem(ItemTest): self.assertEqual(response['move_source_locator'], unicode(source_usage_key)) self.assertEqual(response['parent_locator'], unicode(target_usage_key)) self.assertEqual(response['source_index'], expected_index) + + # Verify parent referance has been changed now. new_parent_loc = self.store.get_parent_location(source_usage_key) + source_item = self.get_item_from_modulestore(source_usage_key) + self.assertEqual(source_item.parent, new_parent_loc) self.assertEqual(new_parent_loc, target_usage_key) self.assertNotEqual(parent_loc, new_parent_loc) + # Assert item is present in children list of target parent and not source parent + target_parent = self.get_item_from_modulestore(target_usage_key) + source_parent = self.get_item_from_modulestore(parent_loc) + self.assertIn(source_usage_key, target_parent.children) + self.assertNotIn(source_usage_key, source_parent.children) + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_move_component(self, store_type): """ @@ -988,7 +999,7 @@ class TestMoveItem(ItemTest): self.assertEqual(response.status_code, 400) response = json.loads(response.content) - self.assertEqual(response['error'], 'You can not move an item into the same parent.') + self.assertEqual(response['error'], 'Item is already present in target location.') self.assertEqual(self.store.get_parent_location(self.html_usage_key), parent_loc) def test_can_not_move_into_itself(self): @@ -1161,6 +1172,80 @@ class TestMoveItem(ItemTest): insert_at ) + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_move_and_discard_changes(self, store_type): + """ + Verifies that discard changes operation brings moved component back to source location and removes the component + from target location. + + Arguments: + store_type (ModuleStoreEnum.Type): Type of modulestore to create test course in. + """ + self.setup_course(default_store=store_type) + + old_parent_loc = self.store.get_parent_location(self.html_usage_key) + + # Check that old_parent_loc is not yet published. + self.assertFalse(self.store.has_item(old_parent_loc, revision=ModuleStoreEnum.RevisionOption.published_only)) + + # Publish old_parent_loc unit + self.client.ajax_post( + reverse_usage_url("xblock_handler", old_parent_loc), + data={'publish': 'make_public'} + ) + + # Check that old_parent_loc is now published. + self.assertTrue(self.store.has_item(old_parent_loc, revision=ModuleStoreEnum.RevisionOption.published_only)) + self.assertFalse(self.store.has_changes(self.store.get_item(old_parent_loc))) + + # Move component html_usage_key in vert2_usage_key + self.assert_move_item(self.html_usage_key, self.vert2_usage_key) + + # Check old_parent_loc becomes in draft mode now. + self.assertTrue(self.store.has_changes(self.store.get_item(old_parent_loc))) + + # Now discard changes in old_parent_loc + self.client.ajax_post( + reverse_usage_url("xblock_handler", old_parent_loc), + data={'publish': 'discard_changes'} + ) + + # Check that old_parent_loc now is reverted to publish. Changes discarded, html_usage_key moved back. + self.assertTrue(self.store.has_item(old_parent_loc, revision=ModuleStoreEnum.RevisionOption.published_only)) + self.assertFalse(self.store.has_changes(self.store.get_item(old_parent_loc))) + + # Now source item should be back in the old parent. + source_item = self.get_item_from_modulestore(self.html_usage_key) + self.assertEqual(source_item.parent, old_parent_loc) + self.assertEqual(self.store.get_parent_location(self.html_usage_key), source_item.parent) + + # Also, check that item is not present in target parent but in source parent + target_parent = self.get_item_from_modulestore(self.vert2_usage_key) + source_parent = self.get_item_from_modulestore(old_parent_loc) + self.assertIn(self.html_usage_key, source_parent.children) + self.assertNotIn(self.html_usage_key, target_parent.children) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_move_item_not_found(self, store_type=ModuleStoreEnum.Type.mongo): + """ + Test that an item not found exception raised when an item is not found when getting the item. + + Arguments: + store_type (ModuleStoreEnum.Type): Type of modulestore to create test course in. + """ + self.setup_course(default_store=store_type) + + data = { + 'move_source_locator': unicode(self.usage_key.course_key.make_usage_key('html', 'html_test')), + 'parent_locator': unicode(self.vert2_usage_key) + } + with self.assertRaises(ItemNotFoundError): + self.client.patch( + reverse('contentstore.views.xblock_handler'), + json.dumps(data), + content_type='application/json' + ) + class TestDuplicateItemWithAsides(ItemTest, DuplicateHelper): """ diff --git a/common/lib/xmodule/xmodule/modulestore/draft_and_published.py b/common/lib/xmodule/xmodule/modulestore/draft_and_published.py index 79f567e66b..82701418e7 100644 --- a/common/lib/xmodule/xmodule/modulestore/draft_and_published.py +++ b/common/lib/xmodule/xmodule/modulestore/draft_and_published.py @@ -3,13 +3,17 @@ This module provides an abstraction for Module Stores that support Draft and Pub """ import threading +import logging from abc import ABCMeta, abstractmethod from contextlib import contextmanager from . import ModuleStoreEnum, BulkOperationsMixin +from .exceptions import ItemNotFoundError # Things w/ these categories should never be marked as version=DRAFT DIRECT_ONLY_CATEGORIES = ['course', 'chapter', 'sequential', 'about', 'static_tab', 'course_info'] +log = logging.getLogger(__name__) + class BranchSettingMixin(object): """ @@ -134,6 +138,61 @@ class ModuleStoreDraftAndPublished(BranchSettingMixin, BulkOperationsMixin): # We remove the branch, because publishing always means copying from draft to published self.signal_handler.send("course_published", course_key=course_key.for_branch(None)) + def update_item_parent(self, item_location, new_parent_location, old_parent_location, user_id, insert_at=None): + """ + Updates item's parent and removes it's reference from old parent. + + Arguments: + item_location (BlockUsageLocator) : Locator of item. + new_parent_location (BlockUsageLocator) : New parent block locator. + old_parent_location (BlockUsageLocator) : Old parent block locator. + user_id (int) : User id. + insert_at (int) : Insert item at the particular index in new parent. + + Returns: + BlockUsageLocator or None: Source item location if updated, otherwise None. + """ + try: + source_item = self.get_item(item_location) # pylint: disable=no-member + old_parent_item = self.get_item(old_parent_location) # pylint: disable=no-member + new_parent_item = self.get_item(new_parent_location) # pylint: disable=no-member + except ItemNotFoundError as exception: + log.error('Unable to find the item : %s', exception.message) + return + + # Remove item from the list of children of old parent. + if source_item.location in old_parent_item.children: + old_parent_item.children.remove(source_item.location) + self.update_item(old_parent_item, user_id) # pylint: disable=no-member + log.info( + '%s removed from %s children', + unicode(source_item.location), + unicode(old_parent_item.location) + ) + + # Add item to new parent at particular location. + if source_item.location not in new_parent_item.children: + if insert_at is not None: + new_parent_item.children.insert(insert_at, source_item.location) + else: + new_parent_item.children.append(source_item.location) + self.update_item(new_parent_item, user_id) # pylint: disable=no-member + log.info( + '%s added to %s children', + unicode(source_item.location), + unicode(new_parent_item.location) + ) + + # Update parent attribute of the item block + source_item.parent = new_parent_location + self.update_item(source_item, user_id) # pylint: disable=no-member + log.info( + '%s parent updated to %s', + unicode(source_item.location), + unicode(new_parent_item.location) + ) + return source_item.location + class UnsupportedRevisionError(ValueError): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index d1ee46412a..c7585a6ecc 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -796,6 +796,15 @@ class DraftModuleStore(MongoModuleStore): # If 2 versions versions exist, we can assume one is a published version. Go ahead and do the delete # of the draft version. if versions_found.count() > 1: + # Moving a child from published parent creates a draft of the parent and moved child. + published_version = [ + version + for version in versions_found + if version.get('_id').get('revision') != MongoRevisionKey.draft + ] + if len(published_version) > 0: + # This change makes sure that parents are updated too i.e. an item will have only one parent. + self.update_parent_if_moved(root_location, published_version[0], delete_draft_only, user_id) self._delete_subtree(root_location, [as_draft], draft_only=True) elif versions_found.count() == 1: # Since this method cannot be called on something in DIRECT_ONLY_CATEGORIES and we call @@ -809,6 +818,28 @@ class DraftModuleStore(MongoModuleStore): delete_draft_only(location) + def update_parent_if_moved(self, original_parent_location, published_version, delete_draft_only, user_id): + """ + Update parent of an item if it has moved. + + Arguments: + original_parent_location (BlockUsageLocator) : Original parent block locator. + published_version (dict) : Published version of the block. + delete_draft_only (function) : A callback function to delete draft children if it was moved. + user_id (int) : User id + """ + for child_location in published_version.get('definition', {}).get('children', []): + item_location = original_parent_location.course_key.make_usage_key_from_deprecated_string(child_location) + try: + source_item = self.get_item(item_location) + except ItemNotFoundError: + log.error('Unable to find the item %s', unicode(item_location)) + return + + if source_item.parent and source_item.parent.block_id != original_parent_location.block_id: + if self.update_item_parent(item_location, original_parent_location, source_item.parent, user_id): + delete_draft_only(Location.from_deprecated_string(child_location)) + def _query_children_for_cache_children(self, course_key, items): # first get non-draft in a round-trip to_process_non_drafts = super(DraftModuleStore, self)._query_children_for_cache_children(course_key, items) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py index 863bd28d53..2f2b45d3ea 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -443,7 +443,10 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli self._get_block_from_structure(published_course_structure, root_block_id) ) block = self._get_block_from_structure(new_structure, root_block_id) + original_parent_location = location.course_key.make_usage_key(root_block_id.type, root_block_id.id) for child_block_id in block.fields.get('children', []): + item_location = location.course_key.make_usage_key(child_block_id.type, child_block_id.id) + self.update_parent_if_moved(item_location, original_parent_location, new_structure, user_id) copy_from_published(child_block_id) copy_from_published(BlockKey.from_usage_key(location)) @@ -454,6 +457,23 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli if index_entry is not None: self._update_head(draft_course_key, index_entry, ModuleStoreEnum.BranchName.draft, new_structure['_id']) + def update_parent_if_moved(self, item_location, original_parent_location, course_structure, user_id): + """ + Update parent of an item if it has moved. + + Arguments: + item_location (BlockUsageLocator) : Locator of item. + original_parent_location (BlockUsageLocator) : Original parent block locator. + course_structure (dict) : course structure of the course. + user_id (int) : User id + """ + parent_block_keys = self._get_parents_from_structure(BlockKey.from_usage_key(item_location), course_structure) + for block_key in parent_block_keys: + # Item's parent is different than its new parent - so it has moved. + if block_key.id != original_parent_location.block_id: + old_parent_location = original_parent_location.course_key.make_usage_key(block_key.type, block_key.id) + self.update_item_parent(item_location, original_parent_location, old_parent_location, user_id) + def force_publish_course(self, course_locator, user_id, commit=False): """ Helper method to forcefully publish a course, 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 feb46f257f..59917387f5 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -1143,6 +1143,378 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.store.get_parent_location(child_location, revision=revision) ) + def verify_item_parent(self, item_location, expected_parent_location, old_parent_location, is_reverted=False): + """ + Verifies that item is placed under expected parent. + + Arguments: + item_location (BlockUsageLocator) : Locator of item. + expected_parent_location (BlockUsageLocator) : Expected parent block locator. + old_parent_location (BlockUsageLocator) : Old parent block locator. + is_reverted (Boolean) : A flag to notify that item was reverted. + """ + with self.store.bulk_operations(self.course.id): + source_item = self.store.get_item(item_location) + old_parent = self.store.get_item(old_parent_location) + expected_parent = self.store.get_item(expected_parent_location) + + self.assertEqual(expected_parent_location, source_item.get_parent().location) + + # If an item is reverted, it means it's actual parent was the one that is the current parent now + # i.e expected_parent_location otherwise old_parent_location. + published_parent_location = expected_parent_location if is_reverted else old_parent_location + + # Check parent locations wrt branches + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + self.assertEqual(expected_parent_location, self.store.get_item(item_location).get_parent().location) + with self.store.branch_setting(ModuleStoreEnum.Branch.published_only): + self.assertEqual(published_parent_location, self.store.get_item(item_location).get_parent().location) + + # Make location specific to published branch for verify_get_parent_locations_results call. + published_parent_location = published_parent_location.for_branch(ModuleStoreEnum.BranchName.published) + + # Verify expected item parent locations + self.verify_get_parent_locations_results([ + (item_location, expected_parent_location, None), + (item_location, expected_parent_location, ModuleStoreEnum.RevisionOption.draft_preferred), + (item_location, published_parent_location, ModuleStoreEnum.RevisionOption.published_only), + ]) + + # Also verify item.parent has correct parent location set. + self.assertEqual(source_item.parent, expected_parent_location) + self.assertEqual(source_item.parent, self.store.get_parent_location(item_location)) + + # Item should be present in new parent's children list but not in old parent's children list. + self.assertIn(item_location, expected_parent.children) + self.assertNotIn(item_location, old_parent.children) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_update_item_parent(self, store_type): + """ + Test that when we move an item from old to new parent, the item should be present in new parent. + """ + self.initdb(store_type) + self._create_block_hierarchy() + + # Publish the course. + self.course = self.store.publish(self.course.location, self.user_id) + + # Move child problem_x1a_1 to vertical_y1a. + item_location = self.problem_x1a_1 + new_parent_location = self.vertical_y1a + old_parent_location = self.vertical_x1a + updated_item_location = self.store.update_item_parent( + item_location, new_parent_location, old_parent_location, self.user_id + ) + self.assertEqual(updated_item_location, item_location) + + self.verify_item_parent( + item_location=item_location, + expected_parent_location=new_parent_location, + old_parent_location=old_parent_location + ) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_move_revert(self, store_type): + """ + Test that when we move an item to new parent and then discard the original parent, the item should be present + back in original parent. + """ + self.initdb(store_type) + self._create_block_hierarchy() + + # Publish the course + self.course = self.store.publish(self.course.location, self.user_id) + + # Move child problem_x1a_1 to vertical_y1a. + item_location = self.problem_x1a_1 + new_parent_location = self.vertical_y1a + old_parent_location = self.vertical_x1a + updated_item_location = self.store.update_item_parent( + item_location, new_parent_location, old_parent_location, self.user_id + ) + self.assertEqual(updated_item_location, item_location) + + self.verify_item_parent( + item_location=item_location, + expected_parent_location=new_parent_location, + old_parent_location=old_parent_location + ) + + # Now discard changes in old_parent_location i.e original parent. + self.store.revert_to_published(old_parent_location, self.user_id) + + self.verify_item_parent( + item_location=item_location, + expected_parent_location=old_parent_location, + old_parent_location=new_parent_location, + is_reverted=True + ) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_move_delete_revert(self, store_type): + """ + Test that when we move an item and delete it and then discard changes for original parent, item should be + present back in original parent. + """ + self.initdb(store_type) + self._create_block_hierarchy() + + # Publish the course + self.course = self.store.publish(self.course.location, self.user_id) + + # Move child problem_x1a_1 to vertical_y1a. + item_location = self.problem_x1a_1 + new_parent_location = self.vertical_y1a + old_parent_location = self.vertical_x1a + updated_item_location = self.store.update_item_parent( + item_location, new_parent_location, old_parent_location, self.user_id + ) + self.assertEqual(updated_item_location, item_location) + + self.verify_item_parent( + item_location=item_location, + expected_parent_location=new_parent_location, + old_parent_location=old_parent_location + ) + + # Now delete the item. + self.store.delete_item(item_location, self.user_id) + + # Now discard changes in old_parent_location i.e original parent. + self.store.revert_to_published(old_parent_location, self.user_id) + + self.verify_item_parent( + item_location=item_location, + expected_parent_location=old_parent_location, + old_parent_location=new_parent_location, + is_reverted=True + ) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_move_revert_move(self, store_type): + """ + Test that when we move an item to new parent and discard changes for the old parent, then the item should be + present in the old parent and then moving an item from old parent to new parent should place that item under + new parent. + """ + self.initdb(store_type) + self._create_block_hierarchy() + + # Publish the course + self.course = self.store.publish(self.course.location, self.user_id) + + # Move child problem_x1a_1 to vertical_y1a. + item_location = self.problem_x1a_1 + new_parent_location = self.vertical_y1a + old_parent_location = self.vertical_x1a + updated_item_location = self.store.update_item_parent( + item_location, new_parent_location, old_parent_location, self.user_id + ) + self.assertEqual(updated_item_location, item_location) + + self.verify_item_parent( + item_location=item_location, + expected_parent_location=new_parent_location, + old_parent_location=old_parent_location + ) + + # Now discard changes in old_parent_location i.e original parent. + self.store.revert_to_published(old_parent_location, self.user_id) + + self.verify_item_parent( + item_location=item_location, + expected_parent_location=old_parent_location, + old_parent_location=new_parent_location, + is_reverted=True + ) + + # Again try to move from x1 to y1 + updated_item_location = self.store.update_item_parent( + item_location, new_parent_location, old_parent_location, self.user_id + ) + self.assertEqual(updated_item_location, item_location) + + self.verify_item_parent( + item_location=item_location, + expected_parent_location=new_parent_location, + old_parent_location=old_parent_location + ) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_move_edited_revert(self, store_type): + """ + Test that when we move an edited item from old parent to new parent and then discard changes in old parent, + item should be placed under original parent with initial state. + """ + self.initdb(store_type) + self._create_block_hierarchy() + + # Publish the course. + self.course = self.store.publish(self.course.location, self.user_id) + + # Move child problem_x1a_1 to vertical_y1a. + item_location = self.problem_x1a_1 + new_parent_location = self.vertical_y1a + old_parent_location = self.vertical_x1a + + problem = self.store.get_item(self.problem_x1a_1) + orig_display_name = problem.display_name + + # Change display name of problem and update just it. + problem.display_name = 'updated' + self.store.update_item(problem, self.user_id) + + updated_problem = self.store.get_item(self.problem_x1a_1) + self.assertEqual(updated_problem.display_name, 'updated') + + # Now, move from x1 to y1. + updated_item_location = self.store.update_item_parent( + item_location, new_parent_location, old_parent_location, self.user_id + ) + self.assertEqual(updated_item_location, item_location) + + self.verify_item_parent( + item_location=item_location, + expected_parent_location=new_parent_location, + old_parent_location=old_parent_location + ) + + # Now discard changes in old_parent_location i.e original parent. + self.store.revert_to_published(old_parent_location, self.user_id) + + # Check that problem has the original name back. + reverted_problem = self.store.get_item(self.problem_x1a_1) + self.assertEqual(orig_display_name, reverted_problem.display_name) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_move_1_moved_1_unchanged(self, store_type): + """ + Test that when we move an item from an old parent which have multiple items then only moved item's parent + is changed while other items are still present inside old parent. + """ + self.initdb(store_type) + self._create_block_hierarchy() + + # Create some children in vertical_x1a + problem_item2 = self.store.create_child(self.user_id, self.vertical_x1a, 'problem', 'Problem_Item2') + + # Publish the course. + self.course = self.store.publish(self.course.location, self.user_id) + + item_location = self.problem_x1a_1 + new_parent_location = self.vertical_y1a + old_parent_location = self.vertical_x1a + + # Move problem_x1a_1 from x1 to y1. + updated_item_location = self.store.update_item_parent( + item_location, new_parent_location, old_parent_location, self.user_id + ) + self.assertEqual(updated_item_location, item_location) + + self.verify_item_parent( + item_location=item_location, + expected_parent_location=new_parent_location, + old_parent_location=old_parent_location + ) + + # Check that problem_item2 is still present in vertical_x1a + problem_item2 = self.store.get_item(problem_item2.location) + self.assertEqual(problem_item2.parent, self.vertical_x1a) + self.assertIn(problem_item2.location, problem_item2.get_parent().children) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_move_1_moved_1_edited(self, store_type): + """ + Test that when we move an item inside an old parent having multiple items, we edit one item and move + other item from old to new parent, then discard changes in old parent would discard the changes of the + edited item and move back the moved item to old location. + """ + self.initdb(store_type) + self._create_block_hierarchy() + + # Create some children in vertical_x1a + problem_item2 = self.store.create_child(self.user_id, self.vertical_x1a, 'problem', 'Problem_Item2') + orig_display_name = problem_item2.display_name + + # Publish the course. + self.course = self.store.publish(self.course.location, self.user_id) + + # Edit problem_item2. + problem_item2.display_name = 'updated' + self.store.update_item(problem_item2, self.user_id) + + updated_problem2 = self.store.get_item(problem_item2.location) + self.assertEqual(updated_problem2.display_name, 'updated') + + item_location = self.problem_x1a_1 + new_parent_location = self.vertical_y1a + old_parent_location = self.vertical_x1a + + # Move problem_x1a_1 from x1 to y1. + updated_item_location = self.store.update_item_parent( + item_location, new_parent_location, old_parent_location, self.user_id + ) + self.assertEqual(updated_item_location, item_location) + + self.verify_item_parent( + item_location=item_location, + expected_parent_location=new_parent_location, + old_parent_location=old_parent_location + ) + + # Now discard changes in old_parent_location i.e original parent. + self.store.revert_to_published(old_parent_location, self.user_id) + + # Check that problem_item2 has the original name back. + reverted_problem2 = self.store.get_item(problem_item2.location) + self.assertEqual(orig_display_name, reverted_problem2.display_name) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_move_1_moved_1_deleted(self, store_type): + """ + Test that when we move an item inside an old parent having multiple items, we delete one item and move + other item from old to new parent, then discard changes in old parent would undo delete the deleted + item and move back the moved item to old location. + """ + self.initdb(store_type) + self._create_block_hierarchy() + + # Create some children in vertical_x1a + problem_item2 = self.store.create_child(self.user_id, self.vertical_x1a, 'problem', 'Problem_Item2') + orig_display_name = problem_item2.display_name + + # Publish the course. + self.course = self.store.publish(self.course.location, self.user_id) + + # Now delete other problem problem_item2. + self.store.delete_item(problem_item2.location, self.user_id) + + # Move child problem_x1a_1 to vertical_y1a. + item_location = self.problem_x1a_1 + new_parent_location = self.vertical_y1a + old_parent_location = self.vertical_x1a + + # Move problem_x1a_1 from x1 to y1. + updated_item_location = self.store.update_item_parent( + item_location, new_parent_location, old_parent_location, self.user_id + ) + self.assertEqual(updated_item_location, item_location) + + self.verify_item_parent( + item_location=item_location, + expected_parent_location=new_parent_location, + old_parent_location=old_parent_location + ) + + # Now discard changes in old_parent_location i.e original parent. + self.store.revert_to_published(old_parent_location, self.user_id) + + # Check that problem_item2 is also back in vertical_x1a + problem_item2 = self.store.get_item(problem_item2.location) + self.assertEqual(problem_item2.parent, self.vertical_x1a) + self.assertIn(problem_item2.location, problem_item2.get_parent().children) + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_get_parent_locations_moved_child(self, default_ms): self.initdb(default_ms)