Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cms/djangoapps/contentstore/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops.. We should probably backport this one-line fix to Teak. Can you split it into a separate commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pomegranited Good idea.

"upstream_container__publishable_entity__published__publish_log_record__publish_log",
).annotate(
ready_to_sync=(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@

from cms.djangoapps.contentstore.rest_api.v2.serializers.downstreams import (
ComponentLinksSerializer,
ContainerLinksSerializer,
PublishableEntityLinksSummarySerializer,
)
from cms.djangoapps.contentstore.rest_api.v2.serializers.home import CourseHomeTabSerializerV2

__all__ = [
'CourseHomeTabSerializerV2',
'ComponentLinksSerializer',
'ContainerLinksSerializer',
'PublishableEntityLinksSummarySerializer',
]
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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']
5 changes: 5 additions & 0 deletions cms/djangoapps/contentstore/rest_api/v2/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
36 changes: 35 additions & 1 deletion cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 | LibraryContainerLocator | 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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, "<html><b>Hello world!</b></html>")
self._publish_library_block(self.html_lib_id)
self._publish_library_block(self.video_lib_id)
Expand All @@ -117,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
Expand Down Expand Up @@ -148,13 +168,29 @@ 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)

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):
"""
Expand Down Expand Up @@ -444,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:
Expand Down Expand Up @@ -562,3 +598,93 @@ 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 = None,
ready_to_sync: bool | None = None,
upstream_container_key: str | None = 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")
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': 2,
'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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Unrelated to this line/file, but part of this issue)

When I delete a Unit from a Subsection (or a Subsection from a Section), the container API correctly returns the updated child list, but the search index still shows the old child Unit/Subsection in the container Card preview.

The content_libraries.api.containers.delete_container and restore_container methods need to send LIBRARY_CONTAINER_UPDATED for each of the affected parent containers, like we do in update_container.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pomegranited Fixed it here: 1d97610

9 changes: 9 additions & 0 deletions openedx/core/djangoapps/content_libraries/api/containers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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:
Expand Down
28 changes: 27 additions & 1 deletion openedx/core/djangoapps/content_libraries/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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,
{
Expand All @@ -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(
Expand Down
Loading