From 8909cb30e754032fb7925178cc53ab78c3bbf8f1 Mon Sep 17 00:00:00 2001 From: connorhaugh Date: Tue, 12 Dec 2023 22:46:38 +0000 Subject: [PATCH] fix: preserve learner state after v2 migration --- cms/djangoapps/contentstore/tasks.py | 2 +- .../core/djangoapps/content_libraries/api.py | 23 +++++++++--- .../djangoapps/content_libraries/tasks.py | 35 ++++++++++++++----- 3 files changed, 47 insertions(+), 13 deletions(-) diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index 8cf1f3172832..646b9adf61de 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -896,7 +896,7 @@ def _create_copy_content_task(v2_library_key, v1_library_key): spin up a celery task to import the V1 Library's content into the V2 library. This utalizes the fact that course and v1 library content is stored almost identically. """ - return v2contentlib_api.import_blocks_create_task(v2_library_key, v1_library_key) + return v2contentlib_api.import_blocks_create_task(v2_library_key, v1_library_key, use_course_key_as_block_id_suffix=False) def _create_metadata(v1_library_key, collection_uuid): diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index 10383de336e5..76a95e18fbf0 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -1214,12 +1214,13 @@ class BaseEdxImportClient(abc.ABC): "video", } - def __init__(self, library_key=None, library=None): + def __init__(self, library_key=None, library=None, use_course_key_as_block_id_suffix=True): """ Initialize an import client for a library. The method accepts either a library object or a key to a library object. """ + self.use_course_key_as_block_id_suffix = use_course_key_as_block_id_suffix if bool(library_key) == bool(library): raise ValueError('Provide at least one of `library_key` or ' '`library`, but not both.') @@ -1249,7 +1250,9 @@ def import_block(self, modulestore_key): """ Import a single modulestore block. """ + print(f'modulestore_key {modulestore_key}') block_data = self.get_block_data(modulestore_key) + print(f'block_data {block_data}') # Get or create the block in the library. # @@ -1261,8 +1264,19 @@ def import_block(self, modulestore_key): str(modulestore_key.course_key).encode() ).digest() )[:16].decode().lower() + print(f'course_key_id {course_key_id}') # Prepend 'c' to allow changing hash without conflicts. - block_id = f"{modulestore_key.block_id}_c{course_key_id}" + + # add the course_key_id if use_course_key_as_suffix is enabled to increase the namespace. + # in some cases, for the preservation of child-parent relationships + # of the same content across modulestore and blockstore, only the block_id is used. + block_id = block_id = ( + f"{modulestore_key.block_id}_c{course_key_id}" + if self.use_course_key_as_block_id_suffix + else f"{modulestore_key.block_id}" + ) + + print(f'block_id {block_id}') log.info('Importing to library block: id=%s', block_id) try: library_block = create_library_block( @@ -1341,6 +1355,7 @@ def __init__(self, modulestore_instance=None, **kwargs): """ Initialize the client with a modulestore instance. """ + print(kwargs.items()) super().__init__(**kwargs) self.modulestore = modulestore_instance or modulestore() @@ -1473,7 +1488,7 @@ def _call(self, method, *args, **kwargs): return response -def import_blocks_create_task(library_key, course_key): +def import_blocks_create_task(library_key, course_key, use_course_key_as_block_id_suffix=True): """ Create a new import block task. @@ -1486,7 +1501,7 @@ def import_blocks_create_task(library_key, course_key): course_id=course_key, ) result = tasks.import_blocks_from_course.apply_async( - args=(import_task.pk, str(course_key)) + args=(import_task.pk, str(course_key), use_course_key_as_block_id_suffix) ) log.info(f"Import block task created: import_task={import_task} " f"celery_task={result.id}") diff --git a/openedx/core/djangoapps/content_libraries/tasks.py b/openedx/core/djangoapps/content_libraries/tasks.py index 3c259aaba956..ca44e0bae700 100644 --- a/openedx/core/djangoapps/content_libraries/tasks.py +++ b/openedx/core/djangoapps/content_libraries/tasks.py @@ -36,6 +36,11 @@ from common.djangoapps.student.auth import has_studio_write_access from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.locator import ( + LibraryLocatorV2, + LibraryUsageLocatorV2, + LibraryLocator as LibraryLocatorV1 +) from openedx.core.djangoapps.content_libraries import api as library_api from openedx.core.djangoapps.xblock.api import load_block from openedx.core.lib import ensure_cms, blockstore_api @@ -45,6 +50,8 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.mixed import MixedModuleStore +from xmodule.modulestore.split_mongo import BlockKey +from xmodule.modulestore.store_utilities import derived_key from . import api from .models import ContentLibraryBlockImportTask @@ -55,7 +62,7 @@ @shared_task(base=LoggedTask) @set_code_owner_attribute -def import_blocks_from_course(import_task_id, course_key_str): +def import_blocks_from_course(import_task_id, course_key_str, use_course_key_as_block_id_suffix=True): """ A Celery task to import blocks from a course through modulestore. """ @@ -72,7 +79,10 @@ def on_progress(block_key, block_num, block_count, exception=None): logger.info('Import block succesful: %s', block_key) import_task.save_progress(block_num / block_count) - edx_client = api.EdxModulestoreImportClient(library=import_task.library) + edx_client = api.EdxModulestoreImportClient( + library=import_task.library, + use_course_key_as_block_id_suffix=use_course_key_as_block_id_suffix + ) edx_client.import_blocks_from_course( course_key, on_progress ) @@ -84,14 +94,23 @@ def _import_block(store, user_id, source_block, dest_parent_key): """ def generate_block_key(source_key, dest_parent_key): """ - Deterministically generate an ID for the new block and return the key + Deterministically generate an ID for the new block and return the key. + Keys are generated such that they appear identical to a v1 library with + the same input block_id, library name, library organization, and parent block using derived_key """ - block_id = ( - dest_parent_key.block_id[:10] + - '-' + - hashlib.sha1(str(source_key).encode('utf-8')).hexdigest()[:10] + if not isinstance(source_key.lib_key, LibraryLocatorV2): + raise ValueError("Input must be an instance of LibraryLocatorV2") + source_key_as_v1_course_key = LibraryLocatorV1(org=source_key.lib_key.org, library=source_key.lib_key.slug) + source_key_usage_id_as_block_key = BlockKey( + type= source_key.block_type, + id=source_key.usage_id, + ) + block_id = derived_key( + source_key_as_v1_course_key, + source_key_usage_id_as_block_key, + BlockKey(dest_parent_key.block_type, dest_parent_key.block_id) ) - return dest_parent_key.context_key.make_usage_key(source_key.block_type, block_id) + return dest_parent_key.context_key.make_usage_key(source_key.block_type, block_id.id) source_key = source_block.scope_ids.usage_id new_block_key = generate_block_key(source_key, dest_parent_key)