Skip to content

Commit

Permalink
feat: keep learner state associated after libraries migration (#33920)
Browse files Browse the repository at this point in the history
* fix: preserve learner state after v2 migration

Co-authored-by: Kyle McCormick <[email protected]>

---------

Co-authored-by: Kyle McCormick <[email protected]>
  • Loading branch information
connorhaugh and kdmccormick authored Dec 18, 2023
1 parent 9e14fa4 commit 2eac2ef
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 18 deletions.
7 changes: 4 additions & 3 deletions cms/djangoapps/contentstore/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -896,14 +896,15 @@ 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):
"""instansiate an index for the V2 lib in the collection"""

print(collection_uuid)

store = modulestore()
v1_library = store.get_library(v1_library_key)
collection = get_collection(collection_uuid).uuid
Expand Down
21 changes: 16 additions & 5 deletions openedx/core/djangoapps/content_libraries/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -1231,12 +1231,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 @@ -1278,8 +1279,18 @@ def import_block(self, modulestore_key):
str(modulestore_key.course_key).encode()
).digest()
)[:16].decode().lower()
# 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.
# The option exists to not use the course key as a suffix because
# in order to preserve learner state in the v1 to v2 libraries migration,
# the v2 and v1 libraries' child block ids must be the same.
block_id = (
# Prepend 'c' to allow changing hash without conflicts.
f"{modulestore_key.block_id}_c{course_key_id}"
if self.use_course_key_as_block_id_suffix
else f"{modulestore_key.block_id}"
)

log.info('Importing to library block: id=%s', block_id)
try:
library_block = create_library_block(
Expand Down Expand Up @@ -1490,7 +1501,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 @@ -1503,7 +1514,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
39 changes: 29 additions & 10 deletions openedx/core/djangoapps/content_libraries/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
from __future__ import annotations

import logging
import hashlib

from celery import shared_task
from celery_utils.logged_task import LoggedTask
Expand All @@ -28,7 +27,9 @@
from opaque_keys.edx.keys import UsageKey
from opaque_keys.edx.locator import (
BlockUsageLocator,
LibraryUsageLocatorV2
LibraryLocatorV2,
LibraryUsageLocatorV2,
LibraryLocator as LibraryLocatorV1
)

from user_tasks.tasks import UserTask, UserTaskStatus
Expand All @@ -45,6 +46,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 +58,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 +75,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 +90,27 @@ 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 TypeError(f"Expected source library key of type LibraryLocatorV2, got {source_key.lib_key} instead.")
source_key_as_v1_course_key = LibraryLocatorV1(
org=source_key.lib_key.org,
library=source_key.lib_key.slug,
branch='library'
)
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 2eac2ef

Please sign in to comment.