From 9bf84b2c459aac34bb98872e296c66ccb6ae4461 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Thu, 20 Feb 2025 13:10:36 -0300 Subject: [PATCH 1/3] feat: add API to return list of downstream blocks for an upstream --- cms/djangoapps/contentstore/models.py | 9 +++++ .../contentstore/rest_api/v2/urls.py | 5 +++ .../rest_api/v2/views/downstreams.py | 30 ++++++++++++++++ .../v2/views/tests/test_downstreams.py | 34 +++++++++++++++++++ cms/djangoapps/contentstore/utils.py | 2 +- .../djangoapps/content/search/documents.py | 17 ++++++++-- .../content/search/tests/test_api.py | 2 ++ .../content/search/tests/test_documents.py | 7 ++++ .../content/search/tests/test_handlers.py | 2 ++ 9 files changed, 105 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/models.py b/cms/djangoapps/contentstore/models.py index 99ff47f9763..a884d0702d8 100644 --- a/cms/djangoapps/contentstore/models.py +++ b/cms/djangoapps/contentstore/models.py @@ -202,6 +202,15 @@ def get_by_downstream_context(cls, downstream_context_key: CourseKey) -> QuerySe "upstream_block__learning_package" ) + @classmethod + def get_by_upstream_usage_key(cls, upstream_usage_key: UsageKey) -> QuerySet["PublishableEntityLink"]: + """ + Get all downstream context keys for given upstream usage key + """ + return cls.objects.filter( + upstream_usage_key=upstream_usage_key, + ) + class LearningContextLinksStatusChoices(models.TextChoices): """ diff --git a/cms/djangoapps/contentstore/rest_api/v2/urls.py b/cms/djangoapps/contentstore/rest_api/v2/urls.py index 690e7879933..95849bc33ec 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/urls.py +++ b/cms/djangoapps/contentstore/rest_api/v2/urls.py @@ -29,6 +29,11 @@ downstreams.UpstreamListView.as_view(), name='upstream-list' ), + re_path( + f'^upstream/{settings.USAGE_KEY_PATTERN}/downstream-links$', + downstreams.DownstreamContextListView.as_view(), + name='downstream-link-list' + ), re_path( fr'^downstreams/{settings.USAGE_KEY_PATTERN}/sync$', downstreams.SyncFromUpstreamView.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 94931acc8ff..76028e44944 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py @@ -40,6 +40,11 @@ 400: Downstream block is not linked to upstream content. 404: Downstream block not found or user lacks permission to edit it. + /api/contentstore/v2/upstream/{usage_key_string}/downstream-links + + GET: List all downstream blocks linked to a library block. + 200: A list of downstream usage_keys linked to the library block. + # NOT YET IMPLEMENTED -- Will be needed for full Libraries Relaunch in ~Teak. /api/contentstore/v2/downstreams /api/contentstore/v2/downstreams?course_id=course-v1:A+B+C&ready_to_sync=true @@ -137,6 +142,31 @@ def get(self, request: _AuthenticatedRequest, course_key_string: str): return Response(serializer.data) +@view_auth_classes() +class DownstreamContextListView(DeveloperErrorViewMixin, APIView): + """ + Serves library block->downstream usage keys + """ + def get(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response: + """ + Fetches downstream links for given publishable entity + """ + try: + usage_key = UsageKey.from_string(usage_key_string) + except InvalidKeyError as exc: + raise ValidationError(detail=f"Malformed usage key: {usage_key_string}") from exc + + downstream_usage_key_list = ( + PublishableEntityLink + .get_by_upstream_usage_key(upstream_usage_key=usage_key) + .values_list("downstream_usage_key", flat=True) + ) + + downstream_usage_key_str_list = [str(usage_key) for usage_key in downstream_usage_key_list] + + return Response(downstream_usage_key_str_list) + + @view_auth_classes(is_authenticated=True) class DownstreamView(DeveloperErrorViewMixin, APIView): """ 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 ec7c842d163..3c7d19753f5 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 @@ -66,6 +66,20 @@ def setUp(self): self.downstream_html_key = BlockFactory.create( category='html', parent=unit, upstream=MOCK_HTML_UPSTREAM_REF, upstream_version=1, ).usage_key + + self.another_course = CourseFactory.create(display_name="Another Course") + another_chapter = BlockFactory.create(category="chapter", parent=self.another_course) + another_sequential = BlockFactory.create(category="sequential", parent=another_chapter) + another_unit = BlockFactory.create(category="vertical", parent=another_sequential) + self.another_video_keys = [] + for _ in range(3): + # Adds 3 videos linked to the same upstream + self.another_video_keys.append( + BlockFactory.create( + category="video", parent=another_unit, upstream=MOCK_UPSTREAM_REF, upstream_version=123, + ).usage_key + ) + self.fake_video_key = self.course.id.make_usage_key("video", "NoSuchVideo") self.superuser = UserFactory(username="superuser", password="password", is_staff=True, is_superuser=True) self.learner = UserFactory(username="learner", password="password") @@ -339,3 +353,23 @@ def test_200_all_upstreams(self): }, ] self.assertListEqual(data, expected) + + +class GetDownstreamContextsTest(_BaseDownstreamViewTestMixin, SharedModuleStoreTestCase): + """ + Test that `GET /api/v2/contentstore/upstream/:usage_key/downstream-links returns list of + linked blocks usage_keys in given upstream entity (i.e. library block). + """ + def call_api(self, usage_key_string): + return self.client.get(f"/api/contentstore/v2/upstream/{usage_key_string}/downstream-links") + + 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(MOCK_UPSTREAM_REF) + assert response.status_code == 200 + data = response.json() + expected = [str(self.downstream_video_key)] + [str(key) for key in self.another_video_keys] + self.assertListEqual(data, expected) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index b7ec8c9d107..42987544f9d 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -2371,7 +2371,7 @@ def get_xblock_render_context(request, block): return "" -def create_or_update_xblock_upstream_link(xblock, course_key: str | CourseKey, created: datetime | None = None): +def create_or_update_xblock_upstream_link(xblock, course_key: str | CourseKey, created: datetime | None = None) -> None: """ Create or update upstream->downstream link in database for given xblock. """ diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 3d739781fe7..da79da6edf3 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -47,9 +47,9 @@ class Fields: context_key = "context_key" org = "org" access_id = "access_id" # .models.SearchAccess.id - # breadcrumbs: an array of {"display_name": "..."} entries. First one is the name of the course/library itself. + # breadcrumbs: an array of {"display_name": "...", "usage_key": "...", "url": "..."} entries. + # First one is the name of the course/library itself. # After that is the name of any parent Section/Subsection/Unit/etc. - # It's a list of dictionaries because for now we just include the name of each but in future we may add their IDs. breadcrumbs = "breadcrumbs" # tags (dictionary) # See https://blog.meilisearch.com/nested-hierarchical-facets-guide/ @@ -199,6 +199,7 @@ def _fields_from_block(block) -> dict: class implementation returns only: {"content": {"display_name": "..."}, "content_type": "..."} """ + block_type = block.scope_ids.block_type block_data = { Fields.usage_key: str(block.usage_key), @@ -213,6 +214,9 @@ class implementation returns only: } # Get the breadcrumbs (course, section, subsection, etc.): if block.usage_key.context_key.is_course: # Getting parent is not yet implemented in Learning Core (for libraries). + # Import here to avoid circular imports + from cms.djangoapps.contentstore.utils import reverse_course_url, reverse_usage_url + cur_block = block while cur_block.parent: if not cur_block.has_cached_parent: @@ -225,6 +229,15 @@ class implementation returns only: } if cur_block.scope_ids.block_type != "course": parent_data["usage_key"] = str(cur_block.usage_key) + # Section and subsections don't have URLs in the CMS + if cur_block.scope_ids.block_type not in ["chapter", "sequential"]: + parent_data["url"] = ( + reverse_course_url("course_handler", cur_block.context_key) + + reverse_usage_url("container_handler", cur_block.usage_key) + ) + else: + parent_data["url"] = reverse_course_url("course_handler", cur_block.context_key) + block_data[Fields.breadcrumbs].insert( 0, parent_data, diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 8063307d61e..2255aa60462 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -85,6 +85,7 @@ def setUp(self): "breadcrumbs": [ { "display_name": "Test Course", + "url": "/course/course-v1:org1+test_course+test_run" }, ], "content": {}, @@ -103,6 +104,7 @@ def setUp(self): "breadcrumbs": [ { "display_name": "Test Course", + "url": "/course/course-v1:org1+test_course+test_run" }, { "display_name": "sequential", diff --git a/openedx/core/djangoapps/content/search/tests/test_documents.py b/openedx/core/djangoapps/content/search/tests/test_documents.py index b4f7eb2b62c..355ec43f26d 100644 --- a/openedx/core/djangoapps/content/search/tests/test_documents.py +++ b/openedx/core/djangoapps/content/search/tests/test_documents.py @@ -158,6 +158,7 @@ def test_problem_block(self): "breadcrumbs": [ { 'display_name': 'Toy Course', + 'url': '/course/course-v1:edX+toy+2012_Fall', }, { 'display_name': 'chapter', @@ -170,6 +171,10 @@ def test_problem_block(self): { 'display_name': 'vertical', 'usage_key': 'block-v1:edX+toy+2012_Fall+type@vertical+block@vertical_test', + 'url': ( + '/course/course-v1:edX+toy+2012_Fall' + '/container/block-v1:edX+toy+2012_Fall+type@vertical+block@vertical_test' + ), }, ], "content": { @@ -208,6 +213,7 @@ def test_html_block(self): "breadcrumbs": [ { 'display_name': 'Toy Course', + 'url': '/course/course-v1:edX+toy+2012_Fall', }, { 'display_name': 'Overview', @@ -251,6 +257,7 @@ def test_video_block_untagged(self): "breadcrumbs": [ { 'display_name': 'Toy Course', + 'url': '/course/course-v1:edX+toy+2012_Fall', }, { 'display_name': 'Overview', diff --git a/openedx/core/djangoapps/content/search/tests/test_handlers.py b/openedx/core/djangoapps/content/search/tests/test_handlers.py index 95b2ecb6f52..cdf526a6bfc 100644 --- a/openedx/core/djangoapps/content/search/tests/test_handlers.py +++ b/openedx/core/djangoapps/content/search/tests/test_handlers.py @@ -72,6 +72,7 @@ def test_create_delete_xblock(self, meilisearch_client): "breadcrumbs": [ { "display_name": "Test Course", + "url": "/course/course-v1:orgA+test_course+test_run", }, ], "content": {}, @@ -92,6 +93,7 @@ def test_create_delete_xblock(self, meilisearch_client): "breadcrumbs": [ { "display_name": "Test Course", + "url": "/course/course-v1:orgA+test_course+test_run", }, { "display_name": "sequential", From 6b27c090346a28ad6d427e172f703f10616b7fb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Fri, 21 Feb 2025 16:34:02 -0300 Subject: [PATCH 2/3] fix: remove url from index --- .../core/djangoapps/content/search/documents.py | 17 ++--------------- .../djangoapps/content/search/tests/test_api.py | 2 -- .../content/search/tests/test_documents.py | 7 ------- .../content/search/tests/test_handlers.py | 2 -- 4 files changed, 2 insertions(+), 26 deletions(-) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index da79da6edf3..3d739781fe7 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -47,9 +47,9 @@ class Fields: context_key = "context_key" org = "org" access_id = "access_id" # .models.SearchAccess.id - # breadcrumbs: an array of {"display_name": "...", "usage_key": "...", "url": "..."} entries. - # First one is the name of the course/library itself. + # breadcrumbs: an array of {"display_name": "..."} entries. First one is the name of the course/library itself. # After that is the name of any parent Section/Subsection/Unit/etc. + # It's a list of dictionaries because for now we just include the name of each but in future we may add their IDs. breadcrumbs = "breadcrumbs" # tags (dictionary) # See https://blog.meilisearch.com/nested-hierarchical-facets-guide/ @@ -199,7 +199,6 @@ def _fields_from_block(block) -> dict: class implementation returns only: {"content": {"display_name": "..."}, "content_type": "..."} """ - block_type = block.scope_ids.block_type block_data = { Fields.usage_key: str(block.usage_key), @@ -214,9 +213,6 @@ class implementation returns only: } # Get the breadcrumbs (course, section, subsection, etc.): if block.usage_key.context_key.is_course: # Getting parent is not yet implemented in Learning Core (for libraries). - # Import here to avoid circular imports - from cms.djangoapps.contentstore.utils import reverse_course_url, reverse_usage_url - cur_block = block while cur_block.parent: if not cur_block.has_cached_parent: @@ -229,15 +225,6 @@ class implementation returns only: } if cur_block.scope_ids.block_type != "course": parent_data["usage_key"] = str(cur_block.usage_key) - # Section and subsections don't have URLs in the CMS - if cur_block.scope_ids.block_type not in ["chapter", "sequential"]: - parent_data["url"] = ( - reverse_course_url("course_handler", cur_block.context_key) + - reverse_usage_url("container_handler", cur_block.usage_key) - ) - else: - parent_data["url"] = reverse_course_url("course_handler", cur_block.context_key) - block_data[Fields.breadcrumbs].insert( 0, parent_data, diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 2255aa60462..8063307d61e 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -85,7 +85,6 @@ def setUp(self): "breadcrumbs": [ { "display_name": "Test Course", - "url": "/course/course-v1:org1+test_course+test_run" }, ], "content": {}, @@ -104,7 +103,6 @@ def setUp(self): "breadcrumbs": [ { "display_name": "Test Course", - "url": "/course/course-v1:org1+test_course+test_run" }, { "display_name": "sequential", diff --git a/openedx/core/djangoapps/content/search/tests/test_documents.py b/openedx/core/djangoapps/content/search/tests/test_documents.py index 355ec43f26d..b4f7eb2b62c 100644 --- a/openedx/core/djangoapps/content/search/tests/test_documents.py +++ b/openedx/core/djangoapps/content/search/tests/test_documents.py @@ -158,7 +158,6 @@ def test_problem_block(self): "breadcrumbs": [ { 'display_name': 'Toy Course', - 'url': '/course/course-v1:edX+toy+2012_Fall', }, { 'display_name': 'chapter', @@ -171,10 +170,6 @@ def test_problem_block(self): { 'display_name': 'vertical', 'usage_key': 'block-v1:edX+toy+2012_Fall+type@vertical+block@vertical_test', - 'url': ( - '/course/course-v1:edX+toy+2012_Fall' - '/container/block-v1:edX+toy+2012_Fall+type@vertical+block@vertical_test' - ), }, ], "content": { @@ -213,7 +208,6 @@ def test_html_block(self): "breadcrumbs": [ { 'display_name': 'Toy Course', - 'url': '/course/course-v1:edX+toy+2012_Fall', }, { 'display_name': 'Overview', @@ -257,7 +251,6 @@ def test_video_block_untagged(self): "breadcrumbs": [ { 'display_name': 'Toy Course', - 'url': '/course/course-v1:edX+toy+2012_Fall', }, { 'display_name': 'Overview', diff --git a/openedx/core/djangoapps/content/search/tests/test_handlers.py b/openedx/core/djangoapps/content/search/tests/test_handlers.py index cdf526a6bfc..95b2ecb6f52 100644 --- a/openedx/core/djangoapps/content/search/tests/test_handlers.py +++ b/openedx/core/djangoapps/content/search/tests/test_handlers.py @@ -72,7 +72,6 @@ def test_create_delete_xblock(self, meilisearch_client): "breadcrumbs": [ { "display_name": "Test Course", - "url": "/course/course-v1:orgA+test_course+test_run", }, ], "content": {}, @@ -93,7 +92,6 @@ def test_create_delete_xblock(self, meilisearch_client): "breadcrumbs": [ { "display_name": "Test Course", - "url": "/course/course-v1:orgA+test_course+test_run", }, { "display_name": "sequential", From 439bc0fe644054642f17a31e1b46870e2152375c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Fri, 21 Feb 2025 16:34:37 -0300 Subject: [PATCH 3/3] fix: return the downstream_usage_key list using a serializer --- .../contentstore/rest_api/v2/serializers/__init__.py | 5 ++++- .../rest_api/v2/serializers/downstreams.py | 12 ++++++++++++ .../contentstore/rest_api/v2/views/downstreams.py | 12 +++++++----- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v2/serializers/__init__.py b/cms/djangoapps/contentstore/rest_api/v2/serializers/__init__.py index 83bc02b685e..98c36357810 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/serializers/__init__.py +++ b/cms/djangoapps/contentstore/rest_api/v2/serializers/__init__.py @@ -1,4 +1,7 @@ """Module for v2 serializers.""" -from cms.djangoapps.contentstore.rest_api.v2.serializers.downstreams import PublishableEntityLinksSerializer +from cms.djangoapps.contentstore.rest_api.v2.serializers.downstreams import ( + PublishableEntityLinksSerializer, + PublishableEntityLinksUsageKeySerializer, +) from cms.djangoapps.contentstore.rest_api.v2.serializers.home import CourseHomeTabSerializerV2 diff --git a/cms/djangoapps/contentstore/rest_api/v2/serializers/downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/serializers/downstreams.py index 930408040e6..03447d567fd 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/serializers/downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/serializers/downstreams.py @@ -26,3 +26,15 @@ def get_ready_to_sync(self, obj): class Meta: model = PublishableEntityLink exclude = ['upstream_block', 'uuid'] + + +class PublishableEntityLinksUsageKeySerializer(serializers.ModelSerializer): + """ + Serializer for returning a string list of the usage keys. + """ + def to_representation(self, instance: PublishableEntityLink) -> str: + return str(instance.downstream_usage_key) + + class Meta: + model = PublishableEntityLink + fields = ('downstream_usage_key') diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py index 76028e44944..a04f04c5b64 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py @@ -76,7 +76,10 @@ from cms.djangoapps.contentstore.helpers import import_static_assets_for_library_sync from cms.djangoapps.contentstore.models import PublishableEntityLink -from cms.djangoapps.contentstore.rest_api.v2.serializers import PublishableEntityLinksSerializer +from cms.djangoapps.contentstore.rest_api.v2.serializers import ( + PublishableEntityLinksSerializer, + PublishableEntityLinksUsageKeySerializer, +) from cms.lib.xblock.upstream_sync import ( BadDownstream, BadUpstream, @@ -156,15 +159,14 @@ def get(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response except InvalidKeyError as exc: raise ValidationError(detail=f"Malformed usage key: {usage_key_string}") from exc - downstream_usage_key_list = ( + links = ( PublishableEntityLink .get_by_upstream_usage_key(upstream_usage_key=usage_key) - .values_list("downstream_usage_key", flat=True) ) - downstream_usage_key_str_list = [str(usage_key) for usage_key in downstream_usage_key_list] + serializer = PublishableEntityLinksUsageKeySerializer(links, many=True) - return Response(downstream_usage_key_str_list) + return Response(serializer.data) @view_auth_classes(is_authenticated=True)