From dfe9cb8380ba4567a3e076b2b46d170ba0aa7dfb Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 25 Sep 2025 21:16:52 +0530 Subject: [PATCH] feat: updates legacy libraries list API to include migration info [FC-0097] (#37286) Adds migration info like `migrated_to_title`, `migrated_to_key` and `is_migrated` fields indicating whether the legacy library was migrated to library v2. If yes, it includes the new library name and key. Users can also filter by migration status using `is_migrated` query param. --- .../rest_api/v1/serializers/home.py | 23 ++- .../contentstore/rest_api/v1/views/home.py | 26 +++- .../rest_api/v1/views/tests/test_home.py | 133 ++++++++++++++++-- cms/djangoapps/contentstore/utils.py | 31 ++-- cms/djangoapps/contentstore/views/course.py | 5 +- cms/djangoapps/modulestore_migrator/api.py | 39 ++++- .../modulestore_migrator/tests/test_api.py | 55 ++++++-- 7 files changed, 266 insertions(+), 46 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py index fdc06e9291..bbc45ddf9a 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py @@ -4,9 +4,8 @@ API Serializers for course home from rest_framework import serializers -from openedx.core.lib.api.serializers import CourseKeyField - from cms.djangoapps.contentstore.rest_api.serializers.common import CourseCommonSerializer +from openedx.core.lib.api.serializers import CourseKeyField class UnsucceededCourseSerializer(serializers.Serializer): @@ -29,6 +28,26 @@ 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 class CourseHomeTabSerializer(serializers.Serializer): diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/home.py b/cms/djangoapps/contentstore/rest_api/v1/views/home.py index 62b5653387..a4e93de9ca 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/home.py @@ -2,14 +2,15 @@ import edx_api_doc_tools as apidocs from django.conf import settings +from organizations import api as org_api from rest_framework.request import Request from rest_framework.response import Response from rest_framework.views import APIView -from organizations import api as org_api + from openedx.core.lib.api.view_utils import view_auth_classes -from ....utils import get_home_context, get_course_context, get_library_context -from ..serializers import StudioHomeSerializer, CourseHomeTabSerializer, LibraryTabSerializer +from ....utils import get_course_context, get_home_context, get_library_context +from ..serializers import CourseHomeTabSerializer, LibraryTabSerializer, StudioHomeSerializer @view_auth_classes(is_authenticated=True) @@ -184,7 +185,17 @@ class HomePageLibrariesView(APIView): "org", apidocs.ParameterLocation.QUERY, description="Query param to filter by course org", - )], + ), + apidocs.query_parameter( + "is_migrated", + bool, + description=( + "Query param to filter by migrated status of library." + " If present (true or false), it will filter by migration status" + " else it will return all legacy libraries." + ), + ) + ], responses={ 200: LibraryTabSerializer, 401: "The requester is not authenticated.", @@ -197,6 +208,13 @@ class HomePageLibrariesView(APIView): **Example Request** GET /api/contentstore/v1/home/libraries + # Returns all legacy libraries + + GET /api/contentstore/v1/home/libraries?is_migrated=true + # Returns legacy libraries that were migrated to library v2 + + GET /api/contentstore/v1/home/libraries?is_migrated=false + # Returns legacy libraries that were not migrated to library v2 **Response Values** diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py index 3e88e04010..cd7592c466 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py @@ -1,18 +1,26 @@ """ Unit tests for home page view. """ -import ddt -import pytz from collections import OrderedDict from datetime import datetime, timedelta + +import ddt +import pytz from django.conf import settings from django.test import override_settings from django.urls import reverse +from opaque_keys.edx.locator import LibraryLocatorV2 +from openedx_learning.api import authoring as authoring_api +from organizations.tests.factories import OrganizationFactory from rest_framework import status -from cms.djangoapps.contentstore.tests.utils import CourseTestCase from cms.djangoapps.contentstore.tests.test_libraries import LibraryTestCase +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 @ddt.ddt @@ -131,7 +139,6 @@ class HomePageCoursesViewTest(CourseTestCase): } self.assertEqual(response.status_code, status.HTTP_200_OK) - print(response.data) self.assertDictEqual(expected_response, response.data) def test_home_page_response_with_api_v2(self): @@ -246,23 +253,121 @@ class HomePageLibrariesViewTest(LibraryTestCase): def setUp(self): super().setUp() + # Create an additional legacy library + self.lib_key_1 = self._create_library(library="lib1") + self.organization = OrganizationFactory() + + # Create a new v2 library + self.lib_key_v2 = LibraryLocatorV2.from_string( + f"lib:{self.organization.short_name}:test-key" + ) + lib_api.create_library( + org=self.organization, + slug=self.lib_key_v2.slug, + title="Test Library", + ) + 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" + authoring_api.create_collection( + learning_package_id=learning_package.id, + key=collection_key, + title="Test Collection", + created_by=self.user.id, + ) + + # Migrate self.lib_key_1 to self.lib_key_v2 + migrator_api.start_migration_to_library( + user=self.user, + source_key=self.source.key, + target_library_key=self.lib_key_v2, + target_collection_slug=collection_key, + composition_level=CompositionLevel.Component.value, + repeat_handling_strategy=RepeatHandlingStrategy.Skip.value, + preserve_url_slugs=True, + forward_source_to_target=False, + ) def test_home_page_libraries_response(self): """Check successful response content""" response = self.client.get(self.url) expected_response = { - "libraries": [{ - 'display_name': 'Test Library', - 'library_key': 'library-v1:org+lib', - 'url': '/library/library-v1:org+lib', - 'org': 'org', - 'number': 'lib', - 'can_edit': True - }], + "libraries": [ + { + 'display_name': 'Test Library', + 'library_key': 'library-v1:org+lib', + 'url': '/library/library-v1:org+lib', + 'org': 'org', + 'number': 'lib', + 'can_edit': True, + 'is_migrated': False, + }, + # Second legacy library was migrated so it will include + # migrated_to_title and migrated_to_key as well + { + 'display_name': 'Test Library', + 'library_key': 'library-v1:org+lib1', + 'url': '/library/library-v1:org+lib1', + 'org': 'org', + 'number': 'lib1', + 'can_edit': True, + 'is_migrated': True, + 'migrated_to_title': 'Test Library', + 'migrated_to_key': 'lib:name0:test-key', + 'migrated_to_collection_key': 'test-collection', + 'migrated_to_collection_title': 'Test Collection', + }, + ] } self.assertEqual(response.status_code, status.HTTP_200_OK) - print(response.data) - self.assertDictEqual(expected_response, response.data) + self.assertDictEqual(expected_response, response.json()) + + # Fetch legacy libraries that were migrated to v2 + response = self.client.get(self.url + '?is_migrated=true') + + expected_response = { + "libraries": [ + { + 'display_name': 'Test Library', + 'library_key': 'library-v1:org+lib1', + 'url': '/library/library-v1:org+lib1', + 'org': 'org', + 'number': 'lib1', + 'can_edit': True, + 'is_migrated': True, + 'migrated_to_title': 'Test Library', + 'migrated_to_key': 'lib:name0:test-key', + 'migrated_to_collection_key': 'test-collection', + 'migrated_to_collection_title': 'Test Collection', + } + ], + } + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertDictEqual(expected_response, response.json()) + + # Fetch legacy libraries that were not migrated to v2 + response = self.client.get(self.url + '?is_migrated=false') + + expected_response = { + "libraries": [ + { + 'display_name': 'Test Library', + 'library_key': 'library-v1:org+lib', + 'url': '/library/library-v1:org+lib', + 'org': 'org', + 'number': 'lib', + 'can_edit': True, + 'is_migrated': False, + }, + ], + } + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertDictEqual(expected_response, response.json()) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index c526111cf2..7140e7f853 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -26,12 +26,13 @@ from lti_consumer.models import CourseAllowPIISharingInLTIFlag from milestones import api as milestones_api from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey, UsageKeyV2 -from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocator, BlockUsageLocator +from opaque_keys.edx.locator import BlockUsageLocator, LibraryContainerLocator, LibraryLocator from openedx_events.content_authoring.data import DuplicatedXBlockData from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED from openedx_events.learning.data import CourseNotificationData from openedx_events.learning.signals import COURSE_NOTIFICATION_REQUESTED from pytz import UTC +from rest_framework.fields import BooleanField from xblock.fields import Scope from cms.djangoapps.contentstore.toggles import ( @@ -61,6 +62,7 @@ from cms.djangoapps.contentstore.toggles import ( ) 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 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 @@ -87,8 +89,8 @@ from common.djangoapps.util.milestones_helpers import ( from common.djangoapps.xblock_django.api import deprecated_xblocks from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from openedx.core import toggles as core_toggles -from openedx.core.djangoapps.content_libraries.api import get_container from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.djangoapps.content_libraries.api import get_container from openedx.core.djangoapps.content_tagging.toggles import is_tagging_feature_disabled from openedx.core.djangoapps.credit.api import get_credit_requirements, is_credit_course from openedx.core.djangoapps.discussions.config.waffle import ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND @@ -1584,12 +1586,12 @@ def get_library_context(request, request_is_json=False): It is used for both DRF and django views. """ from cms.djangoapps.contentstore.views.course import ( + _accessible_libraries_iter, + _format_library_for_view, + _get_course_creator_status, get_allowed_organizations, get_allowed_organizations_for_libraries, user_can_create_organizations, - _accessible_libraries_iter, - _get_course_creator_status, - _format_library_for_view, ) from cms.djangoapps.contentstore.views.library import ( user_can_view_create_library_button, @@ -1598,9 +1600,22 @@ def get_library_context(request, request_is_json=False): user_can_create_library, ) - libraries = _accessible_libraries_iter(request.user) if libraries_v1_enabled() else [] + 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) data = { - 'libraries': [_format_library_for_view(lib, request) for lib in libraries], + 'libraries': [ + _format_library_for_view( + lib, + request, + migrated_to=migration_info.get(lib.location.library_key) + ) + 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 not request_is_json: @@ -1716,9 +1731,7 @@ def get_home_context(request, no_course=False): get_allowed_organizations, get_allowed_organizations_for_libraries, user_can_create_organizations, - _accessible_libraries_iter, _get_course_creator_status, - _format_library_for_view, ) from cms.djangoapps.contentstore.views.library import ( user_can_view_create_library_button, diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index ffb93ed010..fa8769dc0c 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -7,7 +7,7 @@ import logging import random import re import string -from typing import Dict +from typing import Dict, NamedTuple, Optional import django.utils from ccx_keys.locator import CCXLocator @@ -669,7 +669,7 @@ def library_listing(request): return render_to_response('index.html', data) -def _format_library_for_view(library, request): +def _format_library_for_view(library, request, migrated_to: Optional[NamedTuple]): """ Return a dict of the data which the view requires for each library """ @@ -681,6 +681,7 @@ def _format_library_for_view(library, request): '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 {}), } diff --git a/cms/djangoapps/modulestore_migrator/api.py b/cms/djangoapps/modulestore_migrator/api.py index 5f6ca543a0..969cb9d537 100644 --- a/cms/djangoapps/modulestore_migrator/api.py +++ b/cms/djangoapps/modulestore_migrator/api.py @@ -1,10 +1,11 @@ """ API for migration from modulestore to learning core """ -from opaque_keys.edx.locator import LibraryLocatorV2 -from opaque_keys.edx.keys import LearningContextKey -from openedx_learning.api.authoring import get_collection from celery.result import AsyncResult +from opaque_keys.edx.keys import CourseKey, LearningContextKey +from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2 +from openedx_learning.api.authoring import get_collection +from user_tasks.models import UserTaskStatus from openedx.core.djangoapps.content_libraries.api import get_library from openedx.core.types.user import AuthUser @@ -13,9 +14,10 @@ from . import tasks from .data import RepeatHandlingStrategy from .models import ModulestoreSource - __all__ = ( "start_migration_to_library", + "is_successfully_migrated", + "get_migration_info", ) @@ -56,3 +58,32 @@ def start_migration_to_library( preserve_url_slugs=preserve_url_slugs, forward_source_to_target=forward_source_to_target, ) + + +def is_successfully_migrated(source_key: CourseKey | LibraryLocator) -> bool: + """ + Check if the source course/library has been migrated successfully. + """ + return ModulestoreSource.objects.get_or_create(key=str(source_key))[0].migrations.filter( + task_status__state=UserTaskStatus.SUCCEEDED + ).exists() + + +def get_migration_info(source_keys: list[CourseKey | LibraryLocator]) -> dict: + """ + Check if the source course/library has been migrated successfully and return target info + """ + return { + info.key: info + for info in ModulestoreSource.objects.filter( + migrations__task_status__state=UserTaskStatus.SUCCEEDED, key__in=source_keys + ) + .values_list( + 'migrations__target__key', + 'migrations__target__title', + 'migrations__target_collection__key', + 'migrations__target_collection__title', + 'key', + named=True, + ) + } diff --git a/cms/djangoapps/modulestore_migrator/tests/test_api.py b/cms/djangoapps/modulestore_migrator/tests/test_api.py index a130fcd6a7..0942bfe4ad 100644 --- a/cms/djangoapps/modulestore_migrator/tests/test_api.py +++ b/cms/djangoapps/modulestore_migrator/tests/test_api.py @@ -2,22 +2,22 @@ Test cases for the modulestore migrator API. """ +import pytest from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_learning.api import authoring as authoring_api from organizations.tests.factories import OrganizationFactory -import pytest -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from common.djangoapps.student.tests.factories import UserFactory +from cms.djangoapps.contentstore.tests.test_libraries import LibraryTestCase from cms.djangoapps.modulestore_migrator import api from cms.djangoapps.modulestore_migrator.data import CompositionLevel, RepeatHandlingStrategy from cms.djangoapps.modulestore_migrator.models import ModulestoreMigration from cms.djangoapps.modulestore_migrator.tests.factories import ModulestoreSourceFactory +from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangoapps.content_libraries import api as lib_api @pytest.mark.django_db -class TestModulestoreMigratorAPI(ModuleStoreTestCase): +class TestModulestoreMigratorAPI(LibraryTestCase): """ Test cases for the modulestore migrator API. """ @@ -26,16 +26,16 @@ class TestModulestoreMigratorAPI(ModuleStoreTestCase): super().setUp() self.organization = OrganizationFactory() - self.lib_key = LibraryLocatorV2.from_string( + self.lib_key_v2 = LibraryLocatorV2.from_string( f"lib:{self.organization.short_name}:test-key" ) lib_api.create_library( org=self.organization, - slug=self.lib_key.slug, + slug=self.lib_key_v2.slug, title="Test Library", ) - self.library = lib_api.ContentLibrary.objects.get(slug=self.lib_key.slug) - self.learning_package = self.library.learning_package + self.library_v2 = lib_api.ContentLibrary.objects.get(slug=self.lib_key_v2.slug) + self.learning_package = self.library_v2.learning_package def test_start_migration_to_library(self): """ @@ -47,7 +47,7 @@ class TestModulestoreMigratorAPI(ModuleStoreTestCase): api.start_migration_to_library( user=user, source_key=source.key, - target_library_key=self.library.library_key, + target_library_key=self.library_v2.library_key, target_collection_slug=None, composition_level=CompositionLevel.Component.value, repeat_handling_strategy=RepeatHandlingStrategy.Skip.value, @@ -84,7 +84,7 @@ class TestModulestoreMigratorAPI(ModuleStoreTestCase): api.start_migration_to_library( user=user, source_key=source.key, - target_library_key=self.library.library_key, + target_library_key=self.library_v2.library_key, target_collection_slug=collection_key, composition_level=CompositionLevel.Component.value, repeat_handling_strategy=RepeatHandlingStrategy.Skip.value, @@ -106,10 +106,43 @@ class TestModulestoreMigratorAPI(ModuleStoreTestCase): api.start_migration_to_library( user=user, source_key=source.key, - target_library_key=self.library.library_key, + target_library_key=self.library_v2.library_key, target_collection_slug=None, composition_level=CompositionLevel.Component.value, repeat_handling_strategy=RepeatHandlingStrategy.Fork.value, preserve_url_slugs=True, forward_source_to_target=False, ) + + def test_get_migration_info(self): + """ + Test that the API can retrieve migration info. + """ + user = UserFactory() + + collection_key = "test-collection" + authoring_api.create_collection( + learning_package_id=self.learning_package.id, + key=collection_key, + title="Test Collection", + created_by=user.id, + ) + + api.start_migration_to_library( + user=user, + source_key=self.lib_key, + target_library_key=self.library_v2.library_key, + target_collection_slug=collection_key, + composition_level=CompositionLevel.Component.value, + repeat_handling_strategy=RepeatHandlingStrategy.Skip.value, + preserve_url_slugs=True, + forward_source_to_target=True, + ) + with self.assertNumQueries(1): + result = api.get_migration_info([self.lib_key]) + row = result.get(self.lib_key) + assert row is not None + assert row.migrations__target__key == str(self.lib_key_v2) + assert row.migrations__target__title == "Test Library" + assert row.migrations__target_collection__key == collection_key + assert row.migrations__target_collection__title == "Test Collection"