diff --git a/cms/djangoapps/contentstore/rest_api/v2/urls.py b/cms/djangoapps/contentstore/rest_api/v2/urls.py
index ce2a78c0e2a5..5fa954a4ef82 100644
--- a/cms/djangoapps/contentstore/rest_api/v2/urls.py
+++ b/cms/djangoapps/contentstore/rest_api/v2/urls.py
@@ -13,24 +13,11 @@
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(),
diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py
index b08a222ff351..4e168bcaf8af 100644
--- a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py
+++ b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py
@@ -81,7 +81,6 @@
"""
import logging
-import warnings
from attrs import asdict as attrs_asdict
from django.db.models import QuerySet
@@ -101,8 +100,6 @@
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 @@ def _remove_duplicates(self, links: list[EntityLinkBase]) -> list[EntityLinkBase
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 @@ def delete(self, request: _AuthenticatedRequest, usage_key_string: str) -> Respo
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 @@ def delete(self, request: _AuthenticatedRequest, usage_key_string: str) -> Respo
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,
diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py
index 1d7e6b1277de..c03f3f4e336f 100644
--- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py
+++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py
@@ -71,6 +71,9 @@ def _get_sync_status(self, usage_key: str):
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 @@ def _get_downstream_links(
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 @@ def test_unit_sync(self):
'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 @@ def test_unit_sync(self):
'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 @@ def test_unit_sync(self):
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"""
+
+ This is the HTML.
+ multiple choice...
+ multi select...
+
+ """)
+
+ # 2️⃣ Now, lets modify the upstream problem 1:
+ self._set_library_block_olx(
+ self.upstream_problem1["id"],
+ 'multiple choice v2...'
+ )
+ 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"""
+
+ This is the HTML.
+ multiple choice...
+ multi select...
+
+ """)
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 cad103c81185..00db886240ef 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
@@ -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 @@ def call_api(
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):
"""
diff --git a/cms/lib/xblock/upstream_sync.py b/cms/lib/xblock/upstream_sync.py
index bc1148b2cebe..076afef505a7 100644
--- a/cms/lib/xblock/upstream_sync.py
+++ b/cms/lib/xblock/upstream_sync.py
@@ -24,6 +24,7 @@
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locator import LibraryContainerLocator, LibraryUsageLocatorV2
+from opaque_keys.edx.keys import UsageKey
from xblock.exceptions import XBlockNotFoundError
from xblock.fields import Scope, String, Integer, Dict
from xblock.core import XBlockMixin, XBlock
@@ -77,16 +78,15 @@ class UpstreamLink:
"""
upstream_ref: str | None # Reference to the upstream content, e.g., a serialized library block usage key.
upstream_key: LibraryUsageLocatorV2 | LibraryContainerLocator | None # parsed opaque key version of upstream_ref
+ downstream_key: str | None # Key of the downstream object.
version_synced: int | None # Version of the upstream to which the downstream was last synced.
version_available: int | None # Latest version of the upstream that's available, or None if it couldn't be loaded.
version_declined: int | None # Latest version which the user has declined to sync with, if any.
error_message: str | None # If link is valid, None. Otherwise, a localized, human-friendly error message.
+ has_top_level_parent: bool # True if this Upstream link has a top-level parent
@property
- def ready_to_sync(self) -> bool:
- """
- Should we invite the downstream's authors to sync the latest upstream updates?
- """
+ def _is_ready_to_sync_individually(self) -> bool:
return bool(
self.upstream_ref and
self.version_available and
@@ -94,6 +94,60 @@ def ready_to_sync(self) -> bool:
self.version_available > (self.version_declined or 0)
)
+ @property
+ def ready_to_sync(self) -> bool:
+ """
+ Calculates the ready to sync value using the version available.
+ If is a container, also verifies if the children needs sync.
+ """
+ from xmodule.modulestore.django import modulestore
+
+ # If this component/container has top-level parent, so we need to sync the parent
+ if self.has_top_level_parent:
+ return False
+
+ if isinstance(self.upstream_key, LibraryUsageLocatorV2):
+ return self._is_ready_to_sync_individually
+ elif isinstance(self.upstream_key, LibraryContainerLocator):
+ # The container itself has changes to update, it is not necessary to review its children
+ if self._is_ready_to_sync_individually:
+ return True
+
+ def check_children_ready_to_sync(xblock_downstream):
+ """
+ Checks if one of the children of `xblock_downstream` is ready to sync
+ """
+ if not xblock_downstream.has_children:
+ return False
+
+ downstream_children = xblock_downstream.get_children()
+
+ for child in downstream_children:
+ if child.upstream:
+ child_upstream_link = UpstreamLink.get_for_block(child)
+
+ child_ready_to_sync = bool(
+ child_upstream_link.upstream_ref and
+ child_upstream_link.version_available and
+ child_upstream_link.version_available > (child_upstream_link.version_synced or 0) and
+ child_upstream_link.version_available > (child_upstream_link.version_declined or 0)
+ )
+
+ # If one child needs sync, it is not needed to check more children
+ if child_ready_to_sync:
+ return True
+
+ if check_children_ready_to_sync(child):
+ # If one child needs sync, it is not needed to check more children
+ return True
+
+ return False
+ if self.downstream_key is not None:
+ return check_children_ready_to_sync(
+ modulestore().get_item(UsageKey.from_string(self.downstream_key))
+ )
+ return False
+
@property
def upstream_link(self) -> str | None:
"""
@@ -138,10 +192,12 @@ def try_get_for_block(cls, downstream: XBlock, log_error: bool = True) -> t.Self
return cls(
upstream_ref=getattr(downstream, "upstream", ""),
upstream_key=None,
+ downstream_key=str(getattr(downstream, "usage_key", "")),
version_synced=getattr(downstream, "upstream_version", None),
version_available=None,
version_declined=None,
error_message=str(exc),
+ has_top_level_parent=False,
)
@classmethod
@@ -189,15 +245,16 @@ def get_for_block(cls, downstream: XBlock) -> t.Self:
except XBlockNotFoundError as exc:
raise BadUpstream(_("Linked upstream library block was not found in the system")) from exc
version_available = block_meta.published_version_num
- else:
+ elif isinstance(upstream_key, LibraryContainerLocator):
# The upstream is a Container:
- assert isinstance(upstream_key, LibraryContainerLocator)
try:
container_meta = lib_api.get_container(upstream_key)
except lib_api.ContentLibraryContainerNotFound as exc:
raise BadUpstream(_("Linked upstream library container was not found in the system")) from exc
expected_downstream_block_type = container_meta.container_type.olx_tag
version_available = container_meta.published_version_num
+ else:
+ raise BadUpstream(_("Linked `upstream_key` is not a valid key"))
if downstream_type != expected_downstream_block_type:
# Note: generally the upstream and downstream types must match, except that upstream containers
@@ -214,27 +271,44 @@ def get_for_block(cls, downstream: XBlock) -> t.Self:
)
)
- return cls(
+ result = cls(
upstream_ref=downstream.upstream,
upstream_key=upstream_key,
+ downstream_key=str(downstream.usage_key),
version_synced=downstream.upstream_version,
version_available=version_available,
version_declined=downstream.upstream_version_declined,
error_message=None,
+ has_top_level_parent=downstream.top_level_downstream_parent_key is not None,
)
+ return result
-def decline_sync(downstream: XBlock) -> None:
+
+def decline_sync(downstream: XBlock, user_id=None) -> None:
"""
Given an XBlock that is linked to upstream content, mark the latest available update as 'declined' so that its
authors are not prompted (until another upstream version becomes available).
-
- Does not save `downstream` to the store. That is left up to the caller.
+ The function is called recursively to perform the same operation on the children of the `downstream`.
If `downstream` lacks a valid+supported upstream link, this raises an UpstreamLinkException.
"""
- upstream_link = UpstreamLink.get_for_block(downstream) # Can raise UpstreamLinkException
- downstream.upstream_version_declined = upstream_link.version_available
+ if downstream.upstream:
+ from xmodule.modulestore.django import modulestore
+
+ store = modulestore()
+ upstream_link = UpstreamLink.get_for_block(downstream) # Can raise UpstreamLinkException
+ upstream_key = upstream_link.upstream_key
+
+ downstream.upstream_version_declined = upstream_link.version_available
+
+ if isinstance(upstream_key, LibraryContainerLocator) and downstream.has_children:
+ with store.bulk_operations(downstream.usage_key.context_key):
+ children = downstream.get_children()
+ for child in children:
+ decline_sync(child, user_id)
+
+ store.update_item(downstream, user_id)
def sever_upstream_link(downstream: XBlock) -> None: