From 2904d8a2fca89aae8ee13ef04dc4f65b929c265c Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Tue, 24 Jun 2025 11:26:26 +0200 Subject: [PATCH 1/6] fix: missing comma in filter links query for container links --- cms/djangoapps/contentstore/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/models.py b/cms/djangoapps/contentstore/models.py index 2c0a5d42cce9..65f83c23b15f 100644 --- a/cms/djangoapps/contentstore/models.py +++ b/cms/djangoapps/contentstore/models.py @@ -312,7 +312,7 @@ def filter_links( ready_to_sync = link_filter.pop('ready_to_sync', None) result = cls.objects.filter(**link_filter).select_related( "upstream_container__publishable_entity__published__version", - "upstream_container__publishable_entity__learning_package" + "upstream_container__publishable_entity__learning_package", "upstream_container__publishable_entity__published__publish_log_record__publish_log", ).annotate( ready_to_sync=( From 7a656b1ed6650b9eb7427851f78f9f1563249e63 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Tue, 24 Jun 2025 11:26:33 +0200 Subject: [PATCH 2/6] feat: container api links --- .../rest_api/v2/serializers/__init__.py | 2 + .../rest_api/v2/serializers/downstreams.py | 15 +- .../contentstore/rest_api/v2/urls.py | 5 + .../rest_api/v2/views/downstreams.py | 36 ++++- .../v2/views/tests/test_downstreams.py | 143 +++++++++++++++++- 5 files changed, 198 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v2/serializers/__init__.py b/cms/djangoapps/contentstore/rest_api/v2/serializers/__init__.py index 4c275a1976b0..98017ea86f2f 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/serializers/__init__.py +++ b/cms/djangoapps/contentstore/rest_api/v2/serializers/__init__.py @@ -2,6 +2,7 @@ from cms.djangoapps.contentstore.rest_api.v2.serializers.downstreams import ( ComponentLinksSerializer, + ContainerLinksSerializer, PublishableEntityLinksSummarySerializer, ) from cms.djangoapps.contentstore.rest_api.v2.serializers.home import CourseHomeTabSerializerV2 @@ -9,5 +10,6 @@ __all__ = [ 'CourseHomeTabSerializerV2', 'ComponentLinksSerializer', + 'ContainerLinksSerializer', 'PublishableEntityLinksSummarySerializer', ] diff --git a/cms/djangoapps/contentstore/rest_api/v2/serializers/downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/serializers/downstreams.py index 848e9e3a5c7f..5ec27cc176a7 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/serializers/downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/serializers/downstreams.py @@ -4,7 +4,7 @@ from rest_framework import serializers -from cms.djangoapps.contentstore.models import ComponentLink +from cms.djangoapps.contentstore.models import ComponentLink, ContainerLink class ComponentLinksSerializer(serializers.ModelSerializer): @@ -29,3 +29,16 @@ class PublishableEntityLinksSummarySerializer(serializers.Serializer): ready_to_sync_count = serializers.IntegerField(read_only=True) total_count = serializers.IntegerField(read_only=True) last_published_at = serializers.DateTimeField(read_only=True) + + +class ContainerLinksSerializer(serializers.ModelSerializer): + """ + Serializer for publishable container entity links. + """ + upstream_context_title = serializers.CharField(read_only=True) + upstream_version = serializers.IntegerField(read_only=True, source="upstream_version_num") + ready_to_sync = serializers.BooleanField() + + class Meta: + model = ContainerLink + exclude = ['upstream_container', 'uuid'] diff --git a/cms/djangoapps/contentstore/rest_api/v2/urls.py b/cms/djangoapps/contentstore/rest_api/v2/urls.py index 5fa954a4ef82..1d22cb021b5e 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/urls.py +++ b/cms/djangoapps/contentstore/rest_api/v2/urls.py @@ -18,6 +18,11 @@ downstreams.DownstreamListView.as_view(), name="downstreams_list", ), + re_path( + r'^downstream-containers/$', + downstreams.DownstreamContainerListView.as_view(), + name="container_downstreams_list", + ), re_path( fr'^downstreams/{settings.USAGE_KEY_PATTERN}$', downstreams.DownstreamView.as_view(), diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py index 8ac82a9452db..968f590f5d25 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py @@ -95,9 +95,10 @@ from rest_framework.views import APIView from xblock.core import XBlock -from cms.djangoapps.contentstore.models import ComponentLink +from cms.djangoapps.contentstore.models import ComponentLink, ContainerLink from cms.djangoapps.contentstore.rest_api.v2.serializers import ( ComponentLinksSerializer, + ContainerLinksSerializer, PublishableEntityLinksSummarySerializer, ) from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import sync_library_content @@ -364,6 +365,39 @@ def delete(self, request: _AuthenticatedRequest, usage_key_string: str) -> Respo return Response(status=204) +@view_auth_classes() +class DownstreamContainerListView(DeveloperErrorViewMixin, APIView): + """ + List all container blocks which are linked to an upstream context, with optional filtering. + """ + + def get(self, request: _AuthenticatedRequest): + """ + Fetches publishable container entity links for given course key + """ + course_key_string = request.GET.get('course_id') + ready_to_sync = request.GET.get('ready_to_sync') + upstream_container_key = request.GET.get('upstream_container_key') + link_filter: dict[str, CourseKey | UsageKey | bool] = {} + paginator = DownstreamListPaginator() + if course_key_string: + try: + link_filter["downstream_context_key"] = CourseKey.from_string(course_key_string) + except InvalidKeyError as exc: + raise ValidationError(detail=f"Malformed course key: {course_key_string}") from exc + if ready_to_sync is not None: + link_filter["ready_to_sync"] = BooleanField().to_internal_value(ready_to_sync) + if upstream_container_key: + try: + link_filter["upstream_container_key"] = LibraryContainerLocator.from_string(upstream_container_key) + except InvalidKeyError as exc: + raise ValidationError(detail=f"Malformed usage key: {upstream_container_key}") from exc + links = ContainerLink.filter_links(**link_filter) + paginated_links = paginator.paginate_queryset(links, self.request, view=self) + serializer = ContainerLinksSerializer(paginated_links, many=True) + return paginator.get_paginated_response(serializer.data, self.request) + + def _load_accessible_block(user: User, usage_key_string: str, *, require_write_access: bool) -> XBlock: """ Given a logged in-user and a serialized usage key of an upstream-linked XBlock, load it from the ModuleStore, 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 950464839c09..955129fe95e3 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 @@ -14,7 +14,7 @@ from cms.lib.xblock.upstream_sync import BadUpstream, UpstreamLink from cms.djangoapps.contentstore.tests.utils import CourseTestCase from cms.djangoapps.contentstore.xblock_storage_handlers import view_handlers as xblock_view_handlers -from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.keys import ContainerKey, UsageKey from common.djangoapps.student.tests.factories import UserFactory from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase @@ -28,6 +28,9 @@ URL_LIB_BLOCKS = URL_PREFIX + '{lib_key}/blocks/' URL_LIB_BLOCK_PUBLISH = URL_PREFIX + 'blocks/{block_key}/publish/' URL_LIB_BLOCK_OLX = URL_PREFIX + 'blocks/{block_key}/olx/' +URL_LIB_CONTAINER = URL_PREFIX + 'containers/{container_key}/' # Get a container in this library +URL_LIB_CONTAINERS = URL_PREFIX + '{lib_key}/containers/' # Create a new container in this library +URL_LIB_CONTAINER_PUBLISH = URL_LIB_CONTAINER + 'publish/' # Publish changes to the specified container + children def _get_upstream_link_good_and_syncable(downstream): @@ -75,8 +78,14 @@ def setUp(self): )["id"] self.html_lib_id = self._add_block_to_library(self.library_id, "html", "html-baz")["id"] self.video_lib_id = self._add_block_to_library(self.library_id, "video", "video-baz")["id"] + self.unit_id = self._create_container(self.library_id, "unit", "unit-1", "Unit 1")["id"] + self.subsection_id = self._create_container(self.library_id, "subsection", "subsection-1", "Subsection 1")["id"] + self.section_id = self._create_container(self.library_id, "section", "section-1", "Section 1")["id"] self._publish_library_block(self.html_lib_id) self._publish_library_block(self.video_lib_id) + self._publish_container(self.unit_id) + self._publish_container(self.subsection_id) + self._publish_container(self.section_id) self.mock_upstream_link = f"{settings.COURSE_AUTHORING_MICROFRONTEND_URL}/library/{self.library_id}/components?usageKey={self.video_lib_id}" # pylint: disable=line-too-long # noqa: E501 self.course = CourseFactory.create() chapter = BlockFactory.create(category='chapter', parent=self.course) @@ -89,6 +98,15 @@ def setUp(self): self.downstream_html_key = BlockFactory.create( category='html', parent=unit, upstream=self.html_lib_id, upstream_version=1, ).usage_key + self.downstream_chapter_key = BlockFactory.create( + category='chapter', parent=self.course, upstream=self.section_id, upstream_version=1, + ).usage_key + self.downstream_sequential_key = BlockFactory.create( + category='sequential', parent=chapter, upstream=self.subsection_id, upstream_version=1, + ).usage_key + self.downstream_unit_key = BlockFactory.create( + category='vertical', parent=sequential, upstream=self.unit_id, upstream_version=1, + ).usage_key self.another_course = CourseFactory.create(display_name="Another Course") another_chapter = BlockFactory.create(category="chapter", parent=self.another_course) @@ -108,6 +126,8 @@ def setUp(self): self.fake_video_key = self.course.id.make_usage_key("video", "NoSuchVideo") self.learner = UserFactory(username="learner", password="password") + self._update_container(self.unit_id, display_name="Unit 2") + self._publish_container(self.unit_id) self._set_library_block_olx(self.html_lib_id, "Hello world!") self._publish_library_block(self.html_lib_id) self._publish_library_block(self.video_lib_id) @@ -148,6 +168,15 @@ def _publish_library_block(self, block_key, expect_response=200): """ Publish changes from a specified XBlock """ return self._api('post', URL_LIB_BLOCK_PUBLISH.format(block_key=block_key), None, expect_response) + def _publish_container(self, container_key: ContainerKey | str, expect_response=200): + """ Publish all changes in the specified container + children """ + return self._api('post', URL_LIB_CONTAINER_PUBLISH.format(container_key=container_key), None, expect_response) + + def _update_container(self, container_key: ContainerKey | str, display_name: str, expect_response=200): + """ Update a container (unit etc.) """ + data = {"display_name": display_name} + return self._api('patch', URL_LIB_CONTAINER.format(container_key=container_key), data, expect_response) + def _set_library_block_olx(self, block_key, new_olx, expect_response=200): """ Overwrite the OLX of a specific block in the library """ return self._api('post', URL_LIB_BLOCK_OLX.format(block_key=block_key), {"olx": new_olx}, expect_response) @@ -155,6 +184,13 @@ def _set_library_block_olx(self, block_key, new_olx, expect_response=200): def call_api(self, usage_key_string): raise NotImplementedError + def _create_container(self, lib_key, container_type, slug: str | None, display_name: str, expect_response=200): + """ Create a container (unit etc.) """ + data = {"container_type": container_type, "display_name": display_name} + if slug: + data["slug"] = slug + return self._api('post', URL_LIB_CONTAINERS.format(lib_key=lib_key), data, expect_response) + class SharedErrorTestCases(_BaseDownstreamViewTestMixin): """ @@ -562,3 +598,108 @@ def test_200_summary(self): 'last_published_at': self.now.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), }] self.assertListEqual(data, expected) + + +class GetContainerUpstreamViewTest( + _BaseDownstreamViewTestMixin, + SharedModuleStoreTestCase, +): + """ + Test that `GET /api/v2/contentstore/downstream-containers?...` returns list of links based on the provided filter. + """ + def call_api( + self, + course_id: str = None, + ready_to_sync: bool = None, + upstream_container_key: str = None, + ): + data = {} + if course_id is not None: + data["course_id"] = str(course_id) + if ready_to_sync is not None: + data["ready_to_sync"] = str(ready_to_sync) + if upstream_container_key is not None: + data["upstream_container_key"] = str(upstream_container_key) + return self.client.get("/api/contentstore/v2/downstream-containers/", data=data) + + def test_200_all_container_downstreams_for_a_course(self): + """ + Returns all container links for given course + """ + self.client.login(username="superuser", password="password") + self.maxDiff = 20000 + response = self.call_api(course_id=self.course.id) + assert response.status_code == 200 + data = response.json() + date_format = self.now.isoformat().split("+")[0] + 'Z' + expected = [ + { + 'created': date_format, + 'downstream_context_key': str(self.course.id), + 'downstream_usage_key': str(self.downstream_chapter_key), + 'id': 1, + 'ready_to_sync': False, + 'updated': date_format, + 'upstream_context_key': self.library_id, + 'upstream_context_title': self.library_title, + 'upstream_container_key': self.section_id, + 'upstream_version': 1, + 'version_declined': None, + 'version_synced': 1, + }, + { + 'created': date_format, + 'downstream_context_key': str(self.course.id), + 'downstream_usage_key': str(self.downstream_sequential_key), + 'id': 2, + 'ready_to_sync': False, + 'updated': date_format, + 'upstream_context_key': self.library_id, + 'upstream_context_title': self.library_title, + 'upstream_container_key': self.subsection_id, + 'upstream_version': 1, + 'version_declined': None, + 'version_synced': 1, + }, + { + 'created': date_format, + 'downstream_context_key': str(self.course.id), + 'downstream_usage_key': str(self.downstream_unit_key), + 'id': 3, + 'ready_to_sync': True, + 'updated': date_format, + 'upstream_context_key': self.library_id, + 'upstream_context_title': self.library_title, + 'upstream_container_key': self.unit_id, + 'upstream_version': 1, + 'version_declined': None, + 'version_synced': 1 + }, + ] + self.assertListEqual(data["results"], expected) + self.assertEqual(data["count"], 3) + + def test_200_all_downstreams_ready_to_sync(self): + """ + Returns all links that are syncable + """ + self.client.login(username="superuser", password="password") + response = self.call_api(ready_to_sync=True) + assert response.status_code == 200 + data = response.json() + self.assertTrue(all(o["ready_to_sync"] for o in data["results"])) + self.assertEqual(data["count"], 1) + + def test_200_downstream_context_list(self): + """ + Returns all downstream courses for given library block + """ + self.client.login(username="superuser", password="password") + response = self.call_api(upstream_usage_key=self.video_lib_id) + assert response.status_code == 200 + data = response.json() + expected = [str(self.downstream_video_key)] + [str(key) for key in self.another_video_keys] + got = [str(o["downstream_usage_key"]) for o in data["results"]] + self.assertListEqual(got, expected) + self.assertEqual(data["count"], 4) + From cb348b115e363c9ef0dd240d63a08a035a935ebe Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Sat, 14 Jun 2025 13:51:42 +0200 Subject: [PATCH 3/6] test: fix downstream tests --- .../v2/views/tests/test_downstreams.py | 31 +++++-------------- 1 file changed, 8 insertions(+), 23 deletions(-) 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 955129fe95e3..9c7de63000c2 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 @@ -137,7 +137,7 @@ def _api(self, method, url, data, expect_response): """ Call a REST API """ - response = getattr(self.client, method)(url, data, format="json") + response = getattr(self.client, method)(url, data, format="json", content_type="application/json") assert response.status_code == expect_response,\ 'Unexpected response code {}:\n{}'.format(response.status_code, getattr(response, 'data', '(no data)')) return response.data @@ -480,9 +480,9 @@ class GetUpstreamViewTest( """ def call_api( self, - course_id: str = None, - ready_to_sync: bool = None, - upstream_usage_key: str = None, + course_id: str | None = None, + ready_to_sync: bool | None = None, + upstream_usage_key: str | None = None, ): data = {} if course_id is not None: @@ -609,9 +609,9 @@ class GetContainerUpstreamViewTest( """ def call_api( self, - course_id: str = None, - ready_to_sync: bool = None, - upstream_container_key: str = None, + course_id: str | None = None, + ready_to_sync: bool | None = None, + upstream_container_key: str | None = None, ): data = {} if course_id is not None: @@ -627,7 +627,6 @@ def test_200_all_container_downstreams_for_a_course(self): Returns all container links for given course """ self.client.login(username="superuser", password="password") - self.maxDiff = 20000 response = self.call_api(course_id=self.course.id) assert response.status_code == 200 data = response.json() @@ -671,7 +670,7 @@ def test_200_all_container_downstreams_for_a_course(self): 'upstream_context_key': self.library_id, 'upstream_context_title': self.library_title, 'upstream_container_key': self.unit_id, - 'upstream_version': 1, + 'upstream_version': 2, 'version_declined': None, 'version_synced': 1 }, @@ -689,17 +688,3 @@ def test_200_all_downstreams_ready_to_sync(self): data = response.json() self.assertTrue(all(o["ready_to_sync"] for o in data["results"])) self.assertEqual(data["count"], 1) - - def test_200_downstream_context_list(self): - """ - Returns all downstream courses for given library block - """ - self.client.login(username="superuser", password="password") - response = self.call_api(upstream_usage_key=self.video_lib_id) - assert response.status_code == 200 - data = response.json() - expected = [str(self.downstream_video_key)] + [str(key) for key in self.another_video_keys] - got = [str(o["downstream_usage_key"]) for o in data["results"]] - self.assertListEqual(got, expected) - self.assertEqual(data["count"], 4) - From a462864090b75f75239e5c93c9b9010a95293845 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Sat, 14 Jun 2025 17:20:25 +0200 Subject: [PATCH 4/6] fix: lint issues --- cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py index 968f590f5d25..53735ec7c435 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py @@ -378,7 +378,7 @@ def get(self, request: _AuthenticatedRequest): course_key_string = request.GET.get('course_id') ready_to_sync = request.GET.get('ready_to_sync') upstream_container_key = request.GET.get('upstream_container_key') - link_filter: dict[str, CourseKey | UsageKey | bool] = {} + link_filter: dict[str, CourseKey | LibraryContainerLocator | bool] = {} paginator = DownstreamListPaginator() if course_key_string: try: From 1d97610fdafd87eb8311ca9bb12f77f53c90292d Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Tue, 24 Jun 2025 12:16:59 +0200 Subject: [PATCH 5/6] fix: update parent containers index on deleting child container --- .../core/djangoapps/content_libraries/api/containers.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index 8730846c615d..1457586301e1 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -369,6 +369,7 @@ def delete_container( library_key = container_key.lib_key container = _get_container_from_key(container_key) + affected_containers = get_containers_contains_item(container_key) affected_collections = authoring_api.get_entity_collections( container.publishable_entity.learning_package_id, container.key, @@ -395,6 +396,14 @@ def delete_container( background=True, ) ) + # Send events related to the containers that contains the updated container. + # This is to update the children display names used in the section/subsection previews. + for affected_container in affected_containers: + LIBRARY_CONTAINER_UPDATED.send_event( + library_container=LibraryContainerData( + container_key=affected_container.container_key, + ) + ) def restore_container(container_key: LibraryContainerLocator) -> None: From 6e79cd46b131e65a6ae161d6e4b5633aa776c018 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Tue, 24 Jun 2025 12:32:59 +0200 Subject: [PATCH 6/6] test: update parent containers index on delete --- .../content_libraries/tests/test_api.py | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index ee2b90410fbd..6756e4373a88 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -313,6 +313,14 @@ def setUp(self) -> None: "unit", 'unit-1', 'Unit 1' ) + # Create a subsection container + self.subsection1 = api.create_container( + self.lib1.library_key, + api.ContainerType.Subsection, + 'subsection-1', + 'Subsection 1', + None, + ) # Create some library blocks in lib2 self.lib2_problem_block = self._add_block_to_library( self.lib2.library_key, "problem", "problem2", @@ -608,12 +616,19 @@ def test_delete_library_container(self) -> None: ], ) + # Add container under another container + api.update_container_children( + self.subsection1.container_key, + [LibraryContainerLocator.from_string(self.unit1["id"])], + None, + ) event_receiver = mock.Mock() LIBRARY_COLLECTION_UPDATED.connect(event_receiver) + LIBRARY_CONTAINER_UPDATED.connect(event_receiver) api.delete_container(LibraryContainerLocator.from_string(self.unit1["id"])) - assert event_receiver.call_count == 1 + assert event_receiver.call_count == 2 self.assertDictContainsEntries( event_receiver.call_args_list[0].kwargs, { @@ -628,6 +643,17 @@ def test_delete_library_container(self) -> None: ), }, ) + self.assertDictContainsEntries( + event_receiver.call_args_list[1].kwargs, + { + "signal": LIBRARY_CONTAINER_UPDATED, + "sender": None, + "library_container": LibraryContainerData( + container_key=self.subsection1.container_key, + background=False, + ) + }, + ) def test_restore_library_block(self) -> None: api.update_library_collection_items(