fix: return unit/container information

Author: rpenido

cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py

@@ -63,11 +63,11 @@
 """
 
 import logging
+from itertools import groupby
 
 from attrs import asdict as attrs_asdict
 from django.contrib.auth.models import User  # pylint: disable=imported-auth-user
 from django.utils.translation import gettext_lazy as _
-from itertools import groupby
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey, UsageKey
 from rest_framework.exceptions import NotFound, ValidationError
@@ -78,8 +78,8 @@
 
 from cms.djangoapps.contentstore.helpers import import_static_assets_for_library_sync
 from cms.djangoapps.contentstore.models import PublishableEntityLink
-from cms.djangoapps.contentstore.utils import reverse_course_url, reverse_usage_url
 from cms.djangoapps.contentstore.rest_api.v2.serializers import PublishableEntityLinksSerializer
+from cms.djangoapps.contentstore.utils import reverse_course_url, reverse_usage_url
 from cms.lib.xblock.upstream_sync import (
     BadDownstream,
     BadUpstream,
@@ -92,6 +92,7 @@
     sync_from_upstream,
 )
 from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from openedx.core.lib.api.view_utils import (
     DeveloperErrorViewMixin,
     view_auth_classes,
@@ -148,7 +149,7 @@ def get(self, request: _AuthenticatedRequest, course_key_string: str):
 @view_auth_classes()
 class DownstreamContextListView(DeveloperErrorViewMixin, APIView):
     """
-    Serves library block->course->unit links
+    Serves library block->course->container links
     """
     def get(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response:
         """
@@ -166,30 +167,54 @@ def get(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response
                 .values( "downstream_usage_key", "downstream_context_key", "downstream_parent_usage_key")
         )
 
+        context_key_list = set(link["downstream_context_key"] for link in links)
+
+        courses_display_name = dict(
+            CourseOverview.objects.filter(id__in=context_key_list).values_list('id', 'display_name')
+        )
+
         result = []
         for context_key, links_by_context in groupby(links, lambda link: link["downstream_context_key"]):
-            # Pre-fetch the course with all of its children:
-            course = modulestore().get_course(context_key, depth=None)
-            if course is None:
+            if context_key not in courses_display_name:
                 raise BadDownstream(_("Course {context_key} not found").format(context_key=context_key))
+
             course_link = {
                 "id": str(context_key),
-                "display_name": course.display_name,
+                "display_name": courses_display_name[context_key],
                 "url": reverse_course_url("course_handler", context_key),
-                "units": []
+                "containers": []
             }
-            for unit_key, links_by_unit  in groupby(links_by_context, lambda link: link["downstream_parent_usage_key"]):
-                unit_link = {
-                    "id": str(unit_key),
-                    "url": reverse_usage_url("container_handler", unit_key),
-                    "links": []
-                }
-                for downstream_link in links_by_unit:
-                    unit_link["links"].append({
-                        "id": str(downstream_link["downstream_usage_key"]),
-                    })
-                course_link["units"].append(unit_link)
-            result.append(course_link)
+
+            for parent_key, links_by_unit  in groupby(links_by_context, lambda link: link["downstream_parent_usage_key"]):
+                try:
+                    parent = modulestore().get_item(parent_key)
+
+                    parent_link = {
+                        "id": str(parent_key),
+                        "display_name": parent.display_name,
+                        "url": reverse_usage_url("container_handler", parent_key),
+                        "links": []
+                    }
+
+                    for downstream_link in links_by_unit:
+                        parent_link["links"].append({
+                            "id": str(downstream_link["downstream_usage_key"]),
+                        })
+
+                    # Don't include empty containers
+                    if parent_link["links"]:
+                        course_link["containers"].append(parent_link)
+
+                except ItemNotFoundError:
+                    logger.exception(
+                        "Unit %s not found in course %s",
+                        parent_key,
+                        context_key
+                    )
+
+            # Don't include empty courses
+            if course_link["containers"]:
+                result.append(course_link)
 
         return Response(result)
 

cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py

@@ -60,24 +60,27 @@ def setUp(self):
         CourseOverviewFactory.create(id=self.course.id, display_name=self.course.display_name)
         chapter = BlockFactory.create(category='chapter', parent=self.course)
         sequential = BlockFactory.create(category='sequential', parent=chapter)
