feat: Add the top-level parent logic to the ready_to_sync value in the UpstreamLink (#37217)
- Updates `UpstreamLink.ready_to_sync` with the top-level parent logic: In a container, `ready_to_sync` is `True` if the container or any children have changes. - Updates `decline_sync` to decline children recursively.
This commit is contained in:
@@ -13,24 +13,11 @@ urlpatterns = [
|
||||
home.HomePageCoursesViewV2.as_view(),
|
||||
name="courses",
|
||||
),
|
||||
# TODO: Rename this to `downstreams/` after full deprecate `DownstreamComponentsListView`
|
||||
re_path(
|
||||
r'^downstreams-all/$',
|
||||
downstreams.DownstreamListView.as_view(),
|
||||
name="downstreams_list_all",
|
||||
),
|
||||
# [DEPRECATED], use `downstreams-all/` instead.
|
||||
re_path(
|
||||
r'^downstreams/$',
|
||||
downstreams.DownstreamComponentsListView.as_view(),
|
||||
downstreams.DownstreamListView.as_view(),
|
||||
name="downstreams_list",
|
||||
),
|
||||
# [DEPRECATED], use `downstreams-all/` instead.
|
||||
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(),
|
||||
|
||||
@@ -81,7 +81,6 @@ UpstreamLink response schema:
|
||||
"""
|
||||
|
||||
import logging
|
||||
import warnings
|
||||
|
||||
from attrs import asdict as attrs_asdict
|
||||
from django.db.models import QuerySet
|
||||
@@ -101,8 +100,6 @@ from xblock.core import XBlock
|
||||
from cms.djangoapps.contentstore.models import ComponentLink, ContainerLink, EntityLinkBase
|
||||
from cms.djangoapps.contentstore.rest_api.v2.serializers import (
|
||||
PublishableEntityLinkSerializer,
|
||||
ComponentLinksSerializer,
|
||||
ContainerLinksSerializer,
|
||||
PublishableEntityLinksSummarySerializer,
|
||||
)
|
||||
from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import sync_library_content
|
||||
@@ -276,47 +273,6 @@ class DownstreamListView(DeveloperErrorViewMixin, APIView):
|
||||
return unique_links
|
||||
|
||||
|
||||
@view_auth_classes()
|
||||
class DownstreamComponentsListView(DeveloperErrorViewMixin, APIView):
|
||||
"""
|
||||
[DEPRECATED], use DownstreamListView instead.
|
||||
|
||||
List all components which are linked to an upstream context, with optional filtering.
|
||||
"""
|
||||
|
||||
def get(self, request: _AuthenticatedRequest):
|
||||
"""
|
||||
[DEPRECATED], use DownstreamListView.get instead, with `item_type='components'`
|
||||
|
||||
Fetches publishable entity links for given course key
|
||||
"""
|
||||
warnings.warn(
|
||||
'`downstreams/` API is deprecated. Please use `downstreams-all/?item_type=components` instead.',
|
||||
DeprecationWarning, stacklevel=3,
|
||||
)
|
||||
course_key_string = request.GET.get('course_id')
|
||||
ready_to_sync = request.GET.get('ready_to_sync')
|
||||
upstream_usage_key = request.GET.get('upstream_usage_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_usage_key:
|
||||
try:
|
||||
link_filter["upstream_usage_key"] = UsageKey.from_string(upstream_usage_key)
|
||||
except InvalidKeyError as exc:
|
||||
raise ValidationError(detail=f"Malformed usage key: {upstream_usage_key}") from exc
|
||||
links = ComponentLink.filter_links(**link_filter)
|
||||
paginated_links = paginator.paginate_queryset(links, self.request, view=self)
|
||||
serializer = ComponentLinksSerializer(paginated_links, many=True)
|
||||
return paginator.get_paginated_response(serializer.data, self.request)
|
||||
|
||||
|
||||
@view_auth_classes()
|
||||
class DownstreamSummaryView(DeveloperErrorViewMixin, APIView):
|
||||
"""
|
||||
@@ -551,8 +507,10 @@ class SyncFromUpstreamView(DeveloperErrorViewMixin, APIView):
|
||||
Decline the latest updates to the block at {usage_key_string}.
|
||||
"""
|
||||
downstream = _load_accessible_block(request.user, usage_key_string, require_write_access=True)
|
||||
if downstream.upstream is None:
|
||||
raise ValidationError(str(NoUpstream()))
|
||||
try:
|
||||
decline_sync(downstream)
|
||||
decline_sync(downstream, request.user.id)
|
||||
except (NoUpstream, BadUpstream, BadDownstream) as exc:
|
||||
# This is somewhat unexpected. If the upstream link is missing or invalid, then the downstream author
|
||||
# shouldn't have been prompted to accept/decline a sync in the first place. Of course, they could have just
|
||||
@@ -564,51 +522,9 @@ class SyncFromUpstreamView(DeveloperErrorViewMixin, APIView):
|
||||
downstream.upstream,
|
||||
)
|
||||
raise ValidationError(str(exc)) from exc
|
||||
modulestore().update_item(downstream, request.user.id)
|
||||
return Response(status=204)
|
||||
|
||||
|
||||
@view_auth_classes()
|
||||
class DownstreamContainerListView(DeveloperErrorViewMixin, APIView):
|
||||
"""
|
||||
[DEPRECATED], use DownstreamListView instead.
|
||||
|
||||
List all container blocks which are linked to an upstream context, with optional filtering.
|
||||
"""
|
||||
|
||||
def get(self, request: _AuthenticatedRequest):
|
||||
"""
|
||||
[DEPRECATED], use DownstreamListView.get instead, with `item_type='containers'`
|
||||
|
||||
Fetches publishable container entity links for given course key
|
||||
"""
|
||||
warnings.warn(
|
||||
'`downstreams/` API is deprecated. Please use `downstreams-all/?item_type=components` instead.',
|
||||
DeprecationWarning, stacklevel=3,
|
||||
)
|
||||
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,
|
||||
|
||||
@@ -71,6 +71,9 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
|
||||
def _sync_downstream(self, usage_key: str):
|
||||
return self._api('post', f"/api/contentstore/v2/downstreams/{usage_key}/sync", {}, expect_response=200)
|
||||
|
||||
def _decline_sync_downstream(self, usage_key: str):
|
||||
return self._api('delete', f"/api/contentstore/v2/downstreams/{usage_key}/sync", {}, expect_response=204)
|
||||
|
||||
def _get_course_block_olx(self, usage_key: str):
|
||||
data = self._api('get', f'/api/olx-export/v1/xblock/{usage_key}/', {}, expect_response=200)
|
||||
return data["blocks"][data["root_block_id"]]["olx"]
|
||||
@@ -130,7 +133,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
|
||||
data["item_type"] = str(item_type)
|
||||
if use_top_level_parents is not None:
|
||||
data["use_top_level_parents"] = str(use_top_level_parents)
|
||||
return self.client.get("/api/contentstore/v2/downstreams-all/", data=data)
|
||||
return self.client.get("/api/contentstore/v2/downstreams/", data=data)
|
||||
|
||||
def assertXmlEqual(self, xml_str_a: str, xml_str_b: str) -> bool:
|
||||
""" Assert that the given XML strings are equal, ignoring attribute order and some whitespace variations. """
|
||||
@@ -421,9 +424,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
|
||||
'version_available': 2, # <--- not updated since we didn't directly modify the unit
|
||||
'version_synced': 2,
|
||||
'version_declined': None,
|
||||
# FIXME: ready_to_sync should be true, since a child block needs syncing.
|
||||
# This may need to be fixed post-Teak, as syncing the children directly is still possible.
|
||||
'ready_to_sync': False,
|
||||
'ready_to_sync': True, # <--- It's the top-level parent of the block
|
||||
'error_message': None,
|
||||
})
|
||||
|
||||
@@ -434,7 +435,7 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
|
||||
'version_available': 3, # <--- updated since we modified the problem
|
||||
'version_synced': 2,
|
||||
'version_declined': None,
|
||||
'ready_to_sync': True, # <--- updated
|
||||
'ready_to_sync': False, # <-- It has top-level parent, the parent is the one who must synchronize
|
||||
'error_message': None,
|
||||
})
|
||||
|
||||
@@ -829,3 +830,167 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase):
|
||||
data = downstreams.json()
|
||||
self.assertEqual(data["count"], 4)
|
||||
self.assertListEqual(data["results"], expected_downstreams)
|
||||
|
||||
def test_unit_decline_sync(self):
|
||||
"""
|
||||
Test that we can decline sync a unit from the library into the course
|
||||
"""
|
||||
# 1️⃣ Create a "vertical" block in the course based on a "unit" container:
|
||||
downstream_unit = self._create_block_from_upstream(
|
||||
# The API consumer needs to specify "vertical" here, even though upstream is "unit".
|
||||
# In the future we could create a nicer REST API endpoint for this that's not part of
|
||||
# the messy '/xblock/' API and which auto-detects the types based on the upstream_key.
|
||||
block_category="vertical",
|
||||
parent_usage_key=str(self.course_subsection.usage_key),
|
||||
upstream_key=self.upstream_unit["id"],
|
||||
)
|
||||
downstream_unit_block_key = get_block_key_dict(
|
||||
UsageKey.from_string(downstream_unit["locator"]),
|
||||
)
|
||||
children_downstream_keys = self._get_course_block_children(downstream_unit["locator"])
|
||||
downstream_problem1 = children_downstream_keys[1]
|
||||
assert "type@problem" in downstream_problem1
|
||||
status = self._get_sync_status(downstream_unit["locator"])
|
||||
self.assertDictContainsEntries(status, {
|
||||
'upstream_ref': self.upstream_unit["id"], # e.g. 'lct:CL-TEST:testlib:unit:u1'
|
||||
'version_available': 2,
|
||||
'version_synced': 2,
|
||||
'version_declined': None,
|
||||
'ready_to_sync': False,
|
||||
'error_message': None,
|
||||
# 'upstream_link': 'http://course-authoring-mfe/library/lib:CL-TEST:testlib/units/...'
|
||||
})
|
||||
assert status["upstream_link"].startswith("http://course-authoring-mfe/library/")
|
||||
assert status["upstream_link"].endswith(f"/units/{self.upstream_unit['id']}")
|
||||
|
||||
# Check that the downstream container matches our expectations.
|
||||
# Note that:
|
||||
# (1) Every XBlock has an "upstream" field
|
||||
# (2) some "downstream only" fields like weight and max_attempts are omitted.
|
||||
# (3) The "top_level_downstream_parent" is the container created
|
||||
self.assertXmlEqual(self._get_course_block_olx(downstream_unit["locator"]), f"""
|
||||
<vertical
|
||||
display_name="Unit 1 Title"
|
||||
upstream_display_name="Unit 1 Title"
|
||||
upstream="{self.upstream_unit['id']}"
|
||||
upstream_version="2"
|
||||
>
|
||||
<html
|
||||
display_name="Text Content"
|
||||
upstream_display_name="Text Content"
|
||||
editor="visual"
|
||||
upstream="{self.upstream_html1['id']}"
|
||||
upstream_version="2"
|
||||
top_level_downstream_parent_key="{downstream_unit_block_key}"
|
||||
>This is the HTML.</html>
|
||||
<problem
|
||||
display_name="Problem 1 Display Name"
|
||||
upstream_display_name="Problem 1 Display Name"
|
||||
markdown="MD 1"
|
||||
{self.standard_capa_attributes}
|
||||
upstream="{self.upstream_problem1['id']}"
|
||||
upstream_version="2"
|
||||
top_level_downstream_parent_key="{downstream_unit_block_key}"
|
||||
>multiple choice...</problem>
|
||||
<problem
|
||||
display_name="Problem 2 Display Name"
|
||||
upstream_display_name="Problem 2 Display Name"
|
||||
markdown="null"
|
||||
{self.standard_capa_attributes}
|
||||
upstream="{self.upstream_problem2['id']}"
|
||||
upstream_version="2"
|
||||
top_level_downstream_parent_key="{downstream_unit_block_key}"
|
||||
>multi select...</problem>
|
||||
</vertical>
|
||||
""")
|
||||
|
||||
# 2️⃣ Now, lets modify the upstream problem 1:
|
||||
self._set_library_block_olx(
|
||||
self.upstream_problem1["id"],
|
||||
'<problem display_name="Problem 1 NEW name" markdown="updated">multiple choice v2...</problem>'
|
||||
)
|
||||
self._publish_container(self.upstream_unit["id"])
|
||||
|
||||
status = self._get_sync_status(downstream_unit["locator"])
|
||||
self.assertDictContainsEntries(status, {
|
||||
'upstream_ref': self.upstream_unit["id"], # e.g. 'lct:CL-TEST:testlib:unit:u1'
|
||||
'version_available': 2, # <--- not updated since we didn't directly modify the unit
|
||||
'version_synced': 2,
|
||||
'version_declined': None,
|
||||
'ready_to_sync': True, # <--- It's the top-level parent of the block
|
||||
'error_message': None,
|
||||
})
|
||||
|
||||
# Check the upstream/downstream status of [one of] the children
|
||||
self.assertDictContainsEntries(self._get_sync_status(downstream_problem1), {
|
||||
'upstream_ref': self.upstream_problem1["id"],
|
||||
'version_available': 3, # <--- updated since we modified the problem
|
||||
'version_synced': 2,
|
||||
'version_declined': None,
|
||||
'ready_to_sync': False, # <-- It has top-level parent, the parent is the one who must synchronize
|
||||
'error_message': None,
|
||||
})
|
||||
|
||||
# Now, decline the sync
|
||||
self._decline_sync_downstream(downstream_unit["locator"])
|
||||
|
||||
status = self._get_sync_status(downstream_unit["locator"])
|
||||
self.assertDictContainsEntries(status, {
|
||||
'upstream_ref': self.upstream_unit["id"],
|
||||
'version_available': 2,
|
||||
'version_synced': 2,
|
||||
'version_declined': 2,
|
||||
'ready_to_sync': False,
|
||||
'error_message': None,
|
||||
})
|
||||
|
||||
# Check the upstream/downstream status of [one of] the children
|
||||
self.assertDictContainsEntries(self._get_sync_status(downstream_problem1), {
|
||||
'upstream_ref': self.upstream_problem1["id"],
|
||||
'version_available': 3,
|
||||
'version_synced': 2,
|
||||
'version_declined': 3,
|
||||
'ready_to_sync': False,
|
||||
'error_message': None,
|
||||
})
|
||||
|
||||
# Check that the downstream container has not had any changes
|
||||
self.assertXmlEqual(self._get_course_block_olx(downstream_unit["locator"]), f"""
|
||||
<vertical
|
||||
display_name="Unit 1 Title"
|
||||
upstream_display_name="Unit 1 Title"
|
||||
upstream="{self.upstream_unit['id']}"
|
||||
upstream_version="2"
|
||||
upstream_version_declined="2"
|
||||
>
|
||||
<html
|
||||
display_name="Text Content"
|
||||
upstream_display_name="Text Content"
|
||||
editor="visual"
|
||||
upstream="{self.upstream_html1['id']}"
|
||||
upstream_version="2"
|
||||
upstream_version_declined="2"
|
||||
top_level_downstream_parent_key="{downstream_unit_block_key}"
|
||||
>This is the HTML.</html>
|
||||
<problem
|
||||
display_name="Problem 1 Display Name"
|
||||
upstream_display_name="Problem 1 Display Name"
|
||||
markdown="MD 1"
|
||||
{self.standard_capa_attributes}
|
||||
upstream="{self.upstream_problem1['id']}"
|
||||
upstream_version="2"
|
||||
upstream_version_declined="3"
|
||||
top_level_downstream_parent_key="{downstream_unit_block_key}"
|
||||
>multiple choice...</problem>
|
||||
<problem
|
||||
display_name="Problem 2 Display Name"
|
||||
upstream_display_name="Problem 2 Display Name"
|
||||
markdown="null"
|
||||
{self.standard_capa_attributes}
|
||||
upstream="{self.upstream_problem2['id']}"
|
||||
upstream_version="2"
|
||||
upstream_version_declined="2"
|
||||
top_level_downstream_parent_key="{downstream_unit_block_key}"
|
||||
>multi select...</problem>
|
||||
</vertical>
|
||||
""")
|
||||
|
||||
@@ -43,10 +43,12 @@ def _get_upstream_link_good_and_syncable(downstream):
|
||||
return UpstreamLink(
|
||||
upstream_ref=downstream.upstream,
|
||||
upstream_key=UsageKey.from_string(downstream.upstream),
|
||||
downstream_key=str(downstream.usage_key),
|
||||
version_synced=downstream.upstream_version,
|
||||
version_available=(downstream.upstream_version or 0) + 1,
|
||||
version_declined=downstream.upstream_version_declined,
|
||||
error_message=None,
|
||||
has_top_level_parent=False,
|
||||
)
|
||||
|
||||
|
||||
@@ -582,7 +584,7 @@ class GetUpstreamViewTest(
|
||||
data["item_type"] = str(item_type)
|
||||
if use_top_level_parents is not None:
|
||||
data["use_top_level_parents"] = str(use_top_level_parents)
|
||||
return self.client.get("/api/contentstore/v2/downstreams-all/", data=data)
|
||||
return self.client.get("/api/contentstore/v2/downstreams/", data=data)
|
||||
|
||||
def test_200_all_downstreams_for_a_course(self):
|
||||
"""
|
||||
|
||||
Reference in New Issue
Block a user