From ed395e3cba04d93222d1af42f66b4ddf06fba5b8 Mon Sep 17 00:00:00 2001
From: Navin Karkera
Date: Wed, 24 Sep 2025 16:07:48 +0530
Subject: [PATCH 01/23] feat: show item bank ui for migrated legacy library
content
---
xmodule/item_bank_block.py | 68 ++++++++++++++++----------------
xmodule/library_content_block.py | 10 ++++-
2 files changed, 42 insertions(+), 36 deletions(-)
diff --git a/xmodule/item_bank_block.py b/xmodule/item_bank_block.py
index b53617e2c8e4..0fdfcf0fba8b 100644
--- a/xmodule/item_bank_block.py
+++ b/xmodule/item_bank_block.py
@@ -432,6 +432,40 @@ def definition_to_xml(self, resource_fs):
xml_object.set(field_name, str(field.read_from(self)))
return xml_object
+ def author_view(self, context):
+ """
+ Renders the Studio views.
+ Normal studio view: If block is properly configured, displays library status summary
+ Studio container view: displays a preview of all possible children.
+ """
+ fragment = Fragment()
+ root_xblock = context.get('root_xblock')
+ is_root = root_xblock and root_xblock.usage_key == self.usage_key
+ if is_root and self.children:
+ # User has clicked the "View" link. Show a preview of all possible children:
+ context['can_edit_visibility'] = False
+ context['can_move'] = False
+ context['can_collapse'] = True
+ self.render_children(context, fragment, can_reorder=False, can_add=False)
+ else:
+ # We're just on the regular unit page, or we're on the "view" page but no children exist yet.
+ # Show a summary message and instructions.
+ summary_html = loader.render_django_template('templates/item_bank/author_view.html', {
+ # Due to template interpolation limitations, we have to pass some HTML for the link here:
+ "view_link": f'',
+ "blocks": [
+ {"display_name": display_name_with_default(child)}
+ for child in self.get_children()
+ ],
+ "block_count": len(self.children),
+ "max_count": self.max_count,
+ })
+ fragment.add_content(summary_html)
+ # Whether on the main author view or the detailed children view, show a button to add more from the library:
+ add_html = loader.render_django_template('templates/item_bank/author_view_add.html', {})
+ fragment.add_content(add_html)
+ return fragment
+
@classmethod
def get_selected_event_prefix(cls) -> str:
"""
@@ -492,40 +526,6 @@ def validate(self):
)
return validation
- def author_view(self, context):
- """
- Renders the Studio views.
- Normal studio view: If block is properly configured, displays library status summary
- Studio container view: displays a preview of all possible children.
- """
- fragment = Fragment()
- root_xblock = context.get('root_xblock')
- is_root = root_xblock and root_xblock.usage_key == self.usage_key
- if is_root and self.children:
- # User has clicked the "View" link. Show a preview of all possible children:
- context['can_edit_visibility'] = False
- context['can_move'] = False
- context['can_collapse'] = True
- self.render_children(context, fragment, can_reorder=False, can_add=False)
- else:
- # We're just on the regular unit page, or we're on the "view" page but no children exist yet.
- # Show a summary message and instructions.
- summary_html = loader.render_django_template('templates/item_bank/author_view.html', {
- # Due to template interpolation limitations, we have to pass some HTML for the link here:
- "view_link": f'',
- "blocks": [
- {"display_name": display_name_with_default(child)}
- for child in self.get_children()
- ],
- "block_count": len(self.children),
- "max_count": self.max_count,
- })
- fragment.add_content(summary_html)
- # Whether on the main author view or the detailed children view, show a button to add more from the library:
- add_html = loader.render_django_template('templates/item_bank/author_view_add.html', {})
- fragment.add_content(add_html)
- return fragment
-
def format_block_keys_for_analytics(self, block_keys: list[tuple[str, str]]) -> list[dict]:
"""
Implement format_block_keys_for_analytics using the `upstream` link system.
diff --git a/xmodule/library_content_block.py b/xmodule/library_content_block.py
index 52e33108027c..07356d88b886 100644
--- a/xmodule/library_content_block.py
+++ b/xmodule/library_content_block.py
@@ -12,7 +12,7 @@
import json
import logging
-from gettext import ngettext, gettext
+from gettext import gettext, ngettext
import nh3
from django.core.exceptions import ObjectDoesNotExist, PermissionDenied
@@ -23,8 +23,8 @@
from xblock.fields import Integer, Scope, String
from xmodule.capa.responsetypes import registry
-from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.item_bank_block import ItemBankMixin
+from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.validation import StudioValidation, StudioValidationMessage
from xmodule.x_module import XModuleToXBlockMixin
@@ -117,6 +117,12 @@ def author_view(self, context):
Normal studio view: If block is properly configured, displays library status summary
Studio container view: displays a preview of all possible children.
"""
+ from cms.djangoapps.modulestore_migrator.api import is_successfully_migrated
+
+ lib_key = LibraryLocator.from_string(self.source_library_id)
+ if is_successfully_migrated(lib_key):
+ # Show ItemBank UI in this case
+ return super().author_view(context)
fragment = Fragment()
root_xblock = context.get('root_xblock')
is_root = root_xblock and root_xblock.location == self.location
From ddeca53c110bcc17fe8383ece9071f2afb3a0ca1 Mon Sep 17 00:00:00 2001
From: Navin Karkera
Date: Mon, 29 Sep 2025 20:37:21 +0530
Subject: [PATCH 02/23] feat: migrate legacy content block to item bank block
on view in studio
---
cms/djangoapps/modulestore_migrator/api.py | 23 ++++-
xmodule/item_bank_block.py | 57 +++++-------
xmodule/library_content_block.py | 103 ++++++++++++++++++---
xmodule/tests/test_library_content.py | 41 ++++----
4 files changed, 152 insertions(+), 72 deletions(-)
diff --git a/cms/djangoapps/modulestore_migrator/api.py b/cms/djangoapps/modulestore_migrator/api.py
index d084be764a2e..82ea90566a62 100644
--- a/cms/djangoapps/modulestore_migrator/api.py
+++ b/cms/djangoapps/modulestore_migrator/api.py
@@ -3,7 +3,7 @@
"""
from celery.result import AsyncResult
from opaque_keys.edx.keys import CourseKey, LearningContextKey
-from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2
+from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2, LibraryUsageLocatorV2
from openedx_learning.api.authoring import get_collection
from user_tasks.models import UserTaskStatus
@@ -11,13 +11,14 @@
from openedx.core.types.user import AuthUser
from . import tasks
-from .models import ModulestoreSource
+from .models import ModulestoreBlockMigration, ModulestoreSource
__all__ = (
"start_migration_to_library",
"start_bulk_migration_to_library",
"is_successfully_migrated",
"get_migration_info",
+ "get_target_block_usage_keys",
)
@@ -129,3 +130,21 @@ def get_migration_info(source_keys: list[CourseKey | LibraryLocator]) -> dict:
named=True,
)
}
+
+
+def get_target_block_usage_keys(source_key: CourseKey | LibraryLocator) -> dict[str, str]:
+ """
+ Get all target blocks for given list of source keys.
+ """
+ query_set = ModulestoreBlockMigration.objects.filter(
+ overall_migration__source__key=source_key
+ ).values_list('source__key', 'target__key', 'target__learning_package__key')
+
+ def construct_usage_key(row: list[str]):
+ lib_key = LibraryLocatorV2.from_string(row[2])
+ _, block_type, usage_id = row[1].split(":")
+ return str(LibraryUsageLocatorV2(lib_key, block_type=block_type, usage_id=usage_id))
+ # Use LibraryUsageLocatorV2 and construct usage key
+ return {
+ row[0]: construct_usage_key(row) for row in query_set
+ }
diff --git a/xmodule/item_bank_block.py b/xmodule/item_bank_block.py
index 0fdfcf0fba8b..a11393a1a614 100644
--- a/xmodule/item_bank_block.py
+++ b/xmodule/item_bank_block.py
@@ -7,6 +7,7 @@
import logging
import random
from copy import copy
+
from django.conf import settings
from django.utils.functional import classproperty
from lxml import etree
@@ -24,13 +25,13 @@
from xmodule.studio_editable import StudioEditableBlock
from xmodule.util.builtin_assets import add_webpack_js_to_fragment
from xmodule.validation import StudioValidation, StudioValidationMessage
-from xmodule.xml_block import XmlMixin
from xmodule.x_module import (
+ STUDENT_VIEW,
ResourceTemplates,
XModuleMixin,
shim_xmodule_js,
- STUDENT_VIEW,
)
+from xmodule.xml_block import XmlMixin
_ = lambda text: text
@@ -268,20 +269,6 @@ def selected_children(self):
return self.selected
- def format_block_keys_for_analytics(self, block_keys: list[tuple[str, str]]) -> list[dict]:
- """
- Given a list of (block_type, block_id) pairs, prepare the JSON-ready metadata needed for analytics logging.
-
- This is [
- {"usage_key": x, "original_usage_key": y, "original_usage_version": z, "descendants": [...]}
- ]
- where the main list contains all top-level blocks, and descendants contains a *flat* list of all
- descendants of the top level blocks, if any.
-
- Must be implemented in child class.
- """
- raise NotImplementedError
-
@XBlock.handler
def reset_selected_children(self, _, __):
"""
@@ -476,22 +463,7 @@ def get_selected_event_prefix(cls) -> str:
"""
raise NotImplementedError
-
-class ItemBankBlock(ItemBankMixin, XBlock):
- """
- An XBlock which shows a random subset of its children to each learner.
-
- Unlike LegacyLibraryContentBlock, this block does not need to worry about synchronization, capa_type filtering, etc.
- That is all implemented using `upstream` links on each individual child.
- """
- display_name = String(
- display_name=_("Display Name"),
- help=_("The display name for this component."),
- default="Problem Bank",
- scope=Scope.settings,
- )
-
- def validate(self):
+ def _validate(self):
"""
Validates the state of this ItemBankBlock Instance.
"""
@@ -541,6 +513,27 @@ def format_block_keys_for_analytics(self, block_keys: list[tuple[str, str]]) ->
} for block_key in block_keys
]
+
+class ItemBankBlock(ItemBankMixin, XBlock):
+ """
+ An XBlock which shows a random subset of its children to each learner.
+
+ Unlike LegacyLibraryContentBlock, this block does not need to worry about synchronization, capa_type filtering, etc.
+ That is all implemented using `upstream` links on each individual child.
+ """
+ display_name = String(
+ display_name=_("Display Name"),
+ help=_("The display name for this component."),
+ default="Problem Bank",
+ scope=Scope.settings,
+ )
+
+ def validate(self):
+ """
+ Validates the state of this ItemBankBlock Instance.
+ """
+ return self._validate()
+
@classmethod
def get_selected_event_prefix(cls) -> str:
"""
diff --git a/xmodule/library_content_block.py b/xmodule/library_content_block.py
index 07356d88b886..3220dd8df385 100644
--- a/xmodule/library_content_block.py
+++ b/xmodule/library_content_block.py
@@ -20,10 +20,11 @@
from web_fragments.fragment import Fragment
from webob import Response
from xblock.core import XBlock
-from xblock.fields import Integer, Scope, String
+from xblock.fields import Boolean, Scope, String
from xmodule.capa.responsetypes import registry
from xmodule.item_bank_block import ItemBankMixin
+from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.validation import StudioValidation, StudioValidationMessage
from xmodule.x_module import XModuleToXBlockMixin
@@ -90,12 +91,6 @@ class LegacyLibraryContentBlock(ItemBankMixin, XModuleToXBlockMixin, XBlock):
display_name=_("Library Version"),
scope=Scope.settings,
)
- max_count = Integer(
- display_name=_("Count"),
- help=_("Enter the number of components to display to each student. Set it to -1 to display all components."),
- default=1,
- scope=Scope.settings,
- )
capa_type = String(
display_name=_("Problem Type"),
help=_('Choose a problem type to fetch from the library. If "Any Type" is selected no filtering is applied.'),
@@ -103,6 +98,12 @@ class LegacyLibraryContentBlock(ItemBankMixin, XModuleToXBlockMixin, XBlock):
values=_get_capa_types(),
scope=Scope.settings,
)
+ is_migrated_to_v2 = Boolean(
+ # This is a hidden field that stores whether children blocks are migrated to v2 i.e, have upstream set
+ display_name=_("Is Migrated to library v2"),
+ scope=Scope.settings,
+ default=False,
+ )
@property
def source_library_key(self):
@@ -111,16 +112,21 @@ def source_library_key(self):
"""
return LibraryLocator.from_string(self.source_library_id)
+ @property
+ def is_source_lib_migrated_to_v2(self):
+ """
+ Determines whether the source library has been migrated to v2.
+ """
+ from cms.djangoapps.modulestore_migrator.api import is_successfully_migrated
+ return self.source_library_id and is_successfully_migrated(self.source_library_key)
+
def author_view(self, context):
"""
Renders the Studio views.
Normal studio view: If block is properly configured, displays library status summary
Studio container view: displays a preview of all possible children.
"""
- from cms.djangoapps.modulestore_migrator.api import is_successfully_migrated
-
- lib_key = LibraryLocator.from_string(self.source_library_id)
- if is_successfully_migrated(lib_key):
+ if self.is_migrated_to_v2:
# Show ItemBank UI in this case
return super().author_view(context)
fragment = Fragment()
@@ -165,7 +171,14 @@ def non_editable_metadata_fields(self):
non_editable_fields = super().non_editable_metadata_fields
non_editable_fields.extend([
LegacyLibraryContentBlock.source_library_version,
+ LegacyLibraryContentBlock.is_migrated_to_v2,
])
+ if self.is_migrated_to_v2:
+ # If the block is migrated, hide legacy settings to make it similar to the new ItemBankBlock.
+ non_editable_fields.extend([
+ LegacyLibraryContentBlock.capa_type,
+ LegacyLibraryContentBlock.source_library_id,
+ ])
return non_editable_fields
def get_tools(self, to_read_library_content: bool = False) -> 'LegacyLibraryToolsService':
@@ -201,6 +214,12 @@ def upgrade_and_sync(self, request=None, suffix=None): # pylint: disable=unused
Returns 400 if libraray tools or user permission services are not available.
Returns 403/404 if user lacks read access on source library or write access on this block.
"""
+ if self.is_migrated_to_v2:
+ # If the block is already migrated to v2 i.e. ItemBankBlock
+ return Response(
+ _("This block is already migrated to use library v2. You can sync individual blocks now"),
+ status=400
+ )
self._validate_sync_permissions()
if not self.source_library_id:
return Response(_("Source content library has not been specified."), status=400)
@@ -252,6 +271,10 @@ def studio_post_duplicate(self, store, source_block):
if hasattr(super(), 'studio_post_duplicate'):
super().studio_post_duplicate(store, source_block)
+ if self.is_migrated_to_v2:
+ # If the block is already migrated to v2 i.e. ItemBankBlock, Do nothing
+ return True
+
self._validate_sync_permissions()
self.get_tools(to_read_library_content=True).trigger_duplication(source_block=source_block, dest_block=self)
return True # Children have been handled.
@@ -263,9 +286,38 @@ def studio_post_paste(self, store, source_node) -> bool:
if hasattr(super(), 'studio_post_paste'):
super().studio_post_paste(store, source_node)
+ if self.is_migrated_to_v2:
+ # If the block is already migrated to v2 i.e. ItemBankBlock, Do nothing
+ return True
+
self.sync_from_library(upgrade_to_latest=False)
return True # Children have been handled
+ def _v2_update_children_upstream_version(self):
+ """
+ Update the upstream and upstream version fields of all children to point to library v2 version of the legacy
+ library blocks. This essentially converts this legacy block to new ItemBankBlock.
+ """
+ if not self.is_source_lib_migrated_to_v2:
+ return
+ from cms.djangoapps.modulestore_migrator.api import get_target_block_usage_keys
+ blocks = get_target_block_usage_keys(self.source_library_key)
+ store = modulestore()
+ with store.bulk_operations(self.course_id):
+ for child in self.get_children():
+ source_key, _ = self.runtime.modulestore.get_block_original_usage(child.usage_key)
+ child.upstream = blocks.get(source_key)
+ # Since after migration, the component in library is in draft state, we want to make sure that sync icon
+ # appears when it is published
+ child.upstream_version = 0
+ child.save()
+ # Use `modulestore()` instead of `self.runtime.modulestore` to make sure that the XBLOCK_UPDATED signal is
+ # triggered
+ store.update_item(child, None)
+ self.is_migrated_to_v2 = True
+ self.save()
+ store.update_item(self, None)
+
def _validate_library_version(self, validation, lib_tools, version, library_key):
"""
Validates library version
@@ -304,10 +356,20 @@ def _set_validation_error_if_empty(self, validation, summary):
def validate(self):
"""
- Validates the state of this Library Content Block Instance. This
- is the override of the general XBlock method, and it will also ask
- its superclass to validate.
+ Validates the state of this Library Content Block Instance.
+
+ We also use this method to migrate this legacy block to new ItemBankBlock which uses
+ library v2 blocks as children.
"""
+ if self.is_migrated_to_v2:
+ # If the block is already migrated to v2 i.e. ItemBankBlock
+ return self._validate()
+ if self.is_source_lib_migrated_to_v2:
+ # If the source library is migrated but this block still depends on legacy library
+ # Migrate the block by setting upstream field to all children blocks
+ self._v2_update_children_upstream_version()
+ return self._validate()
+
validation = super().validate()
if not isinstance(validation, StudioValidation):
validation = StudioValidation.copy(validation)
@@ -334,7 +396,12 @@ def validate(self):
)
)
return validation
- self._validate_library_version(validation, lib_tools, self.source_library_version, self.source_library_key)
+ self._validate_library_version(
+ validation,
+ lib_tools,
+ self.source_library_version,
+ self.source_library_key
+ )
# Note: we assume children have been synced
# since the last time fields like source_library_id or capa_types were changed.
@@ -396,6 +463,9 @@ def post_editor_saved(self, user, old_metadata, old_content): # pylint: disable
"""
If source library or capa_type have been edited, upgrade library & sync automatically.
"""
+ if self.is_migrated_to_v2:
+ # If the block is already migrated to v2 i.e. ItemBankBlock, Do nothing
+ return True
source_lib_changed = (self.source_library_id != old_metadata.get("source_library_id", ""))
capa_filter_changed = (self.capa_type != old_metadata.get("capa_type", ANY_CAPA_TYPE_VALUE))
if source_lib_changed or capa_filter_changed:
@@ -409,6 +479,9 @@ def format_block_keys_for_analytics(self, block_keys: list[tuple[str, str]]) ->
"""
Implement format_block_keys_for_analytics using the modulestore-specific legacy library original-usage system.
"""
+ if self.is_migrated_to_v2:
+ return super().format_block_keys_for_analytics(block_keys)
+
def summarize_block(usage_key):
""" Basic information about the given block """
orig_key, orig_version = self.runtime.modulestore.get_block_original_usage(usage_key)
diff --git a/xmodule/tests/test_library_content.py b/xmodule/tests/test_library_content.py
index 092606142e1d..18a30ae6fba3 100644
--- a/xmodule/tests/test_library_content.py
+++ b/xmodule/tests/test_library_content.py
@@ -13,7 +13,9 @@
from web_fragments.fragment import Fragment
from xblock.runtime import Runtime as VanillaRuntime
+from common.djangoapps.student.tests.factories import UserFactory
from openedx.core.djangolib.testing.utils import skip_unless_cms
+from xmodule.capa_block import ProblemBlock
from xmodule.library_content_block import ANY_CAPA_TYPE_VALUE, LegacyLibraryContentBlock
from xmodule.library_tools import LegacyLibraryToolsService
from xmodule.modulestore import ModuleStoreEnum
@@ -22,8 +24,6 @@
from xmodule.tests import prepare_block_runtime
from xmodule.validation import StudioValidationMessage
from xmodule.x_module import AUTHOR_VIEW
-from xmodule.capa_block import ProblemBlock
-from common.djangoapps.student.tests.factories import UserFactory
from .test_course_block import DummySystem as TestImportSystem
@@ -133,15 +133,13 @@ def setUp(self):
self._sync_lc_block_from_library()
self.expected_olx = (
- '\n'
- ' \n'
- ' \n'
- ' \n'
- ' \n'
+ f'\n' # noqa: E501
+ f' \n'
+ f' \n'
+ f'