fix: Various Edge-Case Bugs & Data Issues in modulestore_migrator

cms/djangoapps/contentstore/rest_api/v1/serializers/home.py

@@ -28,26 +28,11 @@ class LibraryViewSerializer(serializers.Serializer):
     org = serializers.CharField()
     number = serializers.CharField()
     can_edit = serializers.BooleanField()
-    is_migrated = serializers.SerializerMethodField()
-    migrated_to_title = serializers.CharField(
-        source="migrations__target__title",
-        required=False
-    )
-    migrated_to_key = serializers.CharField(
-        source="migrations__target__key",
-        required=False
-    )
-    migrated_to_collection_key = serializers.CharField(
-        source="migrations__target_collection__key",
-        required=False
-    )
-    migrated_to_collection_title = serializers.CharField(
-        source="migrations__target_collection__title",
-        required=False
-    )
-
-    def get_is_migrated(self, obj):
-        return "migrations__target__key" in obj
+    is_migrated = serializers.BooleanField()
+    migrated_to_title = serializers.CharField(required=False)
+    migrated_to_key = serializers.CharField(required=False)
+    migrated_to_collection_key = serializers.CharField(required=False)
+    migrated_to_collection_title = serializers.CharField(required=False)
 
 
 class CourseHomeTabSerializer(serializers.Serializer):

cms/djangoapps/contentstore/rest_api/v1/views/home.py

@@ -236,7 +236,7 @@ def get(self, request: Request):
                 "number": "CPSPR",
                 "can_edit": true
                 }