-        unit = BlockFactory.create(category='vertical', parent=sequential)
-        self.regular_video_key = BlockFactory.create(category='video', parent=unit).usage_key
+        self.unit = BlockFactory.create(category='vertical', parent=sequential)
+        self.regular_video_key = BlockFactory.create(category='video', parent=self.unit).usage_key
         self.downstream_video_key = BlockFactory.create(
-            category='video', parent=unit, upstream=MOCK_UPSTREAM_REF, upstream_version=123,
+            category='video', parent=self.unit, upstream=MOCK_UPSTREAM_REF, upstream_version=123,
         ).usage_key
         self.downstream_html_key = BlockFactory.create(
-            category='html', parent=unit, upstream=MOCK_HTML_UPSTREAM_REF, upstream_version=1,
+            category='html', parent=self.unit, upstream=MOCK_HTML_UPSTREAM_REF, upstream_version=1,
         ).usage_key
 
         self.another_course = CourseFactory.create(display_name="Another Course")
         CourseOverviewFactory.create(id=self.another_course.id, display_name=self.another_course.display_name)
-        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)
+        another_chapter = BlockFactory.create(category="chapter", parent=self.another_course)
+        another_sequential = BlockFactory.create(category="sequential", parent=another_chapter)
+        self.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
-            BlockFactory.create(
-                category='video', parent=another_unit, upstream=MOCK_UPSTREAM_REF, upstream_version=123,
+            self.another_video_keys.append(
+                BlockFactory.create(
+                    category="video", parent=self.another_unit, upstream=MOCK_UPSTREAM_REF, upstream_version=123,
+                ).usage_key
             )
 
         self.fake_video_key = self.course.id.make_usage_key("video", "NoSuchVideo")
@@ -329,6 +332,7 @@ def test_200_all_upstreams(self):
                 'created': date_format,
                 'downstream_context_key': str(self.course.id),
                 'downstream_usage_key': str(self.downstream_video_key),
+                'downstream_parent_usage_key': str(self.unit.usage_key),
                 'id': 1,
                 'ready_to_sync': False,
                 'updated': date_format,
@@ -342,6 +346,7 @@ def test_200_all_upstreams(self):
                 'created': date_format,
                 'downstream_context_key': str(self.course.id),
                 'downstream_usage_key': str(self.downstream_html_key),
+                'downstream_parent_usage_key': str(self.unit.usage_key),
                 'id': 2,
                 'ready_to_sync': False,
                 'updated': date_format,
@@ -373,14 +378,32 @@ def test_200_downstream_context_list(self):
         data = response.json()
         expected = [
             {
-                'id': str(self.course.id),
-                'display_name': str(self.course.display_name),
-                'url': f'/course/{str(self.course.id)}',
+                "id": str(self.course.id),
+                "display_name": str(self.course.display_name),
+                "url": f"/course/{str(self.course.id)}",
+                "containers": [
+                    {
+                        "id": str(self.unit.usage_key),
+                        "display_name": str(self.unit.display_name),
+                        "url": f"/container/{str(self.unit.usage_key)}",
+                        "links": [
+                            { "id": str(self.downstream_video_key) },
+                        ],
+                    },
+                ],
             },
             {
-                'id': str(self.another_course.id),
-                'display_name': str(self.another_course.display_name),
-                'url': f'/course/{str(self.another_course.id)}',
+                "id": str(self.another_course.id),
+                "display_name": str(self.another_course.display_name),
+                "url": f"/course/{str(self.another_course.id)}",
+                "containers": [
+                    {
+                        "id": str(self.another_unit.usage_key),
+                        "display_name": str(self.another_unit.display_name),
+                        "url": f"/container/{str(self.another_unit.usage_key)}",
+                        "links": [ { "id": str(key) } for key in self.another_video_keys ],
+                    },
+                ],
             },
         ]
         self.assertListEqual(data, expected)

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.
     """
@@ -2383,14 +2383,13 @@ def create_or_update_xblock_upstream_link(xblock, course_key: str | CourseKey, c
     except ObjectDoesNotExist:
         log.error(f"Library component not found for {upstream_usage_key}")
         lib_component = None
-
-    print(f"!!!!!!!!!!!!!!!!! xblock.parent -> {xblock.parent}")
     PublishableEntityLink.update_or_create(
         lib_component,
         upstream_usage_key=xblock.upstream,
         upstream_context_key=str(upstream_usage_key.context_key),
-        downstream_context_key=course_key,
         downstream_usage_key=xblock.usage_key,
+        downstream_parent_usage_key=xblock.parent,
+        downstream_context_key=course_key,
         version_synced=xblock.upstream_version,
         version_declined=xblock.upstream_version_declined,
         created=created,