diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index 019f67a594d9..eb9b109cdd35 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -15,8 +15,8 @@ from xmodule.x_module import STUDIO_VIEW from cms.djangoapps.contentstore.tests.utils import AjaxEnabledTestClient, parse_json -from cms.djangoapps.contentstore.utils import reverse_library_url, reverse_url, reverse_usage_url -from cms.djangoapps.contentstore.views.block import _duplicate_block +from cms.djangoapps.contentstore.utils import reverse_library_url, reverse_url, \ + reverse_usage_url, duplicate_block from cms.djangoapps.contentstore.views.preview import _load_preview_block from cms.djangoapps.contentstore.views.tests.test_library import LIBRARY_REST_URL from cms.djangoapps.course_creators.views import add_user_with_status_granted @@ -947,7 +947,7 @@ def test_persistent_overrides(self, duplicate): if duplicate: # Check that this also works when the RCB is duplicated. self.lc_block = modulestore().get_item( - _duplicate_block(self.course.location, self.lc_block.location, self.user) + duplicate_block(self.course.location, self.lc_block.location, self.user) ) self.problem_in_course = modulestore().get_item(self.lc_block.children[0]) else: @@ -1006,7 +1006,7 @@ def test_duplicated_version(self): # Duplicate self.lc_block: duplicate = store.get_item( - _duplicate_block(self.course.location, self.lc_block.location, self.user) + duplicate_block(self.course.location, self.lc_block.location, self.user) ) # The duplicate should have identical children to the original: self.assertEqual(len(duplicate.children), 1) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index daf5f8dd06fd..9c6049b3513c 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -5,20 +5,28 @@ from collections import defaultdict import logging from contextlib import contextmanager -from datetime import datetime +from datetime import datetime, timezone +from uuid import uuid4 from django.conf import settings from django.urls import reverse from django.utils import translation from django.utils.translation import gettext as _ +from lti_consumer.models import CourseAllowPIISharingInLTIFlag from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import LibraryLocator +from openedx_events.content_authoring.data import DuplicatedXBlockData +from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED from pytz import UTC +from xblock.fields import Scope from cms.djangoapps.contentstore.toggles import exam_setting_view_enabled +from common.djangoapps.edxmako.services import MakoService from common.djangoapps.student import auth +from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole +from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from openedx.core.djangoapps.course_apps.toggles import proctoring_settings_modal_view_enabled from openedx.core.djangoapps.discussions.config.waffle import ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration @@ -29,8 +37,6 @@ from openedx.features.content_type_gating.models import ContentTypeGatingConfig from openedx.features.content_type_gating.partitions import CONTENT_TYPE_GATING_SCHEME from cms.djangoapps.contentstore.toggles import ( - use_new_text_editor, - use_new_video_editor, use_new_advanced_settings_page, use_new_course_outline_page, use_new_export_page, @@ -43,10 +49,13 @@ use_new_updates_page, use_new_video_uploads_page, ) +from cms.djangoapps.contentstore.toggles import use_new_text_editor, use_new_video_editor +from xmodule.library_tools import LibraryToolsService from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order from xmodule.partitions.partitions_service import get_all_partitions_for_course # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.services import SettingsService, ConfigurationService, TeamsConfigurationService log = logging.getLogger(__name__) @@ -921,3 +930,202 @@ def update_course_discussions_settings(course_key): course = store.get_course(course_key) course.discussions_settings['provider_type'] = provider store.update_item(course, course.published_by) + + +def duplicate_block( + parent_usage_key, + duplicate_source_usage_key, + user, + dest_usage_key=None, + display_name=None, + shallow=False, + is_child=False +): + """ + Duplicate an existing xblock as a child of the supplied parent_usage_key. You can + optionally specify what usage key the new duplicate block will use via dest_usage_key. + + If shallow is True, does not copy children. Otherwise, this function calls itself + recursively, and will set the is_child flag to True when dealing with recursed child + blocks. + """ + store = modulestore() + with store.bulk_operations(duplicate_source_usage_key.course_key): + source_item = store.get_item(duplicate_source_usage_key) + if not dest_usage_key: + # Change the blockID to be unique. + dest_usage_key = source_item.location.replace(name=uuid4().hex) + + category = dest_usage_key.block_type + + duplicate_metadata, asides_to_create = gather_block_attributes( + source_item, display_name=display_name, is_child=is_child, + ) + + dest_block = store.create_item( + user.id, + dest_usage_key.course_key, + dest_usage_key.block_type, + block_id=dest_usage_key.block_id, + definition_data=source_item.get_explicitly_set_fields_by_scope(Scope.content), + metadata=duplicate_metadata, + runtime=source_item.runtime, + asides=asides_to_create + ) + + children_handled = False + + if hasattr(dest_block, 'studio_post_duplicate'): + # Allow an XBlock to do anything fancy it may need to when duplicated from another block. + # These blocks may handle their own children or parenting if needed. Let them return booleans to + # let us know if we need to handle these or not. + load_services_for_studio(dest_block.runtime, user) + children_handled = dest_block.studio_post_duplicate(store, source_item) + + # Children are not automatically copied over (and not all xblocks have a 'children' attribute). + # Because DAGs are not fully supported, we need to actually duplicate each child as well. + if source_item.has_children and not shallow and not children_handled: + dest_block.children = dest_block.children or [] + for child in source_item.children: + dupe = duplicate_block(dest_block.location, child, user=user, is_child=True) + if dupe not in dest_block.children: # _duplicate_block may add the child for us. + dest_block.children.append(dupe) + store.update_item(dest_block, user.id) + + # pylint: disable=protected-access + if 'detached' not in source_item.runtime.load_block_type(category)._class_tags: + parent = store.get_item(parent_usage_key) + # If source was already a child of the parent, add duplicate immediately afterward. + # Otherwise, add child to end. + if source_item.location in parent.children: + source_index = parent.children.index(source_item.location) + parent.children.insert(source_index + 1, dest_block.location) + else: + parent.children.append(dest_block.location) + store.update_item(parent, user.id) + + # .. event_implemented_name: XBLOCK_DUPLICATED + XBLOCK_DUPLICATED.send_event( + time=datetime.now(timezone.utc), + xblock_info=DuplicatedXBlockData( + usage_key=dest_block.location, + block_type=dest_block.location.block_type, + source_usage_key=duplicate_source_usage_key, + ) + ) + + return dest_block.location + + +def update_from_source(*, source_block, destination_block, user_id): + """ + Update a block to have all the settings and attributes of another source. + + Copies over all attributes and settings of a source block to a destination + block. Blocks must be the same type. This function does not modify or duplicate + children. + + This function is useful when a block, originally copied from a source block, drifts + and needs to be updated to match the original. + + The modulestore function copy_from_template will copy a block's children recursively, + replacing the target block's children. It does not, however, update any of the target + block's settings. copy_from_template, then, is useful for cases like the Library + Content Block, where the children are the same across all instances, but the settings + may differ. + + By contrast, for cases where we're copying a block that has drifted from its source, + we need to update the target block's settings, but we don't want to replace its children, + or, at least, not only replace its children. update_from_source is useful for these cases. + + This function is meant to be imported by pluggable django apps looking to manage duplicated + sections of a course. It is placed here for lack of a more appropriate location, since this + code has not yet been brought up to the standards in OEP-45. + """ + duplicate_metadata, asides = gather_block_attributes(source_block, display_name=source_block.display_name) + for key, value in duplicate_metadata.items(): + setattr(destination_block, key, value) + for key, value in source_block.get_explicitly_set_fields_by_scope(Scope.content).items(): + setattr(destination_block, key, value) + modulestore().update_item( + destination_block, + user_id, + metadata=duplicate_metadata, + asides=asides, + ) + + +def gather_block_attributes(source_item, display_name=None, is_child=False): + """ + Gather all the attributes of the source block that need to be copied over to a new or updated block. + """ + # Update the display name to indicate this is a duplicate (unless display name provided). + # Can't use own_metadata(), b/c it converts data for JSON serialization - + # not suitable for setting metadata of the new block + duplicate_metadata = {} + for field in source_item.fields.values(): + if field.scope == Scope.settings and field.is_set_on(source_item): + duplicate_metadata[field.name] = field.read_from(source_item) + + if is_child: + display_name = display_name or source_item.display_name or source_item.category + + if display_name is not None: + duplicate_metadata['display_name'] = display_name + else: + if source_item.display_name is None: + duplicate_metadata['display_name'] = _("Duplicate of {0}").format(source_item.category) + else: + duplicate_metadata['display_name'] = _("Duplicate of '{0}'").format(source_item.display_name) + + asides_to_create = [] + for aside in source_item.runtime.get_asides(source_item): + for field in aside.fields.values(): + if field.scope in (Scope.settings, Scope.content,) and field.is_set_on(aside): + asides_to_create.append(aside) + break + + for aside in asides_to_create: + for field in aside.fields.values(): + if field.scope not in (Scope.settings, Scope.content,): + field.delete_from(aside) + return duplicate_metadata, asides_to_create + + +def load_services_for_studio(runtime, user): + """ + Function to set some required services used for XBlock edits and studio_view. + (i.e. whenever we're not loading _prepare_runtime_for_preview.) This is required to make information + about the current user (especially permissions) available via services as needed. + """ + services = { + "user": DjangoXBlockUserService(user), + "studio_user_permissions": StudioPermissionsService(user), + "mako": MakoService(), + "settings": SettingsService(), + "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag), + "teams_configuration": TeamsConfigurationService(), + "library_tools": LibraryToolsService(modulestore(), user.id) + } + + runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access + + +class StudioPermissionsService: + """ + Service that can provide information about a user's permissions. + + Deprecated. To be replaced by a more general authorization service. + + Only used by LibraryContentBlock (and library_tools.py). + """ + def __init__(self, user): + self._user = user + + def can_read(self, course_key): + """ Does the user have read access to the given course/library? """ + return has_studio_read_access(self._user, course_key) + + def can_write(self, course_key): + """ Does the user have read access to the given course/library? """ + return has_studio_write_access(self._user, course_key) diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index 2cadea08ea33..a35230f3b8a5 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -4,19 +4,15 @@ from collections import OrderedDict from datetime import datetime from functools import partial -from uuid import uuid4 from django.conf import settings from django.contrib.auth.decorators import login_required from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.core.exceptions import PermissionDenied from django.http import Http404, HttpResponse, HttpResponseBadRequest -from django.utils.timezone import timezone from django.utils.translation import gettext as _ from django.views.decorators.http import require_http_methods from edx_django_utils.plugins import pluggable_override -from openedx_events.content_authoring.data import DuplicatedXBlockData -from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED from edx_proctoring.api import ( does_backend_support_onboarding, get_exam_by_content_id, @@ -24,7 +20,6 @@ ) from edx_proctoring.exceptions import ProctoredExamNotFoundException from help_tokens.core import HelpUrlExpert -from lti_consumer.models import CourseAllowPIISharingInLTIFlag from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryUsageLocator from pytz import UTC @@ -35,13 +30,11 @@ from cms.djangoapps.contentstore.config.waffle import SHOW_REVIEW_RULES_FLAG from cms.djangoapps.models.settings.course_grading import CourseGradingModel from cms.lib.xblock.authoring_mixin import VISIBILITY_VIEW -from common.djangoapps.edxmako.services import MakoService from common.djangoapps.edxmako.shortcuts import render_to_string from common.djangoapps.static_replace import replace_static_urls from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access from common.djangoapps.util.date_utils import get_default_time_display from common.djangoapps.util.json_request import JsonResponse, expect_json -from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from openedx.core.djangoapps.bookmarks import api as bookmarks_api from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE @@ -49,13 +42,11 @@ from openedx.core.lib.xblock_utils import hash_resource, request_token, wrap_xblock, wrap_xblock_aside from openedx.core.toggles import ENTRANCE_EXAMS from xmodule.course_block import DEFAULT_START_DATE # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.library_tools import LibraryToolsService # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore import EdxJSONEncoder, ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.inheritance import own_metadata # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.services import ConfigurationService, SettingsService, TeamsConfigurationService # lint-amnesty, pylint: disable=wrong-import-order from xmodule.tabs import CourseTabList # lint-amnesty, pylint: disable=wrong-import-order from xmodule.x_module import AUTHOR_VIEW, PREVIEW_VIEWS, STUDENT_VIEW, STUDIO_VIEW # lint-amnesty, pylint: disable=wrong-import-order @@ -68,7 +59,7 @@ get_visibility_partition_info, has_children_visible_to_specific_partition_groups, is_currently_visible_to_students, - is_self_paced + is_self_paced, duplicate_block, load_services_for_studio ) from .helpers import ( create_xblock, @@ -245,11 +236,11 @@ def xblock_handler(request, usage_key_string=None): status=400 ) - dest_usage_key = _duplicate_block( + dest_usage_key = duplicate_block( parent_usage_key, duplicate_source_usage_key, request.user, - request.json.get('display_name'), + display_name=request.json.get('display_name'), ) return JsonResponse({ 'locator': str(dest_usage_key), @@ -277,45 +268,6 @@ def xblock_handler(request, usage_key_string=None): ) -class StudioPermissionsService: - """ - Service that can provide information about a user's permissions. - - Deprecated. To be replaced by a more general authorization service. - - Only used by LibraryContentBlock (and library_tools.py). - """ - def __init__(self, user): - self._user = user - - def can_read(self, course_key): - """ Does the user have read access to the given course/library? """ - return has_studio_read_access(self._user, course_key) - - def can_write(self, course_key): - """ Does the user have read access to the given course/library? """ - return has_studio_write_access(self._user, course_key) - - -def load_services_for_studio(runtime, user): - """ - Function to set some required services used for XBlock edits and studio_view. - (i.e. whenever we're not loading _prepare_runtime_for_preview.) This is required to make information - about the current user (especially permissions) available via services as needed. - """ - services = { - "user": DjangoXBlockUserService(user), - "studio_user_permissions": StudioPermissionsService(user), - "mako": MakoService(), - "settings": SettingsService(), - "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag), - "teams_configuration": TeamsConfigurationService(), - "library_tools": LibraryToolsService(modulestore(), user.id) - } - - runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access - - @require_http_methods("GET") @login_required @expect_json @@ -880,103 +832,6 @@ def _move_item(source_usage_key, target_parent_usage_key, user, target_index=Non return JsonResponse(context) -def _duplicate_block(parent_usage_key, duplicate_source_usage_key, user, display_name=None, is_child=False): - """ - Duplicate an existing xblock as a child of the supplied parent_usage_key. - """ - store = modulestore() - with store.bulk_operations(duplicate_source_usage_key.course_key): - source_item = store.get_item(duplicate_source_usage_key) - # Change the blockID to be unique. - dest_usage_key = source_item.location.replace(name=uuid4().hex) - category = dest_usage_key.block_type - - # Update the display name to indicate this is a duplicate (unless display name provided). - # Can't use own_metadata(), b/c it converts data for JSON serialization - - # not suitable for setting metadata of the new block - duplicate_metadata = {} - for field in source_item.fields.values(): - if field.scope == Scope.settings and field.is_set_on(source_item): - duplicate_metadata[field.name] = field.read_from(source_item) - - if is_child: - display_name = display_name or source_item.display_name or source_item.category - - if display_name is not None: - duplicate_metadata['display_name'] = display_name - else: - if source_item.display_name is None: - duplicate_metadata['display_name'] = _("Duplicate of {0}").format(source_item.category) - else: - duplicate_metadata['display_name'] = _("Duplicate of '{0}'").format(source_item.display_name) - - asides_to_create = [] - for aside in source_item.runtime.get_asides(source_item): - for field in aside.fields.values(): - if field.scope in (Scope.settings, Scope.content,) and field.is_set_on(aside): - asides_to_create.append(aside) - break - - for aside in asides_to_create: - for field in aside.fields.values(): - if field.scope not in (Scope.settings, Scope.content,): - field.delete_from(aside) - - dest_block = store.create_item( - user.id, - dest_usage_key.course_key, - dest_usage_key.block_type, - block_id=dest_usage_key.block_id, - definition_data=source_item.get_explicitly_set_fields_by_scope(Scope.content), - metadata=duplicate_metadata, - runtime=source_item.runtime, - asides=asides_to_create - ) - - children_handled = False - - if hasattr(dest_block, 'studio_post_duplicate'): - # Allow an XBlock to do anything fancy it may need to when duplicated from another block. - # These blocks may handle their own children or parenting if needed. Let them return booleans to - # let us know if we need to handle these or not. - load_services_for_studio(dest_block.runtime, user) - children_handled = dest_block.studio_post_duplicate(store, source_item) - - # Children are not automatically copied over (and not all xblocks have a 'children' attribute). - # Because DAGs are not fully supported, we need to actually duplicate each child as well. - if source_item.has_children and not children_handled: - dest_block.children = dest_block.children or [] - for child in source_item.children: - dupe = _duplicate_block(dest_block.location, child, user=user, is_child=True) - if dupe not in dest_block.children: # _duplicate_block may add the child for us. - dest_block.children.append(dupe) - store.update_item(dest_block, user.id) - - # pylint: disable=protected-access - if 'detached' not in source_item.runtime.load_block_type(category)._class_tags: - parent = store.get_item(parent_usage_key) - # If source was already a child of the parent, add duplicate immediately afterward. - # Otherwise, add child to end. - if source_item.location in parent.children: - source_index = parent.children.index(source_item.location) - parent.children.insert(source_index + 1, dest_block.location) - else: - parent.children.append(dest_block.location) - store.update_item(parent, user.id) - - # .. event_implemented_name: XBLOCK_DUPLICATED - XBLOCK_DUPLICATED.send_event( - time=datetime.now(timezone.utc), - xblock_info=DuplicatedXBlockData( - usage_key=dest_block.location, - block_type=dest_block.location.block_type, - source_usage_key=duplicate_source_usage_key, - ) - ) - - return dest_block.location - - @login_required @expect_json def delete_item(request, usage_key): diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 60d6f68c9adc..8aadd418698a 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -35,9 +35,10 @@ from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order -from ..utils import get_lms_link_for_item, get_sibling_urls, reverse_course_url +from ..utils import get_lms_link_for_item, get_sibling_urls, reverse_course_url, \ + load_services_for_studio from .helpers import get_parent_xblock, is_unit, xblock_type_display_name -from .block import add_container_page_publishing_info, create_xblock_info, load_services_for_studio +from .block import add_container_page_publishing_info, create_xblock_info __all__ = [ 'container_handler', diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index 841e93c656fe..bb827d361c63 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -48,7 +48,7 @@ from xmodule.x_module import STUDENT_VIEW, STUDIO_VIEW from cms.djangoapps.contentstore.tests.utils import CourseTestCase -from cms.djangoapps.contentstore.utils import reverse_course_url, reverse_usage_url +from cms.djangoapps.contentstore.utils import reverse_course_url, reverse_usage_url, duplicate_block, update_from_source from cms.djangoapps.contentstore.views import block as item_module from common.djangoapps.student.tests.factories import StaffFactory, UserFactory from common.djangoapps.xblock_django.models import ( @@ -787,6 +787,30 @@ def verify_name(source_usage_key, parent_usage_key, expected_name, display_name= # Now send a custom display name for the duplicate. verify_name(self.seq_usage_key, self.chapter_usage_key, "customized name", display_name="customized name") + def test_shallow_duplicate(self): + """ + Test that duplicate_block(..., shallow=True) can duplicate a block but ignores its children. + """ + source_course = CourseFactory() + user = UserFactory.create() + source_chapter = BlockFactory(parent=source_course, category='chapter', display_name='Source Chapter') + BlockFactory(parent=source_chapter, category='html', display_name='Child') + # Refresh. + source_chapter = self.store.get_item(source_chapter.location) + self.assertEqual(len(source_chapter.get_children()), 1) + destination_course = CourseFactory() + destination_location = duplicate_block( + parent_usage_key=destination_course.location, + duplicate_source_usage_key=source_chapter.location, + user=user, + display_name=source_chapter.display_name, + shallow=True, + ) + # Refresh here, too, just to be sure. + destination_chapter = self.store.get_item(destination_location) + self.assertEqual(len(destination_chapter.get_children()), 0) + self.assertEqual(destination_chapter.display_name, 'Source Chapter') + @ddt.ddt class TestMoveItem(ItemTest): @@ -3495,3 +3519,111 @@ def test_self_paced_item_visibility_state(self, store_type): # Check that in self paced course content has live state now xblock_info = self._get_xblock_info(chapter.location) self._verify_visibility_state(xblock_info, VisibilityState.live) + + +@patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + lambda self, block: ['test_aside']) +class TestUpdateFromSource(ModuleStoreTestCase): + """ + Test update_from_source. + """ + + def setUp(self): + """ + Set up the runtime for tests. + """ + super().setUp() + key_store = DictKeyValueStore() + field_data = KvsFieldData(key_store) + self.runtime = TestRuntime(services={'field-data': field_data}) + + def create_source_block(self, course): + """ + Create a chapter with all the fixings. + """ + source_block = BlockFactory( + parent=course, + category='course_info', + display_name='Source Block', + metadata={'due': datetime(2010, 11, 22, 4, 0, tzinfo=UTC)}, + ) + + def_id = self.runtime.id_generator.create_definition('html') + usage_id = self.runtime.id_generator.create_usage(def_id) + + aside = AsideTest(scope_ids=ScopeIds('user', 'html', def_id, usage_id), runtime=self.runtime) + aside.field11 = 'html_new_value1' + + # The data attribute is handled in a special manner and should be updated. + source_block.data = '
test
' + # This field is set on the content scope (definition_data), which should be updated. + source_block.items = ['test', 'beep'] + + self.store.update_item(source_block, self.user.id, asides=[aside]) + + # quick sanity checks + source_block = self.store.get_item(source_block.location) + self.assertEqual(source_block.due, datetime(2010, 11, 22, 4, 0, tzinfo=UTC)) + self.assertEqual(source_block.display_name, 'Source Block') + self.assertEqual(source_block.runtime.get_asides(source_block)[0].field11, 'html_new_value1') + self.assertEqual(source_block.data, '
test
') + self.assertEqual(source_block.items, ['test', 'beep']) + + return source_block + + def check_updated(self, source_block, destination_key): + """ + Check that the destination block has been updated to match our source block. + """ + revised = self.store.get_item(destination_key) + self.assertEqual(source_block.display_name, revised.display_name) + self.assertEqual(source_block.due, revised.due) + self.assertEqual(revised.data, source_block.data) + self.assertEqual(revised.items, source_block.items) + + self.assertEqual( + revised.runtime.get_asides(revised)[0].field11, + source_block.runtime.get_asides(source_block)[0].field11, + ) + + @XBlockAside.register_temp_plugin(AsideTest, 'test_aside') + def test_update_from_source(self): + """ + Test that update_from_source updates the destination block. + """ + course = CourseFactory() + user = UserFactory.create() + + source_block = self.create_source_block(course) + + destination_block = BlockFactory(parent=course, category='course_info', display_name='Destination Problem') + update_from_source(source_block=source_block, destination_block=destination_block, user_id=user.id) + self.check_updated(source_block, destination_block.location) + + @XBlockAside.register_temp_plugin(AsideTest, 'test_aside') + def test_update_clobbers(self): + """ + Verify that our update replaces all settings on the block. + """ + course = CourseFactory() + user = UserFactory.create() + + source_block = self.create_source_block(course) + + destination_block = BlockFactory( + parent=course, + category='course_info', + display_name='Destination Chapter', + metadata={'due': datetime(2025, 10, 21, 6, 5, tzinfo=UTC)}, + ) + + def_id = self.runtime.id_generator.create_definition('html') + usage_id = self.runtime.id_generator.create_usage(def_id) + aside = AsideTest(scope_ids=ScopeIds('user', 'html', def_id, usage_id), runtime=self.runtime) + aside.field11 = 'Other stuff' + destination_block.data = '
other stuff
' + destination_block.items = ['other stuff', 'boop'] + self.store.update_item(destination_block, user.id, asides=[aside]) + + update_from_source(source_block=source_block, destination_block=destination_block, user_id=user.id) + self.check_updated(source_block, destination_block.location) diff --git a/xmodule/modulestore/split_mongo/split.py b/xmodule/modulestore/split_mongo/split.py index e76f6c81204d..f72f950a5405 100644 --- a/xmodule/modulestore/split_mongo/split.py +++ b/xmodule/modulestore/split_mongo/split.py @@ -57,7 +57,6 @@ import copy import datetime -import hashlib import logging from collections import defaultdict from importlib import import_module @@ -102,7 +101,7 @@ ) from xmodule.modulestore.split_mongo import BlockKey, CourseEnvelope from xmodule.modulestore.split_mongo.mongo_connection import DuplicateKeyError, DjangoFlexPersistenceBackend -from xmodule.modulestore.store_utilities import DETACHED_XBLOCK_TYPES +from xmodule.modulestore.store_utilities import DETACHED_XBLOCK_TYPES, derived_key from xmodule.partitions.partitions_service import PartitionService from xmodule.util.misc import get_library_or_course_attribute @@ -2369,6 +2368,9 @@ def copy_from_template(self, source_keys, dest_usage, user_id, head_validation=T time this method is called for the same source block and dest_usage, the same resulting block id will be generated. + Note also that this function does not override any of the attributes on the destination + block-- it only replaces the destination block's children. + :param source_keys: a list of BlockUsageLocators. Order is preserved. :param dest_usage: The BlockUsageLocator that will become the parent of an inherited copy @@ -2442,7 +2444,6 @@ def _copy_from_template( for usage_key in source_keys: src_course_key = usage_key.course_key - hashable_source_id = src_course_key.for_version(None) block_key = BlockKey(usage_key.block_type, usage_key.block_id) source_structure = source_structures[src_course_key] @@ -2450,15 +2451,7 @@ def _copy_from_template( raise ItemNotFoundError(usage_key) source_block_info = source_structure['blocks'][block_key] - # Compute a new block ID. This new block ID must be consistent when this - # method is called with the same (source_key, dest_structure) pair - unique_data = "{}:{}:{}".format( - str(hashable_source_id).encode("utf-8"), - block_key.id, - new_parent_block_key.id, - ) - new_block_id = hashlib.sha1(unique_data.encode('utf-8')).hexdigest()[:20] - new_block_key = BlockKey(block_key.type, new_block_id) + new_block_key = derived_key(src_course_key, block_key, new_parent_block_key) # Now clone block_key to new_block_key: new_block_info = copy.deepcopy(source_block_info) diff --git a/xmodule/modulestore/store_utilities.py b/xmodule/modulestore/store_utilities.py index 45082ff43e75..ced5c9728ec0 100644 --- a/xmodule/modulestore/store_utilities.py +++ b/xmodule/modulestore/store_utilities.py @@ -1,5 +1,5 @@ # lint-amnesty, pylint: disable=missing-module-docstring - +import hashlib import logging import re import uuid @@ -7,6 +7,8 @@ from xblock.core import XBlock +from xmodule.modulestore.split_mongo import BlockKey + DETACHED_XBLOCK_TYPES = {name for name, __ in XBlock.load_tagged_classes("detached")} @@ -104,3 +106,31 @@ def get_draft_subtree_roots(draft_nodes): for draft_node in draft_nodes: if draft_node.parent_url not in urls: yield draft_node + + +def derived_key(courselike_source_key, block_key, dest_parent: BlockKey): + """ + Return a new reproducible block ID for a given root, source block, and destination parent. + + When recursively copying a block structure, we need to generate new block IDs for the + blocks. We don't want to use the exact same IDs as we might copy blocks multiple times. + However, we do want to be able to reproduce the same IDs when copying the same block + so that if we ever need to re-copy the block from its source (that is, to update it with + upstream changes) we don't affect any data tied to the ID, such as grades. + + This is used by the copy_from_template function of the modulestore, and can be used by + pluggable django apps that need to copy blocks from one course to another in an + idempotent way. In the future, this should be created into a proper API function + in the spirit of OEP-49. + """ + hashable_source_id = courselike_source_key.for_version(None) + + # Compute a new block ID. This new block ID must be consistent when this + # method is called with the same (source_key, dest_structure) pair + unique_data = "{}:{}:{}".format( + str(hashable_source_id).encode("utf-8"), + block_key.id, + dest_parent.id, + ) + new_block_id = hashlib.sha1(unique_data.encode('utf-8')).hexdigest()[:20] + return BlockKey(block_key.type, new_block_id) diff --git a/xmodule/modulestore/tests/test_store_utilities.py b/xmodule/modulestore/tests/test_store_utilities.py index 1c1c86f4fcd3..a0b5cfcbdbb6 100644 --- a/xmodule/modulestore/tests/test_store_utilities.py +++ b/xmodule/modulestore/tests/test_store_utilities.py @@ -4,11 +4,15 @@ import unittest +from unittest import TestCase from unittest.mock import Mock import ddt -from xmodule.modulestore.store_utilities import draft_node_constructor, get_draft_subtree_roots +from opaque_keys.edx.keys import CourseKey + +from xmodule.modulestore.split_mongo import BlockKey +from xmodule.modulestore.store_utilities import draft_node_constructor, get_draft_subtree_roots, derived_key @ddt.ddt @@ -82,3 +86,43 @@ def test_get_draft_subtree_roots(self, node_arguments_list, expected_roots_urls) subtree_roots_urls = [root.url for root in get_draft_subtree_roots(block_nodes)] # check that we return the expected urls assert set(subtree_roots_urls) == set(expected_roots_urls) + + +mock_block = Mock() +mock_block.id = CourseKey.from_string('course-v1:Beeper+B33P+BOOP') + + +derived_key_scenarios = [ + { + 'courselike_source_key': CourseKey.from_string('course-v1:edX+DemoX+Demo_Course'), + 'block_key': BlockKey('chapter', 'interactive_demonstrations'), + 'parent': mock_block, + 'expected': BlockKey( + 'chapter', '5793ec64e25ed870a7dd', + ), + }, + { + 'courselike_source_key': CourseKey.from_string('course-v1:edX+DemoX+Demo_Course'), + 'block_key': BlockKey('chapter', 'interactive_demonstrations'), + 'parent': BlockKey( + 'chapter', 'thingy', + ), + 'expected': BlockKey( + 'chapter', '599792a5622d85aa41e6', + ), + } +] + + +@ddt.ddt +class TestDerivedKey(TestCase): + """ + Test reproducible block ID generation. + """ + @ddt.data(*derived_key_scenarios) + @ddt.unpack + def test_derived_key(self, courselike_source_key, block_key, parent, expected): + """ + Test that derived_key returns the expected value. + """ + self.assertEqual(derived_key(courselike_source_key, block_key, parent), expected)