feat: store failed migration blocks in modulestore

Author: navinkarkera

cms/djangoapps/modulestore_migrator/migrations/0004_alter_modulestoreblockmigration_target.py

@@ -0,0 +1,24 @@
+# Generated by Django 5.2.7 on 2025-11-19 06:23
+
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ('modulestore_migrator', '0003_modulestoremigration_is_failed'),
+        ('oel_publishing', '0008_alter_draftchangelogrecord_options_and_more'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='modulestoreblockmigration',
+            name='target',
+            field=models.ForeignKey(
+                blank=True,
+                null=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                to='oel_publishing.publishableentity',
+            ),
+        ),
+    ]

cms/djangoapps/modulestore_migrator/models.py

@@ -210,6 +210,8 @@ class ModulestoreBlockMigration(TimeStampedModel):
     target = models.ForeignKey(
         PublishableEntity,
         on_delete=models.CASCADE,
+        null=True,
+        blank=True,
     )
     change_log_record = models.OneToOneField(
         DraftChangeLogRecord,
@@ -222,6 +224,7 @@ class ModulestoreBlockMigration(TimeStampedModel):
     class Meta:
         unique_together = [
             ('overall_migration', 'source'),
+            # By default defining a unique index on a nullable column will only enforce unicity of non-null values.
             ('overall_migration', 'target'),
         ]
 

cms/djangoapps/modulestore_migrator/tasks.py

@@ -154,7 +154,7 @@ def get_existing_target(self, source_key: UsageKey) -> PublishableEntity:
         # NOTE: This is a list of PublishableEntities, but we always return the first one.
         return self.existing_source_to_target_keys[source_key][0]
 
-    def add_migration(self, source_key: UsageKey, target: PublishableEntity) -> None:
+    def add_migration(self, source_key: UsageKey, target: PublishableEntity | None) -> None:
         """Update the context with a new migration (keeps it current)"""
         if source_key not in self.existing_source_to_target_keys:
             self.existing_source_to_target_keys[source_key] = [target]
@@ -166,7 +166,7 @@ def get_existing_target_entity_keys(self, base_key: str) -> set[str]:
             publishable_entity.key
             for publishable_entity_list in self.existing_source_to_target_keys.values()
             for publishable_entity in publishable_entity_list
-            if publishable_entity.key.startswith(base_key)
+            if publishable_entity and publishable_entity.key.startswith(base_key)
         )
 
     @property
@@ -934,10 +934,10 @@ class _MigratedNode:
     This happens, particularly, if the node is above the requested composition level
     but has descendents which are at or below that level.
     """
-    source_to_target: tuple[UsageKey, PublishableEntityVersion] | None
+    source_to_target: tuple[UsageKey, PublishableEntityVersion | None] | None
     children: list[_MigratedNode]
 
-    def all_source_to_target_pairs(self) -> t.Iterable[tuple[UsageKey, PublishableEntityVersion]]:
+    def all_source_to_target_pairs(self) -> t.Iterable[tuple[UsageKey, PublishableEntityVersion | None]]:
         """
         Get all source_key->target_ver pairs via a pre-order traversal.
         """
@@ -1010,7 +1010,7 @@ def _migrate_node(
                     children=[
                         migrated_child.source_to_target[1]
                         for migrated_child in migrated_children if
-                        migrated_child.source_to_target
+                        migrated_child.source_to_target and migrated_child.source_to_target[1]
                     ],
                 )
                 if container_type else
@@ -1021,9 +1021,8 @@ def _migrate_node(
                     title=title,
                 )
             )
-            if target_entity_version:
-                source_to_target = (source_key, target_entity_version)
-                context.add_migration(source_key, target_entity_version.entity)
+            source_to_target = (source_key, target_entity_version)
+            context.add_migration(source_key, target_entity_version.entity if target_entity_version else None)
         else:
             log.warning(
                 f"Cannot migrate node from {context.source_context_key} to {context.target_library_key} "
@@ -1334,7 +1333,7 @@ def _create_migration_artifacts_incrementally(
         ModulestoreBlockMigration.objects.create(
             overall_migration=migration,
             source=block_source,
-            target_id=target_version.entity_id,
+            target_id=target_version.entity_id if target_version else None,
         )
 
         processed += 1

openedx/core/lib/xblock_serializer/block_serializer.py

@@ -79,10 +79,15 @@ def _serialize_block(self, block) -> etree.Element:
         else:
             olx = self._serialize_normal_block(block)
 
-        # The url_name attribute can come either because it was already in the
-        # block's field data, or because this class adds it in the calls above.
-        # However it gets set though, we can remove it here:
-        if not self.write_url_name:
+        if self.write_url_name:
+            # Handles a weird case where url_name is not part of olx.attrib even if it is
+            # set in block. Known case is with openassessment blocks.
+            if "url_name" not in olx.attrib and hasattr(block, "url_name"):
+                olx.attrib['url_name'] = block.url_name
+        else:
+            # The url_name attribute can come either because it was already in the
+            # block's field data, or because this class adds it in the calls above.
+            # However it gets set though, we can remove it here:
             olx.attrib.pop("url_name", None)
 
         # Add copied_from_block and copied_from_version attribute the XBlock's OLX node, to help identify the source of