From 9eba4ba1af49342516655daa4bba5c63f65315dc Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 18 Nov 2025 19:18:25 -0500 Subject: [PATCH 01/10] feat: Some fields in modulestore migratior serializers --- .../rest_api/v1/serializers.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py b/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py index 8c9f15387647..f5ef35ad9231 100644 --- a/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py +++ b/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py @@ -13,6 +13,13 @@ from cms.djangoapps.modulestore_migrator.data import CompositionLevel, RepeatHandlingStrategy from cms.djangoapps.modulestore_migrator.models import ModulestoreMigration, ModulestoreSource +class LibraryMigrationCollectionSerializer(serializers.ModelSerializer): + """ + Serializer for the target collection of a library migration. + """ + class Meta: + model = Collection + fields = ["key", "title"] class ModulestoreMigrationSerializer(serializers.Serializer): """ @@ -50,6 +57,7 @@ class ModulestoreMigrationSerializer(serializers.Serializer): allow_blank=True, default=None, ) + target_collection = LibraryMigrationCollectionSerializer(required=False) forward_source_to_target = serializers.BooleanField( help_text="Forward references of this block source over to the target of this block migration.", required=False, @@ -223,19 +231,11 @@ def get_display_name(self, obj): return self.context["course_names"].get(str(obj.key), None) -class LibraryMigrationCollectionSerializer(serializers.ModelSerializer): - """ - Serializer for the target collection of a library migration. - """ - class Meta: - model = Collection - fields = ["key", "title"] - - class LibraryMigrationCourseSerializer(serializers.ModelSerializer): """ Serializer for the course or legacylibrary migrations to V2 library. """ + task_uuid = serializers.UUIDField(source='task_status.uuid', read_only=True) source = LibraryMigrationCourseSourceSerializer() # type: ignore[assignment] target_collection = LibraryMigrationCollectionSerializer(required=False) state = serializers.SerializerMethodField() @@ -244,6 +244,7 @@ class LibraryMigrationCourseSerializer(serializers.ModelSerializer): class Meta: model = ModulestoreMigration fields = [ + 'task_uuid', 'source', 'target_collection', 'state', From 9dea95c657194f6e4bc30c225cc8e783faae1824 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 19 Nov 2025 20:51:07 -0500 Subject: [PATCH 02/10] feat: Add migration_summary to Migrations --- ..._modulestoremigration_migration_summary.py | 18 ++++++++ cms/djangoapps/modulestore_migrator/models.py | 4 ++ .../rest_api/v1/serializers.py | 12 ++++++ cms/djangoapps/modulestore_migrator/tasks.py | 42 +++++++++++++++++-- .../content_libraries/api/blocks.py | 2 +- 5 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 cms/djangoapps/modulestore_migrator/migrations/0004_modulestoremigration_migration_summary.py diff --git a/cms/djangoapps/modulestore_migrator/migrations/0004_modulestoremigration_migration_summary.py b/cms/djangoapps/modulestore_migrator/migrations/0004_modulestoremigration_migration_summary.py new file mode 100644 index 000000000000..26400c7f42a1 --- /dev/null +++ b/cms/djangoapps/modulestore_migrator/migrations/0004_modulestoremigration_migration_summary.py @@ -0,0 +1,18 @@ +# Generated by Django 5.2.7 on 2025-11-19 21:21 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('modulestore_migrator', '0003_modulestoremigration_is_failed'), + ] + + operations = [ + migrations.AddField( + model_name='modulestoremigration', + name='migration_summary', + field=models.JSONField(default=dict, help_text='Summary that contains number of sections, subsections, units and components migrated'), + ), + ] diff --git a/cms/djangoapps/modulestore_migrator/models.py b/cms/djangoapps/modulestore_migrator/models.py index 810333fc9be9..0d01b8f733a4 100644 --- a/cms/djangoapps/modulestore_migrator/models.py +++ b/cms/djangoapps/modulestore_migrator/models.py @@ -142,6 +142,10 @@ class ModulestoreMigration(models.Model): "is the migration failed?" ), ) + migration_summary = models.JSONField( + default=dict, + help_text=_("Summary that contains number of sections, subsections, units and components migrated"), + ) def __str__(self): return ( diff --git a/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py b/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py index f5ef35ad9231..0900f8d208e7 100644 --- a/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py +++ b/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py @@ -21,6 +21,14 @@ class Meta: model = Collection fields = ["key", "title"] +class MigrationSummarySerializer(serializers.Serializer): + total_blocks = serializers.IntegerField(required=False) + sections = serializers.IntegerField(required=False) + subsections = serializers.IntegerField(required=False) + units = serializers.IntegerField(required=False) + components = serializers.IntegerField(required=False) + unsupported = serializers.IntegerField(required=False) + class ModulestoreMigrationSerializer(serializers.Serializer): """ Serializer for the course or legacylibrary to library V2 import creation API. @@ -68,6 +76,10 @@ class ModulestoreMigrationSerializer(serializers.Serializer): required=False, default=False, ) + migration_summary = MigrationSummarySerializer( + help_text="Summary of the finished migration", + required=False + ) def get_fields(self): fields = super().get_fields() diff --git a/cms/djangoapps/modulestore_migrator/tasks.py b/cms/djangoapps/modulestore_migrator/tasks.py index ef2386c69d34..e56ff805e4f6 100644 --- a/cms/djangoapps/modulestore_migrator/tasks.py +++ b/cms/djangoapps/modulestore_migrator/tasks.py @@ -135,6 +135,7 @@ class _MigrationContext: composition_level: CompositionLevel repeat_handling_strategy: RepeatHandlingStrategy preserve_url_slugs: bool + migration_summary: dict[str, int] created_by: int created_at: datetime @@ -160,6 +161,20 @@ def add_migration(self, source_key: UsageKey, target: PublishableEntity) -> None self.existing_source_to_target_keys[source_key] = [target] else: self.existing_source_to_target_keys[source_key].append(target) + + def add_block_to_summary(self, container_type: ContainerType | None, is_unsupported = False): + """Add a block to the migration summary using the container_type""" + self.migration_summary["total_blocks"] += 1 + if is_unsupported: + self.migration_summary["unsupported"] += 1 + elif container_type is None: + self.migration_summary["components"] += 1 + elif container_type is ContainerType.Unit: + self.migration_summary["units"] += 1 + elif container_type is ContainerType.Subsection: + self.migration_summary["subsections"] += 1 + elif container_type is ContainerType.Section: + self.migration_summary["sections"] += 1 def get_existing_target_entity_keys(self, base_key: str) -> set[str]: return set( @@ -381,6 +396,15 @@ def _import_structure( modulestore_blocks, key=lambda x: x.source.key) } + migration_summary = { + "total_blocks": 0, + "sections": 0, + "subsections": 0, + "units": 0, + "components": 0, + "unsupported": 0, + } + migration_context = _MigrationContext( existing_source_to_target_keys=existing_source_to_target_keys, target_package_id=migration.target.pk, @@ -390,6 +414,7 @@ def _import_structure( composition_level=CompositionLevel(migration.composition_level), repeat_handling_strategy=RepeatHandlingStrategy(migration.repeat_handling_strategy), preserve_url_slugs=migration.preserve_url_slugs, + migration_summary=migration_summary, created_by=status.user_id, created_at=datetime.now(timezone.utc), ) @@ -400,7 +425,7 @@ def _import_structure( source_node=root_node, ) change_log.save() - return change_log, root_migrated_node + return change_log, root_migrated_node, migration_context.migration_summary def _forwarding_content(source_data: _MigrationSourceData) -> None: @@ -618,7 +643,7 @@ def migrate_from_modulestore( # Importing structure of the legacy block status.set_state(MigrationStep.IMPORTING_STRUCTURE.value) - change_log, root_migrated_node = _import_structure( + change_log, root_migrated_node, migration_summary = _import_structure( migration, source_data, target_library, @@ -627,6 +652,7 @@ def migrate_from_modulestore( status, ) migration.change_log = change_log + migration.migration_summary = migration_summary status.increment_completed_steps() status.set_state(MigrationStep.UNSTAGING.value) @@ -652,6 +678,8 @@ def migrate_from_modulestore( if target_collection: _populate_collection(user_id, migration) status.increment_completed_steps() + + migration.save() except Exception as exc: # pylint: disable=broad-exception-caught _set_migrations_to_fail([source_data]) status.fail(str(exc)) @@ -824,7 +852,7 @@ def bulk_migrate_from_modulestore( f"{MigrationStep.STAGING.BULK_MIGRATION_PREFIX} ({source_pk}): " f"{MigrationStep.IMPORTING_STRUCTURE.value}" ) - change_log, root_migrated_node = _import_structure( + change_log, root_migrated_node, migration_summary = _import_structure( source_data.migration, source_data, target_library, @@ -833,6 +861,7 @@ def bulk_migrate_from_modulestore( status, ) source_data.migration.change_log = change_log + source_data.migration.migration_summary = migration_summary status.increment_completed_steps() status.set_state( @@ -916,7 +945,7 @@ def bulk_migrate_from_modulestore( ModulestoreMigration.objects.bulk_update( [x.migration for x in source_data_list], - ["target_collection", "is_failed"], + ["target_collection", "is_failed", "migration_summary"], ) status.increment_completed_steps() except Exception as exc: # pylint: disable=broad-exception-caught @@ -1024,6 +1053,11 @@ def _migrate_node( if target_entity_version: source_to_target = (source_key, target_entity_version) context.add_migration(source_key, target_entity_version.entity) + context.add_block_to_summary(container_type, is_unsupported=target_entity_version is None) + if container_type is None and target_entity_version is None: + # Currently, components with children are not supported, but they appear as unsupported in the summary. + for _ in source_node.getchildren(): + context.add_block_to_summary(None, is_unsupported=True) else: log.warning( f"Cannot migrate node from {context.source_context_key} to {context.target_library_key} " diff --git a/openedx/core/djangoapps/content_libraries/api/blocks.py b/openedx/core/djangoapps/content_libraries/api/blocks.py index be5b296147fd..7dbcbf69b01c 100644 --- a/openedx/core/djangoapps/content_libraries/api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/api/blocks.py @@ -309,7 +309,7 @@ def validate_can_add_block_to_library( block_class = XBlock.load_class(block_type) # Will raise an exception if invalid if block_class.has_children: raise IncompatibleTypesError( - 'The "{block_type}" XBlock (ID: "{block_id}") has children, so it not supported in content libraries', + f'The "{block_type}" XBlock (ID: "{block_id}") has children, so it not supported in content libraries', ) # Make sure the new ID is not taken already: usage_key = LibraryUsageLocatorV2( # type: ignore[abstract] From 2259c81d63581632c68acd3de89e68acbffe22e9 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 20 Nov 2025 17:32:39 -0500 Subject: [PATCH 03/10] feat: Add unsupported reasons to migrations --- ...odulestoremigration_unsupported_reasons.py | 18 ++++++ cms/djangoapps/modulestore_migrator/models.py | 4 ++ .../rest_api/v1/serializers.py | 14 +++++ cms/djangoapps/modulestore_migrator/tasks.py | 46 +++++++++++---- .../modulestore_migrator/tests/test_tasks.py | 57 ++++++++++++++++++- .../content_libraries/api/blocks.py | 6 +- 6 files changed, 132 insertions(+), 13 deletions(-) create mode 100644 cms/djangoapps/modulestore_migrator/migrations/0005_modulestoremigration_unsupported_reasons.py diff --git a/cms/djangoapps/modulestore_migrator/migrations/0005_modulestoremigration_unsupported_reasons.py b/cms/djangoapps/modulestore_migrator/migrations/0005_modulestoremigration_unsupported_reasons.py new file mode 100644 index 000000000000..3aa993eb4b2d --- /dev/null +++ b/cms/djangoapps/modulestore_migrator/migrations/0005_modulestoremigration_unsupported_reasons.py @@ -0,0 +1,18 @@ +# Generated by Django 5.2.7 on 2025-11-20 19:41 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('modulestore_migrator', '0004_modulestoremigration_migration_summary'), + ] + + operations = [ + migrations.AddField( + model_name='modulestoremigration', + name='unsupported_reasons', + field=models.JSONField(default=list, help_text='List of unsuported blocks with the reasons'), + ), + ] diff --git a/cms/djangoapps/modulestore_migrator/models.py b/cms/djangoapps/modulestore_migrator/models.py index 0d01b8f733a4..144d1d0cd27a 100644 --- a/cms/djangoapps/modulestore_migrator/models.py +++ b/cms/djangoapps/modulestore_migrator/models.py @@ -146,6 +146,10 @@ class ModulestoreMigration(models.Model): default=dict, help_text=_("Summary that contains number of sections, subsections, units and components migrated"), ) + unsupported_reasons = models.JSONField( + default=list, + help_text=_("List of unsuported blocks with the reasons"), + ) def __str__(self): return ( diff --git a/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py b/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py index 0900f8d208e7..6788b0d3ecfd 100644 --- a/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py +++ b/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py @@ -13,6 +13,7 @@ from cms.djangoapps.modulestore_migrator.data import CompositionLevel, RepeatHandlingStrategy from cms.djangoapps.modulestore_migrator.models import ModulestoreMigration, ModulestoreSource + class LibraryMigrationCollectionSerializer(serializers.ModelSerializer): """ Serializer for the target collection of a library migration. @@ -21,6 +22,7 @@ class Meta: model = Collection fields = ["key", "title"] + class MigrationSummarySerializer(serializers.Serializer): total_blocks = serializers.IntegerField(required=False) sections = serializers.IntegerField(required=False) @@ -29,6 +31,13 @@ class MigrationSummarySerializer(serializers.Serializer): components = serializers.IntegerField(required=False) unsupported = serializers.IntegerField(required=False) + +class MigrationBlockUnsupportedReasonSerializer(serializers.Serializer): + block_name = serializers.CharField(required=False) + block_type = serializers.CharField(required=False) + reason = serializers.CharField(required=False) + + class ModulestoreMigrationSerializer(serializers.Serializer): """ Serializer for the course or legacylibrary to library V2 import creation API. @@ -80,6 +89,11 @@ class ModulestoreMigrationSerializer(serializers.Serializer): help_text="Summary of the finished migration", required=False ) + unsupported_reasons = MigrationBlockUnsupportedReasonSerializer( + help_text="List of unsupported blocks with the reason", + required=False, + many=True + ) def get_fields(self): fields = super().get_fields() diff --git a/cms/djangoapps/modulestore_migrator/tasks.py b/cms/djangoapps/modulestore_migrator/tasks.py index e56ff805e4f6..e97a3d3c8622 100644 --- a/cms/djangoapps/modulestore_migrator/tasks.py +++ b/cms/djangoapps/modulestore_migrator/tasks.py @@ -136,6 +136,7 @@ class _MigrationContext: repeat_handling_strategy: RepeatHandlingStrategy preserve_url_slugs: bool migration_summary: dict[str, int] + unsupported_reasons: list[dict[str, str]] created_by: int created_at: datetime @@ -161,12 +162,12 @@ def add_migration(self, source_key: UsageKey, target: PublishableEntity) -> None self.existing_source_to_target_keys[source_key] = [target] else: self.existing_source_to_target_keys[source_key].append(target) - - def add_block_to_summary(self, container_type: ContainerType | None, is_unsupported = False): + + def add_block_to_summary(self, container_type: ContainerType | None, is_unsupported=False): """Add a block to the migration summary using the container_type""" self.migration_summary["total_blocks"] += 1 if is_unsupported: - self.migration_summary["unsupported"] += 1 + self.migration_summary["unsupported"] += 1 elif container_type is None: self.migration_summary["components"] += 1 elif container_type is ContainerType.Unit: @@ -176,6 +177,13 @@ def add_block_to_summary(self, container_type: ContainerType | None, is_unsuppor elif container_type is ContainerType.Section: self.migration_summary["sections"] += 1 + def add_unsupported_reason(self, block_name: str, block_type: str, reason: str): + self.unsupported_reasons.append({ + "block_name": block_name, + "block_type": block_type, + "reason": reason, + }) + def get_existing_target_entity_keys(self, base_key: str) -> set[str]: return set( publishable_entity.key @@ -415,6 +423,7 @@ def _import_structure( repeat_handling_strategy=RepeatHandlingStrategy(migration.repeat_handling_strategy), preserve_url_slugs=migration.preserve_url_slugs, migration_summary=migration_summary, + unsupported_reasons=[], created_by=status.user_id, created_at=datetime.now(timezone.utc), ) @@ -425,7 +434,9 @@ def _import_structure( source_node=root_node, ) change_log.save() - return change_log, root_migrated_node, migration_context.migration_summary + migration.migration_summary = migration_context.migration_summary + migration.unsupported_reasons = migration_context.unsupported_reasons + return change_log, root_migrated_node def _forwarding_content(source_data: _MigrationSourceData) -> None: @@ -643,7 +654,7 @@ def migrate_from_modulestore( # Importing structure of the legacy block status.set_state(MigrationStep.IMPORTING_STRUCTURE.value) - change_log, root_migrated_node, migration_summary = _import_structure( + change_log, root_migrated_node = _import_structure( migration, source_data, target_library, @@ -652,7 +663,6 @@ def migrate_from_modulestore( status, ) migration.change_log = change_log - migration.migration_summary = migration_summary status.increment_completed_steps() status.set_state(MigrationStep.UNSTAGING.value) @@ -852,7 +862,7 @@ def bulk_migrate_from_modulestore( f"{MigrationStep.STAGING.BULK_MIGRATION_PREFIX} ({source_pk}): " f"{MigrationStep.IMPORTING_STRUCTURE.value}" ) - change_log, root_migrated_node, migration_summary = _import_structure( + change_log, root_migrated_node = _import_structure( source_data.migration, source_data, target_library, @@ -861,7 +871,6 @@ def bulk_migrate_from_modulestore( status, ) source_data.migration.change_log = change_log - source_data.migration.migration_summary = migration_summary status.increment_completed_steps() status.set_state( @@ -945,7 +954,12 @@ def bulk_migrate_from_modulestore( ModulestoreMigration.objects.bulk_update( [x.migration for x in source_data_list], - ["target_collection", "is_failed", "migration_summary"], + [ + "target_collection", + "is_failed", + "migration_summary", + "unsupported_reasons", + ], ) status.increment_completed_steps() except Exception as exc: # pylint: disable=broad-exception-caught @@ -1056,9 +1070,20 @@ def _migrate_node( context.add_block_to_summary(container_type, is_unsupported=target_entity_version is None) if container_type is None and target_entity_version is None: # Currently, components with children are not supported, but they appear as unsupported in the summary. - for _ in source_node.getchildren(): + for child_node in source_node.getchildren(): context.add_block_to_summary(None, is_unsupported=True) + context.add_unsupported_reason( + child_node.get('display_name'), + child_node.tag, + str(_(f"The block is a child of this unsupported block: {title}")), + ) else: + context.add_block_to_summary(None, is_unsupported=True) + context.add_unsupported_reason( + source_node.get('display_name'), + source_node.tag, + str(_("The block lacks an url_name and thus has no identity.")), + ) log.warning( f"Cannot migrate node from {context.source_context_key} to {context.target_library_key} " f"because it lacks an url_name and thus has no identity: {source_olx}" @@ -1175,6 +1200,7 @@ def _migrate_component( ) except libraries_api.IncompatibleTypesError as e: log.error(f"Error validating block for library {context.target_library_key}: {e}") + context.add_unsupported_reason(title, target_key.block_type, str(e)) return None component = authoring_api.create_component( context.target_package_id, diff --git a/cms/djangoapps/modulestore_migrator/tests/test_tasks.py b/cms/djangoapps/modulestore_migrator/tests/test_tasks.py index 244d435de50a..9436c419c415 100644 --- a/cms/djangoapps/modulestore_migrator/tests/test_tasks.py +++ b/cms/djangoapps/modulestore_migrator/tests/test_tasks.py @@ -36,7 +36,14 @@ ) from openedx.core.djangoapps.content_libraries import api as lib_api - +DEFAULT_MIGRATION_SUMMARY = { + "total_blocks": 0, + "sections": 0, + "subsections": 0, + "units": 0, + "components": 0, + "unsupported": 0, +} @ddt.ddt class TestMigrateFromModulestore(ModuleStoreTestCase): """ @@ -124,6 +131,8 @@ def test_migrate_node_wiki_tag(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) result = _migrate_node( @@ -154,6 +163,8 @@ def test_migrate_node_course_root(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) result = _migrate_node( @@ -186,6 +197,8 @@ def test_migrate_node_library_root(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) result = _migrate_node( context=context, @@ -226,6 +239,8 @@ def test_migrate_node_container_composition_level( preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) result = _migrate_node( @@ -259,6 +274,8 @@ def test_migrate_node_without_url_name(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) result = _migrate_node( @@ -424,6 +441,8 @@ def test_migrate_component_success(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) result = _migrate_component( @@ -469,6 +488,8 @@ def test_migrate_component_with_static_content(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) result = _migrate_component( context=context, @@ -502,6 +523,8 @@ def test_migrate_component_replace_existing_false(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) first_result = _migrate_component( @@ -544,6 +567,8 @@ def test_migrate_component_same_title(self): preserve_url_slugs=False, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) first_result = _migrate_component( @@ -582,6 +607,8 @@ def test_migrate_component_replace_existing_true(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) first_result = _migrate_component( @@ -624,6 +651,8 @@ def test_migrate_component_different_block_types(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) result = _migrate_component( @@ -677,6 +706,8 @@ def test_migrate_component_content_filename_not_in_olx(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) result = _migrate_component( @@ -720,6 +751,8 @@ def test_migrate_component_library_source_key(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) result = _migrate_component( @@ -763,6 +796,8 @@ def test_migrate_component_duplicate_content_integrity_error(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) first_result = _migrate_component( @@ -838,6 +873,8 @@ def test_migrate_container_creates_new_container(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) result = _migrate_container( @@ -880,6 +917,8 @@ def test_migrate_container_different_container_types(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) for container_type, block_type in container_types: @@ -919,6 +958,8 @@ def test_migrate_container_replace_existing_false(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) first_result = _migrate_container( @@ -965,6 +1006,8 @@ def test_migrate_container_same_title(self): preserve_url_slugs=False, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) first_result = _migrate_container( @@ -1025,6 +1068,8 @@ def test_migrate_container_replace_existing_true(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) first_result = _migrate_container( @@ -1069,6 +1114,8 @@ def test_migrate_container_with_library_source_key(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) result = _migrate_container( @@ -1100,6 +1147,8 @@ def test_migrate_container_empty_children_list(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) result = _migrate_container( @@ -1131,6 +1180,8 @@ def test_migrate_container_preserves_child_order(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) children = [] for i in range(3): @@ -1238,6 +1289,8 @@ def test_migrate_container_with_mixed_child_types(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) result = _migrate_container( @@ -1277,6 +1330,8 @@ def test_migrate_container_generates_correct_target_key(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, + migration_summary=DEFAULT_MIGRATION_SUMMARY, + unsupported_reasons=[], ) course_result = _migrate_container( diff --git a/openedx/core/djangoapps/content_libraries/api/blocks.py b/openedx/core/djangoapps/content_libraries/api/blocks.py index 7dbcbf69b01c..eba82071ba0a 100644 --- a/openedx/core/djangoapps/content_libraries/api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/api/blocks.py @@ -309,7 +309,7 @@ def validate_can_add_block_to_library( block_class = XBlock.load_class(block_type) # Will raise an exception if invalid if block_class.has_children: raise IncompatibleTypesError( - f'The "{block_type}" XBlock (ID: "{block_id}") has children, so it not supported in content libraries', + _(f'The "{block_type}" XBlock (ID: "{block_id}") has children, so it not supported in content libraries'), ) # Make sure the new ID is not taken already: usage_key = LibraryUsageLocatorV2( # type: ignore[abstract] @@ -319,7 +319,9 @@ def validate_can_add_block_to_library( ) if _component_exists(usage_key): - raise LibraryBlockAlreadyExists(f"An XBlock with ID '{usage_key}' already exists") + raise LibraryBlockAlreadyExists( + _(f"An XBlock with ID '{usage_key}' already exists"), + ) return content_library, usage_key From 78de351492db80cc3edac45ee9edc3c97a909fb5 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 20 Nov 2025 18:06:10 -0500 Subject: [PATCH 04/10] style: Fix broken lint --- .../modulestore_migrator/rest_api/v1/serializers.py | 6 ++++++ cms/djangoapps/modulestore_migrator/tasks.py | 6 ++++-- cms/djangoapps/modulestore_migrator/tests/test_tasks.py | 4 +++- openedx/core/djangoapps/content_libraries/api/blocks.py | 8 ++++++-- 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py b/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py index 6788b0d3ecfd..22aa9114aa2f 100644 --- a/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py +++ b/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py @@ -24,6 +24,9 @@ class Meta: class MigrationSummarySerializer(serializers.Serializer): + """ + Serializer for a migration summary + """ total_blocks = serializers.IntegerField(required=False) sections = serializers.IntegerField(required=False) subsections = serializers.IntegerField(required=False) @@ -33,6 +36,9 @@ class MigrationSummarySerializer(serializers.Serializer): class MigrationBlockUnsupportedReasonSerializer(serializers.Serializer): + """ + Serializer for an unsupported block reason of a migration + """ block_name = serializers.CharField(required=False) block_type = serializers.CharField(required=False) reason = serializers.CharField(required=False) diff --git a/cms/djangoapps/modulestore_migrator/tasks.py b/cms/djangoapps/modulestore_migrator/tasks.py index e97a3d3c8622..a3d623680670 100644 --- a/cms/djangoapps/modulestore_migrator/tasks.py +++ b/cms/djangoapps/modulestore_migrator/tasks.py @@ -39,7 +39,7 @@ ) from user_tasks.tasks import UserTask, UserTaskStatus from xblock.core import XBlock -from django.utils.translation import gettext_lazy as _ +from django.utils.translation import gettext as _ from common.djangoapps.split_modulestore_django.models import SplitModulestoreCourseIndex from common.djangoapps.util.date_utils import strftime_localized, DEFAULT_DATE_TIME_FORMAT @@ -1075,7 +1075,9 @@ def _migrate_node( context.add_unsupported_reason( child_node.get('display_name'), child_node.tag, - str(_(f"The block is a child of this unsupported block: {title}")), + str(_("The block is a child of this unsupported block: {}").format( + title, + )), ) else: context.add_block_to_summary(None, is_unsupported=True) diff --git a/cms/djangoapps/modulestore_migrator/tests/test_tasks.py b/cms/djangoapps/modulestore_migrator/tests/test_tasks.py index 9436c419c415..53fa9e74dce4 100644 --- a/cms/djangoapps/modulestore_migrator/tests/test_tasks.py +++ b/cms/djangoapps/modulestore_migrator/tests/test_tasks.py @@ -36,7 +36,7 @@ ) from openedx.core.djangoapps.content_libraries import api as lib_api -DEFAULT_MIGRATION_SUMMARY = { +DEFAULT_MIGRATION_SUMMARY = { "total_blocks": 0, "sections": 0, "subsections": 0, @@ -44,6 +44,8 @@ "components": 0, "unsupported": 0, } + + @ddt.ddt class TestMigrateFromModulestore(ModuleStoreTestCase): """ diff --git a/openedx/core/djangoapps/content_libraries/api/blocks.py b/openedx/core/djangoapps/content_libraries/api/blocks.py index eba82071ba0a..ed6ee68042c5 100644 --- a/openedx/core/djangoapps/content_libraries/api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/api/blocks.py @@ -309,7 +309,9 @@ def validate_can_add_block_to_library( block_class = XBlock.load_class(block_type) # Will raise an exception if invalid if block_class.has_children: raise IncompatibleTypesError( - _(f'The "{block_type}" XBlock (ID: "{block_id}") has children, so it not supported in content libraries'), + _('The "{}" XBlock (ID: "{}") has children, so it not supported in content libraries').format( + block_type, block_id + ), ) # Make sure the new ID is not taken already: usage_key = LibraryUsageLocatorV2( # type: ignore[abstract] @@ -320,7 +322,9 @@ def validate_can_add_block_to_library( if _component_exists(usage_key): raise LibraryBlockAlreadyExists( - _(f"An XBlock with ID '{usage_key}' already exists"), + _("An XBlock with ID '{}' already exists").format( + usage_key + ), ) return content_library, usage_key From 5bdd433c1e60b127545621f56c0efe041e58441e Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 21 Nov 2025 18:38:27 -0500 Subject: [PATCH 05/10] refactor: Merge migrations --- ...oremigration_migration_summary_and_more.py} | 7 ++++++- ...modulestoremigration_unsupported_reasons.py | 18 ------------------ 2 files changed, 6 insertions(+), 19 deletions(-) rename cms/djangoapps/modulestore_migrator/migrations/{0004_modulestoremigration_migration_summary.py => 0004_modulestoremigration_migration_summary_and_more.py} (63%) delete mode 100644 cms/djangoapps/modulestore_migrator/migrations/0005_modulestoremigration_unsupported_reasons.py diff --git a/cms/djangoapps/modulestore_migrator/migrations/0004_modulestoremigration_migration_summary.py b/cms/djangoapps/modulestore_migrator/migrations/0004_modulestoremigration_migration_summary_and_more.py similarity index 63% rename from cms/djangoapps/modulestore_migrator/migrations/0004_modulestoremigration_migration_summary.py rename to cms/djangoapps/modulestore_migrator/migrations/0004_modulestoremigration_migration_summary_and_more.py index 26400c7f42a1..cdb63c556f20 100644 --- a/cms/djangoapps/modulestore_migrator/migrations/0004_modulestoremigration_migration_summary.py +++ b/cms/djangoapps/modulestore_migrator/migrations/0004_modulestoremigration_migration_summary_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 5.2.7 on 2025-11-19 21:21 +# Generated by Django 5.2.7 on 2025-11-21 23:29 from django.db import migrations, models @@ -15,4 +15,9 @@ class Migration(migrations.Migration): name='migration_summary', field=models.JSONField(default=dict, help_text='Summary that contains number of sections, subsections, units and components migrated'), ), + migrations.AddField( + model_name='modulestoremigration', + name='unsupported_reasons', + field=models.JSONField(default=list, help_text='List of unsuported blocks with the reasons'), + ), ] diff --git a/cms/djangoapps/modulestore_migrator/migrations/0005_modulestoremigration_unsupported_reasons.py b/cms/djangoapps/modulestore_migrator/migrations/0005_modulestoremigration_unsupported_reasons.py deleted file mode 100644 index 3aa993eb4b2d..000000000000 --- a/cms/djangoapps/modulestore_migrator/migrations/0005_modulestoremigration_unsupported_reasons.py +++ /dev/null @@ -1,18 +0,0 @@ -# Generated by Django 5.2.7 on 2025-11-20 19:41 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('modulestore_migrator', '0004_modulestoremigration_migration_summary'), - ] - - operations = [ - migrations.AddField( - model_name='modulestoremigration', - name='unsupported_reasons', - field=models.JSONField(default=list, help_text='List of unsuported blocks with the reasons'), - ), - ] From 83a7c486a344f5230ba90b3485737ed4d61d847e Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 21 Nov 2025 21:22:41 -0500 Subject: [PATCH 06/10] test: Add tests for the new fields --- cms/djangoapps/modulestore_migrator/tasks.py | 7 +- .../modulestore_migrator/tests/test_tasks.py | 86 +++++++++++++------ 2 files changed, 68 insertions(+), 25 deletions(-) diff --git a/cms/djangoapps/modulestore_migrator/tasks.py b/cms/djangoapps/modulestore_migrator/tasks.py index a3d623680670..01b0daa85ee1 100644 --- a/cms/djangoapps/modulestore_migrator/tasks.py +++ b/cms/djangoapps/modulestore_migrator/tasks.py @@ -667,6 +667,7 @@ def migrate_from_modulestore( status.set_state(MigrationStep.UNSTAGING.value) staged_content.delete() + migration.staged_content = None status.increment_completed_steps() _create_migration_artifacts_incrementally( @@ -689,7 +690,11 @@ def migrate_from_modulestore( _populate_collection(user_id, migration) status.increment_completed_steps() - migration.save() + migration.save(update_fields=[ + "target_collection", + "migration_summary", + "unsupported_reasons", + ]) except Exception as exc: # pylint: disable=broad-exception-caught _set_migrations_to_fail([source_data]) status.fail(str(exc)) diff --git a/cms/djangoapps/modulestore_migrator/tests/test_tasks.py b/cms/djangoapps/modulestore_migrator/tests/test_tasks.py index 53fa9e74dce4..43442038f2a0 100644 --- a/cms/djangoapps/modulestore_migrator/tests/test_tasks.py +++ b/cms/djangoapps/modulestore_migrator/tests/test_tasks.py @@ -3,6 +3,7 @@ """ from unittest.mock import Mock, patch +from copy import deepcopy import ddt from django.utils import timezone from lxml import etree @@ -133,7 +134,7 @@ def test_migrate_node_wiki_tag(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -165,7 +166,7 @@ def test_migrate_node_course_root(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -178,6 +179,8 @@ def test_migrate_node_course_root(self): self.assertIsNone(result.source_to_target) # But should have children processed self.assertEqual(len(result.children), 1) + self.assertEqual(context.unsupported_reasons, []) + self.assertEqual(context.migration_summary, DEFAULT_MIGRATION_SUMMARY) def test_migrate_node_library_root(self): """ @@ -199,7 +202,7 @@ def test_migrate_node_library_root(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) result = _migrate_node( @@ -211,6 +214,15 @@ def test_migrate_node_library_root(self): self.assertIsNone(result.source_to_target) # But should have children processed self.assertEqual(len(result.children), 1) + self.assertEqual(context.unsupported_reasons, []) + self.assertEqual(context.migration_summary, { + "total_blocks": 1, + "sections": 0, + "subsections": 0, + "units": 0, + "components": 1, + "unsupported": 0, + }) @ddt.data( ("chapter", CompositionLevel.Unit, None), @@ -241,7 +253,7 @@ def test_migrate_node_container_composition_level( preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -250,14 +262,26 @@ def test_migrate_node_container_composition_level( source_node=container_node, ) + expected_summary = deepcopy(DEFAULT_MIGRATION_SUMMARY) + if should_migrate: self.assertIsNotNone(result.source_to_target) source_key, _ = result.source_to_target self.assertEqual(source_key.block_type, tag_name) self.assertEqual(source_key.block_id, f"test_{tag_name}") + expected_summary["total_blocks"] += 1 + if tag_name == 'chapter': + expected_summary["sections"] += 1 + elif tag_name == 'sequential': + expected_summary["subsections"] += 1 + elif tag_name == 'vertical': + expected_summary["units"] += 1 else: self.assertIsNone(result.source_to_target) + self.assertEqual(context.migration_summary, expected_summary) + self.assertEqual(context.unsupported_reasons, []) + def test_migrate_node_without_url_name(self): """ Test _migrate_node handles nodes without url_name @@ -276,7 +300,7 @@ def test_migrate_node_without_url_name(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -288,6 +312,20 @@ def test_migrate_node_without_url_name(self): self.assertIsNone(result.source_to_target) self.assertEqual(len(result.children), 0) + self.assertEqual(context.unsupported_reasons, [{ + 'block_name': 'No URL Name', + 'block_type': 'problem', + 'reason': 'The block lacks an url_name and thus has no identity.' + }]) + self.assertEqual(context.migration_summary, { + "total_blocks": 1, + "sections": 0, + "subsections": 0, + "units": 0, + "components": 0, + "unsupported": 1, + }) + def test_migrated_node_all_source_to_target_pairs(self): """ Test _MigratedNode.all_source_to_target_pairs traversal @@ -443,7 +481,7 @@ def test_migrate_component_success(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -490,7 +528,7 @@ def test_migrate_component_with_static_content(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) result = _migrate_component( @@ -525,7 +563,7 @@ def test_migrate_component_replace_existing_false(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -569,7 +607,7 @@ def test_migrate_component_same_title(self): preserve_url_slugs=False, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -609,7 +647,7 @@ def test_migrate_component_replace_existing_true(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -653,7 +691,7 @@ def test_migrate_component_different_block_types(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -708,7 +746,7 @@ def test_migrate_component_content_filename_not_in_olx(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -753,7 +791,7 @@ def test_migrate_component_library_source_key(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -798,7 +836,7 @@ def test_migrate_component_duplicate_content_integrity_error(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -875,7 +913,7 @@ def test_migrate_container_creates_new_container(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -919,7 +957,7 @@ def test_migrate_container_different_container_types(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -960,7 +998,7 @@ def test_migrate_container_replace_existing_false(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -1008,7 +1046,7 @@ def test_migrate_container_same_title(self): preserve_url_slugs=False, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -1070,7 +1108,7 @@ def test_migrate_container_replace_existing_true(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -1116,7 +1154,7 @@ def test_migrate_container_with_library_source_key(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -1149,7 +1187,7 @@ def test_migrate_container_empty_children_list(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -1182,7 +1220,7 @@ def test_migrate_container_preserves_child_order(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) children = [] @@ -1291,7 +1329,7 @@ def test_migrate_container_with_mixed_child_types(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) @@ -1332,7 +1370,7 @@ def test_migrate_container_generates_correct_target_key(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=DEFAULT_MIGRATION_SUMMARY, + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), unsupported_reasons=[], ) From 174f6ab2148dcb8ac9d588158e9a1a77a72ecd3e Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 27 Nov 2025 16:55:05 -0500 Subject: [PATCH 07/10] refactor: Add unsupported children to summary --- cms/djangoapps/modulestore_migrator/tasks.py | 25 ++++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/cms/djangoapps/modulestore_migrator/tasks.py b/cms/djangoapps/modulestore_migrator/tasks.py index 7f9f2f42fdbb..47d2c6f70f9a 100644 --- a/cms/djangoapps/modulestore_migrator/tasks.py +++ b/cms/djangoapps/modulestore_migrator/tasks.py @@ -1069,17 +1069,22 @@ def _migrate_node( ) context.add_block_to_summary(container_type, is_unsupported=target_entity_version is None) - if container_type is None and target_entity_version is None and reason is not None: - # Currently, components with children are not supported + if container_type is None and target_entity_version is None: + # Currently, components with children are not supported, but they appear as unsupported in the summary. children_length = len(source_node.getchildren()) - if children_length: - reason += ( - ngettext( - ' It has {count} children block.', - ' It has {count} children blocks.', - children_length, - ) - ).format(count=children_length) + for _ in range(children_length): + context.add_block_to_summary(None, is_unsupported=True) + + # And add the children count to the reason. + if reason is not None: + if children_length: + reason += ( + ngettext( + ' It has {count} children block.', + ' It has {count} children blocks.', + children_length, + ) + ).format(count=children_length) source_to_target = (source_key, target_entity_version, reason) context.add_migration(source_key, target_entity_version.entity if target_entity_version else None) else: From a8b9f67f905f3b59d527a12ade5db3b8f80e6243 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 27 Nov 2025 17:03:13 -0500 Subject: [PATCH 08/10] style: Fix broken lint --- cms/djangoapps/modulestore_migrator/tasks.py | 6 +++--- cms/djangoapps/modulestore_migrator/tests/test_tasks.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cms/djangoapps/modulestore_migrator/tasks.py b/cms/djangoapps/modulestore_migrator/tasks.py index 47d2c6f70f9a..f314973822e7 100644 --- a/cms/djangoapps/modulestore_migrator/tasks.py +++ b/cms/djangoapps/modulestore_migrator/tasks.py @@ -1068,13 +1068,13 @@ def _migrate_node( ) ) - context.add_block_to_summary(container_type, is_unsupported=target_entity_version is None) - if container_type is None and target_entity_version is None: + context.add_block_to_summary(container_type, is_unsupported=target_entity_version is None) + if container_type is None and target_entity_version is None: # Currently, components with children are not supported, but they appear as unsupported in the summary. children_length = len(source_node.getchildren()) for _ in range(children_length): context.add_block_to_summary(None, is_unsupported=True) - + # And add the children count to the reason. if reason is not None: if children_length: diff --git a/cms/djangoapps/modulestore_migrator/tests/test_tasks.py b/cms/djangoapps/modulestore_migrator/tests/test_tasks.py index ea98407cd60b..38a02ec668d2 100644 --- a/cms/djangoapps/modulestore_migrator/tests/test_tasks.py +++ b/cms/djangoapps/modulestore_migrator/tests/test_tasks.py @@ -631,7 +631,7 @@ def test_migrate_component_replace_existing_false(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) first_result, first_reason = _migrate_component( @@ -985,7 +985,7 @@ def test_migrate_container_creates_new_container(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) result, reason = _migrate_container( @@ -1029,7 +1029,7 @@ def test_migrate_container_different_container_types(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), + migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) for container_type, block_type in container_types: From dba9a97e2917e75c3bec91a416864a11634385dd Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 28 Nov 2025 17:20:17 -0500 Subject: [PATCH 09/10] refactor: Remove all the code related to migration_summary --- ..._modulestoremigration_migration_summary.py | 18 ----- cms/djangoapps/modulestore_migrator/models.py | 4 -- .../rest_api/v1/serializers.py | 30 --------- cms/djangoapps/modulestore_migrator/tasks.py | 38 +---------- .../modulestore_migrator/tests/test_tasks.py | 65 ------------------- 5 files changed, 2 insertions(+), 153 deletions(-) delete mode 100644 cms/djangoapps/modulestore_migrator/migrations/0006_modulestoremigration_migration_summary.py diff --git a/cms/djangoapps/modulestore_migrator/migrations/0006_modulestoremigration_migration_summary.py b/cms/djangoapps/modulestore_migrator/migrations/0006_modulestoremigration_migration_summary.py deleted file mode 100644 index ec4c0f44396f..000000000000 --- a/cms/djangoapps/modulestore_migrator/migrations/0006_modulestoremigration_migration_summary.py +++ /dev/null @@ -1,18 +0,0 @@ -# Generated by Django 5.2.7 on 2025-11-27 18:12 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('modulestore_migrator', '0004_alter_modulestoreblockmigration_target_squashed_0005_modulestoreblockmigration_unsupported_reason'), - ] - - operations = [ - migrations.AddField( - model_name='modulestoremigration', - name='migration_summary', - field=models.JSONField(default=dict, help_text='Summary that contains number of sections, subsections, units and components migrated'), - ), - ] diff --git a/cms/djangoapps/modulestore_migrator/models.py b/cms/djangoapps/modulestore_migrator/models.py index 6be479b82013..149f6dd05d95 100644 --- a/cms/djangoapps/modulestore_migrator/models.py +++ b/cms/djangoapps/modulestore_migrator/models.py @@ -145,10 +145,6 @@ class ModulestoreMigration(models.Model): "is the migration failed?" ), ) - migration_summary = models.JSONField( - default=dict, - help_text=_("Summary that contains number of sections, subsections, units and components migrated"), - ) def __str__(self): return ( diff --git a/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py b/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py index 9a5309d27809..4b787ea5cf1c 100644 --- a/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py +++ b/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py @@ -26,27 +26,6 @@ class Meta: fields = ["key", "title"] -class MigrationSummarySerializer(serializers.Serializer): - """ - Serializer for a migration summary - """ - total_blocks = serializers.IntegerField(required=False) - sections = serializers.IntegerField(required=False) - subsections = serializers.IntegerField(required=False) - units = serializers.IntegerField(required=False) - components = serializers.IntegerField(required=False) - unsupported = serializers.IntegerField(required=False) - - -class MigrationBlockUnsupportedReasonSerializer(serializers.Serializer): - """ - Serializer for an unsupported block reason of a migration - """ - block_name = serializers.CharField(required=False) - block_type = serializers.CharField(required=False) - reason = serializers.CharField(required=False) - - class ModulestoreMigrationSerializer(serializers.Serializer): """ Serializer for the course or legacylibrary to library V2 import creation API. @@ -94,15 +73,6 @@ class ModulestoreMigrationSerializer(serializers.Serializer): required=False, default=False, ) - migration_summary = MigrationSummarySerializer( - help_text="Summary of the finished migration", - required=False - ) - unsupported_reasons = MigrationBlockUnsupportedReasonSerializer( - help_text="List of unsupported blocks with the reason", - required=False, - many=True - ) def get_fields(self): fields = super().get_fields() diff --git a/cms/djangoapps/modulestore_migrator/tasks.py b/cms/djangoapps/modulestore_migrator/tasks.py index f314973822e7..741994f30869 100644 --- a/cms/djangoapps/modulestore_migrator/tasks.py +++ b/cms/djangoapps/modulestore_migrator/tasks.py @@ -136,7 +136,6 @@ class _MigrationContext: composition_level: CompositionLevel repeat_handling_strategy: RepeatHandlingStrategy preserve_url_slugs: bool - migration_summary: dict[str, int] created_by: int created_at: datetime @@ -169,20 +168,6 @@ def add_migration(self, source_key: UsageKey, target: PublishableEntity | None) else: self.existing_source_to_target_keys[source_key].append(target) - def add_block_to_summary(self, container_type: ContainerType | None, is_unsupported=False): - """Add a block to the migration summary using the container_type""" - self.migration_summary["total_blocks"] += 1 - if is_unsupported: - self.migration_summary["unsupported"] += 1 - elif container_type is None: - self.migration_summary["components"] += 1 - elif container_type is ContainerType.Unit: - self.migration_summary["units"] += 1 - elif container_type is ContainerType.Subsection: - self.migration_summary["subsections"] += 1 - elif container_type is ContainerType.Section: - self.migration_summary["sections"] += 1 - def get_existing_target_entity_keys(self, base_key: str) -> set[str]: return set( publishable_entity.key @@ -403,15 +388,6 @@ def _import_structure( modulestore_blocks, key=lambda x: x.source.key) } - migration_summary = { - "total_blocks": 0, - "sections": 0, - "subsections": 0, - "units": 0, - "components": 0, - "unsupported": 0, - } - migration_context = _MigrationContext( existing_source_to_target_keys=existing_source_to_target_keys, target_package_id=migration.target.pk, @@ -421,7 +397,6 @@ def _import_structure( composition_level=CompositionLevel(migration.composition_level), repeat_handling_strategy=RepeatHandlingStrategy(migration.repeat_handling_strategy), preserve_url_slugs=migration.preserve_url_slugs, - migration_summary=migration_summary, created_by=status.user_id, created_at=datetime.now(timezone.utc), ) @@ -432,7 +407,6 @@ def _import_structure( source_node=root_node, ) change_log.save() - migration.migration_summary = migration_context.migration_summary return change_log, root_migrated_node @@ -689,7 +663,6 @@ def migrate_from_modulestore( migration.save(update_fields=[ "target_collection", - "migration_summary", ]) except Exception as exc: # pylint: disable=broad-exception-caught _set_migrations_to_fail([source_data]) @@ -958,7 +931,6 @@ def bulk_migrate_from_modulestore( [ "target_collection", "is_failed", - "migration_summary", ], ) status.increment_completed_steps() @@ -1068,14 +1040,9 @@ def _migrate_node( ) ) - context.add_block_to_summary(container_type, is_unsupported=target_entity_version is None) - if container_type is None and target_entity_version is None: - # Currently, components with children are not supported, but they appear as unsupported in the summary. + if container_type is None and target_entity_version is None and reason is not None: + # Currently, components with children are not supported children_length = len(source_node.getchildren()) - for _ in range(children_length): - context.add_block_to_summary(None, is_unsupported=True) - - # And add the children count to the reason. if reason is not None: if children_length: reason += ( @@ -1088,7 +1055,6 @@ def _migrate_node( source_to_target = (source_key, target_entity_version, reason) context.add_migration(source_key, target_entity_version.entity if target_entity_version else None) else: - context.add_block_to_summary(None, is_unsupported=True) log.warning( f"Cannot migrate node from {context.source_context_key} to {context.target_library_key} " f"because it lacks an url_name and thus has no identity: {source_olx}" diff --git a/cms/djangoapps/modulestore_migrator/tests/test_tasks.py b/cms/djangoapps/modulestore_migrator/tests/test_tasks.py index 38a02ec668d2..abc112e7f74b 100644 --- a/cms/djangoapps/modulestore_migrator/tests/test_tasks.py +++ b/cms/djangoapps/modulestore_migrator/tests/test_tasks.py @@ -3,7 +3,6 @@ """ from unittest.mock import Mock, patch -from copy import deepcopy import ddt from django.utils import timezone from lxml import etree @@ -37,15 +36,6 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, LibraryFactory -DEFAULT_MIGRATION_SUMMARY = { - "total_blocks": 0, - "sections": 0, - "subsections": 0, - "units": 0, - "components": 0, - "unsupported": 0, -} - @ddt.ddt class TestMigrateFromModulestore(ModuleStoreTestCase): @@ -134,7 +124,6 @@ def test_migrate_node_wiki_tag(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) result = _migrate_node( @@ -165,7 +154,6 @@ def test_migrate_node_course_root(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) result = _migrate_node( @@ -177,7 +165,6 @@ def test_migrate_node_course_root(self): self.assertIsNone(result.source_to_target) # But should have children processed self.assertEqual(len(result.children), 1) - self.assertEqual(context.migration_summary, DEFAULT_MIGRATION_SUMMARY) def test_migrate_node_library_root(self): """ @@ -199,7 +186,6 @@ def test_migrate_node_library_root(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) result = _migrate_node( context=context, @@ -210,14 +196,6 @@ def test_migrate_node_library_root(self): self.assertIsNone(result.source_to_target) # But should have children processed self.assertEqual(len(result.children), 1) - self.assertEqual(context.migration_summary, { - "total_blocks": 1, - "sections": 0, - "subsections": 0, - "units": 0, - "components": 1, - "unsupported": 0, - }) @ddt.data( ("chapter", CompositionLevel.Unit, None), @@ -248,7 +226,6 @@ def test_migrate_node_container_composition_level( preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) result = _migrate_node( @@ -256,27 +233,16 @@ def test_migrate_node_container_composition_level( source_node=container_node, ) - expected_summary = deepcopy(DEFAULT_MIGRATION_SUMMARY) - if should_migrate: self.assertIsNotNone(result.source_to_target) source_key, _, reason = result.source_to_target self.assertEqual(source_key.block_type, tag_name) self.assertEqual(source_key.block_id, f"test_{tag_name}") - expected_summary["total_blocks"] += 1 - if tag_name == 'chapter': - expected_summary["sections"] += 1 - elif tag_name == 'sequential': - expected_summary["subsections"] += 1 - elif tag_name == 'vertical': - expected_summary["units"] += 1 self.assertIsNone(reason) else: self.assertIsNone(result.source_to_target) - self.assertEqual(context.migration_summary, expected_summary) - def test_migrate_node_without_url_name(self): """ Test _migrate_node handles nodes without url_name @@ -295,7 +261,6 @@ def test_migrate_node_without_url_name(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) result = _migrate_node( @@ -306,15 +271,6 @@ def test_migrate_node_without_url_name(self): self.assertIsNone(result.source_to_target) self.assertEqual(len(result.children), 0) - self.assertEqual(context.migration_summary, { - "total_blocks": 1, - "sections": 0, - "subsections": 0, - "units": 0, - "components": 0, - "unsupported": 1, - }) - def test_migrate_node_with_children_components(self): """ Test _migrate_node handles nodes with children components @@ -336,7 +292,6 @@ def test_migrate_node_with_children_components(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY) ) result = _migrate_node( @@ -510,7 +465,6 @@ def test_migrate_component_success(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) result, reason = _migrate_component( @@ -553,7 +507,6 @@ def test_migrate_component_failure(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY) ) result, reason = _migrate_component( @@ -596,7 +549,6 @@ def test_migrate_component_with_static_content(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) result, reason = _migrate_component( context=context, @@ -631,7 +583,6 @@ def test_migrate_component_replace_existing_false(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) first_result, first_reason = _migrate_component( @@ -676,7 +627,6 @@ def test_migrate_component_same_title(self): preserve_url_slugs=False, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) first_result, first_reason = _migrate_component( @@ -717,7 +667,6 @@ def test_migrate_component_replace_existing_true(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) first_result, first_reason = _migrate_component( @@ -762,7 +711,6 @@ def test_migrate_component_different_block_types(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) result, reason = _migrate_component( @@ -817,7 +765,6 @@ def test_migrate_component_content_filename_not_in_olx(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) result, reason = _migrate_component( @@ -862,7 +809,6 @@ def test_migrate_component_library_source_key(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) result, reason = _migrate_component( @@ -907,7 +853,6 @@ def test_migrate_component_duplicate_content_integrity_error(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) first_result, first_reason = _migrate_component( @@ -985,7 +930,6 @@ def test_migrate_container_creates_new_container(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) result, reason = _migrate_container( @@ -1029,7 +973,6 @@ def test_migrate_container_different_container_types(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) for container_type, block_type in container_types: @@ -1070,7 +1013,6 @@ def test_migrate_container_replace_existing_false(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) first_result, _ = _migrate_container( @@ -1117,7 +1059,6 @@ def test_migrate_container_same_title(self): preserve_url_slugs=False, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) first_result, _ = _migrate_container( @@ -1178,7 +1119,6 @@ def test_migrate_container_replace_existing_true(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) first_result, _ = _migrate_container( @@ -1223,7 +1163,6 @@ def test_migrate_container_with_library_source_key(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) result, _ = _migrate_container( @@ -1255,7 +1194,6 @@ def test_migrate_container_empty_children_list(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) result, reason = _migrate_container( @@ -1288,7 +1226,6 @@ def test_migrate_container_preserves_child_order(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) children = [] for i in range(3): @@ -1396,7 +1333,6 @@ def test_migrate_container_with_mixed_child_types(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) result, _ = _migrate_container( @@ -1436,7 +1372,6 @@ def test_migrate_container_generates_correct_target_key(self): preserve_url_slugs=True, created_at=timezone.now(), created_by=self.user.id, - migration_summary=deepcopy(DEFAULT_MIGRATION_SUMMARY), ) course_result, _ = _migrate_container( From 0b74fc522c0c8b82b776f3d87d4eb0c6484c6c6d Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 28 Nov 2025 17:29:13 -0500 Subject: [PATCH 10/10] style: Nits on the code --- cms/djangoapps/modulestore_migrator/tasks.py | 28 ++++++------------- .../modulestore_migrator/tests/test_tasks.py | 2 +- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/cms/djangoapps/modulestore_migrator/tasks.py b/cms/djangoapps/modulestore_migrator/tasks.py index 741994f30869..b4f24f149b8e 100644 --- a/cms/djangoapps/modulestore_migrator/tasks.py +++ b/cms/djangoapps/modulestore_migrator/tasks.py @@ -638,7 +638,6 @@ def migrate_from_modulestore( status.set_state(MigrationStep.UNSTAGING.value) staged_content.delete() - migration.staged_content = None status.increment_completed_steps() _create_migration_artifacts_incrementally( @@ -660,10 +659,6 @@ def migrate_from_modulestore( if target_collection: _populate_collection(user_id, migration) status.increment_completed_steps() - - migration.save(update_fields=[ - "target_collection", - ]) except Exception as exc: # pylint: disable=broad-exception-caught _set_migrations_to_fail([source_data]) status.fail(str(exc)) @@ -928,10 +923,7 @@ def bulk_migrate_from_modulestore( ModulestoreMigration.objects.bulk_update( [x.migration for x in source_data_list], - [ - "target_collection", - "is_failed", - ], + ["target_collection", "is_failed"], ) status.increment_completed_steps() except Exception as exc: # pylint: disable=broad-exception-caught @@ -1039,19 +1031,17 @@ def _migrate_node( title=title, ) ) - if container_type is None and target_entity_version is None and reason is not None: # Currently, components with children are not supported children_length = len(source_node.getchildren()) - if reason is not None: - if children_length: - reason += ( - ngettext( - ' It has {count} children block.', - ' It has {count} children blocks.', - children_length, - ) - ).format(count=children_length) + if children_length: + reason += ( + ngettext( + ' It has {count} children block.', + ' It has {count} children blocks.', + children_length, + ) + ).format(count=children_length) source_to_target = (source_key, target_entity_version, reason) context.add_migration(source_key, target_entity_version.entity if target_entity_version else None) else: diff --git a/cms/djangoapps/modulestore_migrator/tests/test_tasks.py b/cms/djangoapps/modulestore_migrator/tests/test_tasks.py index abc112e7f74b..9b4ac4130c59 100644 --- a/cms/djangoapps/modulestore_migrator/tests/test_tasks.py +++ b/cms/djangoapps/modulestore_migrator/tests/test_tasks.py @@ -3,6 +3,7 @@ """ from unittest.mock import Mock, patch + import ddt from django.utils import timezone from lxml import etree @@ -238,7 +239,6 @@ def test_migrate_node_container_composition_level( source_key, _, reason = result.source_to_target self.assertEqual(source_key.block_type, tag_name) self.assertEqual(source_key.block_id, f"test_{tag_name}") - self.assertIsNone(reason) else: self.assertIsNone(result.source_to_target)