From 32b7f27c46b5e2c69b055cc0d3a41f9079c52e80 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 22 Dec 2025 23:19:43 +0530 Subject: [PATCH] feat: api to bulk update legacy library references (#37789) Adds API to fetch all legacy library content blocks that are ready to be updated to use library v2 and convert to item banks. Also adds API to update all the references via a user celery task and to fetch its status. --- cms/djangoapps/cms_user_tasks/signals.py | 12 +- cms/djangoapps/cms_user_tasks/tests.py | 10 ++ cms/djangoapps/contentstore/api/__init__.py | 2 +- .../contentstore/api/tests/test_validation.py | 143 +++++++++++++++++- cms/djangoapps/contentstore/api/urls.py | 12 +- .../api/views/course_validation.py | 61 +++++++- .../contentstore/api/views/utils.py | 17 +++ cms/djangoapps/contentstore/tasks.py | 69 +++++++++ openedx/core/lib/api/serializers.py | 11 ++ xmodule/library_content_block.py | 12 +- 10 files changed, 331 insertions(+), 18 deletions(-) diff --git a/cms/djangoapps/cms_user_tasks/signals.py b/cms/djangoapps/cms_user_tasks/signals.py index 832f6cea4f..6824d1ceb8 100644 --- a/cms/djangoapps/cms_user_tasks/signals.py +++ b/cms/djangoapps/cms_user_tasks/signals.py @@ -19,6 +19,7 @@ from .tasks import send_task_complete_email LOGGER = logging.getLogger(__name__) LIBRARY_CONTENT_TASK_NAME_TEMPLATE = 'updating .*type@library_content.* from library' LIBRARY_IMPORT_TASK_NAME_TEMPLATE = '(.*)?migrate_from_modulestore' +LEGACY_LIB_CONTENT_REF_UPDATE_TASK_TEMPLATE = 'updating legacy library content blocks references of course-v1:.*' @receiver(user_task_stopped, dispatch_uid="cms_user_task_stopped") @@ -63,6 +64,14 @@ def user_task_stopped_handler(sender, **kwargs): # pylint: disable=unused-argum p = re.compile(LIBRARY_IMPORT_TASK_NAME_TEMPLATE) return p.match(task_name) is not None + def is_legacy_library_content_reference_update(task_name: str) -> bool: + """ + Decides whether to suppress an end-of-task email on the basis that the just-ended task + was a legacy library content reference update operation. + """ + p = re.compile(LEGACY_LIB_CONTENT_REF_UPDATE_TASK_TEMPLATE) + return p.match(task_name) is not None + def get_olx_validation_from_artifact(): """ Get olx validation error if available for current task. @@ -96,7 +105,8 @@ def user_task_stopped_handler(sender, **kwargs): # pylint: disable=unused-argum is_library_content_update(task_name) or is_library_backup_task(task_name) or is_library_restore_task(task_name) or - is_library_import_task(task_name) + is_library_import_task(task_name) or + is_legacy_library_content_reference_update(task_name) ) status = kwargs['status'] diff --git a/cms/djangoapps/cms_user_tasks/tests.py b/cms/djangoapps/cms_user_tasks/tests.py index a331bde35e..8a9bc55321 100644 --- a/cms/djangoapps/cms_user_tasks/tests.py +++ b/cms/djangoapps/cms_user_tasks/tests.py @@ -231,6 +231,16 @@ class TestUserTaskStopped(APITestCase): self.assertEqual(len(mail.outbox), 0) + def test_email_not_sent_with_legacy_libary_content_ref_update(self): + """ + Check the signal receiver and email sending. + """ + end_of_task_status = self.status + end_of_task_status.name = "Updating legacy library content blocks references of course-v1:UNIX+UN1+2025_T4" + user_task_stopped.send(sender=UserTaskStatus, status=end_of_task_status) + + self.assertEqual(len(mail.outbox), 0) + def test_email_sent_with_olx_validations_with_config_enabled(self): """ Tests that email is sent with olx validation errors. diff --git a/cms/djangoapps/contentstore/api/__init__.py b/cms/djangoapps/contentstore/api/__init__.py index 6662d35370..da36a1afe4 100644 --- a/cms/djangoapps/contentstore/api/__init__.py +++ b/cms/djangoapps/contentstore/api/__init__.py @@ -1,2 +1,2 @@ """Contentstore API""" -from .views.utils import course_author_access_required +from .views.utils import course_author_access_required, get_ready_to_migrate_legacy_library_content_blocks diff --git a/cms/djangoapps/contentstore/api/tests/test_validation.py b/cms/djangoapps/contentstore/api/tests/test_validation.py index 8da8004772..8bf8e19b62 100644 --- a/cms/djangoapps/contentstore/api/tests/test_validation.py +++ b/cms/djangoapps/contentstore/api/tests/test_validation.py @@ -1,23 +1,26 @@ """ Tests for the course import API views """ - -import factory from datetime import datetime -from django.conf import settings +from unittest.mock import MagicMock, patch +from uuid import uuid4 import ddt +import factory +from django.conf import settings +from django.contrib.auth import get_user_model from django.test.utils import override_settings from django.urls import reverse from rest_framework import status from rest_framework.test import APITestCase -from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.course_modes.tests.factories import CourseModeFactory -from common.djangoapps.student.tests.factories import StaffFactory -from common.djangoapps.student.tests.factories import UserFactory +from common.djangoapps.student.tests.factories import StaffFactory, UserFactory +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory + +User = get_user_model() @ddt.ddt @@ -143,3 +146,129 @@ class CourseValidationViewTest(SharedModuleStoreTestCase, APITestCase): 'is_self_paced': True, } self.assertDictEqual(resp.data, expected_data) + + +class TestMigrationViewSetCreate(SharedModuleStoreTestCase, APITestCase): + """ + Test the MigrationViewSet.create() endpoint. + + Focus: validation, return codes, serialization/deserialization. + """ + @classmethod + def setUpClass(cls): + super().setUpClass() + + cls.course = CourseFactory.create( + display_name='test course', + run="Testing_course", + proctoring_provider='test_proctoring_provider', + proctoring_escalation_email='test@example.com', + ) + cls.course_key = cls.course.id + + cls.password = 'test' + cls.student = UserFactory(username='dummy', password=cls.password) + cls.staff = StaffFactory(course_key=cls.course.id, password=cls.password) + + cls.initialize_course(cls.course) + + @classmethod + def initialize_course(cls, course): + """ + Sets up test course structure. + """ + section = BlockFactory.create( + parent_location=course.location, + category="chapter", + ) + subsection = BlockFactory.create( + parent_location=section.location, + category="sequential", + ) + unit = BlockFactory.create( + parent_location=subsection.location, + category="vertical", + ) + cls.block1 = BlockFactory.create( + parent_location=unit.location, + category="library_content", + ) + cls.block2 = BlockFactory.create( + parent_location=unit.location, + category="library_content", + ) + + @patch('cms.djangoapps.contentstore.api.views.utils.has_course_author_access') + @patch('cms.djangoapps.contentstore.api.views.course_validation.UserTaskStatus') + @patch('xmodule.library_content_block.LegacyLibraryContentBlock.is_ready_to_migrate_to_v2') + def test_create_update_reference_success(self, mock_block, mock_user_task_status, mock_auth): + """ + Test successful migration creation with minimal required fields. + + Validates: + - 201 status code is returned + - Response contains expected serialized fields + - Request data is properly deserialized + - Permission checks are performed for both source and target + """ + mock_auth.return_value = True + + mock_task_status = MagicMock(autospec=True) + mock_task_status.uuid = uuid4() + mock_task_status.state = 'Pending' + mock_task_status.state_text = 'Pending' + mock_task_status.completed_steps = 0 + mock_task_status.total_steps = 10 + mock_task_status.attempts = 1 + mock_task_status.created = '2025-01-01T00:00:00Z' + mock_task_status.modified = '2025-01-01T00:00:00Z' + mock_task_status.artifacts = [] + mock_task_status.migrations.all.return_value = [] + + mock_user_task_status.objects.get.return_value = mock_task_status + + mock_block.return_value = True + + self.client.login(username=self.staff.username, password=self.password) + response = self.client.post( + f'/api/courses/v1/migrate_legacy_content_blocks/{self.course_key}/', + ) + + assert response.status_code == status.HTTP_201_CREATED + + assert 'uuid' in response.data + assert 'state' in response.data + assert 'state_text' in response.data + assert 'completed_steps' in response.data + assert 'total_steps' in response.data + + mock_auth.assert_called_once() + + @patch('cms.djangoapps.contentstore.api.views.utils.has_course_author_access') + @patch('xmodule.library_content_block.LegacyLibraryContentBlock.is_ready_to_migrate_to_v2') + def test_list_ready_to_update_reference_success(self, mock_block, mock_auth): + """ + Test successful migration creation with minimal required fields. + + Validates: + - 201 status code is returned + - Response contains expected serialized fields + - Request data is properly deserialized + - Permission checks are performed for both source and target + """ + mock_auth.return_value = True + mock_block.return_value = True + + self.client.login(username=self.staff.username, password=self.password) + response = self.client.get( + f'/api/courses/v1/migrate_legacy_content_blocks/{self.course_key}/', + ) + + assert response.status_code == status.HTTP_200_OK + + data = response.json() + self.assertListEqual(data, [ + {'usage_key': str(self.block1.location)}, + {'usage_key': str(self.block2.location)}, + ]) + mock_auth.assert_called_once() diff --git a/cms/djangoapps/contentstore/api/urls.py b/cms/djangoapps/contentstore/api/urls.py index e638a59f84..e9791f526e 100644 --- a/cms/djangoapps/contentstore/api/urls.py +++ b/cms/djangoapps/contentstore/api/urls.py @@ -3,18 +3,26 @@ from django.conf import settings from django.urls import re_path +from django.urls.conf import include, path +from rest_framework.routers import SimpleRouter from cms.djangoapps.contentstore.api.views import course_import, course_quality, course_validation - app_name = 'contentstore' +ROUTER = SimpleRouter() +ROUTER.register( + fr'^v1/migrate_legacy_content_blocks/{settings.COURSE_ID_PATTERN}', + course_validation.CourseLegacyLibraryContentMigratorView, + basename='course_ready_to_migrate_legacy_blocks' +) + urlpatterns = [ + path('', include(ROUTER.urls)), re_path(fr'^v0/import/{settings.COURSE_ID_PATTERN}/$', course_import.CourseImportView.as_view(), name='course_import'), re_path(fr'^v1/validation/{settings.COURSE_ID_PATTERN}/$', course_validation.CourseValidationView.as_view(), name='course_validation'), re_path(fr'^v1/quality/{settings.COURSE_ID_PATTERN}/$', course_quality.CourseQualityView.as_view(), name='course_quality'), - ] diff --git a/cms/djangoapps/contentstore/api/views/course_validation.py b/cms/djangoapps/contentstore/api/views/course_validation.py index c55f58d945..d3565fc168 100644 --- a/cms/djangoapps/contentstore/api/views/course_validation.py +++ b/cms/djangoapps/contentstore/api/views/course_validation.py @@ -2,18 +2,27 @@ import logging import dateutil +import edx_api_doc_tools as apidocs +from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication +from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser from pytz import UTC +from rest_framework import serializers, status from rest_framework.generics import GenericAPIView from rest_framework.response import Response +from user_tasks.models import UserTaskStatus +from user_tasks.views import StatusViewSet from cms.djangoapps.contentstore.course_info_model import get_course_updates +from cms.djangoapps.contentstore.tasks import migrate_course_legacy_library_blocks_to_item_bank from cms.djangoapps.contentstore.views.certificates import CertificateManager from common.djangoapps.util.proctoring import requires_escalation_email +from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser +from openedx.core.lib.api.serializers import StatusSerializerWithUuid from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes from xmodule.course_metadata_utils import DEFAULT_GRADING_POLICY # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order -from .utils import course_author_access_required, get_bool_param +from .utils import course_author_access_required, get_bool_param, get_ready_to_migrate_legacy_library_content_blocks log = logging.getLogger(__name__) @@ -346,3 +355,53 @@ class CourseValidationView(DeveloperErrorViewMixin, GenericAPIView): needs_proctoring_escalation_email=requires_escalation_email(course.proctoring_provider), has_proctoring_escalation_email=bool(course.proctoring_escalation_email) ) + + +class CourseLegacyLibraryContentSerializer(serializers.Serializer): + usage_key = serializers.CharField() + + +class CourseLegacyLibraryContentMigratorView(StatusViewSet): + """ + This endpoint is used for migrating legacy library content to the new item bank block library v2. + """ + # DELETE is not allowed, as we want to preserve all task status objects. + # Instead, users can POST to /cancel to cancel running tasks. + http_method_names = ["get", "post"] + authentication_classes = ( + BearerAuthenticationAllowInactiveUser, + JwtAuthentication, + SessionAuthenticationAllowInactiveUser, + ) + serializer_class = StatusSerializerWithUuid + + @apidocs.schema( + responses={ + 200: CourseLegacyLibraryContentSerializer(many=True), + 401: "The requester is not authenticated.", + }, + ) + @course_author_access_required + def list(self, _, course_key): # pylint: disable=arguments-differ + """ + Returns all legacy library content blocks ready to be migrated to new item bank block. + """ + blocks = get_ready_to_migrate_legacy_library_content_blocks(course_key) + serializer = CourseLegacyLibraryContentSerializer(blocks, many=True) + return Response(serializer.data) + + @apidocs.schema( + responses={ + 200: "In case of success, a 200.", + 401: "The requester is not authenticated.", + }, + ) + @course_author_access_required + def create(self, request, course_key): + """ + Migrate all legacy library content blocks to new item bank block. + """ + task = migrate_course_legacy_library_blocks_to_item_bank.delay(request.user.id, str(course_key)) + task_status = UserTaskStatus.objects.get(task_id=task.id) + serializer = self.get_serializer(task_status) + return Response(serializer.data, status=status.HTTP_201_CREATED) diff --git a/cms/djangoapps/contentstore/api/views/utils.py b/cms/djangoapps/contentstore/api/views/utils.py index da92dc08e1..5d7dd8ff06 100644 --- a/cms/djangoapps/contentstore/api/views/utils.py +++ b/cms/djangoapps/contentstore/api/views/utils.py @@ -13,6 +13,7 @@ from common.djangoapps.student.auth import has_course_author_access from openedx.core.djangoapps.util.forms import to_bool from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes from openedx.core.lib.cache_utils import request_cached +from xmodule.library_content_block import LegacyLibraryContentBlock from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order @@ -135,3 +136,19 @@ def course_author_access_required(view): ) return view(self, request, course_key, *args, **kwargs) return _wrapper_view + + +def get_ready_to_migrate_legacy_library_content_blocks(course_key: CourseKey) -> list[LegacyLibraryContentBlock]: + """ + Get the ready to migrate legacy library content blocks for a course. + + Args: + course_key (CourseKey): The key of the course + + Returns: + List[XBlock]: A list of XBlock objects that are marked as ready + """ + store = modulestore() + blocks = store.get_items(course_key, qualifiers={'category': 'library_content'}) + ready_to_migrate_blocks = [block for block in blocks if block.is_ready_to_migrate_to_v2] + return ready_to_migrate_blocks diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index 3ed5cb7b48..ac9cc3d831 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -99,6 +99,7 @@ from .outlines import update_outline_from_modulestore from .outlines_regenerate import CourseOutlineRegenerate from .toggles import bypass_olx_failure_enabled from .utils import course_import_olx_validation_is_enabled +from .api import get_ready_to_migrate_legacy_library_content_blocks User = get_user_model() @@ -2288,3 +2289,71 @@ def _update_result_applies_to_block(result_entry, block_id): return block_category == result_type except Exception: # pylint: disable=broad-except return False + + +class LegacyLibraryContentToItemBank(UserTask): # pylint: disable=abstract-method + """ + Base class for course and library export tasks. + """ + + @classmethod + def generate_name(cls, arguments_dict): + """ + Create a name for this particular import task instance. + + Arguments: + arguments_dict (dict): The arguments given to the task function + + Returns: + str: The generated name + """ + key = arguments_dict['course_key'] + return f'Updating legacy library content blocks references of {key}' + + +def _cancel_old_tasks(course_key: str, user: User, ignore_task_ids: list[str]): + """ + Cancel all old instances of this particular migration task. + """ + task_name = LegacyLibraryContentToItemBank.generate_name({'course_key': course_key}) + tasks_to_cancel = UserTaskStatus.objects.filter( + user=user, + name=task_name, + ).exclude( + # (excluding that aren't running) + state__in=(UserTaskStatus.CANCELED, UserTaskStatus.FAILED, UserTaskStatus.SUCCEEDED) + ).exclude( + task_id__in=ignore_task_ids + ) + for task in tasks_to_cancel: + task.cancel() + + +@shared_task(base=LegacyLibraryContentToItemBank, bind=True) +def migrate_course_legacy_library_blocks_to_item_bank(self, user_id: int, course_key: str): + """ + Migrate legacy course library blocks to Item Bank. + + Depending on the number of blocks and its children blocks this operation can take a significant + amount of time and this is why it is run as a celery task. + """ + ensure_cms("Legacy library content references may only be executed in CMS") + set_code_owner_attribute_from_module(__name__) + _cancel_old_tasks(course_key, self.status.user, [self.status.task_id]) + try: + key = CourseKey.from_string(course_key) + except InvalidKeyError as exc: + LOGGER.exception(f'Invalid course key: {course_key}') + self.status.fail(str(exc)) + return + self.status.set_state(UserTaskStatus.IN_PROGRESS) + blocks = get_ready_to_migrate_legacy_library_content_blocks(key) + store = modulestore() + try: + with store.bulk_operations(key): + for block in blocks: + self.status.set_state(f'Migrating block: {block.usage_key}') + block.v2_update_children_upstream_version(user_id) + except Exception as exc: # pylint: disable=broad-except + LOGGER.exception(f'Error while migrating blocks: {exc}') + self.status.fail(str(exc)) diff --git a/openedx/core/lib/api/serializers.py b/openedx/core/lib/api/serializers.py index 947d5a07f1..dbcf4b3047 100644 --- a/openedx/core/lib/api/serializers.py +++ b/openedx/core/lib/api/serializers.py @@ -6,6 +6,7 @@ Serializers to be used in APIs. from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey from rest_framework import serializers +from user_tasks.serializers import StatusSerializer class CollapsedReferenceSerializer(serializers.HyperlinkedModelSerializer): @@ -70,3 +71,13 @@ class UsageKeyField(serializers.Field): return UsageKey.from_string(data) except InvalidKeyError as err: raise serializers.ValidationError("Invalid usage key") from err + + +class StatusSerializerWithUuid(StatusSerializer): + """ + Serializer for the user task status, including uuid. + """ + + class Meta: + model = StatusSerializer.Meta.model + fields = [*StatusSerializer.Meta.fields, 'uuid'] diff --git a/xmodule/library_content_block.py b/xmodule/library_content_block.py index 5853b3923f..a57e4cd0e1 100644 --- a/xmodule/library_content_block.py +++ b/xmodule/library_content_block.py @@ -136,7 +136,7 @@ class LegacyLibraryContentBlock(ItemBankMixin, XModuleToXBlockMixin, XBlock): ) @property - def is_ready_to_migrated_to_v2(self) -> bool: + def is_ready_to_migrate_to_v2(self) -> bool: """ Returns whether the block can be migrated to v2. """ @@ -315,7 +315,7 @@ class LegacyLibraryContentBlock(ItemBankMixin, XModuleToXBlockMixin, XBlock): self.sync_from_library(upgrade_to_latest=False) return True # Children have been handled - def _v2_update_children_upstream_version(self): + def v2_update_children_upstream_version(self, user_id=None): """ 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. @@ -336,17 +336,17 @@ class LegacyLibraryContentBlock(ItemBankMixin, XModuleToXBlockMixin, XBlock): child.upstream = "" # Use `modulestore()` instead of `self.runtime.modulestore` to make sure that the XBLOCK_UPDATED signal # is triggered - store.update_item(child, None) + store.update_item(child, user_id) self.is_migrated_to_v2 = True self.save() - store.update_item(self, None) + store.update_item(self, user_id) def _validate_library_version(self, validation, lib_tools, version, library_key): """ Validates library version """ latest_version = lib_tools.get_latest_library_version(library_key) - if self.is_ready_to_migrated_to_v2: + if self.is_ready_to_migrate_to_v2: validation.set_summary( StudioValidationMessage( StudioValidationMessage.WARNING, @@ -405,7 +405,7 @@ class LegacyLibraryContentBlock(ItemBankMixin, XModuleToXBlockMixin, XBlock): return Response(_("The block has already been upgraded to version 2"), status=400) # 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() + self.v2_update_children_upstream_version() return Response() def validate(self):