Skip to content

Commit

Permalink
fix: preserve learner state after v2 migration
Browse files Browse the repository at this point in the history
  • Loading branch information
connorhaugh committed Dec 12, 2023
1 parent 2629992 commit 8909cb3
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 13 deletions.
2 changes: 1 addition & 1 deletion cms/djangoapps/contentstore/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
23 changes: 19 additions & 4 deletions openedx/core/djangoapps/content_libraries/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.')
Expand Down Expand Up @@ -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.
#
Expand All @@ -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(
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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.
Expand All @@ -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}")
Expand Down
35 changes: 27 additions & 8 deletions openedx/core/djangoapps/content_libraries/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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.
"""
Expand All @@ -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
)
Expand All @@ -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)
Expand Down

0 comments on commit 8909cb3

Please sign in to comment.