From 26104e3199d7c92065f81e27ae69b01622ddc480 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 24 Dec 2025 16:16:47 -0500 Subject: [PATCH 1/6] feat: Preview migration API --- .../modulestore_migrator/api/read_api.py | 120 +++++++++++++++++- .../rest_api/v1/serializers.py | 15 +++ .../modulestore_migrator/rest_api/v1/urls.py | 2 + .../modulestore_migrator/rest_api/v1/views.py | 81 ++++++++++++ openedx/core/djangoapps/content/search/api.py | 76 ++++++++++- 5 files changed, 291 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/modulestore_migrator/api/read_api.py b/cms/djangoapps/modulestore_migrator/api/read_api.py index 757e1bf55a63..bf765ecc3a7b 100644 --- a/cms/djangoapps/modulestore_migrator/api/read_api.py +++ b/cms/djangoapps/modulestore_migrator/api/read_api.py @@ -5,18 +5,27 @@ import typing as t from uuid import UUID +from django.conf import settings from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.locator import ( LibraryLocatorV2, LibraryUsageLocatorV2, LibraryContainerLocator ) -from openedx_learning.api.authoring import get_draft_version +from openedx_learning.api.authoring import get_draft_version, get_all_drafts from openedx_learning.api.authoring_models import ( PublishableEntityVersion, PublishableEntity, DraftChangeLogRecord ) +from xblock.plugin import PluginMissingError from openedx.core.djangoapps.content_libraries.api import ( - library_component_usage_key, library_container_locator + library_component_usage_key, library_container_locator, + validate_can_add_block_to_library, BlockLimitReachedError, + IncompatibleTypesError, LibraryBlockAlreadyExists, + ContentLibrary +) +from openedx.core.djangoapps.content.search.api import ( + fetch_block_types, + get_all_blocks_from_context, ) from ..data import ( @@ -32,6 +41,7 @@ 'get_forwarding_for_blocks', 'get_migrations', 'get_migration_blocks', + 'preview_migration', ) @@ -242,3 +252,109 @@ def _block_migration_success( target_title=target_title, target_version_num=target_version_num, ) + + +def preview_migration(source_key: str, target_key: str): + """ + Returns a summary preview of the migration given a source key and a target key + on this form: + + ``` + { + "state": "block_limit_reached", + "unsupported_blocks": 0, + "unsupported_percentage": 0, + "blocks_limit": blocks_limit, + "total_blocks": 0, + "total_components": 0, + "sections": 0, + "subsections": 0, + "units": 0, + } + ``` + + List of states: + - 'success': The migration can be carried out in its entirety + - 'partial': The migration will be partial, because there are unsupported blocks. + - 'block_limit_reached': The migration cannot be performed because the block limit per library has been reached. + + TODO: For now, the repeat_handling_strategy is not taken into account. This can be taken into + account for a more advanced summary. + """ + blocks = get_all_blocks_from_context(source_key, ["block_type", "block_id"]) + + unsupported_blocks = [] + total_blocks = 0 + total_components = 0 + sections = 0 + subsections = 0 + units = 0 + blocks_limit = settings.MAX_BLOCKS_PER_CONTENT_LIBRARY + for block in blocks: + block_type = block["block_type"] + block_id = block["block_id"] + total_blocks += 1 + if block_type not in ['chapter', 'sequential', 'vertical']: + total_components += 1 + try: + validate_can_add_block_to_library( + target_key, + block_type, + block_id, + ) + except BlockLimitReachedError: + return { + "state": "block_limit_reached", + "unsupported_blocks": 0, + "unsupported_percentage": 0, + "blocks_limit": blocks_limit, + "total_blocks": 0, + "total_components": 0, + "sections": 0, + "subsections": 0, + "units": 0, + } + except (IncompatibleTypesError, PluginMissingError): + unsupported_blocks.append(block["usage_key"]) + except LibraryBlockAlreadyExists: + # Skip this validation, The block may be repeated in the library, but that's not a bad thing. + pass + elif block_type == "chapter": + sections += 1 + elif block_type == "sequential": + subsections += 1 + elif block_type == "vertical": + units += 1 + + quoted_keys = ','.join(f'"{key}"' for key in unsupported_blocks) + unsupportedBlocksChildren = fetch_block_types( + [ + f'context_key = "{source_key}"', + f'breadcrumbs.usage_key IN [{quoted_keys}]' + ], + ) + unsupported_blocks_count = len(unsupported_blocks) + unsupportedBlocksChildren["estimatedTotalHits"] + + state = "success" + if unsupported_blocks_count: + state = "partial" + + content_library = ContentLibrary.objects.get_by_key(target_key) + target_item_counts = get_all_drafts(content_library.learning_package_id).count() + + unsupported_percentage = (unsupported_blocks_count / total_blocks) * 100 + + if target_item_counts + total_blocks - unsupported_blocks_count > blocks_limit: + state = "block_limit_reached" + + return { + "state": state, + "unsupported_blocks": unsupported_blocks_count, + "unsupported_percentage": unsupported_percentage, + "blocks_limit": blocks_limit, + "total_blocks": total_blocks, + "total_components": total_components, + "sections": sections, + "subsections": subsections, + "units": units, + } diff --git a/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py b/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py index 643d94d2250c..43a0b03ee3c9 100644 --- a/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py +++ b/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py @@ -289,3 +289,18 @@ class BlockMigrationInfoSerializer(serializers.Serializer): source_key = serializers.CharField() target_key = serializers.CharField(allow_null=True) unsupported_reason = serializers.CharField(allow_null=True) + + +class PreviewMigrationSerializer(serializers.Serializer): + """ + Serializer for the preview migration response. + """ + state = serializers.CharField() + unsupported_blocks = serializers.IntegerField() + unsupported_percentage = serializers.FloatField() + blocks_limit = serializers.IntegerField() + total_blocks = serializers.IntegerField() + total_components = serializers.IntegerField() + sections = serializers.IntegerField() + subsections = serializers.IntegerField() + units = serializers.IntegerField() diff --git a/cms/djangoapps/modulestore_migrator/rest_api/v1/urls.py b/cms/djangoapps/modulestore_migrator/rest_api/v1/urls.py index 208825e0c01a..d154d2424763 100644 --- a/cms/djangoapps/modulestore_migrator/rest_api/v1/urls.py +++ b/cms/djangoapps/modulestore_migrator/rest_api/v1/urls.py @@ -10,6 +10,7 @@ LibraryCourseMigrationViewSet, MigrationInfoViewSet, MigrationViewSet, + PreviewMigration, ) ROUTER = SimpleRouter() @@ -25,4 +26,5 @@ path('', include(ROUTER.urls)), path('migration_info/', MigrationInfoViewSet.as_view(), name='migration-info'), path('migration_blocks/', BlockMigrationInfo.as_view(), name='migration-blocks'), + path('migration_preview/', PreviewMigration.as_view(), name='migration-preview'), ] diff --git a/cms/djangoapps/modulestore_migrator/rest_api/v1/views.py b/cms/djangoapps/modulestore_migrator/rest_api/v1/views.py index d55adb6a53e2..8d684a779e0d 100644 --- a/cms/djangoapps/modulestore_migrator/rest_api/v1/views.py +++ b/cms/djangoapps/modulestore_migrator/rest_api/v1/views.py @@ -41,6 +41,7 @@ MigrationInfoResponseSerializer, ModulestoreMigrationSerializer, StatusWithModulestoreMigrationsSerializer, + PreviewMigrationSerializer, ) log = logging.getLogger(__name__) @@ -668,3 +669,83 @@ def get(self, request: Request): ] serializer = BlockMigrationInfoSerializer(data, many=True) return Response(serializer.data) + + +class PreviewMigration(APIView): + """ + Retrieve the summary preview of the migration given a source key and a target key + + It returns the migration block information for each block migrated by a specific task. + + API Endpoints + ------------- + GET /api/modulestore_migrator/v1/migration_preview/ + Retrieve the summary preview of the migration given a source key and a target key + + Query parameters: + source_key (str): Source content key + Example: ?source_key=course-v1:UNIX+UX1+2025_T3 + target_key (str): target content key + Example: ?target_key=lib:UNIX:CIT1 + + Example request: + GET /api/modulestore_migrator/v1/migration_blocks/?source_key=course_key&target_key=library_key + + Example response: + """ + + permission_classes = (IsAuthenticated,) + authentication_classes = ( + BearerAuthenticationAllowInactiveUser, + JwtAuthentication, + SessionAuthenticationAllowInactiveUser, + ) + + @apidocs.schema( + parameters=[ + apidocs.string_parameter( + "target_key", + apidocs.ParameterLocation.QUERY, + description="Target key of the migration", + ), + apidocs.string_parameter( + "source_key", + apidocs.ParameterLocation.QUERY, + description="Source key of the migration", + ), + ], + responses={ + 200: PreviewMigrationSerializer, + 400: "Missing required parameter: target_key/source_key", + 401: "The requester is not authenticated.", + }, + ) + def get(self, request: Request): + """ + Handle the migration info `GET` request + """ + target_key: LibraryLocatorV2 | None + if target_key_param := request.query_params.get("target_key"): + try: + target_key = LibraryLocatorV2.from_string(target_key_param) + except InvalidKeyError: + return Response({"error": f"Bad target_key: {target_key_param}"}, status=400) + else: + return Response({"error": "Target key cannot be blank."}, status=400) + source_key: SourceContextKey | None = None + if source_key_param := request.query_params.get("source_key"): + try: + source_key = CourseLocator.from_string(source_key_param) + except InvalidKeyError: + try: + source_key = LibraryLocator.from_string(source_key_param) + except InvalidKeyError: + return Response({"error": f"Bad source: {source_key_param}"}, status=400) + else: + return Response({"error": "Source key cannot be blank."}, status=400) + + result = migrator_api.preview_migration(source_key, target_key) + + serializer = PreviewMigrationSerializer(result) + + return Response(serializer.data) diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 1771eaa37f88..3bb60597ea99 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -8,7 +8,7 @@ from contextlib import contextmanager, nullcontext from datetime import datetime, timedelta, timezone from functools import wraps -from typing import Callable, Generator +from typing import Callable, Generator, Optional, cast from django.conf import settings from django.contrib.auth import get_user_model @@ -61,6 +61,8 @@ STUDIO_INDEX_SUFFIX = "studio_content" +Filter = str | list[str | list[str]] + if hasattr(settings, "MEILISEARCH_INDEX_PREFIX"): STUDIO_INDEX_NAME = settings.MEILISEARCH_INDEX_PREFIX + STUDIO_INDEX_SUFFIX else: @@ -981,3 +983,75 @@ def generate_user_token_for_studio_search(request): "index_name": STUDIO_INDEX_NAME, "api_key": restricted_api_key, } + + +def force_array(extra_filter: Optional[Filter]) -> list[str]: + """ + Convert a filter value into a list of strings. + + Strings are wrapped in a list, lists are returned as-is (cast to `list[str]`), + and None results in an empty list. + """ + if isinstance(extra_filter, str): + return [extra_filter] + if isinstance(extra_filter, list): + return cast(list[str], extra_filter) + return [] + + +def fetch_block_types(extra_filter: Optional[Filter]): + """ + Fetch the block types facet distribution for the search results. + """ + extra_filter_formatted = force_array(extra_filter) + + client = _get_meilisearch_client() + index = client.get_index(STUDIO_INDEX_NAME) + + response = index.search( + "", + { + "facets": ["block_type"], + "filter": extra_filter_formatted, + "limit": 0, + } + ) + + return response + + +def get_all_blocks_from_context( + context_key: str, + extra_attributes_to_retrieve: Optional[list[str]], +): + """ + Gets all blocks from a context key using a meilisearch search. + Meilisearch works with limits of 1000 maximum; ensuring we obtain all blocks + requires making several queries. + """ + limit = 1000 + offset = 0 + results = [] + + client = _get_meilisearch_client() + index = client.get_index(STUDIO_INDEX_NAME) + + while True: + response = index.search( + "", + { + "filter": [f'context_key = "{context_key}"'], + "limit": limit, + "offset": offset, + "attributesToRetrieve": ["usage_key"] + extra_attributes_to_retrieve, + } + ) + + hits = response["hits"] + if not hits: + break + + results.extend(hits) + offset += limit + + return results From 4701f55deed2cff74dcea5acd042d94acc7f5331 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 24 Dec 2025 18:33:14 -0500 Subject: [PATCH 2/6] style: Add comments --- cms/djangoapps/modulestore_migrator/api/read_api.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/modulestore_migrator/api/read_api.py b/cms/djangoapps/modulestore_migrator/api/read_api.py index bf765ecc3a7b..8d29f24e9ee6 100644 --- a/cms/djangoapps/modulestore_migrator/api/read_api.py +++ b/cms/djangoapps/modulestore_migrator/api/read_api.py @@ -281,6 +281,7 @@ def preview_migration(source_key: str, target_key: str): TODO: For now, the repeat_handling_strategy is not taken into account. This can be taken into account for a more advanced summary. """ + # Get all containers and components from the source key blocks = get_all_blocks_from_context(source_key, ["block_type", "block_id"]) unsupported_blocks = [] @@ -290,6 +291,8 @@ def preview_migration(source_key: str, target_key: str): subsections = 0 units = 0 blocks_limit = settings.MAX_BLOCKS_PER_CONTENT_LIBRARY + + # Builds the summary: counts every container and verify if each component can be added to the library for block in blocks: block_type = block["block_type"] block_id = block["block_id"] @@ -326,6 +329,7 @@ def preview_migration(source_key: str, target_key: str): elif block_type == "vertical": units += 1 + # Gets the count of children of unsupported blocks quoted_keys = ','.join(f'"{key}"' for key in unsupported_blocks) unsupportedBlocksChildren = fetch_block_types( [ @@ -333,17 +337,17 @@ def preview_migration(source_key: str, target_key: str): f'breadcrumbs.usage_key IN [{quoted_keys}]' ], ) + # Final unsupported blocks count unsupported_blocks_count = len(unsupported_blocks) + unsupportedBlocksChildren["estimatedTotalHits"] + unsupported_percentage = (unsupported_blocks_count / total_blocks) * 100 state = "success" if unsupported_blocks_count: state = "partial" + # Checks if this migration reaches the block limit content_library = ContentLibrary.objects.get_by_key(target_key) target_item_counts = get_all_drafts(content_library.learning_package_id).count() - - unsupported_percentage = (unsupported_blocks_count / total_blocks) * 100 - if target_item_counts + total_blocks - unsupported_blocks_count > blocks_limit: state = "block_limit_reached" From 262671f9f1843bcc3fcb0be562ce2c54fc09636c Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Mon, 29 Dec 2025 11:06:17 -0500 Subject: [PATCH 3/6] refactor: Remove children of unsupported blocks from the summary --- .../modulestore_migrator/api/read_api.py | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/cms/djangoapps/modulestore_migrator/api/read_api.py b/cms/djangoapps/modulestore_migrator/api/read_api.py index 8d29f24e9ee6..1957138764ed 100644 --- a/cms/djangoapps/modulestore_migrator/api/read_api.py +++ b/cms/djangoapps/modulestore_migrator/api/read_api.py @@ -254,22 +254,22 @@ def _block_migration_success( ) -def preview_migration(source_key: str, target_key: str): +def preview_migration(source_key: SourceContextKey, target_key: LibraryLocatorV2): """ Returns a summary preview of the migration given a source key and a target key on this form: ``` { - "state": "block_limit_reached", - "unsupported_blocks": 0, - "unsupported_percentage": 0, - "blocks_limit": blocks_limit, - "total_blocks": 0, - "total_components": 0, - "sections": 0, - "subsections": 0, - "units": 0, + "state": "success", + "unsupported_blocks": 4, + "unsupported_percentage": 25, + "blocks_limit": 1000, + "total_blocks": 20, + "total_components": 10, + "sections": 2, + "subsections": 3, + "units": 5, } ``` @@ -282,7 +282,7 @@ def preview_migration(source_key: str, target_key: str): account for a more advanced summary. """ # Get all containers and components from the source key - blocks = get_all_blocks_from_context(source_key, ["block_type", "block_id"]) + blocks = get_all_blocks_from_context(str(source_key), ["block_type", "block_id"]) unsupported_blocks = [] total_blocks = 0 @@ -292,7 +292,7 @@ def preview_migration(source_key: str, target_key: str): units = 0 blocks_limit = settings.MAX_BLOCKS_PER_CONTENT_LIBRARY - # Builds the summary: counts every container and verify if each component can be added to the library + # Builds the summary: counts every container and verify if each component can be added to the library for block in blocks: block_type = block["block_type"] block_id = block["block_id"] @@ -338,7 +338,9 @@ def preview_migration(source_key: str, target_key: str): ], ) # Final unsupported blocks count - unsupported_blocks_count = len(unsupported_blocks) + unsupportedBlocksChildren["estimatedTotalHits"] + unsupported_blocks_count = len(unsupported_blocks) + total_blocks -= unsupportedBlocksChildren["estimatedTotalHits"] + total_components -= unsupportedBlocksChildren["estimatedTotalHits"] unsupported_percentage = (unsupported_blocks_count / total_blocks) * 100 state = "success" @@ -347,6 +349,7 @@ def preview_migration(source_key: str, target_key: str): # Checks if this migration reaches the block limit content_library = ContentLibrary.objects.get_by_key(target_key) + assert content_library.learning_package_id is not None target_item_counts = get_all_drafts(content_library.learning_package_id).count() if target_item_counts + total_blocks - unsupported_blocks_count > blocks_limit: state = "block_limit_reached" From 0cfb1e85ce532b8d106d769f26fb2210f2d47497 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Mon, 29 Dec 2025 14:05:39 -0500 Subject: [PATCH 4/6] test: Tests added for search api, REST API and migration api new functions --- .../modulestore_migrator/api/read_api.py | 3 +- .../modulestore_migrator/rest_api/v1/views.py | 5 + .../modulestore_migrator/tests/test_api.py | 175 +++++++++++++++++- .../tests/test_rest_api.py | 145 +++++++++++++++ .../content/search/tests/test_api.py | 60 ++++++ 5 files changed, 386 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/modulestore_migrator/api/read_api.py b/cms/djangoapps/modulestore_migrator/api/read_api.py index 1957138764ed..310f9caefb5a 100644 --- a/cms/djangoapps/modulestore_migrator/api/read_api.py +++ b/cms/djangoapps/modulestore_migrator/api/read_api.py @@ -261,7 +261,7 @@ def preview_migration(source_key: SourceContextKey, target_key: LibraryLocatorV2 ``` { - "state": "success", + "state": "partial", "unsupported_blocks": 4, "unsupported_percentage": 25, "blocks_limit": 1000, @@ -338,6 +338,7 @@ def preview_migration(source_key: SourceContextKey, target_key: LibraryLocatorV2 ], ) # Final unsupported blocks count + # The unsupported children are subtracted from the totals since they have already been counted in the first query. unsupported_blocks_count = len(unsupported_blocks) total_blocks -= unsupportedBlocksChildren["estimatedTotalHits"] total_components -= unsupportedBlocksChildren["estimatedTotalHits"] diff --git a/cms/djangoapps/modulestore_migrator/rest_api/v1/views.py b/cms/djangoapps/modulestore_migrator/rest_api/v1/views.py index 8d684a779e0d..fa46a2c57005 100644 --- a/cms/djangoapps/modulestore_migrator/rest_api/v1/views.py +++ b/cms/djangoapps/modulestore_migrator/rest_api/v1/views.py @@ -744,6 +744,11 @@ def get(self, request: Request): else: return Response({"error": "Source key cannot be blank."}, status=400) + lib_api.require_permission_for_library_key( + target_key, + request.user, + lib_api.permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, + ) result = migrator_api.preview_migration(source_key, target_key) serializer = PreviewMigrationSerializer(result) diff --git a/cms/djangoapps/modulestore_migrator/tests/test_api.py b/cms/djangoapps/modulestore_migrator/tests/test_api.py index c9e2fc3b587b..05a4992c8a98 100644 --- a/cms/djangoapps/modulestore_migrator/tests/test_api.py +++ b/cms/djangoapps/modulestore_migrator/tests/test_api.py @@ -3,7 +3,8 @@ """ import pytest -from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2 +from unittest.mock import patch +from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2, CourseLocator from openedx_learning.api import authoring as authoring_api from organizations.tests.factories import OrganizationFactory @@ -58,6 +59,7 @@ def setUp(self): ] # We load this last so that it has an updated list of children. self.lib_v1 = self.store.get_library(self.lib_key_v1) + self.course_key = CourseLocator.from_string('course-v1:TestOrg+TestCourse+TestRun') def test_start_migration_to_library(self): """ @@ -566,3 +568,174 @@ def test_migration_api_for_various_scenarios(self): forwarded_blocks = api.get_forwarding_for_blocks(all_source_usage_keys) assert forwarded_blocks[self.source_html_keys[1]].target_key.context_key == self.lib_key_v2_1 assert forwarded_blocks[self.source_unit_keys[1]].target_key.context_key == self.lib_key_v2_1 + + @patch('cms.djangoapps.modulestore_migrator.api.read_api.fetch_block_types') + @patch('cms.djangoapps.modulestore_migrator.api.read_api.get_all_blocks_from_context') + def test_preview_migration_success(self, mock_get_blocks, mock_fetch_block_types): + mock_get_blocks.return_value = [ + { + 'block_type': 'html', + 'block_id': '16', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@html+block@16', + }, + { + 'block_type': 'video', + 'block_id': '17', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@video+block@17', + }, + { + 'block_type': 'chapter', + 'block_id': '18', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@chapter+block@18', + }, + { + 'block_type': 'sequential', + 'block_id': '19', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@19', + }, + { + 'block_type': 'vertical', + 'block_id': '20', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@20', + }, + ] + mock_fetch_block_types.return_value = { + "estimatedTotalHits": 0, + } + + expected = { + "state": "success", + "unsupported_blocks": 0, + "unsupported_percentage": 0, + "blocks_limit": 1000, + "total_blocks": 5, + "total_components": 2, + "sections": 1, + "subsections": 1, + "units": 1, + } + + with self.settings(MAX_BLOCKS_PER_CONTENT_LIBRARY=1000): + results = api.preview_migration(self.course_key, self.lib_key_v2_1) + assert results == expected + + @patch('cms.djangoapps.modulestore_migrator.api.read_api.fetch_block_types') + @patch('cms.djangoapps.modulestore_migrator.api.read_api.get_all_blocks_from_context') + def test_preview_migration_partial(self, mock_get_blocks, mock_fetch_block_types): + mock_get_blocks.return_value = [ + { + 'block_type': 'html', + 'block_id': '16', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@html+block@16', + }, + { + 'block_type': 'video', + 'block_id': '17', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@video+block@17', + }, + { + 'block_type': 'chapter', + 'block_id': '18', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@chapter+block@18', + }, + { + 'block_type': 'sequential', + 'block_id': '19', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@19', + }, + { + 'block_type': 'vertical', + 'block_id': '20', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@20', + }, + { + 'block_type': 'invalid', + 'block_id': '21', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@invalid+block@21', + }, # Invalid + { + 'block_type': 'item_bank', + 'block_id': '22', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@item_bank+block@22', + }, # Invalid + { + 'block_type': 'html', + 'block_id': '23', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@html+block@23', + }, # Child of item bank + { + 'block_type': 'html', + 'block_id': '24', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@html+block@24', + }, # Child of item bank + ] + mock_fetch_block_types.return_value = { + "estimatedTotalHits": 2, + } + + # The unsupported children are not included in the summary + expected = { + "state": "partial", + "unsupported_blocks": 2, + "unsupported_percentage": 28.57142857142857, + "blocks_limit": 1000, + "total_blocks": 7, + "total_components": 4, + "sections": 1, + "subsections": 1, + "units": 1, + } + + with self.settings(MAX_BLOCKS_PER_CONTENT_LIBRARY=1000): + results = api.preview_migration(self.course_key, self.lib_key_v2_1) + assert results == expected + + @patch('cms.djangoapps.modulestore_migrator.api.read_api.fetch_block_types') + @patch('cms.djangoapps.modulestore_migrator.api.read_api.get_all_blocks_from_context') + def test_preview_migration_block_limit(self, mock_get_blocks, mock_fetch_block_types): + mock_get_blocks.return_value = [ + { + 'block_type': 'html', + 'block_id': '16', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@html+block@16', + }, + { + 'block_type': 'video', + 'block_id': '17', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@video+block@17', + }, + { + 'block_type': 'chapter', + 'block_id': '18', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@chapter+block@18', + }, + { + 'block_type': 'sequential', + 'block_id': '19', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@19', + }, + { + 'block_type': 'vertical', + 'block_id': '20', + 'usage_key': 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@20', + }, + ] + mock_fetch_block_types.return_value = { + "estimatedTotalHits": 0, + } + + expected = { + "state": "block_limit_reached", + "unsupported_blocks": 0, + "unsupported_percentage": 0, + "blocks_limit": 4, + "total_blocks": 5, + "total_components": 2, + "sections": 1, + "subsections": 1, + "units": 1, + } + + with self.settings(MAX_BLOCKS_PER_CONTENT_LIBRARY=4): + results = api.preview_migration(self.course_key, self.lib_key_v2_1) + assert results == expected diff --git a/cms/djangoapps/modulestore_migrator/tests/test_rest_api.py b/cms/djangoapps/modulestore_migrator/tests/test_rest_api.py index 83c214015ed9..78472d41098c 100644 --- a/cms/djangoapps/modulestore_migrator/tests/test_rest_api.py +++ b/cms/djangoapps/modulestore_migrator/tests/test_rest_api.py @@ -32,6 +32,7 @@ BulkMigrationViewSet, MigrationInfoViewSet, MigrationViewSet, + PreviewMigration, ) from openedx.core.djangoapps.content_libraries import api as lib_api @@ -1109,3 +1110,147 @@ def test_get_block_migration_info_without_library_access(self, mock_lib_api): response = self.view(request) assert response.status_code == status.HTTP_403_FORBIDDEN + + +class TestPreviewMigration(TestCase): + """ + Test the PreviewMigration.get() endpoint. + """ + def setUp(self): + """Set up test fixtures.""" + self.factory = APIRequestFactory() + self.view = PreviewMigration.as_view() + + self.user = User.objects.create_user( + username='testuser', + email='testuser@test.com', + password='password' + ) + + @patch('cms.djangoapps.modulestore_migrator.rest_api.v1.views.migrator_api') + @patch('cms.djangoapps.modulestore_migrator.rest_api.v1.views.lib_api') + def test_preview_migration_success(self, mock_lib_api, mock_migrator_api): + """ + Test successful retrieval of preview migration. + """ + mock_lib_api.require_permission_for_library_key.return_value = None + + expected = { + "state": "partial", + "unsupported_blocks": 4, + "unsupported_percentage": 25, + "blocks_limit": 1000, + "total_blocks": 20, + "total_components": 10, + "sections": 2, + "subsections": 3, + "units": 5, + } + + mock_migrator_api.preview_migration.return_value = expected + + request = self.factory.get( + '/api/modulestore_migrator/v1/migration_preview/', + { + 'target_key': 'lib:TestOrg:TestLibrary', + 'source_key': 'course-v1:TestOrg+TestCourse+TestRun', + } + ) + force_authenticate(request, user=self.user) + + response = self.view(request) + + assert response.status_code == status.HTTP_200_OK + + for key, value in expected.items(): + assert response.data[key] == value + + @patch('cms.djangoapps.modulestore_migrator.rest_api.v1.views.lib_api') + def test_preview_migration_without_library_access(self, mock_lib_api): + """ + Test that users without library view access get 403 Forbidden. + """ + mock_lib_api.require_permission_for_library_key.side_effect = PermissionDenied( + "User lacks permission to view this library" + ) + + request = self.factory.get( + '/api/modulestore_migrator/v1/migration_preview/', + { + 'target_key': 'lib:TestOrg:TestLibrary', + 'source_key': 'course-v1:TestOrg+TestCourse+TestRun', + } + ) + force_authenticate(request, user=self.user) + + response = self.view(request) + + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_preview_migration_missing_target_key(self): + """ + Test that missing target_key parameter returns 400 Bad Request. + """ + request = self.factory.get( + '/api/modulestore_migrator/v1/migration_preview/', + { + 'source_key': 'course-v1:TestOrg+TestCourse+TestRun', + } + ) + force_authenticate(request, user=self.user) + + response = self.view(request) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert 'target' in response.data.get('error', '').lower() + + def test_preview_migration_invalid_target_key(self): + """ + Test that invalid target_key returns 400 Bad Request. + """ + request = self.factory.get( + '/api/modulestore_migrator/v1/migration_preview/', + { + 'target_key': 'not-a-valid-key', + 'source_key': 'course-v1:TestOrg+TestCourse+TestRun', + } + ) + force_authenticate(request, user=self.user) + + response = self.view(request) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + + def test_preview_migration_missing_source_key(self): + """ + Test that missing target_key parameter returns 400 Bad Request. + """ + request = self.factory.get( + '/api/modulestore_migrator/v1/migration_preview/', + { + 'target_key': 'lib:TestOrg:TestLibrary', + } + ) + force_authenticate(request, user=self.user) + + response = self.view(request) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert 'source' in response.data.get('error', '').lower() + + def test_preview_migration_invalid_source_key(self): + """ + Test that invalid target_key returns 400 Bad Request. + """ + request = self.factory.get( + '/api/modulestore_migrator/v1/migration_preview/', + { + 'target_key': 'lib:TestOrg:TestLibrary', + 'source_key': 'not-a-valid-key', + } + ) + force_authenticate(request, user=self.user) + + response = self.view(request) + + assert response.status_code == status.HTTP_400_BAD_REQUEST diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index e1b6f8fe16b0..47e95cb4b759 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -21,6 +21,7 @@ from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangoapps.content_libraries import api as library_api from openedx.core.djangoapps.content_tagging import api as tagging_api +from openedx.core.djangoapps.content.search import api as search_api from openedx.core.djangoapps.content.course_overviews.api import CourseOverview from openedx.core.djangolib.testing.utils import skip_unless_cms from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase @@ -1233,3 +1234,62 @@ def test_section_in_usbsections(self, mock_meilisearch) -> None: ], any_order=True, ) + + @override_settings(MEILISEARCH_ENABLED=True) + def test_fetch_block_types(self, mock_meilisearch): + mock_index = mock_meilisearch.return_value.get_index.return_value + search_api.fetch_block_types('context_key = test') + + mock_index.search.assert_called_once_with( + "", + { + "facets": ["block_type"], + "filter": ['context_key = test'], + "limit": 0, + } + ) + + @override_settings(MEILISEARCH_ENABLED=True) + def test_get_all_blocks_from_context(self, mock_meilisearch): + mock_index = mock_meilisearch.return_value.get_index.return_value + expected_result = [ + {"usage_key": "block-v1:test+type@html+block@1"}, + {"usage_key": "block-v1:test+type@video+block@2"}, + ] + + # Simulate two pages: one with results and one empty (while loop ends) + mock_index.search.side_effect = [ + { + "hits": expected_result, + }, + { + "hits": [], + }, + ] + + result = search_api.get_all_blocks_from_context( + context_key="course-v1:TestOrg+TestCourse+TestRun", + extra_attributes_to_retrieve=["display_name"], + ) + + assert result == expected_result + assert mock_index.search.call_count == 2 + mock_index.search.assert_any_call( + "", + { + "filter": ['context_key = "course-v1:TestOrg+TestCourse+TestRun"'], + "limit": 1000, + "offset": 0, + "attributesToRetrieve": ["usage_key", "display_name"], + } + ) + + mock_index.search.assert_any_call( + "", + { + "filter": ['context_key = "course-v1:TestOrg+TestCourse+TestRun"'], + "limit": 1000, + "offset": 1000, + "attributesToRetrieve": ["usage_key", "display_name"], + } + ) From 3a8661818132b27633ae778fb3c3ca90b27b6d5e Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 30 Dec 2025 11:34:36 -0500 Subject: [PATCH 5/6] fix: Fix broken tests --- openedx/core/djangoapps/content/search/tests/test_api.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 47e95cb4b759..3c441999e4a3 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -21,7 +21,6 @@ from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangoapps.content_libraries import api as library_api from openedx.core.djangoapps.content_tagging import api as tagging_api -from openedx.core.djangoapps.content.search import api as search_api from openedx.core.djangoapps.content.course_overviews.api import CourseOverview from openedx.core.djangolib.testing.utils import skip_unless_cms from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase @@ -1237,8 +1236,10 @@ def test_section_in_usbsections(self, mock_meilisearch) -> None: @override_settings(MEILISEARCH_ENABLED=True) def test_fetch_block_types(self, mock_meilisearch): + from openedx.core.djangoapps.content.search.api import fetch_block_types + mock_index = mock_meilisearch.return_value.get_index.return_value - search_api.fetch_block_types('context_key = test') + fetch_block_types('context_key = test') mock_index.search.assert_called_once_with( "", @@ -1251,6 +1252,8 @@ def test_fetch_block_types(self, mock_meilisearch): @override_settings(MEILISEARCH_ENABLED=True) def test_get_all_blocks_from_context(self, mock_meilisearch): + from openedx.core.djangoapps.content.search.api import get_all_blocks_from_context + mock_index = mock_meilisearch.return_value.get_index.return_value expected_result = [ {"usage_key": "block-v1:test+type@html+block@1"}, @@ -1267,7 +1270,7 @@ def test_get_all_blocks_from_context(self, mock_meilisearch): }, ] - result = search_api.get_all_blocks_from_context( + result = get_all_blocks_from_context( context_key="course-v1:TestOrg+TestCourse+TestRun", extra_attributes_to_retrieve=["display_name"], ) From 5f9b576f5d1bb3896b98f0b75e07069ff5c587ed Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 31 Dec 2025 09:32:23 -0500 Subject: [PATCH 6/6] style: Nits on the code --- .../modulestore_migrator/api/read_api.py | 2 +- openedx/core/djangoapps/content/search/api.py | 24 +++++++++---------- .../content/search/tests/test_api.py | 6 +++-- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/cms/djangoapps/modulestore_migrator/api/read_api.py b/cms/djangoapps/modulestore_migrator/api/read_api.py index 310f9caefb5a..caed2c8c9055 100644 --- a/cms/djangoapps/modulestore_migrator/api/read_api.py +++ b/cms/djangoapps/modulestore_migrator/api/read_api.py @@ -352,7 +352,7 @@ def preview_migration(source_key: SourceContextKey, target_key: LibraryLocatorV2 content_library = ContentLibrary.objects.get_by_key(target_key) assert content_library.learning_package_id is not None target_item_counts = get_all_drafts(content_library.learning_package_id).count() - if target_item_counts + total_blocks - unsupported_blocks_count > blocks_limit: + if (target_item_counts + total_blocks - unsupported_blocks_count) > blocks_limit: state = "block_limit_reached" return { diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 3bb60597ea99..217dfbd88dc3 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -5,10 +5,11 @@ import logging import time +from collections.abc import Iterator from contextlib import contextmanager, nullcontext from datetime import datetime, timedelta, timezone from functools import wraps -from typing import Callable, Generator, Optional, cast +from typing import Callable, Generator, cast from django.conf import settings from django.contrib.auth import get_user_model @@ -985,7 +986,7 @@ def generate_user_token_for_studio_search(request): } -def force_array(extra_filter: Optional[Filter]) -> list[str]: +def force_array(extra_filter: Filter | None = None) -> list[str]: """ Convert a filter value into a list of strings. @@ -999,7 +1000,7 @@ def force_array(extra_filter: Optional[Filter]) -> list[str]: return [] -def fetch_block_types(extra_filter: Optional[Filter]): +def fetch_block_types(extra_filter: Filter | None = None): """ Fetch the block types facet distribution for the search results. """ @@ -1022,16 +1023,15 @@ def fetch_block_types(extra_filter: Optional[Filter]): def get_all_blocks_from_context( context_key: str, - extra_attributes_to_retrieve: Optional[list[str]], -): + extra_attributes_to_retrieve: list[str] | None = None, +) -> Iterator[dict]: """ - Gets all blocks from a context key using a meilisearch search. + Lazily yields all blocks for a given context key using Meilisearch pagination. Meilisearch works with limits of 1000 maximum; ensuring we obtain all blocks requires making several queries. """ limit = 1000 offset = 0 - results = [] client = _get_meilisearch_client() index = client.get_index(STUDIO_INDEX_NAME) @@ -1043,15 +1043,13 @@ def get_all_blocks_from_context( "filter": [f'context_key = "{context_key}"'], "limit": limit, "offset": offset, - "attributesToRetrieve": ["usage_key"] + extra_attributes_to_retrieve, + "attributesToRetrieve": ["usage_key"] + (extra_attributes_to_retrieve or []), } ) - hits = response["hits"] - if not hits: + yield from response["hits"] + + if response["estimatedTotalHits"] <= offset + limit: break - results.extend(hits) offset += limit - - return results diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 3c441999e4a3..f0213eae75a2 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -1264,16 +1264,18 @@ def test_get_all_blocks_from_context(self, mock_meilisearch): mock_index.search.side_effect = [ { "hits": expected_result, + "estimatedTotalHits": 1200, }, { "hits": [], + "estimatedTotalHits": 1200, }, ] - result = get_all_blocks_from_context( + result = list(get_all_blocks_from_context( context_key="course-v1:TestOrg+TestCourse+TestRun", extra_attributes_to_retrieve=["display_name"], - ) + )) assert result == expected_result assert mock_index.search.call_count == 2