-            ],        }
+            ],
         ```
         """
 

cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py

@@ -18,7 +18,6 @@
 from cms.djangoapps.contentstore.tests.utils import CourseTestCase
 from cms.djangoapps.modulestore_migrator import api as migrator_api
 from cms.djangoapps.modulestore_migrator.data import CompositionLevel, RepeatHandlingStrategy
-from cms.djangoapps.modulestore_migrator.tests.factories import ModulestoreSourceFactory
 from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
 from openedx.core.djangoapps.content_libraries import api as lib_api
 
@@ -253,8 +252,9 @@ class HomePageLibrariesViewTest(LibraryTestCase):
 
     def setUp(self):
         super().setUp()
-        # Create an additional legacy library
+        # Create an two additional legacy libaries
         self.lib_key_1 = self._create_library(library="lib1")
+        self.lib_key_2 = self._create_library(library="lib2")
         self.organization = OrganizationFactory()
 
         # Create a new v2 library
@@ -269,7 +269,6 @@ def setUp(self):
         library = lib_api.ContentLibrary.objects.get(slug=self.lib_key_v2.slug)
         learning_package = library.learning_package
         # Create a migration source for the legacy library
-        self.source = ModulestoreSourceFactory(key=self.lib_key_1)
         self.url = reverse("cms.djangoapps.contentstore:v1:libraries")
         # Create a collection to migrate this library to
         collection_key = "test-collection"
@@ -280,20 +279,32 @@ def setUp(self):
             created_by=self.user.id,
         )
 
-        # Migrate self.lib_key_1 to self.lib_key_v2
+        # Migrate both lib_key_1 and lib_key_2 to v2
+        # Only make lib_key_1 a "forwarding" migration.
         migrator_api.start_migration_to_library(
             user=self.user,
-            source_key=self.source.key,
+            source_key=self.lib_key_1,
             target_library_key=self.lib_key_v2,
             target_collection_slug=collection_key,
-            composition_level=CompositionLevel.Component.value,
-            repeat_handling_strategy=RepeatHandlingStrategy.Skip.value,
+            composition_level=CompositionLevel.Component,
+            repeat_handling_strategy=RepeatHandlingStrategy.Skip,
+            preserve_url_slugs=True,
+            forward_source_to_target=True,
+        )
+        migrator_api.start_migration_to_library(
+            user=self.user,
+            source_key=self.lib_key_2,
+            target_library_key=self.lib_key_v2,
+            target_collection_slug=collection_key,
+            composition_level=CompositionLevel.Component,
+            repeat_handling_strategy=RepeatHandlingStrategy.Skip,
             preserve_url_slugs=True,
             forward_source_to_target=False,
         )
 
     def test_home_page_libraries_response(self):
-        """Check successful response content"""
+        """Check sucessful response content"""
+        self.maxDiff = None
         response = self.client.get(self.url)
 
         expected_response = {
@@ -322,6 +333,17 @@ def test_home_page_libraries_response(self):
                     'migrated_to_collection_key': 'test-collection',
                     'migrated_to_collection_title': 'Test Collection',
                 },
+                # Third library was migrated, but not with forwarding.
+                # So, it appears just like the unmigrated library.
+                {
+                    'display_name': 'Test Library',
+                    'library_key': 'library-v1:org+lib2',
+                    'url': '/library/library-v1:org+lib2',
+                    'org': 'org',
+                    'number': 'lib2',
+                    'can_edit': True,
+                    'is_migrated': False,
+                },
             ]
         }
 
@@ -366,6 +388,15 @@ def test_home_page_libraries_response(self):
                     'can_edit': True,
                     'is_migrated': False,
                 },
+                {
+                    'display_name': 'Test Library',
+                    'library_key': 'library-v1:org+lib2',
+                    'url': '/library/library-v1:org+lib2',
+                    'org': 'org',
+                    'number': 'lib2',
+                    'can_edit': True,
+                    'is_migrated': False,
+                },
             ],
         }
 

cms/djangoapps/contentstore/utils.py

@@ -54,7 +54,8 @@
 )
 from cms.djangoapps.models.settings.course_grading import CourseGradingModel
 from cms.djangoapps.models.settings.course_metadata import CourseMetadata
-from cms.djangoapps.modulestore_migrator.api import get_migration_info
+from cms.djangoapps.modulestore_migrator import api as migrator_api
+from cms.djangoapps.modulestore_migrator.data import ModulestoreMigration
 from common.djangoapps.course_action_state.managers import CourseActionStateItemNotFoundError
 from common.djangoapps.course_action_state.models import CourseRerunState, CourseRerunUIStateManager
 from common.djangoapps.course_modes.models import CourseMode
@@ -1572,13 +1573,12 @@ def request_response_format_is_json(request, response_format):
 
 def get_library_context(request, request_is_json=False):
     """
-    Utils is used to get context of course home library tab.
-    It is used for both DRF and django views.
+    Utils is used to get context of course home library tab. Returned in DRF view.
     """
     from cms.djangoapps.contentstore.views.course import (
         _accessible_libraries_iter,
-        _format_library_for_view,
         _get_course_creator_status,
+        format_library_for_view,
         get_allowed_organizations,
         get_allowed_organizations_for_libraries,
         user_can_create_organizations,
@@ -1590,21 +1590,25 @@ def get_library_context(request, request_is_json=False):
         user_can_create_library,
     )
 
+    is_migrated: bool | None  # None means: do not filter on is_migrated
+    if (is_migrated_param := request.GET.get('is_migrated')) is not None:
+        is_migrated = BooleanField().to_internal_value(is_migrated_param)
+    else:
+        is_migrated = None
     libraries = list(_accessible_libraries_iter(request.user) if libraries_v1_enabled() else [])
-    library_keys = [lib.location.library_key for lib in libraries]
-    migration_info = get_migration_info(library_keys)
-    is_migrated_filter = request.GET.get('is_migrated', None)
+    migration_info: dict[LibraryLocator, ModulestoreMigration | None] = {
+        lib.id: migrator_api.get_forwarding(lib.id)
+        for lib in libraries
+    }
     data = {
         'libraries': [
-            _format_library_for_view(
+            format_library_for_view(
                 lib,
                 request,
-                migrated_to=migration_info.get(lib.location.library_key)
+                migration=migration_info[lib.id],
             )
             for lib in libraries
-            if is_migrated_filter is None or (
-                BooleanField().to_internal_value(is_migrated_filter) == (lib.location.library_key in migration_info)
-            )
+            if is_migrated is None or is_migrated == bool(migration_info[lib.id])
         ]
     }
 
@@ -1713,8 +1717,7 @@ def format_in_process_course_view(uca):
 
 def get_home_context(request, no_course=False):
     """
-    Utils is used to get context of course home.
-    It is used for both DRF and django views.
+    Utils is used to get context of course home. Returned by DRF view.
     """
 
     from cms.djangoapps.contentstore.views.course import (

cms/djangoapps/contentstore/views/course.py

@@ -7,7 +7,7 @@
 import random
 import re
 import string
-from typing import Dict, NamedTuple, Optional
+from typing import Dict
 
 import django.utils
 from ccx_keys.locator import CCXLocator
@@ -44,6 +44,7 @@
 from cms.djangoapps.models.settings.course_grading import CourseGradingModel
 from cms.djangoapps.models.settings.course_metadata import CourseMetadata
 from cms.djangoapps.models.settings.encoder import CourseSettingsEncoder
+from cms.djangoapps.modulestore_migrator.data import ModulestoreMigration
 from cms.djangoapps.contentstore.api.views.utils import get_bool_param
 from common.djangoapps.course_action_state.managers import CourseActionStateItemNotFoundError
 from common.djangoapps.course_action_state.models import CourseRerunState, CourseRerunUIStateManager
@@ -670,19 +671,27 @@ def library_listing(request):
     )
 
 
-def _format_library_for_view(library, request, migrated_to: Optional[NamedTuple]):
+def format_library_for_view(library, request, migration: ModulestoreMigration | None):
     """
     Return a dict of the data which the view requires for each library
     """
-
+    migration_info = {}
+    if migration:
+        migration_info = {
+            'migrated_to_key': migration.target_key,
+            'migrated_to_title': migration.target_title,
+            'migrated_to_collection_key': migration.target_collection_slug,
+            'migrated_to_collection_title': migration.target_collection_title,
+        }
     return {
         'display_name': library.display_name,
         'library_key': str(library.location.library_key),
         'url': reverse_library_url('library_handler', str(library.location.library_key)),
         'org': library.display_org_with_default,
         'number': library.display_number_with_default,
         'can_edit': has_studio_write_access(request.user, library.location.library_key),
-        **(migrated_to._asdict() if migrated_to is not None else {}),
+        'is_migrated': migration is not None,
+        **migration_info,
     }
 
 

cms/djangoapps/modulestore_migrator/admin.py

@@ -147,8 +147,8 @@ def start_migration_task(
                     source_key=source.key,
                     target_library_key=target_library_key,
                     target_collection_slug=target_collection_slug,
-                    composition_level=form.cleaned_data['composition_level'],
-                    repeat_handling_strategy=form.cleaned_data['repeat_handling_strategy'],
+                    composition_level=CompositionLevel(form.cleaned_data['composition_level']),
+                    repeat_handling_strategy=RepeatHandlingStrategy(form.cleaned_data['repeat_handling_strategy']),
                     preserve_url_slugs=form.cleaned_data['preserve_url_slugs'],
                     forward_source_to_target=form.cleaned_data['forward_to_target'],
                 )