diff --git a/.github/workflows/check-consistent-dependencies.yml b/.github/workflows/check-consistent-dependencies.yml index 82298af70a..c3f35d92a0 100644 --- a/.github/workflows/check-consistent-dependencies.yml +++ b/.github/workflows/check-consistent-dependencies.yml @@ -34,7 +34,7 @@ jobs: echo "RELEVANT=true" >> "$GITHUB_ENV" fi - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 if: ${{ env.RELEVANT == 'true' }} - uses: actions/setup-python@v5 diff --git a/.github/workflows/check-for-tutorial-prs.yml b/.github/workflows/check-for-tutorial-prs.yml index dc8d8557e5..1dc4d38609 100644 --- a/.github/workflows/check-for-tutorial-prs.yml +++ b/.github/workflows/check-for-tutorial-prs.yml @@ -23,7 +23,7 @@ jobs: name: provide helpful bot comment steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Comment PR uses: thollander/actions-comment-pull-request@v2 diff --git a/.github/workflows/check_python_dependencies.yml b/.github/workflows/check_python_dependencies.yml index 281e26589d..7b93a545cd 100644 --- a/.github/workflows/check_python_dependencies.yml +++ b/.github/workflows/check_python_dependencies.yml @@ -13,7 +13,7 @@ jobs: steps: - name: Checkout Repository - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Set up Python uses: actions/setup-python@v5 diff --git a/.github/workflows/ci-static-analysis.yml b/.github/workflows/ci-static-analysis.yml index d989ff9db2..d2513ba210 100644 --- a/.github/workflows/ci-static-analysis.yml +++ b/.github/workflows/ci-static-analysis.yml @@ -13,7 +13,7 @@ jobs: os: ["ubuntu-24.04"] steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: Set up Python uses: actions/setup-python@v5 with: diff --git a/.github/workflows/compile-python-requirements.yml b/.github/workflows/compile-python-requirements.yml index 21cb80083f..8673cc3c23 100644 --- a/.github/workflows/compile-python-requirements.yml +++ b/.github/workflows/compile-python-requirements.yml @@ -19,7 +19,7 @@ jobs: steps: - name: Check out target branch - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: ref: "${{ inputs.branch }}" @@ -44,7 +44,7 @@ jobs: - name: Make a PR id: make-pr - uses: peter-evans/create-pull-request@v6 + uses: peter-evans/create-pull-request@v7 with: branch: "${{ github.triggering_actor }}/compile-python-deps" branch-suffix: short-commit-hash diff --git a/.github/workflows/js-tests.yml b/.github/workflows/js-tests.yml index 6df9cee794..94a1368e96 100644 --- a/.github/workflows/js-tests.yml +++ b/.github/workflows/js-tests.yml @@ -18,7 +18,7 @@ jobs: - "3.11" steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: Fetch master to compare coverage run: git fetch --depth=1 origin master diff --git a/.github/workflows/lint-imports.yml b/.github/workflows/lint-imports.yml index e3c59ec093..baf914298b 100644 --- a/.github/workflows/lint-imports.yml +++ b/.github/workflows/lint-imports.yml @@ -13,7 +13,7 @@ jobs: steps: - name: Check out branch - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Set up Python uses: actions/setup-python@v5 diff --git a/.github/workflows/migrations-check.yml b/.github/workflows/migrations-check.yml index 84e334d688..cd4d09589c 100644 --- a/.github/workflows/migrations-check.yml +++ b/.github/workflows/migrations-check.yml @@ -70,7 +70,7 @@ jobs: docker exec ${{ job.services.mongo.id }} mongosh --host 127.0.0.1 --username edxapp --password password --eval 'use edxapp; db.adminCommand("ping");' edxapp - name: Checkout repo - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Setup Python ${{ matrix.python-version }} uses: actions/setup-python@v5 diff --git a/.github/workflows/pylint-checks.yml b/.github/workflows/pylint-checks.yml index dd90fd05d9..abc51eb91b 100644 --- a/.github/workflows/pylint-checks.yml +++ b/.github/workflows/pylint-checks.yml @@ -31,7 +31,7 @@ jobs: name: pylint ${{ matrix.module-name }} steps: - name: Check out repo - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Install required system packages run: sudo apt-get update && sudo apt-get install libxmlsec1-dev diff --git a/.github/workflows/quality-checks.yml b/.github/workflows/quality-checks.yml index 3c80c1fac8..3f4cbeeb4d 100644 --- a/.github/workflows/quality-checks.yml +++ b/.github/workflows/quality-checks.yml @@ -19,7 +19,7 @@ jobs: node-version: [20] steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 with: fetch-depth: 2 diff --git a/.github/workflows/semgrep.yml b/.github/workflows/semgrep.yml index d880d73517..520cd23a67 100644 --- a/.github/workflows/semgrep.yml +++ b/.github/workflows/semgrep.yml @@ -22,7 +22,7 @@ jobs: - "3.11" steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 with: fetch-depth: 1 diff --git a/.github/workflows/static-assets-check.yml b/.github/workflows/static-assets-check.yml index 502dddce08..43cb597c16 100644 --- a/.github/workflows/static-assets-check.yml +++ b/.github/workflows/static-assets-check.yml @@ -35,7 +35,7 @@ jobs: steps: - name: Checkout repo - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Setup Python ${{ matrix.python-version }} uses: actions/setup-python@v5 diff --git a/.github/workflows/unit-test-shards.json b/.github/workflows/unit-test-shards.json index 784f607f06..827366365f 100644 --- a/.github/workflows/unit-test-shards.json +++ b/.github/workflows/unit-test-shards.json @@ -238,6 +238,7 @@ "cms/djangoapps/cms_user_tasks/", "cms/djangoapps/course_creators/", "cms/djangoapps/export_course_metadata/", + "cms/djangoapps/modulestore_migrator/", "cms/djangoapps/maintenance/", "cms/djangoapps/models/", "cms/djangoapps/pipeline_js/", diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 81f65eda77..d8b8c26cd0 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -68,7 +68,7 @@ jobs: steps: - name: checkout repo - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: install system requirements run: | @@ -149,7 +149,7 @@ jobs: collect-and-verify: runs-on: ubuntu-24.04 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: Setup Python uses: actions/setup-python@v5 with: @@ -225,9 +225,9 @@ jobs: runs-on: ubuntu-24.04 needs: [run-tests] steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: collect pytest warnings files - uses: actions/download-artifact@v4 + uses: actions/download-artifact@v5 with: pattern: pytest-warnings-json-* merge-multiple: true @@ -278,7 +278,7 @@ jobs: - 3.11 steps: - name: Checkout repo - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Setup Python ${{ matrix.python-version }} uses: actions/setup-python@v5 @@ -286,7 +286,7 @@ jobs: python-version: ${{ matrix.python-version }} - name: Download all artifacts - uses: actions/download-artifact@v4 + uses: actions/download-artifact@v5 with: pattern: coverage-* merge-multiple: true @@ -301,4 +301,4 @@ jobs: coverage combine reports/* coverage report coverage xml - - uses: codecov/codecov-action@v4 + - uses: codecov/codecov-action@v5 diff --git a/.github/workflows/units-test-scripts-structures-pruning.yml b/.github/workflows/units-test-scripts-structures-pruning.yml index cbf9da8f5c..14a01b5923 100644 --- a/.github/workflows/units-test-scripts-structures-pruning.yml +++ b/.github/workflows/units-test-scripts-structures-pruning.yml @@ -17,7 +17,7 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Set up Python uses: actions/setup-python@v5 diff --git a/.github/workflows/units-test-scripts-user-retirement.yml b/.github/workflows/units-test-scripts-user-retirement.yml index f1b2b2c539..889c43a64a 100644 --- a/.github/workflows/units-test-scripts-user-retirement.yml +++ b/.github/workflows/units-test-scripts-user-retirement.yml @@ -17,7 +17,7 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Set up Python uses: actions/setup-python@v5 diff --git a/.github/workflows/update-geolite-database.yml b/.github/workflows/update-geolite-database.yml index 08716b8ad9..484fa167a3 100644 --- a/.github/workflows/update-geolite-database.yml +++ b/.github/workflows/update-geolite-database.yml @@ -24,7 +24,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout Repository - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Download GeoLite tar file run: | diff --git a/.github/workflows/upgrade-one-python-dependency.yml b/.github/workflows/upgrade-one-python-dependency.yml index 84a00266e9..3f9678593c 100644 --- a/.github/workflows/upgrade-one-python-dependency.yml +++ b/.github/workflows/upgrade-one-python-dependency.yml @@ -32,7 +32,7 @@ jobs: steps: - name: Check out target branch - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: ref: "${{ inputs.branch }}" @@ -82,7 +82,7 @@ jobs: - name: Make a PR id: make-pr - uses: peter-evans/create-pull-request@v6 + uses: peter-evans/create-pull-request@v7 with: branch: "${{ github.triggering_actor }}/upgrade-${{ inputs.package }}" branch-suffix: short-commit-hash diff --git a/.github/workflows/verify-dunder-init.yml b/.github/workflows/verify-dunder-init.yml index c398c50673..c3248def2f 100644 --- a/.github/workflows/verify-dunder-init.yml +++ b/.github/workflows/verify-dunder-init.yml @@ -12,7 +12,7 @@ jobs: steps: - name: Check out branch - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Ensure git is installed run: | diff --git a/.readthedocs.yaml b/.readthedocs.yaml index c0db64e5e1..fdef59eb56 100644 --- a/.readthedocs.yaml +++ b/.readthedocs.yaml @@ -1,7 +1,7 @@ version: 2 build: - os: "ubuntu-22.04" + os: "ubuntu-lts-latest" tools: python: "3.11" diff --git a/cms/djangoapps/contentstore/migrations/0013_componentlink_downstream_is_modified_and_more.py b/cms/djangoapps/contentstore/migrations/0013_componentlink_downstream_is_modified_and_more.py new file mode 100644 index 0000000000..c119387ce7 --- /dev/null +++ b/cms/djangoapps/contentstore/migrations/0013_componentlink_downstream_is_modified_and_more.py @@ -0,0 +1,23 @@ +# Generated by Django 4.2.24 on 2025-09-23 19:47 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('contentstore', '0012_componentlink_top_level_parent_and_more'), + ] + + operations = [ + migrations.AddField( + model_name='componentlink', + name='downstream_is_modified', + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name='containerlink', + name='downstream_is_modified', + field=models.BooleanField(default=False), + ), + ] diff --git a/cms/djangoapps/contentstore/models.py b/cms/djangoapps/contentstore/models.py index fd98ec44fe..47a450cde8 100644 --- a/cms/djangoapps/contentstore/models.py +++ b/cms/djangoapps/contentstore/models.py @@ -108,6 +108,7 @@ class EntityLinkBase(models.Model): top_level_parent = models.ForeignKey("ContainerLink", on_delete=models.SET_NULL, null=True, blank=True) version_synced = models.IntegerField() version_declined = models.IntegerField(null=True, blank=True) + downstream_is_modified = models.BooleanField(default=False) created = manual_date_time_field() updated = manual_date_time_field() @@ -257,6 +258,7 @@ class ComponentLink(EntityLinkBase): version_synced: int, top_level_parent_usage_key: UsageKey | None = None, version_declined: int | None = None, + downstream_is_modified: bool = False, created: datetime | None = None, ) -> "ComponentLink": """ @@ -281,6 +283,7 @@ class ComponentLink(EntityLinkBase): 'version_synced': version_synced, 'version_declined': version_declined, 'top_level_parent': top_level_parent, + 'downstream_is_modified': downstream_is_modified, } if upstream_block: new_values['upstream_block'] = upstream_block @@ -482,6 +485,7 @@ class ContainerLink(EntityLinkBase): version_synced: int, top_level_parent_usage_key: UsageKey | None = None, version_declined: int | None = None, + downstream_is_modified: bool = False, created: datetime | None = None, ) -> "ContainerLink": """ @@ -506,6 +510,7 @@ class ContainerLink(EntityLinkBase): 'version_synced': version_synced, 'version_declined': version_declined, 'top_level_parent': top_level_parent, + 'downstream_is_modified': downstream_is_modified, } if upstream_container_id: new_values['upstream_container_id'] = upstream_container_id 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/rest_api/v2/views/tests/test_downstream_sync_integration.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py index 8eba50c829..e5cd063e81 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py @@ -384,6 +384,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): 'updated': date_format, 'upstream_key': self.upstream_html1["id"], 'upstream_type': 'component', + 'downstream_is_modified': False, }, { 'id': 2, @@ -400,7 +401,8 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): 'created': date_format, 'updated': date_format, 'upstream_key': self.upstream_problem1["id"], - 'upstream_type': 'component' + 'upstream_type': 'component', + 'downstream_is_modified': False, }, { 'id': 3, @@ -417,7 +419,8 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): 'created': date_format, 'updated': date_format, 'upstream_key': self.upstream_problem2["id"], - 'upstream_type': 'component' + 'upstream_type': 'component', + 'downstream_is_modified': False, }, { 'id': 1, @@ -434,7 +437,8 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): 'created': date_format, 'updated': date_format, 'upstream_key': self.upstream_unit["id"], - 'upstream_type': 'container' + 'upstream_type': 'container', + 'downstream_is_modified': False, } ] data = downstreams.json() @@ -533,6 +537,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): 'updated': date_format, 'upstream_key': self.upstream_html1["id"], 'upstream_type': 'component', + 'downstream_is_modified': False, }, { 'id': 2, @@ -549,7 +554,8 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): 'created': date_format, 'updated': date_format, 'upstream_key': self.upstream_problem1["id"], - 'upstream_type': 'component' + 'upstream_type': 'component', + 'downstream_is_modified': False, }, { 'id': 3, @@ -566,7 +572,8 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): 'created': date_format, 'updated': date_format, 'upstream_key': self.upstream_problem2["id"], - 'upstream_type': 'component' + 'upstream_type': 'component', + 'downstream_is_modified': False, }, { 'id': 1, @@ -583,7 +590,8 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): 'created': date_format, 'updated': date_format, 'upstream_key': self.upstream_unit["id"], - 'upstream_type': 'container' + 'upstream_type': 'container', + 'downstream_is_modified': False, } ] data = downstreams.json() @@ -681,6 +689,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): 'updated': date_format, 'upstream_key': self.upstream_html1["id"], 'upstream_type': 'component', + 'downstream_is_modified': False, }, { 'id': 2, @@ -697,7 +706,8 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): 'created': date_format, 'updated': date_format, 'upstream_key': self.upstream_problem1["id"], - 'upstream_type': 'component' + 'upstream_type': 'component', + 'downstream_is_modified': False, }, { 'id': 4, @@ -714,7 +724,8 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): 'created': date_format, 'updated': date_format, 'upstream_key': upstream_problem3["id"], - 'upstream_type': 'component' + 'upstream_type': 'component', + 'downstream_is_modified': False, }, { 'id': 1, @@ -731,7 +742,8 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): 'created': date_format, 'updated': date_format, 'upstream_key': self.upstream_unit["id"], - 'upstream_type': 'container' + 'upstream_type': 'container', + 'downstream_is_modified': False, } ] data = downstreams.json() @@ -810,6 +822,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): 'updated': date_format, 'upstream_key': self.upstream_html1["id"], 'upstream_type': 'component', + 'downstream_is_modified': False, }, { 'id': 2, @@ -826,7 +839,8 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): 'created': date_format, 'updated': date_format, 'upstream_key': self.upstream_problem1["id"], - 'upstream_type': 'component' + 'upstream_type': 'component', + 'downstream_is_modified': False, }, { 'id': 4, @@ -843,7 +857,8 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): 'created': date_format, 'updated': date_format, 'upstream_key': upstream_problem3["id"], - 'upstream_type': 'component' + 'upstream_type': 'component', + 'downstream_is_modified': False, }, { 'id': 1, @@ -860,7 +875,8 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): 'created': date_format, 'updated': date_format, 'upstream_key': self.upstream_unit["id"], - 'upstream_type': 'container' + 'upstream_type': 'container', + 'downstream_is_modified': False, } ] data = downstreams.json() @@ -1047,6 +1063,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): Test that we can sync a html from a library into a course. """ # 1️⃣ First, create the html in the course, using the upstream problem as a template: + date_format = self.now.isoformat().split("+")[0] + 'Z' downstream_html1 = self._create_block_from_upstream( block_category="html", parent_usage_key=str(self.course_subsection.usage_key), @@ -1079,6 +1096,34 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): >This is the HTML. """) + # Check that: The downstream links are created as expected for the component + downstreams = self._get_downstream_links( + course_id=str(self.course.id) + ) + expected_downstreams = [ + { + 'id': 1, + 'upstream_context_title': self.library_title, + 'upstream_version': 2, + 'ready_to_sync': False, + 'ready_to_sync_from_children': False, + 'upstream_context_key': self.library_id, + 'downstream_usage_key': downstream_html1["locator"], + 'downstream_context_key': str(self.course.id), + 'top_level_parent_usage_key': None, + 'version_synced': 2, + 'version_declined': None, + 'created': date_format, + 'updated': date_format, + 'upstream_key': self.upstream_html1["id"], + 'upstream_type': 'component', + 'downstream_is_modified': False, + }, + ] + data = downstreams.json() + self.assertEqual(data["count"], 1) + self.assertListEqual(data["results"], expected_downstreams) + # 2️⃣ Now, lets modify the upstream html AND the downstream display_name: self._update_course_block_fields(downstream_html1["locator"], { "display_name": "New Text Content", @@ -1111,9 +1156,36 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): 'version_declined': None, 'ready_to_sync': True, # <--- updated 'error_message': None, - 'is_modified': True, + 'is_modified': True, # <--- updated }) + downstreams = self._get_downstream_links( + course_id=str(self.course.id) + ) + expected_downstreams = [ + { + 'id': 1, + 'upstream_context_title': self.library_title, + 'upstream_version': 3, # <--- updated + 'ready_to_sync': True, # <--- updated + 'ready_to_sync_from_children': False, + 'upstream_context_key': self.library_id, + 'downstream_usage_key': downstream_html1["locator"], + 'downstream_context_key': str(self.course.id), + 'top_level_parent_usage_key': None, + 'version_synced': 2, + 'version_declined': None, + 'created': date_format, + 'updated': date_format, + 'upstream_key': self.upstream_html1["id"], + 'upstream_type': 'component', + 'downstream_is_modified': True, # <--- updated + }, + ] + data = downstreams.json() + self.assertEqual(data["count"], 1) + self.assertListEqual(data["results"], expected_downstreams) + # 3️⃣ Now, sync and check the resulting OLX of the downstream self._sync_downstream(downstream_html1["locator"]) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py index bab601e9c0..b8f2c8f410 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py @@ -675,6 +675,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -692,6 +693,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -709,6 +711,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': str(self.top_level_downstream_unit.usage_key), + 'downstream_is_modified': False, }, { 'created': date_format, @@ -726,6 +729,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': str(self.top_level_downstream_chapter.usage_key), + 'downstream_is_modified': False, }, { 'created': date_format, @@ -743,6 +747,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -760,6 +765,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -777,6 +783,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -794,6 +801,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -811,6 +819,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -828,6 +837,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': str(self.top_level_downstream_chapter.usage_key), + 'downstream_is_modified': False, }, { 'created': date_format, @@ -845,6 +855,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': str(self.top_level_downstream_chapter.usage_key), + 'downstream_is_modified': False, }, ] self.assertListEqual(data["results"], expected) @@ -884,6 +895,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -901,6 +913,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -918,6 +931,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': str(self.top_level_downstream_unit.usage_key), + 'downstream_is_modified': False, }, { 'created': date_format, @@ -935,6 +949,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': str(self.top_level_downstream_chapter.usage_key), + 'downstream_is_modified': False, }, ] self.assertListEqual(data["results"], expected) @@ -969,6 +984,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -986,6 +1002,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -1003,6 +1020,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -1020,6 +1038,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -1037,6 +1056,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -1054,6 +1074,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': str(self.top_level_downstream_chapter.usage_key), + 'downstream_is_modified': False, }, { 'created': date_format, @@ -1071,6 +1092,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': str(self.top_level_downstream_chapter.usage_key), + 'downstream_is_modified': False, }, ] self.assertListEqual(data["results"], expected) @@ -1170,6 +1192,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -1187,6 +1210,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -1204,6 +1228,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -1221,6 +1246,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, ] print(data["results"]) @@ -1267,6 +1293,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -1284,6 +1311,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -1301,6 +1329,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, ] self.assertListEqual(data["results"], expected) @@ -1354,6 +1383,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -1371,6 +1401,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, { 'created': date_format, @@ -1388,6 +1419,7 @@ class GetUpstreamViewTest( 'version_declined': None, 'version_synced': 1, 'top_level_parent_usage_key': None, + 'downstream_is_modified': False, }, ] self.assertListEqual(data["results"], expected) diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index a72d14e931..08d858c550 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -180,7 +180,9 @@ class CourseAdvanceSettingViewTest(CourseTestCase, MilestonesTestCaseMixin): """ advanced_settings_link_html = f"Advanced Settings".encode('utf-8') - with override_settings(FEATURES={'DISABLE_ADVANCED_SETTINGS': disable_advanced_settings}): + with override_settings(FEATURES={ + 'DISABLE_ADVANCED_SETTINGS': disable_advanced_settings, + }): for handler in ( 'import_handler', 'export_handler', diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index c526111cf2..3ca1d20bf5 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, @@ -2413,6 +2426,7 @@ def _create_or_update_component_link(created: datetime | None, xblock): top_level_parent_usage_key=top_level_parent_usage_key, version_synced=xblock.upstream_version, version_declined=xblock.upstream_version_declined, + downstream_is_modified=len(getattr(xblock, "downstream_customized", [])) > 0, created=created, ) @@ -2445,6 +2459,7 @@ def _create_or_update_container_link(created: datetime | None, xblock): version_synced=xblock.upstream_version, top_level_parent_usage_key=top_level_parent_usage_key, version_declined=xblock.upstream_version_declined, + downstream_is_modified=len(getattr(xblock, "downstream_customized", [])) > 0, created=created, ) 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/__init__.py b/cms/djangoapps/modulestore_migrator/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cms/djangoapps/modulestore_migrator/admin.py b/cms/djangoapps/modulestore_migrator/admin.py new file mode 100644 index 0000000000..8eef778531 --- /dev/null +++ b/cms/djangoapps/modulestore_migrator/admin.py @@ -0,0 +1,192 @@ +""" +A nice little admin interface for migrating courses and libraries from modulstore to Learning Core. +""" +import logging + +from django import forms +from django.contrib import admin, messages +from django.contrib.admin.helpers import ActionForm +from django.db import models + + +from opaque_keys import InvalidKeyError +from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryLocatorV2 +from user_tasks.models import UserTaskStatus + +from openedx.core.types.http import AuthenticatedHttpRequest + +from . import api +from .data import CompositionLevel, RepeatHandlingStrategy +from .models import ModulestoreSource, ModulestoreMigration, ModulestoreBlockSource, ModulestoreBlockMigration + + +log = logging.getLogger(__name__) + + +class StartMigrationTaskForm(ActionForm): + """ + Params for start_migration_task admin adtion, displayed next the "Go" button. + """ + target_key = forms.CharField(label="Target library or collection key →", required=False) + repeat_handling_strategy = forms.ChoiceField( + label="How to handle existing content? →", + choices=RepeatHandlingStrategy.supported_choices, + required=False, + ) + preserve_url_slugs = forms.BooleanField(label="Preserve current slugs? →", required=False, initial=True) + forward_to_target = forms.BooleanField(label="Forward references? →", required=False) + composition_level = forms.ChoiceField( + label="Aggregate up to →", choices=CompositionLevel.supported_choices, required=False + ) + + +def task_status_details(obj: ModulestoreMigration) -> str: + """ + Return the state and, if available, details of the status of the migration. + """ + details: str | None = None + if obj.task_status.state == UserTaskStatus.FAILED: + # Calling fail(msg) from a task should automatically generates an "Error" artifact with that msg. + # https://django-user-tasks.readthedocs.io/en/latest/user_tasks.html#user_tasks.models.UserTaskStatus.fail + if error_artifacts := obj.task_status.artifacts.filter(name="Error"): + if error_text := error_artifacts.order_by("-created").first().text: + details = error_text + elif obj.task_status.state == UserTaskStatus.SUCCEEDED: + details = f"Migrated {obj.block_migrations.count()} blocks" + return f"{obj.task_status.state}: {details}" if details else obj.task_status.state + + +migration_admin_fields = ( + "target", + "target_collection", + "task_status", + # The next line works, but django-stubs incorrectly thinks that these should all be strings, + # so we will need to use type:ignore below. + task_status_details, + "composition_level", + "repeat_handling_strategy", + "preserve_url_slugs", + "change_log", + "staged_content", +) + + +class ModulestoreMigrationInline(admin.TabularInline): + """ + Readonly table within the ModulestoreSource page; each row is a Migration from this Source. + """ + model = ModulestoreMigration + fk_name = "source" + show_change_link = True + readonly_fields = migration_admin_fields # type: ignore[assignment] + ordering = ("-task_status__created",) + + def has_add_permission(self, _request, _obj): + return False + + +class ModulestoreBlockSourceInline(admin.TabularInline): + """ + Readonly table within the ModulestoreSource page; each row is a BlockSource. + """ + model = ModulestoreBlockSource + fk_name = "overall_source" + readonly_fields = ( + "key", + "forwarded" + ) + + def has_add_permission(self, _request, _obj): + return False + + +@admin.register(ModulestoreSource) +class ModulestoreSourceAdmin(admin.ModelAdmin): + """ + Admin interface for source legacy libraries and courses. + """ + readonly_fields = ("forwarded",) + list_display = ("id", "key", "forwarded") + actions = ["start_migration_task"] + action_form = StartMigrationTaskForm + inlines = [ModulestoreMigrationInline, ModulestoreBlockSourceInline] + + @admin.action(description="Start migration for selected sources") + def start_migration_task( + self, + request: AuthenticatedHttpRequest, + queryset: models.QuerySet[ModulestoreSource], + ) -> None: + """ + Start a migration for each selected source + """ + form = StartMigrationTaskForm(request.POST) + form.is_valid() + target_key_string = form.cleaned_data['target_key'] + if not target_key_string: + messages.add_message(request, messages.ERROR, "Target key is required") + return + try: + target_library_key = LibraryLocatorV2.from_string(target_key_string) + target_collection_slug = None + except InvalidKeyError: + try: + target_collection_key = LibraryCollectionLocator.from_string(target_key_string) + target_library_key = target_collection_key.lib_key + target_collection_slug = target_collection_key.collection_id + except InvalidKeyError: + messages.add_message(request, messages.ERROR, f"Invalid target key: {target_key_string}") + return + started = 0 + total = 0 + for source in queryset: + total += 1 + try: + api.start_migration_to_library( + user=request.user, + 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'], + preserve_url_slugs=form.cleaned_data['preserve_url_slugs'], + forward_source_to_target=form.cleaned_data['forward_to_target'], + ) + except Exception as exc: # pylint: disable=broad-except + message = f"Failed to start migration {source.key} -> {target_key_string}" + messages.add_message(request, messages.ERROR, f"{message}: {exc}") + log.exception(message) + continue + started += 1 + click_in = "Click into the source objects to see migration details." + + if not started: + messages.add_message(request, messages.WARNING, f"Failed to start {total} migration(s).") + if started < total: + messages.add_message(request, messages.WARNING, f"Started {started} of {total} migration(s). {click_in}") + else: + messages.add_message(request, messages.INFO, f"Started {started} migration(s). {click_in}") + + +class ModulestoreBlockMigrationInline(admin.TabularInline): + """ + Readonly table witin the Migration admin; each row is a block + """ + model = ModulestoreBlockMigration + fk_name = "overall_migration" + readonly_fields = ( + "source", + "target", + "change_log_record", + ) + list_display = ("id", *readonly_fields) + + +@admin.register(ModulestoreMigration) +class ModulestoreMigrationAdmin(admin.ModelAdmin): + """ + Readonly admin page for viewing Migrations + """ + readonly_fields = ("source", *migration_admin_fields) # type: ignore[assignment] + list_display = ("id", "source", *migration_admin_fields) # type: ignore[assignment] + inlines = [ModulestoreBlockMigrationInline] diff --git a/cms/djangoapps/modulestore_migrator/api.py b/cms/djangoapps/modulestore_migrator/api.py new file mode 100644 index 0000000000..969cb9d537 --- /dev/null +++ b/cms/djangoapps/modulestore_migrator/api.py @@ -0,0 +1,89 @@ +""" +API for migration from modulestore to learning core +""" +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 + +from . import tasks +from .data import RepeatHandlingStrategy +from .models import ModulestoreSource + +__all__ = ( + "start_migration_to_library", + "is_successfully_migrated", + "get_migration_info", +) + + +def start_migration_to_library( + *, + user: AuthUser, + source_key: LearningContextKey, + target_library_key: LibraryLocatorV2, + target_collection_slug: str | None = None, + composition_level: str, + repeat_handling_strategy: str, + preserve_url_slugs: bool, + forward_source_to_target: bool, +) -> AsyncResult: + """ + Import a course or legacy library into a V2 library (or, a collection within a V2 library). + """ + # Can raise NotImplementedError for the Fork strategy + assert RepeatHandlingStrategy(repeat_handling_strategy).is_implemented() + + source, _ = ModulestoreSource.objects.get_or_create(key=source_key) + target_library = get_library(target_library_key) + # get_library ensures that the library is connected to a learning package. + target_package_id: int = target_library.learning_package_id # type: ignore[assignment] + target_collection_id = None + + if target_collection_slug: + target_collection_id = get_collection(target_package_id, target_collection_slug).id + + return tasks.migrate_from_modulestore.delay( + user_id=user.id, + source_pk=source.id, + target_package_pk=target_package_id, + target_library_key=str(target_library_key), + target_collection_pk=target_collection_id, + composition_level=composition_level, + repeat_handling_strategy=repeat_handling_strategy, + 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/apps.py b/cms/djangoapps/modulestore_migrator/apps.py new file mode 100644 index 0000000000..37c2b66d6f --- /dev/null +++ b/cms/djangoapps/modulestore_migrator/apps.py @@ -0,0 +1,13 @@ +""" +App configurations +""" + +from django.apps import AppConfig + + +class ModulestoreMigratorConfig(AppConfig): + """ + App for importing legacy content from the modulestore. + """ + + name = 'cms.djangoapps.modulestore_migrator' diff --git a/cms/djangoapps/modulestore_migrator/constants.py b/cms/djangoapps/modulestore_migrator/constants.py new file mode 100644 index 0000000000..ec7740ef19 --- /dev/null +++ b/cms/djangoapps/modulestore_migrator/constants.py @@ -0,0 +1,6 @@ +""" +Constants +""" + +CONTENT_STAGING_PURPOSE_PREFIX = "modulestore_migrator" +CONTENT_STAGING_PURPOSE_TEMPLATE = CONTENT_STAGING_PURPOSE_PREFIX + "({source_key})" diff --git a/cms/djangoapps/modulestore_migrator/data.py b/cms/djangoapps/modulestore_migrator/data.py new file mode 100644 index 0000000000..c2c5f0c29f --- /dev/null +++ b/cms/djangoapps/modulestore_migrator/data.py @@ -0,0 +1,81 @@ +""" +Value objects +""" +from __future__ import annotations + +from enum import Enum + +from openedx.core.djangoapps.content_libraries.api import ContainerType + + +class CompositionLevel(Enum): + """ + Enumeration of composition levels for legacy content. + + Defined in increasing order of complexity so that `is_higher_than` works correctly. + """ + # Components are individual XBlocks, e.g. Problem + Component = 'component' + + # Container types currently supported by Content Libraries + Unit = ContainerType.Unit.value + Subsection = ContainerType.Subsection.value + Section = ContainerType.Section.value + + @property + def is_container(self) -> bool: + return self is not self.Component + + def is_higher_than(self, other: 'CompositionLevel') -> bool: + """ + Is this composition level 'above' (more complex than) the other? + """ + levels: list[CompositionLevel] = list(self.__class__) + return levels.index(self) > levels.index(other) + + @classmethod + def supported_choices(cls) -> list[tuple[str, str]]: + """ + Returns all supported composition levels as a list of tuples, + for use in a Django Models ChoiceField. + """ + return [ + (composition_level.value, composition_level.name) + for composition_level in cls + ] + + +class RepeatHandlingStrategy(Enum): + """ + Enumeration of repeat handling strategies for imported content. + """ + Skip = 'skip' + Fork = 'fork' + Update = 'update' + + @classmethod + def supported_choices(cls) -> list[tuple[str, str]]: + """ + Returns all supported repeat handling strategies as a list of tuples, + for use in a Django Models ChoiceField. + """ + return [ + (strategy.value, strategy.name) + for strategy in cls + ] + + @classmethod + def default(cls) -> RepeatHandlingStrategy: + """ + Returns the default repeat handling strategy. + """ + return cls.Skip + + def is_implemented(self) -> bool: + """ + Returns True if the repeat handling strategy is implemented. + """ + if self == self.Fork: + raise NotImplementedError("Forking is not implemented yet.") + + return True diff --git a/cms/djangoapps/modulestore_migrator/migrations/0001_initial.py b/cms/djangoapps/modulestore_migrator/migrations/0001_initial.py new file mode 100644 index 0000000000..42d1df5bad --- /dev/null +++ b/cms/djangoapps/modulestore_migrator/migrations/0001_initial.py @@ -0,0 +1,108 @@ +# Generated by Django 4.2.24 on 2025-09-10 15:14 + +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import model_utils.fields +import opaque_keys.edx.django.models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ('content_staging', '0006_alter_userclipboard_source_usage_key'), + ('oel_collections', '0005_alter_collection_options_alter_collection_enabled'), + ('oel_publishing', '0008_alter_draftchangelogrecord_options_and_more'), + ('user_tasks', '0004_url_textfield'), + ] + + operations = [ + migrations.CreateModel( + name='ModulestoreBlockMigration', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')), + ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')), + ('change_log_record', models.OneToOneField(null=True, on_delete=django.db.models.deletion.SET_NULL, to='oel_publishing.draftchangelogrecord')), + ], + ), + migrations.CreateModel( + name='ModulestoreMigration', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('source_version', models.CharField(blank=True, help_text='Migrated content version, the hash of published content version', max_length=255, null=True)), + ('composition_level', models.CharField(choices=[('component', 'Component'), ('unit', 'Unit'), ('subsection', 'Subsection'), ('section', 'Section')], default='component', help_text='Maximum hierachy level at which content should be aggregated in target library', max_length=255)), + ('repeat_handling_strategy', models.CharField(choices=[('skip', 'Skip'), ('fork', 'Fork'), ('update', 'Update')], default='skip', help_text='If a piece of content already exists in the content library, choose how to handle it.', max_length=24)), + ('preserve_url_slugs', models.BooleanField(default=False, help_text='Should the migration preserve the location IDs of the existing blocks?If not, then new, unique human-readable IDs will be generated based on the block titles.')), + ('change_log', models.ForeignKey(help_text='Changelog entry in the target learning package which records this migration', null=True, on_delete=django.db.models.deletion.SET_NULL, to='oel_publishing.draftchangelog')), + ], + ), + migrations.CreateModel( + name='ModulestoreSource', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('key', opaque_keys.edx.django.models.LearningContextKeyField(help_text='Key of the content source (a course or a legacy library)', max_length=255, unique=True)), + ('forwarded', models.OneToOneField(blank=True, help_text='If set, the system will forward references of this source over to the target of this migration', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='forwards', to='modulestore_migrator.modulestoremigration')), + ], + ), + migrations.AddField( + model_name='modulestoremigration', + name='source', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='migrations', to='modulestore_migrator.modulestoresource'), + ), + migrations.AddField( + model_name='modulestoremigration', + name='staged_content', + field=models.OneToOneField(help_text='Modulestore content is processed and staged before importing it to a learning packge. We temporarily save the staged content to allow for troubleshooting of failed migrations.', null=True, on_delete=django.db.models.deletion.SET_NULL, to='content_staging.stagedcontent'), + ), + migrations.AddField( + model_name='modulestoremigration', + name='target', + field=models.ForeignKey(help_text='Content will be imported into this library', on_delete=django.db.models.deletion.CASCADE, to='oel_publishing.learningpackage'), + ), + migrations.AddField( + model_name='modulestoremigration', + name='target_collection', + field=models.ForeignKey(blank=True, help_text='Optional - Collection (within the target library) into which imported content will be grouped', null=True, on_delete=django.db.models.deletion.SET_NULL, to='oel_collections.collection'), + ), + migrations.AddField( + model_name='modulestoremigration', + name='task_status', + field=models.OneToOneField(help_text='Tracks the status of the task which is executing this migration', on_delete=django.db.models.deletion.RESTRICT, to='user_tasks.usertaskstatus'), + ), + migrations.CreateModel( + name='ModulestoreBlockSource', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')), + ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')), + ('key', opaque_keys.edx.django.models.UsageKeyField(help_text='Original usage key of the XBlock that has been imported.', max_length=255)), + ('forwarded', models.OneToOneField(help_text='If set, the system will forward references of this block source over to the target of this block migration', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='forwards', to='modulestore_migrator.modulestoreblockmigration')), + ('overall_source', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='blocks', to='modulestore_migrator.modulestoresource')), + ], + options={ + 'abstract': False, + }, + ), + migrations.AddField( + model_name='modulestoreblockmigration', + name='overall_migration', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='block_migrations', to='modulestore_migrator.modulestoremigration'), + ), + migrations.AddField( + model_name='modulestoreblockmigration', + name='source', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='modulestore_migrator.modulestoreblocksource'), + ), + migrations.AddField( + model_name='modulestoreblockmigration', + name='target', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='oel_publishing.publishableentity'), + ), + migrations.AlterUniqueTogether( + name='modulestoreblockmigration', + unique_together={('overall_migration', 'source'), ('overall_migration', 'target')}, + ), + ] diff --git a/cms/djangoapps/modulestore_migrator/migrations/__init__.py b/cms/djangoapps/modulestore_migrator/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cms/djangoapps/modulestore_migrator/models.py b/cms/djangoapps/modulestore_migrator/models.py new file mode 100644 index 0000000000..05bea5bfe1 --- /dev/null +++ b/cms/djangoapps/modulestore_migrator/models.py @@ -0,0 +1,224 @@ +""" +Models for the modulestore migration tool. +""" +from __future__ import annotations + +from django.contrib.auth import get_user_model +from django.db import models +from django.utils.translation import gettext_lazy as _ +from user_tasks.models import UserTaskStatus + +from model_utils.models import TimeStampedModel +from opaque_keys.edx.django.models import ( + LearningContextKeyField, + UsageKeyField, +) +from openedx_learning.api.authoring_models import ( + LearningPackage, PublishableEntity, Collection, DraftChangeLog, DraftChangeLogRecord +) + +from .data import CompositionLevel, RepeatHandlingStrategy + +User = get_user_model() + + +class ModulestoreSource(models.Model): + """ + A legacy learning context (course or library) which can be a source of a migration. + """ + key = LearningContextKeyField( + max_length=255, + unique=True, + help_text=_('Key of the content source (a course or a legacy library)'), + ) + forwarded = models.OneToOneField( + 'modulestore_migrator.ModulestoreMigration', + null=True, + blank=True, + on_delete=models.SET_NULL, + help_text=_('If set, the system will forward references of this source over to the target of this migration'), + related_name="forwards", + ) + + def __str__(self): + return f"{self.__class__.__name__}('{self.key}')" + + __repr__ = __str__ + + +class ModulestoreMigration(models.Model): + """ + Tracks the action of a user importing a Modulestore-based course or legacy library into a + learning-core based learning package + + Notes: + * As of Ulmo, a learning package is always associated with a v2 content library, but we + will not bake that assumption into this model) + * Each Migration is tied to a single UserTaskStatus, which connects it to a user and + contains the progress of the import. + * A single ModulestoreSource may very well have multiple ModulestoreMigrations; however, + at most one of them with be the "authoritative" migration, as indicated by `forwarded`. + """ + + ## MIGRATION SPECIFICATION + source = models.ForeignKey( + ModulestoreSource, + on_delete=models.CASCADE, + related_name="migrations", + ) + source_version = models.CharField( + max_length=255, + blank=True, + null=True, + help_text=_('Migrated content version, the hash of published content version'), + ) + composition_level = models.CharField( + max_length=255, + choices=CompositionLevel.supported_choices(), + default=CompositionLevel.Component.value, + help_text=_('Maximum hierachy level at which content should be aggregated in target library'), + ) + repeat_handling_strategy = models.CharField( + choices=RepeatHandlingStrategy.supported_choices(), + default=RepeatHandlingStrategy.default().value, + max_length=24, + help_text=_( + "If a piece of content already exists in the content library, choose how to handle it." + ), + ) + preserve_url_slugs = models.BooleanField( + default=False, + help_text=_( + "Should the migration preserve the location IDs of the existing blocks?" + "If not, then new, unique human-readable IDs will be generated based on the block titles." + ), + ) + target = models.ForeignKey( + LearningPackage, + on_delete=models.CASCADE, + help_text=_('Content will be imported into this library'), + ) + target_collection = models.ForeignKey( + Collection, + on_delete=models.SET_NULL, + null=True, + blank=True, + help_text=_('Optional - Collection (within the target library) into which imported content will be grouped'), + ) + + ## MIGRATION ARTIFACTS + task_status = models.OneToOneField( + UserTaskStatus, + on_delete=models.RESTRICT, + help_text=_("Tracks the status of the task which is executing this migration"), + ) + change_log = models.ForeignKey( + DraftChangeLog, + on_delete=models.SET_NULL, + null=True, + help_text=_("Changelog entry in the target learning package which records this migration"), + ) + staged_content = models.OneToOneField( + "content_staging.StagedContent", + null=True, + on_delete=models.SET_NULL, # Staged content is liable to be deleted in order to save space + help_text=_( + "Modulestore content is processed and staged before importing it to a learning packge. " + "We temporarily save the staged content to allow for troubleshooting of failed migrations." + ) + ) + + def __str__(self): + return ( + f"{self.__class__.__name__} #{self.pk}: " + f"{self.source.key} → {self.target_collection or self.target}" + ) + + def __repr__(self): + return ( + f"{self.__class__.__name__}(" + f"id={self.id}, source='{self.source}'," + f"target='{self.target_collection or self.target}')" + ) + + +class ModulestoreBlockSource(TimeStampedModel): + """ + A legacy block usage (in a course or library) which can be a source of a block migration. + """ + overall_source = models.ForeignKey( + ModulestoreSource, + on_delete=models.CASCADE, + related_name="blocks", + ) + key = UsageKeyField( + max_length=255, + help_text=_('Original usage key of the XBlock that has been imported.'), + ) + forwarded = models.OneToOneField( + 'modulestore_migrator.ModulestoreBlockMigration', + null=True, + on_delete=models.SET_NULL, + help_text=_( + 'If set, the system will forward references of this block source over to the target of this block migration' + ), + related_name="forwards", + ) + unique_together = [("overall_source", "key")] + + def __str__(self): + return f"{self.__class__.__name__}('{self.key}')" + + __repr__ = __str__ + + +class ModulestoreBlockMigration(TimeStampedModel): + """ + The migration of a single legacy block into a learning package. + + Is always tied to a greater overall ModulestoreMigration. + + Note: + * A single ModulestoreBlockSource may very well have multiple ModulestoreBlockMigrations; however, + at most one of them with be the "authoritative" migration, as indicated by `forwarded`. + This will coincide with the `overall_migration` being pointed to by `forwarded` as well. + """ + overall_migration = models.ForeignKey( + ModulestoreMigration, + on_delete=models.CASCADE, + related_name="block_migrations", + ) + source = models.ForeignKey( + ModulestoreBlockSource, + on_delete=models.CASCADE, + ) + target = models.ForeignKey( + PublishableEntity, + on_delete=models.CASCADE, + ) + change_log_record = models.OneToOneField( + DraftChangeLogRecord, + # a changelog record can be pruned, which would set this to NULL, but not delete the + # entire import record + null=True, + on_delete=models.SET_NULL, + ) + + class Meta: + unique_together = [ + ('overall_migration', 'source'), + ('overall_migration', 'target'), + ] + + def __str__(self): + return ( + f"{self.__class__.__name__} #{self.pk}: " + f"{self.source.key} → {self.target}" + ) + + def __repr__(self): + return ( + f"{self.__class__.__name__}(" + f"id={self.id}, source='{self.source}'," + f"target='{self.target}')" + ) diff --git a/cms/djangoapps/modulestore_migrator/rest_api/__init__.py b/cms/djangoapps/modulestore_migrator/rest_api/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cms/djangoapps/modulestore_migrator/rest_api/urls.py b/cms/djangoapps/modulestore_migrator/rest_api/urls.py new file mode 100644 index 0000000000..63bdb1aa1b --- /dev/null +++ b/cms/djangoapps/modulestore_migrator/rest_api/urls.py @@ -0,0 +1,13 @@ +""" +Course to Library Import API URLs. +""" + +from django.urls import include, path + +from .v1 import urls as v1_urls + +app_name = 'modulestore_migrator' + +urlpatterns = [ + path('v1/', include(v1_urls)), +] diff --git a/cms/djangoapps/modulestore_migrator/rest_api/v1/__init__.py b/cms/djangoapps/modulestore_migrator/rest_api/v1/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py b/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py new file mode 100644 index 0000000000..beb72729cd --- /dev/null +++ b/cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py @@ -0,0 +1,126 @@ +""" +Serializers for the Course to Library Import API. +""" + +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import LearningContextKey +from opaque_keys.edx.locator import LibraryLocatorV2 +from rest_framework import serializers +from user_tasks.serializers import StatusSerializer + +from cms.djangoapps.modulestore_migrator.data import CompositionLevel, RepeatHandlingStrategy +from cms.djangoapps.modulestore_migrator.models import ModulestoreMigration + + +class ModulestoreMigrationSerializer(serializers.ModelSerializer): + """ + Serializer for the course to library import creation API. + """ + + source = serializers.CharField( # type: ignore[assignment] + help_text="The source course or legacy library key to import from.", + required=True, + ) + target = serializers.CharField( + help_text="The target library key to import into.", + required=True, + ) + composition_level = serializers.ChoiceField( + help_text="The composition level to import the content at.", + choices=CompositionLevel.supported_choices(), + required=False, + default=CompositionLevel.Component.value, + ) + repeat_handling_strategy = serializers.ChoiceField( + help_text="If a piece of content already exists in the content library, choose how to handle it.", + choices=RepeatHandlingStrategy.supported_choices(), + required=False, + default=RepeatHandlingStrategy.Skip.value, + ) + preserve_url_slugs = serializers.BooleanField( + help_text="If true, current slugs will be preserved.", + required=False, + default=True, + ) + target_collection_slug = serializers.CharField( + help_text="The target collection slug within the library to import into. Optional.", + required=False, + allow_blank=True, + ) + forward_source_to_target = serializers.BooleanField( + help_text="Forward references of this block source over to the target of this block migration.", + required=False, + default=False, + ) + + class Meta: + model = ModulestoreMigration + fields = [ + 'source', + 'target', + 'target_collection_slug', + 'composition_level', + 'repeat_handling_strategy', + 'preserve_url_slugs', + 'forward_source_to_target', + ] + + def get_fields(self): + fields = super().get_fields() + request = self.context.get('request') + if request and request.method != 'POST': + fields.pop('target', None) + fields.pop('target_collection_slug', None) + return fields + + def validate_source(self, value): + """ + Validate the source key format. + """ + try: + return LearningContextKey.from_string(value) + except InvalidKeyError as exc: + raise serializers.ValidationError(f"Invalid source key: {str(exc)}") from exc + + def validate_target(self, value): + """ + Validate the target library key format. + """ + try: + return LibraryLocatorV2.from_string(value) + except InvalidKeyError as exc: + raise serializers.ValidationError(f"Invalid target library key: {str(exc)}") from exc + + def get_forward_source_to_target(self, obj: ModulestoreMigration): + """ + Check if the source block was forwarded to the target. + """ + return obj.id == obj.source.forwarded_id + + def to_representation(self, instance): + """ + Override to customize the serialized representation.""" + data = super().to_representation(instance) + # Custom logic for forward_source_to_target during serialization + data['forward_source_to_target'] = self.get_forward_source_to_target(instance) + return data + + +class StatusWithModulestoreMigrationSerializer(StatusSerializer): + """ + Serializer for the import task status. + """ + + parameters = ModulestoreMigrationSerializer(source='modulestoremigration') + + class Meta: + model = StatusSerializer.Meta.model + fields = [*StatusSerializer.Meta.fields, 'uuid', 'parameters'] + + def get_fields(self): + """ + Remove unwanted fields + """ + fields = super().get_fields() + fields.pop('name', None) + return fields diff --git a/cms/djangoapps/modulestore_migrator/rest_api/v1/urls.py b/cms/djangoapps/modulestore_migrator/rest_api/v1/urls.py new file mode 100644 index 0000000000..22d509f169 --- /dev/null +++ b/cms/djangoapps/modulestore_migrator/rest_api/v1/urls.py @@ -0,0 +1,11 @@ +""" +Course to Library Import API v1 URLs. +""" + +from rest_framework.routers import SimpleRouter +from .views import MigrationViewSet + +ROUTER = SimpleRouter() +ROUTER.register(r'migrations', MigrationViewSet) + +urlpatterns = ROUTER.urls diff --git a/cms/djangoapps/modulestore_migrator/rest_api/v1/views.py b/cms/djangoapps/modulestore_migrator/rest_api/v1/views.py new file mode 100644 index 0000000000..feeafb1b40 --- /dev/null +++ b/cms/djangoapps/modulestore_migrator/rest_api/v1/views.py @@ -0,0 +1,137 @@ +""" +API v1 views. +""" +import logging + +from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication +from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser +from rest_framework.permissions import IsAdminUser +from rest_framework.response import Response +from rest_framework import status +from user_tasks.models import UserTaskStatus +from user_tasks.views import StatusViewSet + +from cms.djangoapps.modulestore_migrator.api import start_migration_to_library +from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser + +from .serializers import ModulestoreMigrationSerializer, StatusWithModulestoreMigrationSerializer + + +log = logging.getLogger(__name__) + + +class MigrationViewSet(StatusViewSet): + """ + Import course content from modulestore into a content library. + + This viewset handles the import process, including creating the import task and + retrieving the status of the import task. Meant to be used by admin users only. + + API Endpoints + ------------ + POST /api/modulestore_migrator/v1/migrations/ + Start the import process. + + Request body: + { + "source": "", + "target": "", + "composition_level": "", # Optional, defaults to "component" + "target_collection_slug": "", # Optional + "repeat_handling_strategy": "" # Optional, defaults to Skip + "preserve_url_slugs": "" # Optional, defaults to true + } + + Example request: + { + "source": "course-v1:edX+DemoX+2014_T1", + "target": "library-v1:org1+lib_1", + "composition_level": "unit", + "repeat_handling_strategy": "update", + "preserve_url_slugs": true + } + + Example response: + { + "state": "Succeeded", + "state_text": "Succeeded", # Translation into the current language of the current state + "completed_steps": 11, + "total_steps": 11, + "attempts": 1, + "created": "2025-05-14T22:24:37.048539Z", + "modified": "2025-05-14T22:24:59.128068Z", + "artifacts": [], + "uuid": "3de23e5d-fd34-4a6f-bf02-b183374120f0", + "parameters": { + "source": "course-v1:OpenedX+DemoX+DemoCourse", + "composition_level": "unit", + "repeat_handling_strategy": "update", + "preserve_url_slugs": true + } + } + + GET /api/modulestore_migrator/v1/migrations// + Get the status of the import task. + + Example response: + { + "state": "Importing staged content structure", + "state_text": "Importing staged content structure", + "completed_steps": 6, + "total_steps": 11, + "attempts": 1, + "created": "2025-05-14T22:24:37.048539Z", + "modified": "2025-05-14T22:24:59.128068Z", + "artifacts": [], + "uuid": "3de23e5d-fd34-4a6f-bf02-b183374120f0", + "parameters": { + "source": "course-v1:OpenedX+DemoX+DemoCourse2", + "composition_level": "component", + "repeat_handling_strategy": "skip", + "preserve_url_slugs": false + } + } + """ + + permission_classes = (IsAdminUser,) + authentication_classes = ( + BearerAuthenticationAllowInactiveUser, + JwtAuthentication, + SessionAuthenticationAllowInactiveUser, + ) + serializer_class = StatusWithModulestoreMigrationSerializer + + def get_queryset(self): + """ + Override the default queryset to filter by the import event and user. + """ + return StatusViewSet.queryset.filter(modulestoremigration__isnull=False, user=self.request.user) + + def create(self, request, *args, **kwargs): + """ + Handle the import task creation. + """ + + serializer_data = ModulestoreMigrationSerializer(data=request.data) + serializer_data.is_valid(raise_exception=True) + validated_data = serializer_data.validated_data + + try: + task = start_migration_to_library( + user=request.user, + source_key=validated_data['source'], + target_library_key=validated_data['target'], + target_collection_slug=validated_data['target_collection_slug'], + composition_level=validated_data['composition_level'], + repeat_handling_strategy=validated_data['repeat_handling_strategy'], + preserve_url_slugs=validated_data['preserve_url_slugs'], + forward_source_to_target=validated_data['forward_source_to_target'], + ) + except NotImplementedError as e: + log.exception(str(e)) + return Response({'error': str(e)}, status=status.HTTP_400_BAD_REQUEST) + + task_status = UserTaskStatus.objects.get(task_id=task.id) + serializer = self.get_serializer(task_status) + + return Response(serializer.data, status=status.HTTP_201_CREATED) diff --git a/cms/djangoapps/modulestore_migrator/tasks.py b/cms/djangoapps/modulestore_migrator/tasks.py new file mode 100644 index 0000000000..806f251d63 --- /dev/null +++ b/cms/djangoapps/modulestore_migrator/tasks.py @@ -0,0 +1,750 @@ +""" +Tasks for the modulestore_migrator +""" +from __future__ import annotations + +import mimetypes +import os +import typing as t +from dataclasses import dataclass +from datetime import datetime, timezone +from enum import Enum + +from celery import shared_task +from celery.utils.log import get_task_logger +from django.core.exceptions import ObjectDoesNotExist +from django.utils.text import slugify +from edx_django_utils.monitoring import set_code_owner_attribute_from_module +from lxml import etree +from lxml.etree import _ElementTree as XmlTree +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey, UsageKey +from opaque_keys.edx.locator import ( + CourseLocator, LibraryLocator, + LibraryLocatorV2, LibraryUsageLocatorV2, LibraryContainerLocator +) +from openedx_learning.api import authoring as authoring_api +from openedx_learning.api.authoring_models import ( + Collection, + Component, + ComponentType, + LearningPackage, + PublishableEntity, + PublishableEntityVersion, +) +from user_tasks.tasks import UserTask, UserTaskStatus + +from openedx.core.djangoapps.content_libraries.api import ContainerType, get_library +from openedx.core.djangoapps.content_libraries import api as libraries_api +from openedx.core.djangoapps.content_staging import api as staging_api +from xmodule.modulestore import exceptions as modulestore_exceptions +from xmodule.modulestore.django import modulestore +from common.djangoapps.split_modulestore_django.models import SplitModulestoreCourseIndex + +from .constants import CONTENT_STAGING_PURPOSE_TEMPLATE +from .data import CompositionLevel, RepeatHandlingStrategy +from .models import ModulestoreSource, ModulestoreMigration, ModulestoreBlockSource, ModulestoreBlockMigration + + +log = get_task_logger(__name__) + + +class MigrationStep(Enum): + """ + Strings representation the state of an in-progress modulestore-to-learning-core import. + + We use these values to set UserTaskStatus.state. + The other possible UserTaskStatus.state values are the built-in ones: + UserTaskStatus.{PENDING,FAILED,CANCELED,SUCCEEDED}. + """ + VALIDATING_INPUT = 'Validating migration parameters' + CANCELLING_OLD = 'Cancelling any redundant migration tasks' + LOADING = 'Loading legacy content from ModulesStore' + STAGING = 'Staging legacy content for import' + PARSING = 'Parsing staged OLX' + IMPORTING_ASSETS = 'Importing staged files and resources' + IMPORTING_STRUCTURE = 'Importing staged content structure' + UNSTAGING = 'Cleaning staged content' + MAPPING_OLD_TO_NEW = 'Saving map of legacy content to migrated content' + FORWARDING = 'Forwarding legacy content to migrated content' + POPULATING_COLLECTION = 'Assigning imported items to the specified collection' + + +class _MigrationTask(UserTask): + """ + Base class for migrate_to_modulestore + """ + + @staticmethod + def calculate_total_steps(arguments_dict): + """ + Get number of in-progress steps in importing process, as shown in the UI. + """ + return len(list(MigrationStep)) + + +@dataclass(frozen=True) +class _MigrationContext: + """ + Context for the migration process. + """ + existing_source_to_target_keys: dict[ # Note: It's intended to be mutable to reflect changes during migration. + UsageKey, PublishableEntity + ] + target_package_id: int + target_library_key: LibraryLocatorV2 + source_context_key: CourseKey # Note: This includes legacy LibraryLocators, which are sneakily CourseKeys. + content_by_filename: dict[str, int] + composition_level: CompositionLevel + repeat_handling_strategy: RepeatHandlingStrategy + preserve_url_slugs: bool + created_by: int + created_at: datetime + + def is_already_migrated(self, source_key: UsageKey) -> bool: + return source_key in self.existing_source_to_target_keys + + def get_existing_target(self, source_key: UsageKey) -> PublishableEntity: + return self.existing_source_to_target_keys[source_key] + + def add_migration(self, source_key: UsageKey, target: PublishableEntity) -> None: + """Update the context with a new migration (keeps it current)""" + self.existing_source_to_target_keys[source_key] = target + + def get_existing_target_entity_keys(self, base_key: str) -> set[str]: + return set( + publishable_entity.key for _, publishable_entity in + self.existing_source_to_target_keys.items() + if publishable_entity.key.startswith(base_key) + ) + + @property + def should_skip_strategy(self) -> bool: + """ + Determines whether the repeat handling strategy should skip the entity. + """ + return self.repeat_handling_strategy is RepeatHandlingStrategy.Skip + + @property + def should_update_strategy(self) -> bool: + """ + Determines whether the repeat handling strategy should update the entity. + """ + return self.repeat_handling_strategy is RepeatHandlingStrategy.Update + + +@shared_task(base=_MigrationTask, bind=True) +# Note: The decorator @set_code_owner_attribute cannot be used here because the UserTaskMixin +# does stack inspection and can't handle additional decorators. +def migrate_from_modulestore( + self: _MigrationTask, + *, + user_id: int, + source_pk: int, + target_package_pk: int, + target_library_key: str, + target_collection_pk: int, + repeat_handling_strategy: str, + preserve_url_slugs: bool, + composition_level: str, + forward_source_to_target: bool, +) -> None: + """ + Import a course or legacy library into a learning package. + + Currently, the target learning package must be associated with a V2 content library, but that + restriction may be loosened in the future as more types of learning packages are developed. + """ + # pylint: disable=too-many-statements + # This is a large function, but breaking it up futher would probably not + # make it any easier to understand. + + set_code_owner_attribute_from_module(__name__) + + status: UserTaskStatus = self.status + status.set_state(MigrationStep.VALIDATING_INPUT.value) + try: + source = ModulestoreSource.objects.get(pk=source_pk) + target_package = LearningPackage.objects.get(pk=target_package_pk) + target_library = get_library(LibraryLocatorV2.from_string(target_library_key)) + target_collection = Collection.objects.get(pk=target_collection_pk) if target_collection_pk else None + except (ObjectDoesNotExist, InvalidKeyError) as exc: + status.fail(str(exc)) + return + + # The Model is used for Course and Legacy Library + course_index = SplitModulestoreCourseIndex.objects.filter(course_id=source.key).first() + if isinstance(source.key, CourseLocator): + source_root_usage_key = source.key.make_usage_key('course', 'course') + source_version = course_index.published_version if course_index else None + elif isinstance(source.key, LibraryLocator): + source_root_usage_key = source.key.make_usage_key('library', 'library') + source_version = course_index.library_version if course_index else None + else: + status.fail( + f"Not a valid source context key: {source.key}. " + "Source key must reference a course or a legacy library." + ) + return + + migration = ModulestoreMigration.objects.create( + source=source, + source_version=source_version, + composition_level=composition_level, + repeat_handling_strategy=repeat_handling_strategy, + preserve_url_slugs=preserve_url_slugs, + target=target_package, + target_collection=target_collection, + task_status=status, + ) + status.increment_completed_steps() + + status.set_state(MigrationStep.CANCELLING_OLD.value) + # In order to prevent a user from accidentally starting a bunch of identical import tasks... + migrations_to_cancel = ModulestoreMigration.objects.filter( + # get all Migration tasks by this user with the same source and target + task_status__user=status.user, + source=source, + target=target_package, + ).select_related('task_status').exclude( + # (excluding that aren't running) + task_status__state__in=(UserTaskStatus.CANCELED, UserTaskStatus.FAILED, UserTaskStatus.SUCCEEDED) + ).exclude( + # (excluding this migration itself) + id=migration.id + ) + # ... and cancel their tasks and clean away their staged content. + for migration_to_cancel in migrations_to_cancel: + if migration_to_cancel.task_status: + migration_to_cancel.task_status.cancel() + if migration_to_cancel.staged_content: + migration_to_cancel.staged_content.delete() + status.increment_completed_steps() + + status.set_state(MigrationStep.LOADING) + try: + legacy_root = modulestore().get_item(source_root_usage_key) + except modulestore_exceptions.ItemNotFoundError as exc: + status.fail(f"Failed to load source item '{source_root_usage_key}' from ModuleStore: {exc}") + return + if not legacy_root: + status.fail(f"Could not find source item '{source_root_usage_key}' in ModuleStore") + return + status.increment_completed_steps() + + status.set_state(MigrationStep.STAGING.value) + staged_content = staging_api.stage_xblock_temporarily( + block=legacy_root, + user_id=status.user.pk, + purpose=CONTENT_STAGING_PURPOSE_TEMPLATE.format(source_key=source.key), + ) + migration.staged_content = staged_content + status.increment_completed_steps() + + status.set_state(MigrationStep.PARSING.value) + parser = etree.XMLParser(strip_cdata=False) + try: + root_node = etree.fromstring(staged_content.olx, parser=parser) + except etree.ParseError as exc: + status.fail(f"Failed to parse source OLX (from staged content with id = {staged_content.id}): {exc}") + status.increment_completed_steps() + + status.set_state(MigrationStep.IMPORTING_ASSETS.value) + content_by_filename: dict[str, int] = {} + now = datetime.now(tz=timezone.utc) + for staged_content_file_data in staging_api.get_staged_content_static_files(staged_content.id): + old_path = staged_content_file_data.filename + file_data = staging_api.get_staged_content_static_file_data(staged_content.id, old_path) + if not file_data: + log.error( + f"Staged content {staged_content.id} included referenced file {old_path}, " + "but no file data was found." + ) + continue + filename = os.path.basename(old_path) + media_type_str = mimetypes.guess_type(filename)[0] or "application/octet-stream" + media_type = authoring_api.get_or_create_media_type(media_type_str) + content_by_filename[filename] = authoring_api.get_or_create_file_content( + migration.target_id, + media_type.id, + data=file_data, + created=now, + ).id + status.increment_completed_steps() + + status.set_state(MigrationStep.IMPORTING_STRUCTURE.value) + + # "key" is locally unique across all PublishableEntities within + # a given LearningPackage. + # We use this mapping to ensure that we don't create duplicate + # PublishableEntities during the migration process for a given LearningPackage. + existing_source_to_target_keys = { + block.source.key: block.target for block in ModulestoreBlockMigration.objects.filter( + overall_migration__target=migration.target.id + ) + } + + migration_context = _MigrationContext( + existing_source_to_target_keys=existing_source_to_target_keys, + target_package_id=target_package_pk, + target_library_key=target_library.key, + source_context_key=source_root_usage_key.course_key, + content_by_filename=content_by_filename, + composition_level=CompositionLevel(composition_level), + repeat_handling_strategy=RepeatHandlingStrategy(repeat_handling_strategy), + preserve_url_slugs=preserve_url_slugs, + created_by=status.user_id, + created_at=datetime.now(timezone.utc), + ) + + with authoring_api.bulk_draft_changes_for(migration.target.id) as change_log: + root_migrated_node = _migrate_node( + context=migration_context, + source_node=root_node, + ) + change_log.save() + migration.change_log = change_log + status.increment_completed_steps() + + status.set_state(MigrationStep.UNSTAGING.value) + staged_content.delete() + status.increment_completed_steps() + + _create_migration_artifacts_incrementally( + root_migrated_node=root_migrated_node, + source=source, + migration=migration, + status=status, + ) + + block_migrations = ModulestoreBlockMigration.objects.filter(overall_migration=migration) + status.increment_completed_steps() + + status.set_state(MigrationStep.FORWARDING.value) + if forward_source_to_target: + block_sources_to_block_migrations = { + block_migration.source: block_migration for block_migration in block_migrations + } + for block_source, block_migration in block_sources_to_block_migrations.items(): + block_source.forwarded = block_migration + block_source.save() + + source.forwarded = migration + source.save() + status.increment_completed_steps() + + status.set_state(MigrationStep.POPULATING_COLLECTION.value) + if target_collection: + block_target_pks: list[int] = list( + ModulestoreBlockMigration.objects.filter( + overall_migration=migration + ).values_list('target_id', flat=True) + ) + if block_target_pks: + authoring_api.add_to_collection( + learning_package_id=target_package_pk, + key=target_collection.key, + entities_qset=PublishableEntity.objects.filter(id__in=block_target_pks), + created_by=user_id, + ) + else: + log.warning("No target entities found to add to collection") + status.increment_completed_steps() + + +@dataclass(frozen=True) +class _MigratedNode: + """ + A node in the source tree, its target (if migrated), and any migrated children. + + Note that target_version can equal None even when there migrated children. + 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 + children: list[_MigratedNode] + + def all_source_to_target_pairs(self) -> t.Iterable[tuple[UsageKey, PublishableEntityVersion]]: + """ + Get all source_key->target_ver pairs via a pre-order traversal. + """ + if self.source_to_target: + yield self.source_to_target + for child in self.children: + yield from child.all_source_to_target_pairs() + + +def _migrate_node( + *, + context: _MigrationContext, + source_node: XmlTree, +) -> _MigratedNode: + """ + Migrate an OLX node (source_node) from a legacy course or library (context.source_context_key) + to a learning package (context.target_library). If the node is a container, create it in the + target if it is at or above the requested composition_level; otherwise, just import its contents. + Recursively apply the same logic to all children. + """ + # The OLX tag will map to one of the following... + # * A wiki tag --> Ignore + # * A recognized container type --> Migrate children, and import container if requested. + # * A legacy library root --> Migrate children, but NOT the root itself. + # * A course root --> Migrate children, but NOT the root itself (for Ulmo, at least. Future + # releases may support treating the Course as an importable container). + # * Something else --> Try to import it as a component. If that fails, then it's either an un- + # supported component type, or it's an XBlock with dynamic children, which we + # do not support in libraries as of Ulmo. + should_migrate_node: bool + should_migrate_children: bool + container_type: ContainerType | None # if None, it's a Component + if source_node.tag == "wiki": + return _MigratedNode(None, []) + try: + container_type = ContainerType.from_source_olx_tag(source_node.tag) + except ValueError: + container_type = None + if source_node.tag in {"course", "library"}: + should_migrate_node = False + should_migrate_children = True + else: + should_migrate_node = True + should_migrate_children = False + else: + node_level = CompositionLevel(container_type.value) + should_migrate_node = not node_level.is_higher_than(context.composition_level) + should_migrate_children = True + migrated_children: list[_MigratedNode] = [] + if should_migrate_children: + migrated_children = [ + _migrate_node( + context=context, + source_node=source_node_child, + ) + for source_node_child in source_node.getchildren() + ] + source_to_target: tuple[UsageKey, PublishableEntityVersion] | None = None + if should_migrate_node: + source_olx = etree.tostring(source_node).decode('utf-8') + if source_block_id := source_node.get('url_name'): + source_key: UsageKey = context.source_context_key.make_usage_key(source_node.tag, source_block_id) + title = source_node.get('display_name', source_block_id) + target_entity_version = ( + _migrate_container( + context=context, + source_key=source_key, + container_type=container_type, + title=title, + children=[ + migrated_child.source_to_target[1] + for migrated_child in migrated_children if + migrated_child.source_to_target + ], + ) + if container_type else + _migrate_component( + context=context, + source_key=source_key, + olx=source_olx, + title=title, + ) + ) + if target_entity_version: + source_to_target = (source_key, target_entity_version) + context.add_migration(source_key, target_entity_version.entity) + else: + 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}" + ) + return _MigratedNode(source_to_target=source_to_target, children=migrated_children) + + +def _migrate_container( + *, + context: _MigrationContext, + source_key: UsageKey, + container_type: ContainerType, + title: str, + children: list[PublishableEntityVersion], +) -> PublishableEntityVersion: + """ + Create, update, or replace a container in a library based on a source key and children. + + (We assume that the destination is a library rather than some other future kind of learning + package, but let's keep than an internal assumption.) + """ + target_key = _get_distinct_target_container_key( + context, + source_key, + container_type, + title, + ) + try: + container = libraries_api.get_container(target_key) + container_exists = True + except libraries_api.ContentLibraryContainerNotFound: + container_exists = False + if PublishableEntity.objects.filter( + learning_package_id=context.target_package_id, + key=target_key.container_id, + ).exists(): + libraries_api.restore_container(container_key=target_key) + container = libraries_api.get_container(target_key) + else: + container = libraries_api.create_container( + library_key=context.target_library_key, + container_type=container_type, + slug=target_key.container_id, + title=title, + created=context.created_at, + user_id=context.created_by, + ) + if container_exists and context.should_skip_strategy: + return PublishableEntityVersion.objects.get( + entity_id=container.container_pk, + version_num=container.draft_version_num, + ) + return authoring_api.create_next_container_version( + container.container_pk, + title=title, + entity_rows=[ + authoring_api.ContainerEntityRow(entity_pk=child.entity_id, version_pk=None) + for child in children + ], + created=context.created_at, + created_by=context.created_by, + container_version_cls=container_type.container_model_classes[1], + ).publishable_entity_version + + +def _migrate_component( + *, + context: _MigrationContext, + source_key: UsageKey, + olx: str, + title: str, +) -> PublishableEntityVersion | None: + """ + Create, update, or replace a component in a library based on a source key and OLX. + + (We assume that the destination is a library rather than some other future kind of learning + package, but let's keep than an internal assumption.) + """ + component_type = authoring_api.get_or_create_component_type("xblock.v1", source_key.block_type) + + target_key = _get_distinct_target_usage_key( + context, + source_key, + component_type, + title, + ) + + try: + component = authoring_api.get_components(context.target_package_id).get( + component_type=component_type, + local_key=target_key.block_id, + ) + component_existed = True + # Do we have a specific method for this? + component_deleted = not component.versioning.draft + except Component.DoesNotExist: + component_existed = False + component_deleted = False + try: + libraries_api.validate_can_add_block_to_library( + context.target_library_key, target_key.block_type, target_key.block_id + ) + except libraries_api.IncompatibleTypesError as e: + log.error(f"Error validating block for library {context.target_library_key}: {e}") + return None + component = authoring_api.create_component( + context.target_package_id, + component_type=component_type, + local_key=target_key.block_id, + created=context.created_at, + created_by=context.created_by, + ) + + # Component existed and we do not replace it and it is not deleted previously + if component_existed and not component_deleted and context.should_skip_strategy: + return component.versioning.draft.publishable_entity_version + + # If component existed and was deleted or we have to replace the current version + # Create the new component version for it + component_version = libraries_api.set_library_block_olx(target_key, new_olx_str=olx) + for filename, content_pk in context.content_by_filename.items(): + filename_no_ext, _ = os.path.splitext(filename) + if filename_no_ext not in olx: + continue + new_path = f"static/{filename}" + authoring_api.create_component_version_content( + component_version.pk, content_pk, key=new_path + ) + return component_version.publishable_entity_version + + +def _get_distinct_target_container_key( + context: _MigrationContext, + source_key: UsageKey, + container_type: ContainerType, + title: str, +) -> LibraryContainerLocator: + """ + Find a unique key for block_id by appending a unique identifier if necessary. + + Args: + context (_MigrationContext): The migration context. + source_key (UsageKey): The source key. + container_type (ContainerType): The container type. + title (str): The title. + + Returns: + LibraryContainerLocator: The target container key. + """ + # Check if we already processed this block + if context.is_already_migrated(source_key): + existing_version = context.get_existing_target(source_key) + + return LibraryContainerLocator( + context.target_library_key, + container_type.value, + existing_version.key + ) + # Generate new unique block ID + base_slug = ( + source_key.block_id + if context.preserve_url_slugs + else (slugify(title) or source_key.block_id) + ) + unique_slug = _find_unique_slug(context, base_slug) + + return LibraryContainerLocator( + context.target_library_key, + container_type.value, + unique_slug + ) + + +def _get_distinct_target_usage_key( + context: _MigrationContext, + source_key: UsageKey, + component_type: ComponentType, + title: str, +) -> LibraryUsageLocatorV2: + """ + Find a unique key for block_id by appending a unique identifier if necessary. + + Args: + context: The migration context + source_key: The original usage key from the source + component_type: The component type string + olx: The OLX content of the component + + Returns: + A unique LibraryUsageLocatorV2 for the target + + Raises: + ValueError: If source_key is invalid + """ + # Check if we already processed this block + if context.is_already_migrated(source_key): + log.debug(f"Block {source_key} already exists, reusing existing target") + existing_target = context.get_existing_target(source_key) + block_id = existing_target.component.local_key + + # mypy thinks LibraryUsageLocatorV2 is abstract. It's not. + return LibraryUsageLocatorV2( # type: ignore[abstract] + context.target_library_key, + source_key.block_type, + block_id + ) + + # Generate new unique block ID + base_slug = ( + source_key.block_id + if context.preserve_url_slugs + else (slugify(title) or source_key.block_id) + ) + unique_slug = _find_unique_slug(context, base_slug, component_type) + + # mypy thinks LibraryUsageLocatorV2 is abstract. It's not. + return LibraryUsageLocatorV2( # type: ignore[abstract] + context.target_library_key, + source_key.block_type, + unique_slug + ) + + +def _find_unique_slug( + context: _MigrationContext, + base_slug: str, + component_type: ComponentType | None = None, + max_attempts: int = 1000 +) -> str: + """ + Find a unique slug by appending incrementing numbers if necessary. + Using batch querying to avoid multiple database roundtrips. + + Args: + component_type: The component type to check against + base_slug: The base slug to make unique + max_attempts: Maximum number of attempts to prevent infinite loops + + Returns: + A unique slug string + + Raises: + RuntimeError: If unable to find unique slug within max_attempts + """ + if not component_type: + base_key = base_slug + else: + base_key = f"{component_type}:{base_slug}" + + existing_publishable_entity_keys = context.get_existing_target_entity_keys(base_key) + + # Check if base slug is available + if base_key not in existing_publishable_entity_keys: + return base_slug + + # Try numbered variations until we find one that doesn't exist + for i in range(1, max_attempts + 1): + candidate_slug = f"{base_slug}_{i}" + candidate_key = f"{component_type}:{candidate_slug}" if component_type else candidate_slug + + if candidate_key not in existing_publishable_entity_keys: + return candidate_slug + + raise RuntimeError(f"Unable to find unique slug after {max_attempts} attempts for base: {base_slug}") + + +def _create_migration_artifacts_incrementally( + root_migrated_node: _MigratedNode, + source: ModulestoreSource, + migration: ModulestoreMigration, + status: UserTaskStatus +) -> None: + """ + Create ModulestoreBlockSource and ModulestoreBlockMigration objects incrementally. + """ + nodes = tuple(root_migrated_node.all_source_to_target_pairs()) + total_nodes = len(nodes) + processed = 0 + + for source_usage_key, target_version in root_migrated_node.all_source_to_target_pairs(): + block_source, _ = ModulestoreBlockSource.objects.get_or_create( + overall_source=source, + key=source_usage_key + ) + + ModulestoreBlockMigration.objects.create( + overall_migration=migration, + source=block_source, + target_id=target_version.entity_id, + ) + + processed += 1 + if processed % 10 == 0 or processed == total_nodes: + status.set_state( + f"{MigrationStep.MAPPING_OLD_TO_NEW.value} ({processed}/{total_nodes})" + ) diff --git a/cms/djangoapps/modulestore_migrator/tests/__init__.py b/cms/djangoapps/modulestore_migrator/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cms/djangoapps/modulestore_migrator/tests/factories.py b/cms/djangoapps/modulestore_migrator/tests/factories.py new file mode 100644 index 0000000000..e506257efc --- /dev/null +++ b/cms/djangoapps/modulestore_migrator/tests/factories.py @@ -0,0 +1,21 @@ +""" +Factories for creating test data for the modulestore migrator. +""" +import uuid + +import factory +from opaque_keys.edx.keys import LearningContextKey + +from cms.djangoapps.modulestore_migrator.models import ModulestoreSource + + +class ModulestoreSourceFactory(factory.django.DjangoModelFactory): + """ + Factory for creating ModulestoreSource instances. + """ + class Meta: + model = ModulestoreSource + + @factory.lazy_attribute + def key(self): + return LearningContextKey.from_string(f"course-v1:edX+DemoX+{uuid.uuid4()}") diff --git a/cms/djangoapps/modulestore_migrator/tests/test_api.py b/cms/djangoapps/modulestore_migrator/tests/test_api.py new file mode 100644 index 0000000000..0942bfe4ad --- /dev/null +++ b/cms/djangoapps/modulestore_migrator/tests/test_api.py @@ -0,0 +1,148 @@ +""" +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 + +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(LibraryTestCase): + """ + Test cases for the modulestore migrator API. + """ + + def setUp(self): + super().setUp() + + self.organization = OrganizationFactory() + 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", + ) + 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): + """ + Test that the API can start a migration to a library. + """ + source = ModulestoreSourceFactory() + user = UserFactory() + + api.start_migration_to_library( + user=user, + source_key=source.key, + target_library_key=self.library_v2.library_key, + target_collection_slug=None, + composition_level=CompositionLevel.Component.value, + repeat_handling_strategy=RepeatHandlingStrategy.Skip.value, + preserve_url_slugs=True, + forward_source_to_target=False, + ) + + modulestoremigration = ModulestoreMigration.objects.get() + assert modulestoremigration.source.key == source.key + assert ( + modulestoremigration.composition_level == CompositionLevel.Component.value + ) + assert modulestoremigration.repeat_handling_strategy == RepeatHandlingStrategy.Skip.value + assert modulestoremigration.preserve_url_slugs is True + assert modulestoremigration.task_status is not None + assert modulestoremigration.task_status.user == user + + def test_start_migration_to_library_with_collection(self): + """ + Test that the API can start a migration to a library with a target collection. + """ + + source = ModulestoreSourceFactory() + 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=source.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=False, + ) + + modulestoremigration = ModulestoreMigration.objects.get() + assert modulestoremigration.target_collection.key == collection_key + + def test_forking_is_not_implemented(self): + """ + Test that the API raises NotImplementedError for the Fork strategy. + """ + source = ModulestoreSourceFactory() + user = UserFactory() + + with pytest.raises(NotImplementedError): + api.start_migration_to_library( + user=user, + source_key=source.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" diff --git a/cms/djangoapps/modulestore_migrator/tests/test_tasks.py b/cms/djangoapps/modulestore_migrator/tests/test_tasks.py new file mode 100644 index 0000000000..b1c5890e11 --- /dev/null +++ b/cms/djangoapps/modulestore_migrator/tests/test_tasks.py @@ -0,0 +1,1430 @@ +""" +Tests for the modulestore_migrator tasks +""" + +from unittest.mock import Mock +import ddt +from django.utils import timezone +from lxml import etree +from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2 +from openedx_learning.api.authoring_models import Collection, PublishableEntityVersion +from openedx_learning.api import authoring as authoring_api +from organizations.tests.factories import OrganizationFactory +from user_tasks.models import UserTaskArtifact +from user_tasks.tasks import UserTaskStatus +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +from common.djangoapps.student.tests.factories import UserFactory +from cms.djangoapps.modulestore_migrator.data import CompositionLevel, RepeatHandlingStrategy +from cms.djangoapps.modulestore_migrator.models import ( + ModulestoreMigration, + ModulestoreSource, +) +from cms.djangoapps.modulestore_migrator.tasks import ( + _migrate_component, + _migrate_container, + _migrate_node, + _MigratedNode, + _MigrationContext, + _MigrationTask, + migrate_from_modulestore, + MigrationStep, +) +from openedx.core.djangoapps.content_libraries import api as lib_api + + +@ddt.ddt +class TestMigrateFromModulestore(ModuleStoreTestCase): + """ + Test the migrate_from_modulestore task + """ + + def setUp(self): + super().setUp() + self.user = UserFactory() + self.organization = OrganizationFactory(short_name="testorg") + self.lib_key = LibraryLocatorV2.from_string( + f"lib:{self.organization.short_name}:test-key" + ) + lib_api.create_library( + org=self.organization, + slug=self.lib_key.slug, + title="Test Library", + ) + self.library = lib_api.ContentLibrary.objects.get(slug=self.lib_key.slug) + self.learning_package = self.library.learning_package + self.course = CourseFactory( + org=self.organization.short_name, + course="TestCourse", + run="TestRun", + display_name="Test Course", + ) + self.collection = Collection.objects.create( + learning_package=self.learning_package, + key="test_collection", + title="Test Collection", + ) + + def _get_task_status_fail_message(self, status): + """ + Helper method to get the failure message from a UserTaskStatus object. + """ + if status.state == UserTaskStatus.FAILED: + return UserTaskArtifact.objects.get(status=status, name="Error").text + return None + + def test_migrate_node_wiki_tag(self): + """ + Test _migrate_node ignores wiki tags + """ + wiki_node = etree.fromstring("") + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + result = _migrate_node( + context=context, + source_node=wiki_node, + ) + + self.assertIsNone(result.source_to_target) + self.assertEqual(len(result.children), 0) + + def test_migrate_node_course_root(self): + """ + Test _migrate_node handles course root + """ + course_node = etree.fromstring( + '' + '' + "" + ) + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + result = _migrate_node( + context=context, + source_node=course_node, + ) + + # Course root should not be migrated + self.assertIsNone(result.source_to_target) + # But should have children processed + self.assertEqual(len(result.children), 1) + + def test_migrate_node_library_root(self): + """ + Test _migrate_node handles library root + """ + library_node = etree.fromstring( + '' + '' + "" + ) + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + result = _migrate_node( + context=context, + source_node=library_node, + ) + + # Library root should not be migrated + self.assertIsNone(result.source_to_target) + # But should have children processed + self.assertEqual(len(result.children), 1) + + @ddt.data( + ("chapter", CompositionLevel.Unit, None), + ("sequential", CompositionLevel.Unit, None), + ("vertical", CompositionLevel.Unit, True), + ("chapter", CompositionLevel.Section, True), + ("sequential", CompositionLevel.Section, True), + ("vertical", CompositionLevel.Section, True), + ) + @ddt.unpack + def test_migrate_node_container_composition_level( + self, tag_name, composition_level, should_migrate + ): + """ + Test _migrate_node respects composition level for containers + """ + container_node = etree.fromstring( + f'<{tag_name} url_name="test_{tag_name}" display_name="Test {tag_name.title()}" />' + ) + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=composition_level, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + result = _migrate_node( + context=context, + source_node=container_node, + ) + + 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}") + else: + self.assertIsNone(result.source_to_target) + + def test_migrate_node_without_url_name(self): + """ + Test _migrate_node handles nodes without url_name + """ + node_without_url_name = etree.fromstring( + '' + ) + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + result = _migrate_node( + context=context, + source_node=node_without_url_name, + ) + + self.assertIsNone(result.source_to_target) + self.assertEqual(len(result.children), 0) + + def test_migrated_node_all_source_to_target_pairs(self): + """ + Test _MigratedNode.all_source_to_target_pairs traversal + """ + mock_version1 = Mock(spec=PublishableEntityVersion) + mock_version2 = Mock(spec=PublishableEntityVersion) + mock_version3 = Mock(spec=PublishableEntityVersion) + + key1 = self.course.id.make_usage_key("problem", "problem1") + key2 = self.course.id.make_usage_key("problem", "problem2") + key3 = self.course.id.make_usage_key("problem", "problem3") + + child_node = _MigratedNode(source_to_target=(key3, mock_version3), children=[]) + parent_node = _MigratedNode( + source_to_target=(key1, mock_version1), + children=[ + _MigratedNode(source_to_target=(key2, mock_version2), children=[]), + child_node, + ], + ) + + pairs = list(parent_node.all_source_to_target_pairs()) + + self.assertEqual(len(pairs), 3) + self.assertEqual(pairs[0][0], key1) + self.assertEqual(pairs[1][0], key2) + self.assertEqual(pairs[2][0], key3) + + def test_migrate_from_modulestore_invalid_source(self): + """ + Test migrate_from_modulestore with invalid source + """ + task = migrate_from_modulestore.apply_async( + kwargs={ + "user_id": self.user.id, + "source_pk": 999999, # Non-existent source + "target_package_pk": self.learning_package.id, + "target_library_key": str(self.lib_key), + "target_collection_pk": self.collection.id, + "repeat_handling_strategy": RepeatHandlingStrategy.Skip.value, + "preserve_url_slugs": True, + "composition_level": CompositionLevel.Unit.value, + "forward_source_to_target": False, + } + ) + + status = UserTaskStatus.objects.get(task_id=task.id) + self.assertEqual(status.state, UserTaskStatus.FAILED) + self.assertEqual(self._get_task_status_fail_message(status), "ModulestoreSource matching query does not exist.") + + def test_migrate_from_modulestore_invalid_target_package(self): + """ + Test migrate_from_modulestore with invalid target package + """ + source = ModulestoreSource.objects.create( + key=self.course.id, + ) + + task = migrate_from_modulestore.apply_async( + kwargs={ + "user_id": self.user.id, + "source_pk": source.id, + "target_package_pk": 999999, # Non-existent package + "target_library_key": str(self.lib_key), + "target_collection_pk": self.collection.id, + "repeat_handling_strategy": RepeatHandlingStrategy.Skip.value, + "preserve_url_slugs": True, + "composition_level": CompositionLevel.Unit.value, + "forward_source_to_target": False, + } + ) + + status = UserTaskStatus.objects.get(task_id=task.id) + self.assertEqual(status.state, UserTaskStatus.FAILED) + self.assertEqual(self._get_task_status_fail_message(status), "LearningPackage matching query does not exist.") + + def test_migrate_from_modulestore_invalid_collection(self): + """ + Test migrate_from_modulestore with invalid collection + """ + source = ModulestoreSource.objects.create( + key=self.course.id, + ) + + task = migrate_from_modulestore.apply_async( + kwargs={ + "user_id": self.user.id, + "source_pk": source.id, + "target_package_pk": self.learning_package.id, + "target_library_key": str(self.lib_key), + "target_collection_pk": 999999, # Non-existent collection + "repeat_handling_strategy": RepeatHandlingStrategy.Skip.value, + "preserve_url_slugs": True, + "composition_level": CompositionLevel.Unit.value, + "forward_source_to_target": False, + } + ) + + status = UserTaskStatus.objects.get(task_id=task.id) + self.assertEqual(status.state, UserTaskStatus.FAILED) + self.assertEqual(self._get_task_status_fail_message(status), "Collection matching query does not exist.") + + def test_migration_task_calculate_total_steps(self): + """ + Test _MigrationTask.calculate_total_steps returns correct count + """ + total_steps = _MigrationTask.calculate_total_steps({}) + expected_steps = len(list(MigrationStep)) + self.assertEqual(total_steps, expected_steps) + + def test_migrate_component_success(self): + """ + Test _migrate_component successfully creates a new component + """ + source_key = self.course.id.make_usage_key("problem", "test_problem") + olx = '' + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + result = _migrate_component( + context=context, + source_key=source_key, + olx=olx, + title="test_problem" + ) + + self.assertIsNotNone(result) + self.assertIsInstance(result, PublishableEntityVersion) + + self.assertEqual( + "problem", result.componentversion.component.component_type.name + ) + + def test_migrate_component_with_static_content(self): + """ + Test _migrate_component with static file content + """ + source_key = self.course.id.make_usage_key("problem", "test_problem_with_image") + olx = '

See image: test_image.png

' + + media_type = authoring_api.get_or_create_media_type("image/png") + test_content = authoring_api.get_or_create_file_content( + self.learning_package.id, + media_type.id, + data=b"fake_image_data", + created=timezone.now(), + ) + content_by_filename = {"test_image.png": test_content.id} + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename=content_by_filename, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + result = _migrate_component( + context=context, + source_key=source_key, + olx=olx, + title="test_problem" + ) + + self.assertIsNotNone(result) + + component_content = result.componentversion.componentversioncontent_set.filter( + key="static/test_image.png" + ).first() + self.assertIsNotNone(component_content) + self.assertEqual(component_content.content_id, test_content.id) + + def test_migrate_component_replace_existing_false(self): + """ + Test _migrate_component with replace_existing=False returns existing component + """ + source_key = self.course.id.make_usage_key("problem", "existing_problem") + olx = '' + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + first_result = _migrate_component( + context=context, + source_key=source_key, + olx=olx, + title="test_problem" + ) + + context.existing_source_to_target_keys[source_key] = first_result.entity + + second_result = _migrate_component( + context=context, + source_key=source_key, + olx='', + title="updated_problem" + ) + + self.assertEqual(first_result.entity_id, second_result.entity_id) + self.assertEqual(first_result.version_num, second_result.version_num) + + def test_migrate_component_same_title(self): + """ + Test _migrate_component for two components with the same title + + Using preserve_url_slugs=False to create a new component with + a different URL slug based on the component's Title. + """ + source_key_1 = self.course.id.make_usage_key("problem", "existing_problem_1") + source_key_2 = self.course.id.make_usage_key("problem", "existing_problem_2") + olx = '' + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=False, + created_at=timezone.now(), + created_by=self.user.id, + ) + + first_result = _migrate_component( + context=context, + source_key=source_key_1, + olx=olx, + title="test_problem" + ) + + context.existing_source_to_target_keys[source_key_1] = first_result.entity + + second_result = _migrate_component( + context=context, + source_key=source_key_2, + olx=olx, + title="test_problem" + ) + + self.assertNotEqual(first_result.entity_id, second_result.entity_id) + self.assertNotEqual(first_result.entity.key, second_result.entity.key) + + def test_migrate_component_replace_existing_true(self): + """ + Test _migrate_component with replace_existing=True creates new version + """ + source_key = self.course.id.make_usage_key("problem", "replaceable_problem") + original_olx = '' + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Update, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + first_result = _migrate_component( + context=context, + source_key=source_key, + olx=original_olx, + title="original" + ) + + context.existing_source_to_target_keys[source_key] = first_result.entity + + updated_olx = '' + second_result = _migrate_component( + context=context, + source_key=source_key, + olx=updated_olx, + title="updated" + ) + + self.assertEqual(first_result.entity_id, second_result.entity_id) + self.assertNotEqual(first_result.version_num, second_result.version_num) + + def test_migrate_component_different_block_types(self): + """ + Test _migrate_component with different block types + """ + block_types = ["problem", "html", "video", "discussion"] + + for block_type in block_types: + source_key = self.course.id.make_usage_key(block_type, f"test_{block_type}") + olx = f'<{block_type} display_name="Test {block_type.title()}">' + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + result = _migrate_component( + context=context, + source_key=source_key, + olx=olx, + title="test" + ) + + self.assertIsNotNone(result, f"Failed to migrate {block_type}") + + self.assertEqual( + block_type, result.componentversion.component.component_type.name + ) + + def test_migrate_component_content_filename_not_in_olx(self): + """ + Test _migrate_component ignores content files not referenced in OLX + """ + source_key = self.course.id.make_usage_key( + "problem", "test_problem_selective_content" + ) + olx = '

See image: referenced.png

' + + media_type = authoring_api.get_or_create_media_type("image/png") + referenced_content = authoring_api.get_or_create_file_content( + self.learning_package.id, + media_type.id, + data=b"referenced_image_data", + created=timezone.now(), + ) + unreferenced_content = authoring_api.get_or_create_file_content( + self.learning_package.id, + media_type.id, + data=b"unreferenced_image_data", + created=timezone.now(), + ) + + content_by_filename = { + "referenced.png": referenced_content.id, + "unreferenced.png": unreferenced_content.id, + } + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename=content_by_filename, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + result = _migrate_component( + context=context, + source_key=source_key, + olx=olx, + title="test_problem" + ) + + self.assertIsNotNone(result) + + referenced_content_exists = ( + result.componentversion.componentversioncontent_set.filter( + key="static/referenced.png" + ).exists() + ) + unreferenced_content_exists = ( + result.componentversion.componentversioncontent_set.filter( + key="static/unreferenced.png" + ).exists() + ) + + self.assertTrue(referenced_content_exists) + self.assertFalse(unreferenced_content_exists) + + def test_migrate_component_library_source_key(self): + """ + Test _migrate_component with library source key + """ + library_key = LibraryLocator(org="TestOrg", library="TestLibrary") + source_key = library_key.make_usage_key("problem", "library_problem") + olx = '' + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + result = _migrate_component( + context=context, + source_key=source_key, + olx=olx, + title="library_problem" + ) + + self.assertIsNotNone(result) + + self.assertEqual( + "problem", result.componentversion.component.component_type.name + ) + + def test_migrate_component_duplicate_content_integrity_error(self): + """ + Test _migrate_component handles IntegrityError when content already exists + """ + source_key = self.course.id.make_usage_key( + "problem", "test_problem_duplicate_content" + ) + olx = '

See image: duplicate.png

' + + media_type = authoring_api.get_or_create_media_type("image/png") + test_content = authoring_api.get_or_create_file_content( + self.learning_package.id, + media_type.id, + data=b"test_image_data", + created=timezone.now(), + ) + content_by_filename = {"duplicate.png": test_content.id} + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename=content_by_filename, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Update, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + first_result = _migrate_component( + context=context, + source_key=source_key, + olx=olx, + title="test_problem" + ) + + context.existing_source_to_target_keys[source_key] = first_result.entity + + second_result = _migrate_component( + context=context, + source_key=source_key, + olx=olx, + title="test_problem" + ) + + self.assertIsNotNone(first_result) + self.assertIsNotNone(second_result) + self.assertEqual(first_result.entity_id, second_result.entity_id) + + def test_migrate_container_creates_new_container(self): + """ + Test _migrate_container creates a new container when none exists + """ + source_key = self.course.id.make_usage_key("vertical", "test_vertical") + + child_component_1 = authoring_api.create_component( + self.learning_package.id, + component_type=authoring_api.get_or_create_component_type( + "xblock.v1", "problem" + ), + local_key="child_problem_1", + created=timezone.now(), + created_by=self.user.id, + ) + child_version_1 = authoring_api.create_next_component_version( + child_component_1.pk, + content_to_replace={}, + created=timezone.now(), + created_by=self.user.id, + ) + + child_component_2 = authoring_api.create_component( + self.learning_package.id, + component_type=authoring_api.get_or_create_component_type( + "xblock.v1", "html" + ), + local_key="child_html_1", + created=timezone.now(), + created_by=self.user.id, + ) + child_version_2 = authoring_api.create_next_component_version( + child_component_2.pk, + content_to_replace={}, + created=timezone.now(), + created_by=self.user.id, + ) + + children = [ + child_version_1.publishable_entity_version, + child_version_2.publishable_entity_version, + ] + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + result = _migrate_container( + context=context, + source_key=source_key, + container_type=lib_api.ContainerType.Unit, + title="Test Vertical", + children=children, + ) + + self.assertIsInstance(result, PublishableEntityVersion) + + container_version = result.containerversion + self.assertEqual(container_version.title, "Test Vertical") + + entity_rows = container_version.entity_list.entitylistrow_set.all() + self.assertEqual(len(entity_rows), 2) + + child_entity_ids = {row.entity_id for row in entity_rows} + expected_entity_ids = {child.entity_id for child in children} + self.assertEqual(child_entity_ids, expected_entity_ids) + + def test_migrate_container_different_container_types(self): + """ + Test _migrate_container works with different container types + """ + container_types = [ + (lib_api.ContainerType.Unit, "vertical"), + (lib_api.ContainerType.Subsection, "sequential"), + (lib_api.ContainerType.Section, "chapter"), + ] + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + for container_type, block_type in container_types: + with self.subTest(container_type=container_type, block_type=block_type): + source_key = self.course.id.make_usage_key( + block_type, f"test_{block_type}" + ) + + result = _migrate_container( + context=context, + source_key=source_key, + container_type=container_type, + title=f"Test {block_type.title()}", + children=[], + ) + + self.assertIsNotNone(result) + + container_version = result.containerversion + self.assertEqual(container_version.title, f"Test {block_type.title()}") + + def test_migrate_container_replace_existing_false(self): + """ + Test _migrate_container returns existing container when replace_existing=False + """ + source_key = self.course.id.make_usage_key("vertical", "existing_vertical") + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + first_result = _migrate_container( + context=context, + source_key=source_key, + container_type=lib_api.ContainerType.Unit, + title="Original Title", + children=[], + ) + + context.existing_source_to_target_keys[source_key] = first_result.entity + + second_result = _migrate_container( + context=context, + source_key=source_key, + container_type=lib_api.ContainerType.Unit, + title="Updated Title", + children=[], + ) + + self.assertEqual(first_result.entity_id, second_result.entity_id) + self.assertEqual(first_result.version_num, second_result.version_num) + + container_version = second_result.containerversion + self.assertEqual(container_version.title, "Original Title") + + def test_migrate_container_same_title(self): + """ + Test _migrate_container for two containers with the same title + + Using preserve_url_slugs=False to create a new Unit with + a different URL slug based on the container's Title. + """ + source_key_1 = self.course.id.make_usage_key("vertical", "human_readable_vertical_1") + source_key_2 = self.course.id.make_usage_key("vertical", "human_readable_vertical_2") + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=False, + created_at=timezone.now(), + created_by=self.user.id, + ) + + first_result = _migrate_container( + context=context, + source_key=source_key_1, + container_type=lib_api.ContainerType.Unit, + title="Original Human Readable Title", + children=[], + ) + + context.existing_source_to_target_keys[source_key_1] = first_result.entity + + second_result = _migrate_container( + context=context, + source_key=source_key_2, + container_type=lib_api.ContainerType.Unit, + title="Original Human Readable Title", + children=[], + ) + + self.assertNotEqual(first_result.entity_id, second_result.entity_id) + self.assertNotEqual(first_result.entity.key, second_result.entity.key) + # Make sure the current logic from tasts::_find_unique_slug is used + self.assertEqual(second_result.entity.key, first_result.entity.key + "_1") + + container_version = second_result.containerversion + self.assertEqual(container_version.title, "Original Human Readable Title") + + def test_migrate_container_replace_existing_true(self): + """ + Test _migrate_container creates new version when replace_existing=True + """ + source_key = self.course.id.make_usage_key("vertical", "replaceable_vertical") + + child_component = authoring_api.create_component( + self.learning_package.id, + component_type=authoring_api.get_or_create_component_type( + "xblock.v1", "problem" + ), + local_key="child_problem", + created=timezone.now(), + created_by=self.user.id, + ) + child_version = authoring_api.create_next_component_version( + child_component.pk, + content_to_replace={}, + created=timezone.now(), + created_by=self.user.id, + ) + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Update, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + first_result = _migrate_container( + context=context, + source_key=source_key, + container_type=lib_api.ContainerType.Unit, + title="Original Title", + children=[], + ) + + context.existing_source_to_target_keys[source_key] = first_result.entity + + second_result = _migrate_container( + context=context, + source_key=source_key, + container_type=lib_api.ContainerType.Unit, + title="Updated Title", + children=[child_version.publishable_entity_version], + ) + + self.assertEqual(first_result.entity_id, second_result.entity_id) + self.assertNotEqual(first_result.version_num, second_result.version_num) + + container_version = second_result.containerversion + self.assertEqual(container_version.title, "Updated Title") + self.assertEqual(container_version.entity_list.entitylistrow_set.count(), 1) + + def test_migrate_container_with_library_source_key(self): + """ + Test _migrate_container with library source key + """ + library_key = LibraryLocator(org="TestOrg", library="TestLibrary") + source_key = library_key.make_usage_key("vertical", "library_vertical") + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + result = _migrate_container( + context=context, + source_key=source_key, + container_type=lib_api.ContainerType.Unit, + title="Library Vertical", + children=[], + ) + + self.assertIsNotNone(result) + + container_version = result.containerversion + self.assertEqual(container_version.title, "Library Vertical") + + def test_migrate_container_empty_children_list(self): + """ + Test _migrate_container handles empty children list + """ + source_key = self.course.id.make_usage_key("vertical", "empty_vertical") + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + result = _migrate_container( + context=context, + source_key=source_key, + container_type=lib_api.ContainerType.Unit, + title="Empty Vertical", + children=[], + ) + + self.assertIsNotNone(result) + + container_version = result.containerversion + self.assertEqual(container_version.entity_list.entitylistrow_set.count(), 0) + + def test_migrate_container_preserves_child_order(self): + """ + Test _migrate_container preserves the order of children + """ + source_key = self.course.id.make_usage_key("vertical", "ordered_vertical") + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + children = [] + for i in range(3): + child_component = authoring_api.create_component( + self.learning_package.id, + component_type=authoring_api.get_or_create_component_type( + "xblock.v1", "problem" + ), + local_key=f"child_problem_{i}", + created=timezone.now(), + created_by=self.user.id, + ) + child_version = authoring_api.create_next_component_version( + child_component.pk, + content_to_replace={}, + created=timezone.now(), + created_by=self.user.id, + ) + children.append(child_version.publishable_entity_version) + + result = _migrate_container( + context=context, + source_key=source_key, + container_type=lib_api.ContainerType.Unit, + title="Ordered Vertical", + children=children, + ) + + container_version = result.containerversion + entity_rows = list( + container_version.entity_list.entitylistrow_set.order_by("order_num") + ) + + self.assertEqual(len(entity_rows), 3) + for i, (expected_child, actual_row) in enumerate(zip(children, entity_rows)): + self.assertEqual(expected_child.entity_id, actual_row.entity_id) + + def test_migrate_container_with_mixed_child_types(self): + """ + Test _migrate_container with children of different component types + """ + source_key = self.course.id.make_usage_key("vertical", "mixed_vertical") + + problem_component = authoring_api.create_component( + self.learning_package.id, + component_type=authoring_api.get_or_create_component_type( + "xblock.v1", "problem" + ), + local_key="mixed_problem", + created=timezone.now(), + created_by=self.user.id, + ) + problem_version = authoring_api.create_next_component_version( + problem_component.pk, + content_to_replace={}, + created=timezone.now(), + created_by=self.user.id, + ) + + html_component = authoring_api.create_component( + self.learning_package.id, + component_type=authoring_api.get_or_create_component_type( + "xblock.v1", "html" + ), + local_key="mixed_html", + created=timezone.now(), + created_by=self.user.id, + ) + html_version = authoring_api.create_next_component_version( + html_component.pk, + content_to_replace={}, + created=timezone.now(), + created_by=self.user.id, + ) + + video_component = authoring_api.create_component( + self.learning_package.id, + component_type=authoring_api.get_or_create_component_type( + "xblock.v1", "video" + ), + local_key="mixed_video", + created=timezone.now(), + created_by=self.user.id, + ) + video_version = authoring_api.create_next_component_version( + video_component.pk, + content_to_replace={}, + created=timezone.now(), + created_by=self.user.id, + ) + + children = [ + problem_version.publishable_entity_version, + html_version.publishable_entity_version, + video_version.publishable_entity_version, + ] + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + result = _migrate_container( + context=context, + source_key=source_key, + container_type=lib_api.ContainerType.Unit, + title="Mixed Content Vertical", + children=children, + ) + + self.assertIsNotNone(result) + + container_version = result.containerversion + self.assertEqual(container_version.entity_list.entitylistrow_set.count(), 3) + + child_entity_ids = set( + container_version.entity_list.entitylistrow_set.values_list( + "entity_id", flat=True + ) + ) + expected_entity_ids = {child.entity_id for child in children} + self.assertEqual(child_entity_ids, expected_entity_ids) + + def test_migrate_container_generates_correct_target_key(self): + """ + Test _migrate_container generates correct target key from source key + """ + course_source_key = self.course.id.make_usage_key("vertical", "test_vertical") + context = _MigrationContext( + existing_source_to_target_keys={}, + target_package_id=self.learning_package.id, + target_library_key=self.library.library_key, + source_context_key=self.course.id, + content_by_filename={}, + composition_level=CompositionLevel.Unit, + repeat_handling_strategy=RepeatHandlingStrategy.Skip, + preserve_url_slugs=True, + created_at=timezone.now(), + created_by=self.user.id, + ) + + course_result = _migrate_container( + context=context, + source_key=course_source_key, + container_type=lib_api.ContainerType.Unit, + title="Course Vertical", + children=[], + ) + context.add_migration(course_source_key, course_result.entity) + + library_key = LibraryLocator(org="TestOrg", library="TestLibrary") + library_source_key = library_key.make_usage_key("vertical", "test_vertical") + + library_result = _migrate_container( + context=context, + source_key=library_source_key, + container_type=lib_api.ContainerType.Unit, + title="Library Vertical", + children=[], + ) + + self.assertIsNotNone(course_result) + self.assertIsNotNone(library_result) + self.assertNotEqual(course_result.entity_id, library_result.entity_id) + + def test_migrate_from_modulestore_success_course(self): + """ + Test successful migration from course to library + """ + source = ModulestoreSource.objects.create(key=self.course.id) + + task = migrate_from_modulestore.apply_async( + kwargs={ + "user_id": self.user.id, + "source_pk": source.id, + "target_package_pk": self.learning_package.id, + "target_library_key": str(self.lib_key), + "target_collection_pk": self.collection.id, + "repeat_handling_strategy": RepeatHandlingStrategy.Skip.value, + "preserve_url_slugs": True, + "composition_level": CompositionLevel.Unit.value, + "forward_source_to_target": False, + } + ) + + status = UserTaskStatus.objects.get(task_id=task.id) + self.assertEqual(status.state, UserTaskStatus.SUCCEEDED) + + migration = ModulestoreMigration.objects.get( + source=source, target=self.learning_package + ) + self.assertEqual(migration.composition_level, CompositionLevel.Unit.value) + self.assertEqual(migration.repeat_handling_strategy, RepeatHandlingStrategy.Skip.value) + + def test_migrate_from_modulestore_library_validation_failure(self): + """ + Test migration from legacy library fails when modulestore content doesn't exist + """ + library_key = LibraryLocator(org="TestOrg", library="TestLibrary") + + source = ModulestoreSource.objects.create(key=library_key) + + task = migrate_from_modulestore.apply_async( + kwargs={ + "user_id": self.user.id, + "source_pk": source.id, + "target_package_pk": self.learning_package.id, + "target_library_key": str(self.lib_key), + "target_collection_pk": None, + "repeat_handling_strategy": RepeatHandlingStrategy.Update.value, + "preserve_url_slugs": True, + "composition_level": CompositionLevel.Section.value, + "forward_source_to_target": True, + } + ) + + status = UserTaskStatus.objects.get(task_id=task.id) + + # Should fail at loading step since we don't have real modulestore content + self.assertEqual(status.state, UserTaskStatus.FAILED) + self.assertEqual( + self._get_task_status_fail_message(status), + "Failed to load source item 'lib-block-v1:TestOrg+TestLibrary+type@library+block@library' " + "from ModuleStore: library-v1:TestOrg+TestLibrary+branch@library" + ) + + def test_migrate_from_modulestore_invalid_source_key_type(self): + """ + Test migration with invalid source key type + """ + invalid_key = LibraryLocatorV2.from_string("lib:testorg:invalid") + source = ModulestoreSource.objects.create(key=invalid_key) + + task = migrate_from_modulestore.apply_async( + kwargs={ + "user_id": self.user.id, + "source_pk": source.id, + "target_package_pk": self.learning_package.id, + "target_library_key": str(self.lib_key), + "target_collection_pk": self.collection.id, + "repeat_handling_strategy": RepeatHandlingStrategy.Skip.value, + "preserve_url_slugs": True, + "composition_level": CompositionLevel.Unit.value, + "forward_source_to_target": False, + } + ) + + status = UserTaskStatus.objects.get(task_id=task.id) + self.assertEqual(status.state, UserTaskStatus.FAILED) + self.assertEqual( + self._get_task_status_fail_message(status), + f"Not a valid source context key: {invalid_key}. Source key must reference a course or a legacy library." + ) + + def test_migrate_from_modulestore_nonexistent_modulestore_item(self): + """ + Test migration when modulestore item doesn't exist + """ + nonexistent_course_key = CourseKey.from_string( + "course-v1:NonExistent+Course+Run" + ) + source = ModulestoreSource.objects.create(key=nonexistent_course_key) + + task = migrate_from_modulestore.apply_async( + kwargs={ + "user_id": self.user.id, + "source_pk": source.id, + "target_package_pk": self.learning_package.id, + "target_library_key": str(self.lib_key), + "target_collection_pk": self.collection.id, + "repeat_handling_strategy": RepeatHandlingStrategy.Skip.value, + "preserve_url_slugs": True, + "composition_level": CompositionLevel.Unit.value, + "forward_source_to_target": False, + } + ) + + status = UserTaskStatus.objects.get(task_id=task.id) + self.assertEqual(status.state, UserTaskStatus.FAILED) + self.assertEqual( + self._get_task_status_fail_message(status), + "Failed to load source item 'block-v1:NonExistent+Course+Run+type@course+block@course' " + "from ModuleStore: course-v1:NonExistent+Course+Run+branch@draft-branch" + ) + + def test_migrate_from_modulestore_task_status_progression(self): + """Test that task status progresses through expected steps""" + source = ModulestoreSource.objects.create(key=self.course.id) + + task = migrate_from_modulestore.apply_async( + kwargs={ + "user_id": self.user.id, + "source_pk": source.id, + "target_package_pk": self.learning_package.id, + "target_library_key": str(self.lib_key), + "target_collection_pk": self.collection.id, + "repeat_handling_strategy": RepeatHandlingStrategy.Skip.value, + "preserve_url_slugs": True, + "composition_level": CompositionLevel.Unit.value, + "forward_source_to_target": False, + } + ) + + status = UserTaskStatus.objects.get(task_id=task.id) + + # Should either succeed or fail, but should have progressed past validation + self.assertIn(status.state, [UserTaskStatus.SUCCEEDED, UserTaskStatus.FAILED]) + + migration = ModulestoreMigration.objects.get( + source=source, target=self.learning_package + ) + self.assertEqual(migration.task_status, status) + + def test_migrate_from_modulestore_multiple_users_no_interference(self): + """ + Test that migrations by different users don't interfere with each other + """ + source = ModulestoreSource.objects.create(key=self.course.id) + other_user = UserFactory() + + task1 = migrate_from_modulestore.apply_async( + kwargs={ + "user_id": self.user.id, + "source_pk": source.id, + "target_package_pk": self.learning_package.id, + "target_library_key": str(self.lib_key), + "target_collection_pk": self.collection.id, + "repeat_handling_strategy": RepeatHandlingStrategy.Skip.value, + "preserve_url_slugs": True, + "composition_level": CompositionLevel.Unit.value, + "forward_source_to_target": False, + } + ) + + task2 = migrate_from_modulestore.apply_async( + kwargs={ + "user_id": other_user.id, + "source_pk": source.id, + "target_package_pk": self.learning_package.id, + "target_library_key": str(self.lib_key), + "target_collection_pk": self.collection.id, + "repeat_handling_strategy": RepeatHandlingStrategy.Skip.value, + "preserve_url_slugs": True, + "composition_level": CompositionLevel.Unit.value, + "forward_source_to_target": False, + } + ) + + status1 = UserTaskStatus.objects.get(task_id=task1.id) + status2 = UserTaskStatus.objects.get(task_id=task2.id) + + self.assertEqual(status1.user, self.user) + self.assertEqual(status2.user, other_user) + + # The first task should not be cancelled since it's from a different user + self.assertNotEqual(status1.state, UserTaskStatus.CANCELED) diff --git a/cms/envs/common.py b/cms/envs/common.py index 20db8ba750..99db8900d3 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1005,6 +1005,7 @@ INSTALLED_APPS = [ 'openedx.core.djangoapps.course_groups', # not used in cms (yet), but tests run 'cms.djangoapps.xblock_config.apps.XBlockConfig', 'cms.djangoapps.export_course_metadata.apps.ExportCourseMetadataConfig', + 'cms.djangoapps.modulestore_migrator', # New (Learning-Core-based) XBlock runtime 'openedx.core.djangoapps.xblock.apps.StudioXBlockAppConfig', diff --git a/cms/envs/help_tokens.ini b/cms/envs/help_tokens.ini index 08cd73b30e..8d6b48fd50 100644 --- a/cms/envs/help_tokens.ini +++ b/cms/envs/help_tokens.ini @@ -32,7 +32,7 @@ group_configurations = course_author:course_features/content_experiments/content container = course_author:developing_course/course_components.html#components-that-contain-other-components video = course_author:video/index.html certificates = course_author:set_up_course/studio_add_course_information/studio_creating_certificates.html -content_highlights = course_author:developing_course/course_sections.html#set-section-highlights-for-weekly-course-highlight-messages +content_highlights = course_author:developing_course/course_sections.html#set-course-section-highlights image_accessibility = course_author:accessibility/best_practices_course_content_dev.html#use-best-practices-for-describing-images social_sharing = course_author:developing_course/social_sharing.html diff --git a/cms/envs/test.py b/cms/envs/test.py index 9a294bf10b..5b6b4facbe 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -373,3 +373,5 @@ SINGLE_LEARNER_COURSE_REGRADE_ROUTING_KEY = "edx.lms.core.default" SOFTWARE_SECURE_VERIFICATION_ROUTING_KEY = "edx.lms.core.default" STATIC_ROOT_BASE = "/edx/var/edxapp/staticfiles" STATIC_URL_BASE = "/static/" + +CATALOG_MICROFRONTEND_URL = "http://catalog-mfe" diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 831f71747d..d50f6b4bbe 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -577,6 +577,7 @@ function($, _, Backbone, gettext, BasePage, const headerElement = xblockElement.find('.xblock-header-primary'); const upstreamBlockId = headerElement.data('upstream-ref'); const upstreamBlockVersionSynced = headerElement.data('version-synced'); + const isLocallyModified = headerElement.data('is-modified'); try { if (this.options.isIframeEmbed) { @@ -586,9 +587,11 @@ function($, _, Backbone, gettext, BasePage, payload: { downstreamBlockId: xblockInfo.get('id'), displayName: xblockInfo.get('display_name'), - isVertical: xblockInfo.isVertical(), + isContainer: false, upstreamBlockId, upstreamBlockVersionSynced, + isLocallyModified: isLocallyModified === 'True', + blockType: xblockInfo.get('category'), } }, document.referrer ); diff --git a/cms/static/sass/course-unit-mfe-iframe-bundle.scss b/cms/static/sass/course-unit-mfe-iframe-bundle.scss index b53210484a..6c9b43cf0b 100644 --- a/cms/static/sass/course-unit-mfe-iframe-bundle.scss +++ b/cms/static/sass/course-unit-mfe-iframe-bundle.scss @@ -470,6 +470,14 @@ body, &.xblock-iframe-content { height: 100%; + .xblock-title { + margin-bottom: 1.5em !important; + font-size: 1.5em; + font-weight: bold; + margin-block-start: 0.83em; + margin-block-end: 0.83em; + } + // Reset the max-height to allow the settings list to grow .wrapper-comp-settings .list-input.settings-list { max-height: unset; diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index 2c57872e10..4fb3c71803 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -93,6 +93,7 @@ can_unlink = upstream_info.upstream_ref and not upstream_info.has_top_level_pare % if upstream_info.upstream_ref: data-upstream-ref = ${upstream_info.upstream_ref} data-version-synced = ${upstream_info.version_synced} + data-is-modified = ${upstream_info.is_modified} %endif >
diff --git a/cms/urls.py b/cms/urls.py index 50ea7b3add..c60b56c3bd 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -141,6 +141,8 @@ urlpatterns = oauth2_urlpatterns + [ # rest api for course import/export path('api/courses/', include('cms.djangoapps.contentstore.api.urls', namespace='courses_api') ), + path('api/modulestore_migrator/', + include('cms.djangoapps.modulestore_migrator.rest_api.urls', namespace='modulestore_migrator_api')), re_path(fr'^export/{COURSELIKE_KEY_PATTERN}$', contentstore_views.export_handler, name='export_handler'), re_path(fr'^export_output/{COURSELIKE_KEY_PATTERN}$', contentstore_views.export_output_handler, diff --git a/common/djangoapps/util/course.py b/common/djangoapps/util/course.py index fa82f9c4aa..abef18a326 100644 --- a/common/djangoapps/util/course.py +++ b/common/djangoapps/util/course.py @@ -9,6 +9,7 @@ from urllib.parse import urlencode from django.conf import settings from opaque_keys.edx.keys import CourseKey, UsageKey +from lms.djangoapps.branding.toggles import use_catalog_mfe from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx_filters.learning.filters import CourseAboutPageURLRequested @@ -50,22 +51,25 @@ def get_link_for_about_page(course): 'SOCIAL_SHARING_SETTINGS', getattr(settings, 'SOCIAL_SHARING_SETTINGS', {}) ).get('CUSTOM_COURSE_URLS') + + if use_catalog_mfe(): + about_base_url = settings.CATALOG_MICROFRONTEND_URL + else: + about_base_url = configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL) + if is_social_sharing_enabled and course.social_sharing_url: course_about_url = course.social_sharing_url elif settings.FEATURES.get('ENABLE_MKTG_SITE') and getattr(course, 'marketing_url', None): course_about_url = course.marketing_url else: - course_about_url = '{about_base_url}/courses/{course_key}/about'.format( - about_base_url=configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL), - course_key=str(course.id), - ) + course_about_url = f'{about_base_url}/courses/{course.id}/about' - ## .. filter_implemented_name: CourseAboutPageURLRequested - ## .. filter_type: org.openedx.learning.course_about.page.url.requested.v1 - course_about_url, _ = CourseAboutPageURLRequested.run_filter( - url=course_about_url, - org=course.id.org, - ) + ## .. filter_implemented_name: CourseAboutPageURLRequested + ## .. filter_type: org.openedx.learning.course_about.page.url.requested.v1 + course_about_url, _ = CourseAboutPageURLRequested.run_filter( + url=course_about_url, + org=course.id.org, + ) return course_about_url diff --git a/common/djangoapps/util/tests/test_course.py b/common/djangoapps/util/tests/test_course.py index 1c476b9756..f0baf55814 100644 --- a/common/djangoapps/util/tests/test_course.py +++ b/common/djangoapps/util/tests/test_course.py @@ -5,6 +5,7 @@ from unittest import mock import ddt from django.conf import settings +from django.test import override_settings from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from common.djangoapps.util.course import get_link_for_about_page @@ -51,17 +52,18 @@ class TestCourseSharingLinks(ModuleStoreTestCase): """ mock_settings = { 'FEATURES': { - 'ENABLE_MKTG_SITE': enable_mktg_site + 'ENABLE_MKTG_SITE': enable_mktg_site, }, 'SOCIAL_SHARING_SETTINGS': { 'CUSTOM_COURSE_URLS': enable_social_sharing - }, + } } - with mock.patch.multiple('django.conf.settings', **mock_settings): - course_sharing_link = get_link_for_about_page( - self.course_overview if use_overview else self.course - ) + with override_settings(ENABLE_CATALOG_MICROFRONTEND=False): + with mock.patch.multiple('django.conf.settings', **mock_settings): + course_sharing_link = get_link_for_about_page( + self.course_overview if use_overview else self.course + ) return course_sharing_link @@ -126,3 +128,24 @@ class TestCourseSharingLinks(ModuleStoreTestCase): use_overview=False, ) assert actual_course_sharing_link == expected_course_sharing_link + + @ddt.data( + ( + True, + f'{settings.CATALOG_MICROFRONTEND_URL}/courses/course-v1:test_org+test_number+test_run/about' + ), + ( + False, + f'{settings.LMS_ROOT_URL}/courses/course-v1:test_org+test_number+test_run/about' + ) + ) + @ddt.unpack + def test_sharing_link_with_new_course_about_page( + self, catalog_mfe_enabled, expected_course_sharing_link + ): + """ + Verify the method gives correct course sharing url when new course about page is used. + """ + with override_settings(ENABLE_CATALOG_MICROFRONTEND=catalog_mfe_enabled): + actual_course_sharing_link = get_link_for_about_page(self.course_overview) + assert actual_course_sharing_link == expected_course_sharing_link diff --git a/common/templates/xblock_v2/xblock_iframe.html b/common/templates/xblock_v2/xblock_iframe.html index cd3096aa46..9bf0ab2df9 100644 --- a/common/templates/xblock_v2/xblock_iframe.html +++ b/common/templates/xblock_v2/xblock_iframe.html @@ -196,6 +196,11 @@ event listeners below, in certain situations. Resetting it to the default "auto" skirts the problem.-->
+ {% if show_title %} +
+ {{ display_name | safe }} +
+ {% endif %} {{ fragment.body_html | safe }} diff --git a/db_keyword_overrides.yml b/db_keyword_overrides.yml index f2dda1e593..2932198016 100644 --- a/db_keyword_overrides.yml +++ b/db_keyword_overrides.yml @@ -27,6 +27,8 @@ MYSQL: - UserOrgTag.key - UserPreference.key - XAPILRSConfiguration.key + - ModulestoreSource.key + - ModulestoreBlockSource.key SNOWFLAKE: - CourseOverview.start - HistoricalCourseOverview.start diff --git a/docs/decisions/0024-course-component-types.rst b/docs/decisions/0024-course-component-types.rst new file mode 100644 index 0000000000..fd343ceacd --- /dev/null +++ b/docs/decisions/0024-course-component-types.rst @@ -0,0 +1,144 @@ +Course Component Types Page +########################### + +**Status**: Proposed +**Date**: 2025‑06‑02 +**Target release**: Verawood + +----- + +Problem Statement +***************** + +Course authors currently enable or disable XBlocks for each course via Advanced Settings → "Advanced modules" and the corresponding JSON configuration fields. This user interface is rather difficult for the average user to navigate: it ties the Studio UI to low‐level block keys that the user must locate in documentation or block code, and it does not display any block metadata (documentation, support level, etc.). + +The new **Course Component Types page** (see *Figure 1*) provides a catalog-style interface where authors can browse, search, and enable blocks. To support this page, we must: + +* Store canonical metadata for each installed XBlock. +* Store activation for each course. +* Safely transfer the existing configuration. +* Synchronize the functionality of the new block addition flow from the Course Component Types page and the old one through Advanced modules until it is removed. + +Architectural diagrams (*Figures 2 and 3*) illustrate the interaction during execution. + + +Decision +******** + +Domain Models +============= + +.. list-table:: Domain Models + :widths: 25 35 40 + :header-rows: 1 + + * - Model + - Responsibility + - Key Fields + * - **ComponentType** + - Canonical catalog record for an XBlock type. Contains fields for global overrides specified in the subtitle, description, documentation urls block. + - ``title``, ``slug`` (entry‑point name), ``enabled`` *(global default)*, ``support_level`` *(global default)*, ``component_type`` (``common`` | ``advanced`` | ``external``, default=``advanced``), ``created``, ``updated``, ``is_experimental``, ``title`` (optional override), ``subtitle`` (optional override), ``description`` (optional override), ``documentation_url`` (optional override). + * - **CourseComponentType** + - Join between ``ComponentType`` and ``CourseKey`` storing per‑course enablement. + - ``id``, ``course_key``, ``content_block``, ``enabled``, ``created``, ``updated``, ``configurable_fields`` (JSON). + +Both models live in the existing app ``common.djangoapps.xblock_django``. A unique constraint ``(course_key, content_block)`` prevents duplicates. + +Bootstrap and migration +======================= + +* Created json with a list of default component types. (The list of component types and data about them can be viewed here_) (All other component types will be marked as experimental during migration) +* Created a migration that fills in the records in `ComponentType` according to the specified json and creates records for each course in CourseComponentType for the enabled component types. +* Created a migration/management command that scans all types of components added to the Advanced modules list and creates corresponding entries in ComponentType if a component with such a slug exists/is available in entry_points. Creates entries in CourseComponentType according to the enabled component types in courses. + +.. _here: https://openedx.atlassian.net/wiki/spaces/COMM/database/4499341322 + + +Runtime APIs +============ + +.. list-table:: Runtime APIs + :widths: 30 10 60 + :header-rows: 1 + + * - Endpoint + - Method + - Purpose + * - ``/api/course_component_types/v1//`` + - GET + - Return **all enabled component types** for a course. Supports ``?component_type=common|advanced|external`` filter. + * - ``/api/course_component_types/v1//`` + - POST + - Add a new component type to the course. The request body must contain ``slug`` (entry‑point name). If the component type is enabled globally, it will be enabled for the course. If the component type is not enabled globally, it shouldn't be added to the course. + * - ``/xblock//`` (configurable_fields_info|metadata_info) + - GET + - **XBlock Info Handler** (*Fig. 3*) to return ``metadata``(``title``, ``subtitle``, ``description`` etc.) or data about ``configurable_fields`` like a field name, type, value, help etc. + * - ``/api/course_component_types/v1///`` + - POST + - Persist author edits to component type specific configuration fields (dynamic schema) and store to ``CourseComponentType`` as JSON. + + +Serializers source immutable metadata from ``ComponentType``, then layer per‑course overrides from ``CourseComponentType``. + +New mixin +========= + +* **``StudioConfigurableXBlockMixin``** Adds and lists the configuration fields of the component. These fields are also added to the non_editable_fields of the block so that they cannot be changed from the edit form on the unit page. The list of configuration fields can be overridden in the child classes of the corresponding blocks. The mixin also adds default values for metadata fields such as title, subtitle, description, and documentation links. At the same time, it provides an interface for obtaining the values of these fields, as they can be overwritten by the administrator in BlockConfig (shown in the diagram). + +Waffle Flag `...enable_course_component_types_page` +=================================================== + +.. list-table:: Waffle Flag ``...enable_course_component_types_page`` + :header-rows: 1 + + * - Flag state + - Behaviour + * - **Enabled** + - "Course Component Types" appears under *Content* menu; Course Component Types page is accessible. + * - **Disabled** + - Legacy behaviour intact, Course Component Types page is hidden. + +Advanced Modules Field Deprecation +================================== + +The "Advanced module list" field is hidden by default as an deprecated field. It is displayed after clicking the "Show deprecated settings" button and is marked as deprecated. The "Advanced module list" field still retains its full functionality but will be removed over time. + + +Consequences +************ + +* Every new installed XBlock must be added to the ``ComponentType`` table. +* When a user adds a new component type to the Advanced modules list, a corresponding entry with a link to the course is created in CourseComponentType. +* The "Course Component Types" page is discoverable and provides a better UX for course authors. +* If a component type is not enabled in the "Advanced Modules" list, it will be hidden from the course author on the Studio unit page. They will not be able to add it to the course, but any components of that type that have already been added will continue to work. (This is the same as the current behavior.) +* The new API endpoints allow for dynamic configuration of component types and retrieval of metadata. +* The new mixin allows for easy addition of configuration fields to XBlocks and provides a consistent interface for metadata. +* Many existing component types will be marked as experimental during migration, allowing for a gradual transition to the new system. +* The "Advanced module list" field is deprecated, and its functionality will be removed in the future. +* Many new DB entries will be created during the migration, but this is a one‑time cost. + + +Rejected Alternatives +********************* + +* **Hardcoded list of common blocks**: This would not allow for extensibility or dynamic configuration. To many configuration levels, it would be difficult to maintain and extend. +* **Extend existing XBlockConfiguration model**: The current implementation of XBlockConfiguration and related models(XBlockStudioConfigurationFlag, XBlockStudioConfiguration) has complex logic and rather strange behavior (when adding a block to XBlockStudioConfiguration, all other blocks disappear on the unit page, including standard ones (html, problem, video), and there is no way to enable them separately). Also, since these are fairly old models, such a significant refactoring could cause significant problems with existing data. +* **Ability to change block metadata fields on course level**: There is no need for this level, as it is unlikely that information such as component type name, description, or documentation links will need to be changed from course to course. + +References +********** + +* **Figure 1** – *Course Component Types page*. + +.. image:: images/course_component_types_page_design.png + :alt: Course Component Types page + +* **Figure 2** – *Course Component Types API*. + +.. image:: images/course_component_types_api_diagram.png + :alt: Course Component Types API + + +* **Figure 3** – *Interaction diagram of the content block’s sidebar tabs*. +.. image:: images/course_component_types_system_diagram.png + :alt: Interaction diagram of the content block’s sidebar tabs diff --git a/docs/images/сourse_сomponent_types_api_diagram.png b/docs/images/сourse_сomponent_types_api_diagram.png new file mode 100644 index 0000000000..a26a7cebda Binary files /dev/null and b/docs/images/сourse_сomponent_types_api_diagram.png differ diff --git a/docs/images/сourse_сomponent_types_page_design.png b/docs/images/сourse_сomponent_types_page_design.png new file mode 100644 index 0000000000..1f1fa71261 Binary files /dev/null and b/docs/images/сourse_сomponent_types_page_design.png differ diff --git a/docs/images/сourse_сomponent_types_system_diagram.png b/docs/images/сourse_сomponent_types_system_diagram.png new file mode 100644 index 0000000000..d45367d728 Binary files /dev/null and b/docs/images/сourse_сomponent_types_system_diagram.png differ diff --git a/lms/djangoapps/branding/test_toggles.py b/lms/djangoapps/branding/test_toggles.py index 92bde93a71..a6797b8090 100644 --- a/lms/djangoapps/branding/test_toggles.py +++ b/lms/djangoapps/branding/test_toggles.py @@ -19,5 +19,5 @@ class TestBrandingToggles(TestCase): """ Test the use_catalog_mfe toggle. """ - with override_settings(FEATURES={'ENABLE_CATALOG_MICROFRONTEND': enabled}): + with override_settings(ENABLE_CATALOG_MICROFRONTEND=enabled): assert use_catalog_mfe() == enabled diff --git a/lms/djangoapps/branding/toggles.py b/lms/djangoapps/branding/toggles.py index ba7b3407d7..c981bbe30b 100644 --- a/lms/djangoapps/branding/toggles.py +++ b/lms/djangoapps/branding/toggles.py @@ -8,8 +8,8 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_ def use_catalog_mfe(): """ - Determine if Catalog MFE is enabled, replacing student_dashboard + Returns a boolean = true if the Catalog MFE is enabled. """ return configuration_helpers.get_value( - 'ENABLE_CATALOG_MICROFRONTEND', settings.FEATURES['ENABLE_CATALOG_MICROFRONTEND'] + 'ENABLE_CATALOG_MICROFRONTEND', getattr(settings, 'ENABLE_CATALOG_MICROFRONTEND', False) ) diff --git a/lms/djangoapps/courseware/tests/test_courses.py b/lms/djangoapps/courseware/tests/test_courses.py index 804f903779..72465b299a 100644 --- a/lms/djangoapps/courseware/tests/test_courses.py +++ b/lms/djangoapps/courseware/tests/test_courses.py @@ -88,7 +88,7 @@ class CoursesTest(ModuleStoreTestCase): assert not error.value.access_response.has_access @ddt.data( - (GET_COURSE_WITH_ACCESS, 2), + (GET_COURSE_WITH_ACCESS, 1), (GET_COURSE_OVERVIEW_WITH_ACCESS, 0), ) @ddt.unpack diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 1b199df151..5b3ed4bb8d 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -47,6 +47,7 @@ from rest_framework import status from rest_framework.decorators import api_view, throttle_classes from rest_framework.response import Response from rest_framework.throttling import UserRateThrottle +from rest_framework.fields import BooleanField from web_fragments.fragment import Fragment from xmodule.course_block import ( COURSE_VISIBILITY_PUBLIC, @@ -1576,6 +1577,9 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True, disable_sta Returns an HttpResponse with HTML content for the xBlock with the given usage_key. The returned HTML is a chromeless rendering of the xBlock (excluding content of the containing courseware). """ + if not disable_staff_debug_info: + disable_staff_debug_info = BooleanField().to_internal_value(request.GET.get('disable_staff_debug_info', False)) + usage_key = UsageKey.from_string(usage_key_string) usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key)) diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 347879105a..8f32991272 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -156,8 +156,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest assert mock_block_structure_create.call_count == 1 @ddt.data( - (ModuleStoreEnum.Type.split, 2, 42, True), - (ModuleStoreEnum.Type.split, 2, 42, False), + (ModuleStoreEnum.Type.split, 1, 42, True), + (ModuleStoreEnum.Type.split, 1, 42, False), ) @ddt.unpack def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): @@ -168,7 +168,7 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self._apply_recalculate_subsection_grade() @ddt.data( - (ModuleStoreEnum.Type.split, 2, 42), + (ModuleStoreEnum.Type.split, 1, 42), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -200,16 +200,17 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.course.id): self.store.update_item(sequential, self.user.id) - # Make sure the signal is sent for only the 2 accessible sequentials. + # Make sure the signal is sent for only the 1 accessible sequentials. + # Update: draft branch content shouldn't be accessible self._apply_recalculate_subsection_grade() - assert mock_subsection_signal.call_count == 2 + assert mock_subsection_signal.call_count == 1 sequentials_signalled = { args[1]['subsection_grade'].location for args in mock_subsection_signal.call_args_list } self.assertSetEqual( sequentials_signalled, - {self.sequential.location, accessible_seq.location}, + {self.sequential.location}, ) @patch('lms.djangoapps.grades.signals.signals.SUBSECTION_SCORE_CHANGED.send') @@ -255,7 +256,7 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest UserPartition.scheme_extensions = None @ddt.data( - (ModuleStoreEnum.Type.split, 2, 42), + (ModuleStoreEnum.Type.split, 1, 42), ) @ddt.unpack def test_persistent_grades_on_course(self, default_store, num_mongo_queries, num_sql_queries): diff --git a/lms/djangoapps/learner_home/test_views.py b/lms/djangoapps/learner_home/test_views.py index 5b09893971..8722629371 100644 --- a/lms/djangoapps/learner_home/test_views.py +++ b/lms/djangoapps/learner_home/test_views.py @@ -61,6 +61,7 @@ from xmodule.modulestore.tests.factories import CourseFactory ENTERPRISE_ENABLED = "ENABLE_ENTERPRISE_INTEGRATION" +@ddt.ddt class TestGetPlatformSettings(TestCase): """Tests for get_platform_settings""" @@ -88,6 +89,18 @@ class TestGetPlatformSettings(TestCase): }, ) + @ddt.data( + (True, f'{settings.CATALOG_MICROFRONTEND_URL}/courses'), + (False, '/courses'), + ) + @ddt.unpack + def test_link_with_new_catalog_page(self, catalog_mfe_enabled, expected_catalog_link): + """ + Test that the catalog link is constructed correctly based on the MFE flags. + """ + with override_settings(ENABLE_CATALOG_MICROFRONTEND=catalog_mfe_enabled): + assert get_platform_settings()["courseSearchUrl"] == expected_catalog_link + @ddt.ddt class TestGetUserAccountConfirmationInfo(SharedModuleStoreTestCase): diff --git a/lms/djangoapps/learner_home/views.py b/lms/djangoapps/learner_home/views.py index b97f9a20f4..40962721b3 100644 --- a/lms/djangoapps/learner_home/views.py +++ b/lms/djangoapps/learner_home/views.py @@ -41,6 +41,7 @@ from common.djangoapps.util.course import ( from common.djangoapps.util.milestones_helpers import ( get_pre_requisite_courses_not_completed, ) +from lms.djangoapps.branding import toggles from lms.djangoapps.bulk_email.models import Optout from lms.djangoapps.bulk_email.models_api import is_bulk_email_feature_enabled from lms.djangoapps.commerce.utils import EcommerceService @@ -71,10 +72,14 @@ logger = logging.getLogger(__name__) def get_platform_settings(): """Get settings used for platform level connections: emails, url routes, etc.""" + course_search_url = marketing_link("COURSES") + if toggles.use_catalog_mfe(): + course_search_url = f"{settings.CATALOG_MICROFRONTEND_URL}/courses" + return { "supportEmail": settings.DEFAULT_FEEDBACK_EMAIL, "billingEmail": settings.PAYMENT_SUPPORT_EMAIL, - "courseSearchUrl": marketing_link("COURSES"), + "courseSearchUrl": course_search_url, } diff --git a/lms/envs/common.py b/lms/envs/common.py index 046b7cfef3..3dde7156b9 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3217,6 +3217,10 @@ ORA_MICROFRONTEND_URL = None # .. setting_default: None # .. setting_description: Base URL of the exams dashboard micro-frontend for instructors. EXAMS_DASHBOARD_MICROFRONTEND_URL = None +# .. setting_name: CATALOG_MICROFRONTEND_URL +# .. setting_default: None +# .. setting_description: Base URL of the micro-frontend-based course catalog page. +CATALOG_MICROFRONTEND_URL = None # .. setting_name: DISCUSSION_SPAM_URLS # .. setting_default: [] diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index e2fc7fb6c2..b15533855c 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -396,6 +396,7 @@ COMMUNICATIONS_MICROFRONTEND_URL = 'http://localhost:1984' AUTHN_MICROFRONTEND_URL = 'http://localhost:1999' AUTHN_MICROFRONTEND_DOMAIN = 'localhost:1999' EXAMS_DASHBOARD_MICROFRONTEND_URL = 'http://localhost:2020' +CATALOG_MICROFRONTEND_URL = 'http://localhost:1998/catalog' ################### FRONTEND APPLICATION DISCUSSIONS ################### DISCUSSIONS_MICROFRONTEND_URL = 'http://localhost:2002' diff --git a/lms/envs/test.py b/lms/envs/test.py index 958a54be01..218a7e8461 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -577,6 +577,7 @@ DISCUSSIONS_MICROFRONTEND_URL = "http://discussions-mfe" LEARNER_HOME_MICROFRONTEND_URL = "http://learner-home-mfe" ORA_GRADING_MICROFRONTEND_URL = "http://ora-grading-mfe" ORA_MICROFRONTEND_URL = "http://ora-mfe" +CATALOG_MICROFRONTEND_URL = "http://catalog-mfe" ########################## limiting dashboard courses ###################### diff --git a/mypy.ini b/mypy.ini index 72937e1034..982e500b3a 100644 --- a/mypy.ini +++ b/mypy.ini @@ -9,6 +9,7 @@ files = cms/lib/xblock/upstream_sync.py, cms/lib/xblock/upstream_sync_container.py, cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py, + cms/djangoapps/modulestore_migrator, openedx/core/djangoapps/content/learning_sequences, # FIXME: need to solve type issues and add 'search' app here: # openedx/core/djangoapps/content/search, diff --git a/openedx/core/djangoapps/content/block_structure/manager.py b/openedx/core/djangoapps/content/block_structure/manager.py index 49f423ce7a..b6a447bdf2 100644 --- a/openedx/core/djangoapps/content/block_structure/manager.py +++ b/openedx/core/djangoapps/content/block_structure/manager.py @@ -6,6 +6,8 @@ BlockStructures. from contextlib import contextmanager +from xmodule.modulestore import ModuleStoreEnum + from .exceptions import BlockStructureNotFound, TransformerDataIncompatible, UsageKeyNotInBlockStructure from .factory import BlockStructureFactory from .store import BlockStructureStore @@ -104,7 +106,6 @@ class BlockStructureManager: self.store, ) BlockStructureTransformers.verify_versions(block_structure) - except (BlockStructureNotFound, TransformerDataIncompatible): if user and getattr(user, "known", True): # This bypasses the runtime access checks. When we are populating the course blocks cache, @@ -133,10 +134,16 @@ class BlockStructureManager: the modulestore. """ with self._bulk_operations(): - block_structure = BlockStructureFactory.create_from_modulestore( - self.root_block_usage_key, - self.modulestore, - ) + # Always uses published-only branch regardless of CMS or LMS context. + with self.modulestore.branch_setting( + ModuleStoreEnum.Branch.published_only, + self.root_block_usage_key.course_key + ): + block_structure = BlockStructureFactory.create_from_modulestore( + self.root_block_usage_key, + self.modulestore, + ) + BlockStructureTransformers.collect(block_structure) self.store.add(block_structure) return block_structure diff --git a/openedx/core/djangoapps/content/block_structure/tests/helpers.py b/openedx/core/djangoapps/content/block_structure/tests/helpers.py index f0a20554e6..df53b33d7b 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/helpers.py +++ b/openedx/core/djangoapps/content/block_structure/tests/helpers.py @@ -110,6 +110,13 @@ class MockModulestore: """ yield + @contextmanager + def branch_setting(self, branch_settings, course_id=None): # pylint: disable=unused-argument + """ + A context manager for temporarily setting a store's branch value on the current thread. + """ + yield + class MockCache: """ diff --git a/openedx/core/djangoapps/content/block_structure/tests/test_manager.py b/openedx/core/djangoapps/content/block_structure/tests/test_manager.py index 493e4c3471..1c4060871c 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/test_manager.py +++ b/openedx/core/djangoapps/content/block_structure/tests/test_manager.py @@ -4,8 +4,11 @@ Tests for manager.py import pytest import ddt +from unittest.mock import MagicMock from django.test import TestCase +from xmodule.modulestore import ModuleStoreEnum + from ..block_structure import BlockStructureBlockData from ..exceptions import UsageKeyNotInBlockStructure from ..manager import BlockStructureManager @@ -216,3 +219,37 @@ class TestBlockStructureManager(UsageKeyFactoryMixin, ChildrenMapTestMixin, Test self.bs_manager.clear() self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) assert TestTransformer1.collect_call_count == 2 + + def test_update_collected_branch_context_integration(self): + """ + Integration test to verify the published-only branch context works end-to-end. + """ + # Track branch setting calls on our mock modulestore + attr_name = 'branch_setting' + original_branch_setting = getattr(self.modulestore, attr_name, None) + branch_setting_calls = [] + + def mock_branch_setting(branch, course_key): + branch_setting_calls.append((branch, course_key)) + # Return a proper context manager that does nothing + return MagicMock(__enter__=MagicMock(), __exit__=MagicMock()) + + # Add the branch_setting method to our mock modulestore + setattr(self.modulestore, attr_name, mock_branch_setting) + + try: + with mock_registered_transformers(self.registered_transformers): + self.bs_manager.get_collected() + + # Verify branch_setting was called with the correct parameters + self.assertEqual(len(branch_setting_calls), 1) + branch, course_key = branch_setting_calls[0] + self.assertEqual(branch, ModuleStoreEnum.Branch.published_only) + self.assertEqual(course_key, self.block_key_factory(0).course_key) + + finally: + # Restore original method if it existed + if original_branch_setting is not None: + setattr(self.modulestore, attr_name, original_branch_setting) + elif hasattr(self.modulestore, attr_name): + delattr(self.modulestore, attr_name) diff --git a/openedx/core/djangoapps/content_libraries/api/container_metadata.py b/openedx/core/djangoapps/content_libraries/api/container_metadata.py index 65779fafb4..53726ca49d 100644 --- a/openedx/core/djangoapps/content_libraries/api/container_metadata.py +++ b/openedx/core/djangoapps/content_libraries/api/container_metadata.py @@ -9,7 +9,18 @@ from django.db.models import QuerySet from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryUsageLocatorV2 from openedx_learning.api import authoring as authoring_api -from openedx_learning.api.authoring_models import Container, Component, PublishableEntity +from openedx_learning.api.authoring_models import ( + Component, + Container, + ContainerVersion, + Unit, + UnitVersion, + Subsection, + SubsectionVersion, + Section, + SectionVersion, + PublishableEntity, +) from openedx.core.djangoapps.content_tagging.api import get_object_tag_counts from openedx.core.djangoapps.xblock.api import get_component_from_usage_key @@ -36,6 +47,25 @@ class ContainerType(Enum): Subsection = "subsection" Section = "section" + @property + def container_model_classes(self) -> tuple[type[Container], type[ContainerVersion]]: + """ + Get the container, containerversion subclasses associated with this type. + @@TODO Is this what we want, a hard mapping between container_types and Container classes? + * If so, then expand on this pattern, so that all ContainerType logic is contained within + this class, and get rid of the match-case statements that are all over the content_libraries + app. + * If not, then figure out what to do instead. + """ + match self: + case self.Unit: + return (Unit, UnitVersion) + case self.Subsection: + return (Subsection, SubsectionVersion) + case self.Section: + return (Section, SectionVersion) + raise TypeError(f"unexpected ContainerType: {self!r}") + @property def olx_tag(self) -> str: """ diff --git a/openedx/core/djangoapps/xblock/rest_api/views.py b/openedx/core/djangoapps/xblock/rest_api/views.py index a71fb6cfd5..4135fcbf6c 100644 --- a/openedx/core/djangoapps/xblock/rest_api/views.py +++ b/openedx/core/djangoapps/xblock/rest_api/views.py @@ -18,6 +18,7 @@ from rest_framework import permissions, serializers from rest_framework.decorators import api_view, permission_classes # lint-amnesty, pylint: disable=unused-import from rest_framework.exceptions import PermissionDenied, AuthenticationFailed, NotFound from rest_framework.response import Response +from rest_framework.fields import BooleanField from rest_framework.views import APIView from xblock.django.request import DjangoWebobRequest, webob_to_django_response from xblock.exceptions import NoSuchUsage @@ -100,6 +101,10 @@ def embed_block_view(request, usage_key: UsageKeyV2, view_name: str): Unstable - may change after Sumac """ # Check if a specific version has been requested. TODO: move this to a URL path param like the other views? + show_title = request.GET.get('show_title', False) + if show_title is not None: + show_title = BooleanField().to_internal_value(show_title) + try: version = VersionConverter().to_python(request.GET.get("version")) except ValueError as exc: @@ -147,6 +152,8 @@ def embed_block_view(request, usage_key: UsageKeyV2, view_name: str): 'view_name': view_name, 'is_development': settings.DEBUG, 'oa_manifest': new_oa_manifest, + 'display_name': block.display_name, + 'show_title': show_title, } response = render(request, 'xblock_v2/xblock_iframe.html', context, content_type='text/html') diff --git a/openedx/core/types/user.py b/openedx/core/types/user.py index 9eb63edba3..e680a33b42 100644 --- a/openedx/core/types/user.py +++ b/openedx/core/types/user.py @@ -7,4 +7,5 @@ import typing as t import django.contrib.auth.models +AuthUser: t.TypeAlias = django.contrib.auth.models.User User: t.TypeAlias = django.contrib.auth.models.User | django.contrib.auth.models.AnonymousUser diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 1c0130ee76..3c36ffafcd 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -42,7 +42,7 @@ django-stubs<6 # The team that owns this package will manually bump this package rather than having it pulled in automatically. # This is to allow them to better control its deployment and to do it in a process that works better # for them. -edx-enterprise==6.4.3 +edx-enterprise==6.4.4 # Date: 2023-07-26 # Our legacy Sass code is incompatible with anything except this ancient libsass version. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 09b92b3d46..a1cf0468f6 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -473,7 +473,7 @@ edx-drf-extensions==10.6.0 # edxval # enterprise-integrated-channels # openedx-learning -edx-enterprise==6.4.3 +edx-enterprise==6.4.4 # via # -c requirements/constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 5d4b6a11de..e430c306ae 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -747,7 +747,7 @@ edx-drf-extensions==10.6.0 # edxval # enterprise-integrated-channels # openedx-learning -edx-enterprise==6.4.3 +edx-enterprise==6.4.4 # via # -c requirements/constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 68e585bdb6..4b1c89dfdb 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -557,7 +557,7 @@ edx-drf-extensions==10.6.0 # edxval # enterprise-integrated-channels # openedx-learning -edx-enterprise==6.4.3 +edx-enterprise==6.4.4 # via # -c requirements/constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 19dd5915c0..cd72761e6f 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -578,7 +578,7 @@ edx-drf-extensions==10.6.0 # edxval # enterprise-integrated-channels # openedx-learning -edx-enterprise==6.4.3 +edx-enterprise==6.4.4 # via # -c requirements/constraints.txt # -r requirements/edx/base.txt