diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index a4ece6c85d59..e9f599772d3e 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -9,6 +9,7 @@ from attrs import frozen, Factory from django.conf import settings +from django.contrib.auth import get_user_model from django.utils.translation import gettext as _ from opaque_keys.edx.keys import AssetKey, CourseKey, UsageKey from opaque_keys.edx.locator import DefinitionLocator, LocalId @@ -22,6 +23,7 @@ from xmodule.xml_block import XmlMixin from cms.djangoapps.models.settings.course_grading import CourseGradingModel +from cms.lib.xblock.upstream_sync import UpstreamLink, BadUpstream, BadDownstream, fetch_customizable_fields from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers import openedx.core.djangoapps.content_staging.api as content_staging_api import openedx.core.djangoapps.content_tagging.api as content_tagging_api @@ -30,6 +32,10 @@ log = logging.getLogger(__name__) + +User = get_user_model() + + # Note: Grader types are used throughout the platform but most usages are simply in-line # strings. In addition, new grader types can be defined on the fly anytime one is needed # (because they're just strings). This dict is an attempt to constrain the sprawl in Studio. @@ -282,9 +288,10 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) -> node, parent_xblock, store, - user_id=request.user.id, + user=request.user, slug_hint=user_clipboard.source_usage_key.block_id, copied_from_block=str(user_clipboard.source_usage_key), + copied_from_version_num=user_clipboard.content.version_num, tags=user_clipboard.content.tags, ) # Now handle static files that need to go into Files & Uploads: @@ -293,7 +300,6 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) -> staged_content_id=user_clipboard.content.id, static_files=static_files, ) - return new_xblock, notices @@ -302,12 +308,15 @@ def _import_xml_node_to_parent( parent_xblock: XBlock, # The modulestore we're using store, - # The ID of the user who is performing this operation - user_id: int, + # The user who is performing this operation + user: User, # Hint to use as usage ID (block_id) for the new XBlock slug_hint: str | None = None, # UsageKey of the XBlock that this one is a copy of copied_from_block: str | None = None, + # Positive int version of source block, if applicable (e.g., library block). + # Zero if not applicable (e.g., course block). + copied_from_version_num: int = 0, # Content tags applied to the source XBlock(s) tags: dict[str, str] | None = None, ) -> XBlock: @@ -373,12 +382,32 @@ def _import_xml_node_to_parent( raise NotImplementedError("We don't yet support pasting XBlocks with children") temp_xblock.parent = parent_key if copied_from_block: - # Store a reference to where this block was copied from, in the 'copied_from_block' field (AuthoringMixin) - temp_xblock.copied_from_block = copied_from_block + # Try to link the pasted block (downstream) to the copied block (upstream). + temp_xblock.upstream = copied_from_block + try: + UpstreamLink.get_for_block(temp_xblock) + except (BadDownstream, BadUpstream): + # Usually this will fail. For example, if the copied block is a modulestore course block, it can't be an + # upstream. That's fine! Instead, we store a reference to where this block was copied from, in the + # 'copied_from_block' field (from AuthoringMixin). + temp_xblock.upstream = None + temp_xblock.copied_from_block = copied_from_block + else: + # But if it doesn't fail, then populate the `upstream_version` field based on what was copied. Note that + # this could be the latest published version, or it could be an an even newer draft version. + temp_xblock.upstream_version = copied_from_version_num + # Also, fetch upstream values (`upstream_display_name`, etc.). + # Recall that the copied block could be a draft. So, rather than fetching from the published upstream (which + # could be older), fetch from the copied block itself. That way, if an author customizes a field, but then + # later wants to restore it, it will restore to the value that the field had when the block was pasted. Of + # course, if the author later syncs updates from a *future* published upstream version, then that will fetch + # new values from the published upstream content. + fetch_customizable_fields(upstream=temp_xblock, downstream=temp_xblock, user=user) + # Save the XBlock into modulestore. We need to save the block and its parent for this to work: - new_xblock = store.update_item(temp_xblock, user_id, allow_not_found=True) + new_xblock = store.update_item(temp_xblock, user.id, allow_not_found=True) parent_xblock.children.append(new_xblock.location) - store.update_item(parent_xblock, user_id) + store.update_item(parent_xblock, user.id) children_handled = False if hasattr(new_xblock, 'studio_post_paste'): @@ -394,7 +423,7 @@ def _import_xml_node_to_parent( child_node, new_xblock, store, - user_id=user_id, + user=user, copied_from_block=str(child_copied_from), tags=tags, ) diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py index d72707ed7836..f2e8b6ef431b 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py @@ -103,6 +103,18 @@ def get_assets_url(self, obj): return None +class UpstreamLinkSerializer(serializers.Serializer): + """ + Serializer holding info for syncing a block with its upstream (eg, a library block). + """ + upstream_ref = serializers.CharField() + version_synced = serializers.IntegerField() + version_available = serializers.IntegerField(allow_null=True) + version_declined = serializers.IntegerField(allow_null=True) + error_message = serializers.CharField(allow_null=True) + ready_to_sync = serializers.BooleanField() + + class ChildVerticalContainerSerializer(serializers.Serializer): """ Serializer for representing a xblock child of vertical container. @@ -113,6 +125,7 @@ class ChildVerticalContainerSerializer(serializers.Serializer): block_type = serializers.CharField() user_partition_info = serializers.DictField() user_partitions = serializers.ListField() + upstream_link = UpstreamLinkSerializer(allow_null=True) actions = serializers.SerializerMethodField() validation_messages = MessageValidation(many=True) render_error = serializers.CharField() diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py index d3fc37198213..7cac074a433f 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py @@ -70,6 +70,8 @@ def setup_xblock(self): parent=self.vertical.location, category="html", display_name="Html Content 2", + upstream="lb:FakeOrg:FakeLib:html:FakeBlock", + upstream_version=5, ) def create_block(self, parent, category, display_name, **kwargs): @@ -193,6 +195,7 @@ def test_children_content(self): "name": self.html_unit_first.display_name_with_default, "block_id": str(self.html_unit_first.location), "block_type": self.html_unit_first.location.block_type, + "upstream_link": None, "user_partition_info": expected_user_partition_info, "user_partitions": expected_user_partitions, "actions": { @@ -218,12 +221,21 @@ def test_children_content(self): "can_delete": True, "can_manage_tags": True, }, + "upstream_link": { + "upstream_ref": "lb:FakeOrg:FakeLib:html:FakeBlock", + "version_synced": 5, + "version_available": None, + "version_declined": None, + "error_message": "Linked library item was not found in the system", + "ready_to_sync": False, + }, "user_partition_info": expected_user_partition_info, "user_partitions": expected_user_partitions, "validation_messages": [], "render_error": "", }, ] + self.maxDiff = None self.assertEqual(response.data["children"], expected_response) def test_not_valid_usage_key_string(self): diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py b/cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py index 670b94afbbe0..0798c341cc1c 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py @@ -20,6 +20,7 @@ ContainerHandlerSerializer, VerticalContainerSerializer, ) +from cms.lib.xblock.upstream_sync import UpstreamLink from openedx.core.lib.api.view_utils import view_auth_classes from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order @@ -198,6 +199,7 @@ def get(self, request: Request, usage_key_string: str): "block_type": "drag-and-drop-v2", "user_partition_info": {}, "user_partitions": {} + "upstream_link": null, "actions": { "can_copy": true, "can_duplicate": true, @@ -215,6 +217,13 @@ def get(self, request: Request, usage_key_string: str): "block_type": "video", "user_partition_info": {}, "user_partitions": {} + "upstream_link": { + "upstream_ref": "lb:org:mylib:video:404", + "version_synced": 16 + "version_available": null, + "error_message": "Linked library item not found: lb:org:mylib:video:404", + "ready_to_sync": false, + }, "actions": { "can_copy": true, "can_duplicate": true, @@ -232,6 +241,13 @@ def get(self, request: Request, usage_key_string: str): "block_type": "html", "user_partition_info": {}, "user_partitions": {}, + "upstream_link": { + "upstream_ref": "lb:org:mylib:html:abcd", + "version_synced": 43, + "version_available": 49, + "error_message": null, + "ready_to_sync": true, + }, "actions": { "can_copy": true, "can_duplicate": true, @@ -267,6 +283,7 @@ def get(self, request: Request, usage_key_string: str): child_info = modulestore().get_item(child) user_partition_info = get_visibility_partition_info(child_info, course=course) user_partitions = get_user_partition_info(child_info, course=course) + upstream_link = UpstreamLink.try_get_for_block(child_info) validation_messages = get_xblock_validation_messages(child_info) render_error = get_xblock_render_error(request, child_info) @@ -277,6 +294,12 @@ def get(self, request: Request, usage_key_string: str): "block_type": child_info.location.block_type, "user_partition_info": user_partition_info, "user_partitions": user_partitions, + "upstream_link": ( + # If the block isn't linked to an upstream (which is by far the most common case) then just + # make this field null, which communicates the same info, but with less noise. + upstream_link.to_json() if upstream_link.upstream_ref + else None + ), "validation_messages": validation_messages, "render_error": render_error, }) diff --git a/cms/djangoapps/contentstore/rest_api/v2/urls.py b/cms/djangoapps/contentstore/rest_api/v2/urls.py index ad61cc937015..3e653d07fbcf 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/urls.py +++ b/cms/djangoapps/contentstore/rest_api/v2/urls.py @@ -1,15 +1,31 @@ """Contenstore API v2 URLs.""" -from django.urls import path - -from cms.djangoapps.contentstore.rest_api.v2.views import HomePageCoursesViewV2 +from django.conf import settings +from django.urls import path, re_path +from cms.djangoapps.contentstore.rest_api.v2.views import home, downstreams app_name = "v2" urlpatterns = [ path( "home/courses", - HomePageCoursesViewV2.as_view(), + home.HomePageCoursesViewV2.as_view(), name="courses", ), + # TODO: Potential future path. + # re_path( + # fr'^downstreams/$', + # downstreams.DownstreamsListView.as_view(), + # name="downstreams_list", + # ), + re_path( + fr'^downstreams/{settings.USAGE_KEY_PATTERN}$', + downstreams.DownstreamView.as_view(), + name="downstream" + ), + re_path( + fr'^downstreams/{settings.USAGE_KEY_PATTERN}/sync$', + downstreams.SyncFromUpstreamView.as_view(), + name="sync_from_upstream" + ), ] diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/__init__.py b/cms/djangoapps/contentstore/rest_api/v2/views/__init__.py index 73ddde98440c..e69de29bb2d1 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/__init__.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/__init__.py @@ -1,3 +0,0 @@ -"""Module for v2 views.""" - -from cms.djangoapps.contentstore.rest_api.v2.views.home import HomePageCoursesViewV2 diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py new file mode 100644 index 000000000000..5079698082be --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py @@ -0,0 +1,251 @@ +""" +API Views for managing & syncing links between upstream & downstream content + +API paths (We will move these into proper api_doc_tools annotations soon +https://github.com/openedx/edx-platform/issues/35653): + + /api/contentstore/v2/downstreams/{usage_key_string} + + GET: Inspect a single downstream block's link to upstream content. + 200: Upstream link details successfully fetched. Returns UpstreamLink (may contain an error_message). + 404: Downstream block not found or user lacks permission to edit it. + + DELETE: Sever a single downstream block's link to upstream content. + 204: Block successfully unlinked (or it wasn't linked in the first place). No response body. + 404: Downstream block not found or user lacks permission to edit it. + + PUT: Establish or modify a single downstream block's link to upstream content. An authoring client could use this + endpoint to add library content in a two-step process, specifically: (1) add a blank block to a course, then + (2) link it to a content library with ?sync=True. + REQUEST BODY: { + "upstream_ref": str, // reference to upstream block (eg, library block usage key) + "sync": bool, // whether to sync in upstream content (False by default) + } + 200: Downstream block's upstream link successfully edited (and synced, if requested). Returns UpstreamLink. + 400: upstream_ref is malformed, missing, or inaccessible. + 400: Content at upstream_ref does not support syncing. + 404: Downstream block not found or user lacks permission to edit it. + + /api/contentstore/v2/downstreams/{usage_key_string}/sync + + POST: Sync a downstream block with upstream content. + 200: Downstream block successfully synced with upstream content. + 400: Downstream block is not linked to upstream content. + 400: Upstream is malformed, missing, or inaccessible. + 400: Upstream block does not support syncing. + 404: Downstream block not found or user lacks permission to edit it. + + DELETE: Decline an available sync for a downstream block. + 204: Sync successfuly dismissed. No response body. + 400: Downstream block is not linked to upstream content. + 404: Downstream block not found or user lacks permission to edit it. + + # 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 + GET: List downstream blocks that can be synced, filterable by course or sync-readiness. + 200: A paginated list of applicable & accessible downstream blocks. Entries are UpstreamLinks. + +UpstreamLink response schema: + { + "upstream_ref": string? + "version_synced": string?, + "version_available": string?, + "version_declined": string?, + "error_message": string?, + "ready_to_sync": Boolean + } +""" +import logging + +from django.contrib.auth.models import User # pylint: disable=imported-auth-user +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import UsageKey +from rest_framework.exceptions import NotFound, ValidationError +from rest_framework.request import Request +from rest_framework.response import Response +from rest_framework.views import APIView +from xblock.core import XBlock + +from cms.lib.xblock.upstream_sync import ( + UpstreamLink, UpstreamLinkException, NoUpstream, BadUpstream, BadDownstream, + fetch_customizable_fields, sync_from_upstream, decline_sync, sever_upstream_link +) +from common.djangoapps.student.auth import has_studio_write_access, has_studio_read_access +from openedx.core.lib.api.view_utils import ( + DeveloperErrorViewMixin, + view_auth_classes, +) +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.exceptions import ItemNotFoundError + + +logger = logging.getLogger(__name__) + + +class _AuthenticatedRequest(Request): + """ + Alias for the `Request` class which tells mypy to assume that `.user` is not an AnonymousUser. + + Using this does NOT ensure the request is actually authenticated-- + you will some other way to ensure that, such as `@view_auth_classes(is_authenticated=True)`. + """ + user: User + + +# TODO: Potential future view. +# @view_auth_classes(is_authenticated=True) +# class DownstreamListView(DeveloperErrorViewMixin, APIView): +# """ +# List all blocks which are linked to upstream content, with optional filtering. +# """ +# def get(self, request: _AuthenticatedRequest) -> Response: +# """ +# Handle the request. +# """ +# course_key_string = request.GET['course_id'] +# syncable = request.GET['ready_to_sync'] +# ... + + +@view_auth_classes(is_authenticated=True) +class DownstreamView(DeveloperErrorViewMixin, APIView): + """ + Inspect or manage an XBlock's link to upstream content. + """ + def get(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response: + """ + Inspect an XBlock's link to upstream content. + """ + downstream = _load_accessible_block(request.user, usage_key_string, require_write_access=False) + return Response(UpstreamLink.try_get_for_block(downstream).to_json()) + + def put(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response: + """ + Edit an XBlock's link to upstream content. + """ + downstream = _load_accessible_block(request.user, usage_key_string, require_write_access=True) + new_upstream_ref = request.data.get("upstream_ref") + + # Set `downstream.upstream` so that we can try to sync and/or fetch. + # Note that, if this fails and we raise a 4XX, then we will not call modulstore().update_item, + # thus preserving the former value of `downstream.upstream`. + downstream.upstream = new_upstream_ref + sync_param = request.data.get("sync", "false").lower() + if sync_param not in ["true", "false"]: + raise ValidationError({"sync": "must be 'true' or 'false'"}) + try: + if sync_param == "true": + sync_from_upstream(downstream=downstream, user=request.user) + else: + # Even if we're not syncing (i.e., updating the downstream's values with the upstream's), we still need + # to fetch the upstream's customizable values and store them as hidden fields on the downstream. This + # ensures that downstream authors can restore defaults based on the upstream. + fetch_customizable_fields(downstream=downstream, user=request.user) + except BadDownstream as exc: + logger.exception( + "'%s' is an invalid downstream; refusing to set its upstream to '%s'", + usage_key_string, + new_upstream_ref, + ) + raise ValidationError(str(exc)) from exc + except BadUpstream as exc: + logger.exception( + "'%s' is an invalid upstream reference; refusing to set it as upstream of '%s'", + new_upstream_ref, + usage_key_string, + ) + raise ValidationError({"upstream_ref": str(exc)}) from exc + except NoUpstream as exc: + raise ValidationError({"upstream_ref": "value missing"}) from exc + modulestore().update_item(downstream, request.user.id) + # Note: We call `get_for_block` (rather than `try_get_for_block`) because if anything is wrong with the + # upstream at this point, then that is completely unexpected, so it's appropriate to let the 500 happen. + return Response(UpstreamLink.get_for_block(downstream).to_json()) + + def delete(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response: + """ + Sever an XBlock's link to upstream content. + """ + downstream = _load_accessible_block(request.user, usage_key_string, require_write_access=True) + try: + sever_upstream_link(downstream) + except NoUpstream as exc: + logger.exception( + "Tried to DELETE upstream link of '%s', but it wasn't linked to anything in the first place. " + "Will do nothing. ", + usage_key_string, + ) + else: + modulestore().update_item(downstream, request.user.id) + return Response(status=204) + + +@view_auth_classes(is_authenticated=True) +class SyncFromUpstreamView(DeveloperErrorViewMixin, APIView): + """ + Accept or decline an opportunity to sync a downstream block from its upstream content. + """ + + def post(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response: + """ + Pull latest updates to the block at {usage_key_string} from its linked upstream content. + """ + downstream = _load_accessible_block(request.user, usage_key_string, require_write_access=True) + try: + sync_from_upstream(downstream, request.user) + except UpstreamLinkException as exc: + logger.exception( + "Could not sync from upstream '%s' to downstream '%s'", + downstream.upstream, + usage_key_string, + ) + raise ValidationError(detail=str(exc)) from exc + modulestore().update_item(downstream, request.user.id) + # Note: We call `get_for_block` (rather than `try_get_for_block`) because if anything is wrong with the + # upstream at this point, then that is completely unexpected, so it's appropriate to let the 500 happen. + return Response(UpstreamLink.get_for_block(downstream).to_json()) + + def delete(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response: + """ + Decline the latest updates to the block at {usage_key_string}. + """ + downstream = _load_accessible_block(request.user, usage_key_string, require_write_access=True) + try: + decline_sync(downstream) + 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 + # hit the HTTP API anyway, or they could be viewing a Studio page which hasn't been refreshed in a while. + # So, it's a 400, not a 500. + logger.exception( + "Tried to decline a sync to downstream '%s', but the upstream link '%s' is invalid.", + usage_key_string, + downstream.upstream, + ) + raise ValidationError(str(exc)) from exc + modulestore().update_item(downstream, request.user.id) + return Response(status=204) + + +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, + raising a DRF-friendly exception if anything goes wrong. + + Raises NotFound if usage key is malformed, if the user lacks access, or if the block doesn't exist. + """ + not_found = NotFound(detail=f"Block not found or not accessible: {usage_key_string}") + try: + usage_key = UsageKey.from_string(usage_key_string) + except InvalidKeyError as exc: + raise ValidationError(detail=f"Malformed block usage key: {usage_key_string}") from exc + if require_write_access and not has_studio_write_access(user, usage_key.context_key): + raise not_found + if not has_studio_read_access(user, usage_key.context_key): + raise not_found + try: + block = modulestore().get_item(usage_key) + except ItemNotFoundError as exc: + raise not_found from exc + return block 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 new file mode 100644 index 000000000000..616035473e7e --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py @@ -0,0 +1,266 @@ +""" +Unit tests for /api/contentstore/v2/downstreams/* JSON APIs. +""" +from unittest.mock import patch + +from cms.lib.xblock.upstream_sync import UpstreamLink, BadUpstream +from common.djangoapps.student.tests.factories import UserFactory +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory + +from .. import downstreams as downstreams_views + + +MOCK_UPSTREAM_REF = "mock-upstream-ref" +MOCK_UPSTREAM_ERROR = "your LibraryGPT subscription has expired" + + +def _get_upstream_link_good_and_syncable(downstream): + return UpstreamLink( + upstream_ref=downstream.upstream, + version_synced=downstream.upstream_version, + version_available=(downstream.upstream_version or 0) + 1, + version_declined=downstream.upstream_version_declined, + error_message=None, + ) + + +def _get_upstream_link_bad(_downstream): + raise BadUpstream(MOCK_UPSTREAM_ERROR) + + +class _DownstreamViewTestMixin: + """ + Shared data and error test cases. + """ + + def setUp(self): + """ + Create a simple course with one unit and two videos, one of which is linked to an "upstream". + """ + super().setUp() + self.course = CourseFactory.create() + 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.downstream_video_key = BlockFactory.create( + category='video', parent=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") + + def call_api(self, usage_key_string): + raise NotImplementedError + + def test_404_downstream_not_found(self): + """ + Do we raise 404 if the specified downstream block could not be loaded? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.fake_video_key) + assert response.status_code == 404 + assert "not found" in response.data["developer_message"] + + def test_404_downstream_not_accessible(self): + """ + Do we raise 404 if the user lacks read access on the specified downstream block? + """ + self.client.login(username="learner", password="password") + response = self.call_api(self.downstream_video_key) + assert response.status_code == 404 + assert "not found" in response.data["developer_message"] + + +class GetDownstreamViewTest(_DownstreamViewTestMixin, SharedModuleStoreTestCase): + """ + Test that `GET /api/v2/contentstore/downstreams/...` inspects a downstream's link to an upstream. + """ + def call_api(self, usage_key_string): + return self.client.get(f"/api/contentstore/v2/downstreams/{usage_key_string}") + + @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_good_and_syncable) + def test_200_good_upstream(self): + """ + Does the happy path work? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.downstream_video_key) + assert response.status_code == 200 + assert response.data['upstream_ref'] == MOCK_UPSTREAM_REF + assert response.data['error_message'] is None + assert response.data['ready_to_sync'] is True + + @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_bad) + def test_200_bad_upstream(self): + """ + If the upstream link is broken, do we still return 200, but with an error message in body? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.downstream_video_key) + assert response.status_code == 200 + assert response.data['upstream_ref'] == MOCK_UPSTREAM_REF + assert response.data['error_message'] == MOCK_UPSTREAM_ERROR + assert response.data['ready_to_sync'] is False + + def test_200_no_upstream(self): + """ + If the upstream link is missing, do we still return 200, but with an error message in body? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.regular_video_key) + assert response.status_code == 200 + assert response.data['upstream_ref'] is None + assert "is not linked" in response.data['error_message'] + assert response.data['ready_to_sync'] is False + + +class PutDownstreamViewTest(_DownstreamViewTestMixin, SharedModuleStoreTestCase): + """ + Test that `PUT /api/v2/contentstore/downstreams/...` edits a downstream's link to an upstream. + """ + def call_api(self, usage_key_string, sync: str | None = None): + return self.client.put( + f"/api/contentstore/v2/downstreams/{usage_key_string}", + data={ + "upstream_ref": MOCK_UPSTREAM_REF, + **({"sync": sync} if sync else {}), + }, + content_type="application/json", + ) + + @patch.object(downstreams_views, "fetch_customizable_fields") + @patch.object(downstreams_views, "sync_from_upstream") + @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_good_and_syncable) + def test_200_with_sync(self, mock_sync, mock_fetch): + """ + Does the happy path work (with sync=True)? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.regular_video_key, sync='true') + assert response.status_code == 200 + video_after = modulestore().get_item(self.regular_video_key) + assert mock_sync.call_count == 1 + assert mock_fetch.call_count == 0 + assert video_after.upstream == MOCK_UPSTREAM_REF + + @patch.object(downstreams_views, "fetch_customizable_fields") + @patch.object(downstreams_views, "sync_from_upstream") + @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_good_and_syncable) + def test_200_no_sync(self, mock_sync, mock_fetch): + """ + Does the happy path work (with sync=False)? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.regular_video_key, sync='false') + assert response.status_code == 200 + video_after = modulestore().get_item(self.regular_video_key) + assert mock_sync.call_count == 0 + assert mock_fetch.call_count == 1 + assert video_after.upstream == MOCK_UPSTREAM_REF + + @patch.object(downstreams_views, "fetch_customizable_fields", side_effect=BadUpstream(MOCK_UPSTREAM_ERROR)) + def test_400(self, sync: str): + """ + Do we raise a 400 if the provided upstream reference is malformed or not accessible? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.downstream_video_key) + assert response.status_code == 400 + assert MOCK_UPSTREAM_ERROR in response.data['developer_message']['upstream_ref'] + video_after = modulestore().get_item(self.regular_video_key) + assert video_after.upstream is None + + +class DeleteDownstreamViewTest(_DownstreamViewTestMixin, SharedModuleStoreTestCase): + """ + Test that `DELETE /api/v2/contentstore/downstreams/...` severs a downstream's link to an upstream. + """ + def call_api(self, usage_key_string): + return self.client.delete(f"/api/contentstore/v2/downstreams/{usage_key_string}") + + @patch.object(downstreams_views, "sever_upstream_link") + @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_good_and_syncable) + def test_204(self, mock_sever): + """ + Does the happy path work? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.downstream_video_key) + assert response.status_code == 204 + assert mock_sever.call_count == 1 + + @patch.object(downstreams_views, "sever_upstream_link") + def test_204_no_upstream(self, mock_sever): + """ + If there's no upsream, do we still happily return 204? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.regular_video_key) + assert response.status_code == 204 + assert mock_sever.call_count == 1 + + +class _DownstreamSyncViewTestMixin(_DownstreamViewTestMixin): + """ + Shared tests between the /api/contentstore/v2/downstreams/.../sync endpoints. + """ + + @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_bad) + def test_400_bad_upstream(self): + """ + If the upstream link is bad, do we get a 400? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.downstream_video_key) + assert response.status_code == 400 + assert MOCK_UPSTREAM_ERROR in response.data["developer_message"][0] + + def test_400_no_upstream(self): + """ + If the upstream link is missing, do we get a 400? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.regular_video_key) + assert response.status_code == 400 + assert "is not linked" in response.data["developer_message"][0] + + +class PostDownstreamSyncViewTest(_DownstreamSyncViewTestMixin, SharedModuleStoreTestCase): + """ + Test that `POST /api/v2/contentstore/downstreams/.../sync` initiates a sync from the linked upstream. + """ + def call_api(self, usage_key_string): + return self.client.post(f"/api/contentstore/v2/downstreams/{usage_key_string}/sync") + + @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_good_and_syncable) + @patch.object(downstreams_views, "sync_from_upstream") + def test_200(self, mock_sync_from_upstream): + """ + Does the happy path work? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.downstream_video_key) + assert response.status_code == 200 + assert mock_sync_from_upstream.call_count == 1 + + +class DeleteDownstreamSyncViewtest(_DownstreamSyncViewTestMixin, SharedModuleStoreTestCase): + """ + Test that `DELETE /api/v2/contentstore/downstreams/.../sync` declines a sync from the linked upstream. + """ + def call_api(self, usage_key_string): + return self.client.delete(f"/api/contentstore/v2/downstreams/{usage_key_string}/sync") + + @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_good_and_syncable) + @patch.object(downstreams_views, "decline_sync") + def test_204(self, mock_decline_sync): + """ + Does the happy path work? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.downstream_video_key) + assert response.status_code == 204 + assert mock_decline_sync.call_count == 1 diff --git a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py index 7979a422a331..5706b44e2cec 100644 --- a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py +++ b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py @@ -7,11 +7,13 @@ from opaque_keys.edx.keys import UsageKey from rest_framework.test import APIClient from openedx_tagging.core.tagging.models import Tag +from organizations.models import Organization from xmodule.modulestore.django import contentstore, modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, upload_file_to_course from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory, ToyCourseFactory, LibraryFactory from cms.djangoapps.contentstore.utils import reverse_usage_url +from openedx.core.djangoapps.content_libraries import api as library_api from openedx.core.djangoapps.content_tagging import api as tagging_api CLIPBOARD_ENDPOINT = "/api/content-staging/v1/clipboard/" @@ -391,6 +393,78 @@ def test_paste_with_assets(self): assert source_pic2_hash != dest_pic2_hash # Because there was a conflict, this file was unchanged. +class ClipboardPasteFromV2LibraryTestCase(ModuleStoreTestCase): + """ + Test Clipboard Paste functionality with a "new" (as of Sumac) library + """ + + def setUp(self): + """ + Set up a v2 Content Library and a library content block + """ + super().setUp() + self.client = APIClient() + self.client.login(username=self.user.username, password=self.user_password) + self.store = modulestore() + + self.library = library_api.create_library( + library_type=library_api.COMPLEX, + org=Organization.objects.create(name="Test Org", short_name="CL-TEST"), + slug="lib", + title="Library", + ) + + self.lib_block_key = library_api.create_library_block(self.library.key, "problem", "p1").usage_key # v==1 + library_api.set_library_block_olx(self.lib_block_key, """ + + + + + Wrong + Right + + + + """) # v==2 + library_api.publish_changes(self.library.key) + library_api.set_library_block_olx(self.lib_block_key, """ + + + + + Wrong + Right + + + + """) # v==3 + lib_block_meta = library_api.get_library_block(self.lib_block_key) + assert lib_block_meta.published_version_num == 2 + assert lib_block_meta.draft_version_num == 3 + + self.course = CourseFactory.create(display_name='Course') + + def test_paste_from_library_creates_link(self): + """ + When we copy a v2 lib block into a course, the dest block should be linked up to the lib block. + """ + copy_response = self.client.post(CLIPBOARD_ENDPOINT, {"usage_key": str(self.lib_block_key)}, format="json") + assert copy_response.status_code == 200 + + paste_response = self.client.post(XBLOCK_ENDPOINT, { + "parent_locator": str(self.course.usage_key), + "staged_content": "clipboard", + }, format="json") + assert paste_response.status_code == 200 + + new_block_key = UsageKey.from_string(paste_response.json()["locator"]) + new_block = modulestore().get_item(new_block_key) + assert new_block.upstream == str(self.lib_block_key) + assert new_block.upstream_version == 3 + assert new_block.upstream_display_name == "MCQ-draft" + assert new_block.upstream_max_attempts == 5 + + class ClipboardPasteFromV1LibraryTestCase(ModuleStoreTestCase): """ Test Clipboard Paste functionality with legacy (v1) library content diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index 79137cfde11f..e4d37f942331 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -539,7 +539,8 @@ def _create_block(request): # Paste from the user's clipboard (content_staging app clipboard, not browser clipboard) into 'usage_key': try: created_xblock, notices = import_staged_content_from_user_clipboard( - parent_key=usage_key, request=request + parent_key=usage_key, + request=request, ) except Exception: # pylint: disable=broad-except log.exception( diff --git a/cms/envs/common.py b/cms/envs/common.py index 63221ee0b0a4..7297adae8354 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -127,6 +127,7 @@ from lms.djangoapps.lms_xblock.mixin import LmsBlockMixin from cms.lib.xblock.authoring_mixin import AuthoringMixin +from cms.lib.xblock.upstream_sync import UpstreamSyncMixin from xmodule.modulestore.edit_info import EditInfoMixin from openedx.core.djangoapps.theming.helpers_dirs import ( get_themes_unchecked, @@ -995,6 +996,7 @@ XModuleMixin, EditInfoMixin, AuthoringMixin, + UpstreamSyncMixin, ) # .. setting_name: XBLOCK_EXTRA_MIXINS diff --git a/cms/lib/xblock/test/test_upstream_sync.py b/cms/lib/xblock/test/test_upstream_sync.py new file mode 100644 index 000000000000..5db020393eab --- /dev/null +++ b/cms/lib/xblock/test/test_upstream_sync.py @@ -0,0 +1,337 @@ +""" +Test CMS's upstream->downstream syncing system +""" +import ddt + +from organizations.api import ensure_organization +from organizations.models import Organization + +from cms.lib.xblock.upstream_sync import ( + UpstreamLink, + sync_from_upstream, decline_sync, fetch_customizable_fields, sever_upstream_link, + NoUpstream, BadUpstream, BadDownstream, +) +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.content_libraries import api as libs +from openedx.core.djangoapps.xblock import api as xblock +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory + + +@ddt.ddt +class UpstreamTestCase(ModuleStoreTestCase): + """ + Tests the upstream_sync mixin, data object, and Python APIs. + """ + + def setUp(self): + """ + Create a simple course with one unit, and simple V2 library with two blocks. + """ + super().setUp() + course = CourseFactory.create() + chapter = BlockFactory.create(category='chapter', parent=course) + sequential = BlockFactory.create(category='sequential', parent=chapter) + self.unit = BlockFactory.create(category='vertical', parent=sequential) + + ensure_organization("TestX") + self.library = libs.create_library( + org=Organization.objects.get(short_name="TestX"), + slug="TestLib", + title="Test Upstream Library", + ) + self.upstream_key = libs.create_library_block(self.library.key, "html", "test-upstream").usage_key + libs.create_library_block(self.library.key, "video", "video-upstream") + + upstream = xblock.load_block(self.upstream_key, self.user) + upstream.display_name = "Upstream Title V2" + upstream.data = "Upstream content V2" + upstream.save() + + def test_sync_bad_downstream(self): + """ + Syncing into an unsupported downstream (such as a another Content Library block) raises BadDownstream, but + doesn't affect the block. + """ + downstream_lib_block_key = libs.create_library_block(self.library.key, "html", "bad-downstream").usage_key + downstream_lib_block = xblock.load_block(downstream_lib_block_key, self.user) + downstream_lib_block.display_name = "Another lib block" + downstream_lib_block.data = "another lib block" + downstream_lib_block.upstream = str(self.upstream_key) + downstream_lib_block.save() + + with self.assertRaises(BadDownstream): + sync_from_upstream(downstream_lib_block, self.user) + + assert downstream_lib_block.display_name == "Another lib block" + assert downstream_lib_block.data == "another lib block" + + def test_sync_no_upstream(self): + """ + Trivial case: Syncing a block with no upstream is a no-op + """ + block = BlockFactory.create(category='html', parent=self.unit) + block.display_name = "Block Title" + block.data = "Block content" + + with self.assertRaises(NoUpstream): + sync_from_upstream(block, self.user) + + assert block.display_name == "Block Title" + assert block.data == "Block content" + assert not block.upstream_display_name + + @ddt.data( + ("not-a-key-at-all", ".*is malformed.*"), + ("course-v1:Oops+ItsA+CourseKey", ".*is malformed.*"), + ("block-v1:The+Wrong+KindOfUsageKey+type@html+block@nope", ".*is malformed.*"), + ("lb:TestX:NoSuchLib:html:block-id", ".*not found in the system.*"), + ("lb:TestX:TestLib:video:should-be-html-but-is-a-video", ".*type mismatch.*"), + ("lb:TestX:TestLib:html:no-such-html", ".*not found in the system.*"), + ) + @ddt.unpack + def test_sync_bad_upstream(self, upstream, message_regex): + """ + Syncing with a bad upstream raises BadUpstream, but doesn't affect the block + """ + block = BlockFactory.create(category='html', parent=self.unit, upstream=upstream) + block.display_name = "Block Title" + block.data = "Block content" + + with self.assertRaisesRegex(BadUpstream, message_regex): + sync_from_upstream(block, self.user) + + assert block.display_name == "Block Title" + assert block.data == "Block content" + assert not block.upstream_display_name + + def test_sync_not_accessible(self): + """ + Syncing with an block that exists, but is inaccessible, raises BadUpstream + """ + downstream = BlockFactory.create(category='html', parent=self.unit, upstream=str(self.upstream_key)) + user_who_cannot_read_upstream = UserFactory.create(username="rando", is_staff=False, is_superuser=False) + with self.assertRaisesRegex(BadUpstream, ".*could not be loaded.*") as exc: + sync_from_upstream(downstream, user_who_cannot_read_upstream) + + def test_sync_updates_happy_path(self): + """ + Can we sync updates from a content library block to a linked out-of-date course block? + """ + downstream = BlockFactory.create(category='html', parent=self.unit, upstream=str(self.upstream_key)) + + # Initial sync + sync_from_upstream(downstream, self.user) + assert downstream.upstream_version == 2 # Library blocks start at version 2 (v1 is the empty new block) + assert downstream.upstream_display_name == "Upstream Title V2" + assert downstream.display_name == "Upstream Title V2" + assert downstream.data == "Upstream content V2" + + # Upstream updates + upstream = xblock.load_block(self.upstream_key, self.user) + upstream.display_name = "Upstream Title V3" + upstream.data = "Upstream content V3" + upstream.save() + + # Follow-up sync. Assert that updates are pulled into downstream. + sync_from_upstream(downstream, self.user) + assert downstream.upstream_version == 3 + assert downstream.upstream_display_name == "Upstream Title V3" + assert downstream.display_name == "Upstream Title V3" + assert downstream.data == "Upstream content V3" + + def test_sync_updates_to_modified_content(self): + """ + If we sync to modified content, will it preserve customizable fields, but overwrite the rest? + """ + downstream = BlockFactory.create(category='html', parent=self.unit, upstream=str(self.upstream_key)) + + # Initial sync + sync_from_upstream(downstream, self.user) + assert downstream.upstream_display_name == "Upstream Title V2" + assert downstream.display_name == "Upstream Title V2" + assert downstream.data == "Upstream content V2" + + # Upstream updates + upstream = xblock.load_block(self.upstream_key, self.user) + upstream.display_name = "Upstream Title V3" + upstream.data = "Upstream content V3" + upstream.save() + + # Downstream modifications + downstream.display_name = "Downstream Title Override" # "safe" customization + downstream.data = "Downstream content override" # "unsafe" override + downstream.save() + + # Follow-up sync. Assert that updates are pulled into downstream, but customizations are saved. + sync_from_upstream(downstream, self.user) + assert downstream.upstream_display_name == "Upstream Title V3" + assert downstream.display_name == "Downstream Title Override" # "safe" customization survives + assert downstream.data == "Upstream content V3" # "unsafe" override is gone + + # For the Content Libraries Relaunch Beta, we do not yet need to support this edge case. + # See "PRESERVING DOWNSTREAM CUSTOMIZATIONS and RESTORING UPSTREAM DEFAULTS" in cms/lib/xblock/upstream_sync.py. + # + # def test_sync_to_downstream_with_subtle_customization(self): + # """ + # Edge case: If our downstream customizes a field, but then the upstream is changed to match the + # customization do we still remember that the downstream field is customized? That is, + # if the upstream later changes again, do we retain the downstream customization (rather than + # following the upstream update?) + # """ + # # Start with an uncustomized downstream block. + # downstream = BlockFactory.create(category='html', parent=self.unit, upstream=str(self.upstream_key)) + # sync_from_upstream(downstream, self.user) + # assert downstream.downstream_customized == [] + # assert downstream.display_name == downstream.upstream_display_name == "Upstream Title V2" + # + # # Then, customize our downstream title. + # downstream.display_name = "Title V3" + # downstream.save() + # assert downstream.downstream_customized == ["display_name"] + # + # # Syncing should retain the customization. + # sync_from_upstream(downstream, self.user) + # assert downstream.upstream_version == 2 + # assert downstream.upstream_display_name == "Upstream Title V2" + # assert downstream.display_name == "Title V3" + # + # # Whoa, look at that, the upstream has updated itself to the exact same title... + # upstream = xblock.load_block(self.upstream_key, self.user) + # upstream.display_name = "Title V3" + # upstream.save() + # + # # ...which is reflected when we sync. + # sync_from_upstream(downstream, self.user) + # assert downstream.upstream_version == 3 + # assert downstream.upstream_display_name == downstream.display_name == "Title V3" + # + # # But! Our downstream knows that its title is still customized. + # assert downstream.downstream_customized == ["display_name"] + # # So, if the upstream title changes again... + # upstream.display_name = "Title V4" + # upstream.save() + # + # # ...then the downstream title should remain put. + # sync_from_upstream(downstream, self.user) + # assert downstream.upstream_version == 4 + # assert downstream.upstream_display_name == "Title V4" + # assert downstream.display_name == "Title V3" + # + # # Finally, if we "de-customize" the display_name field, then it should go back to syncing normally. + # downstream.downstream_customized = [] + # upstream.display_name = "Title V5" + # upstream.save() + # sync_from_upstream(downstream, self.user) + # assert downstream.upstream_version == 5 + # assert downstream.upstream_display_name == downstream.display_name == "Title V5" + + @ddt.data(None, "Title From Some Other Upstream Version") + def test_fetch_customizable_fields(self, initial_upstream_display_name): + """ + Can we fetch a block's upstream field values without syncing it? + + Test both with and without a pre-"fetched" upstrema values on the downstream. + """ + downstream = BlockFactory.create(category='html', parent=self.unit) + downstream.upstream_display_name = initial_upstream_display_name + downstream.display_name = "Some Title" + downstream.data = "Some content" + + # Note that we're not linked to any upstream. fetch_customizable_fields shouldn't care. + assert not downstream.upstream + assert not downstream.upstream_version + + # fetch! + upstream = xblock.load_block(self.upstream_key, self.user) + fetch_customizable_fields(upstream=upstream, downstream=downstream, user=self.user) + + # Ensure: fetching doesn't affect the upstream link (or lack thereof). + assert not downstream.upstream + assert not downstream.upstream_version + + # Ensure: fetching doesn't affect actual content or settings. + assert downstream.display_name == "Some Title" + assert downstream.data == "Some content" + + # Ensure: fetching DOES set the upstream_* fields. + assert downstream.upstream_display_name == "Upstream Title V2" + + def test_prompt_and_decline_sync(self): + """ + Is the user prompted for sync when it's available? Does declining remove the prompt until a new sync is ready? + """ + # Initial conditions (pre-sync) + downstream = BlockFactory.create(category='html', parent=self.unit, upstream=str(self.upstream_key)) + link = UpstreamLink.get_for_block(downstream) + assert link.version_synced is None + assert link.version_declined is None + assert link.version_available == 2 # Library block with content starts at version 2 + assert link.ready_to_sync is True + + # Initial sync to V2 + sync_from_upstream(downstream, self.user) + link = UpstreamLink.get_for_block(downstream) + assert link.version_synced == 2 + assert link.version_declined is None + assert link.version_available == 2 + assert link.ready_to_sync is False + + # Upstream updated to V3 + upstream = xblock.load_block(self.upstream_key, self.user) + upstream.data = "Upstream content V3" + upstream.save() + link = UpstreamLink.get_for_block(downstream) + assert link.version_synced == 2 + assert link.version_declined is None + assert link.version_available == 3 + assert link.ready_to_sync is True + + # Decline to sync to V3 -- ready_to_sync becomes False. + decline_sync(downstream) + link = UpstreamLink.get_for_block(downstream) + assert link.version_synced == 2 + assert link.version_declined == 3 + assert link.version_available == 3 + assert link.ready_to_sync is False + + # Upstream updated to V4 -- ready_to_sync becomes True again. + upstream = xblock.load_block(self.upstream_key, self.user) + upstream.data = "Upstream content V4" + upstream.save() + link = UpstreamLink.get_for_block(downstream) + assert link.version_synced == 2 + assert link.version_declined == 3 + assert link.version_available == 4 + assert link.ready_to_sync is True + + def test_sever_upstream_link(self): + """ + Does sever_upstream_link correctly disconnect a block from its upstream? + """ + # Start with a course block that is linked+synced to a content library block. + downstream = BlockFactory.create(category='html', parent=self.unit, upstream=str(self.upstream_key)) + sync_from_upstream(downstream, self.user) + + # (sanity checks) + assert downstream.upstream == str(self.upstream_key) + assert downstream.upstream_version == 2 + assert downstream.upstream_display_name == "Upstream Title V2" + assert downstream.display_name == "Upstream Title V2" + assert downstream.data == "Upstream content V2" + assert downstream.copied_from_block is None + + # Now, disconnect the course block. + sever_upstream_link(downstream) + + # All upstream metadata has been wiped out. + assert downstream.upstream is None + assert downstream.upstream_version is None + assert downstream.upstream_display_name is None + + # BUT, the content which was synced into the upstream remains. + assert downstream.display_name == "Upstream Title V2" + assert downstream.data == "Upstream content V2" + + # AND, we have recorded the old upstream as our copied_from_block. + assert downstream.copied_from_block == str(self.upstream_key) diff --git a/cms/lib/xblock/upstream_sync.py b/cms/lib/xblock/upstream_sync.py new file mode 100644 index 000000000000..785cc7dc7e36 --- /dev/null +++ b/cms/lib/xblock/upstream_sync.py @@ -0,0 +1,451 @@ +""" +Synchronize content and settings from upstream blocks to their downstream usages. + +At the time of writing, we assume that for any upstream-downstream linkage: +* The upstream is a Component from a Learning Core-backed Content Library. +* The downstream is a block of matching type in a SplitModuleStore-backed Course. +* They are both on the same Open edX instance. + +HOWEVER, those assumptions may loosen in the future. So, we consider these to be INTERNAL ASSUMPIONS that should not be +exposed through this module's public Python interface. +""" +from __future__ import annotations + +import logging +import typing as t +from dataclasses import dataclass, asdict + +from django.core.exceptions import PermissionDenied +from django.utils.translation import gettext_lazy as _ +from rest_framework.exceptions import NotFound +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.locator import LibraryUsageLocatorV2 +from xblock.exceptions import XBlockNotFoundError +from xblock.fields import Scope, String, Integer +from xblock.core import XBlockMixin, XBlock + +if t.TYPE_CHECKING: + from django.contrib.auth.models import User # pylint: disable=imported-auth-user + + +logger = logging.getLogger(__name__) + + +class UpstreamLinkException(Exception): + """ + Raised whenever we try to inspect, sync-from, fetch-from, or delete a block's link to upstream content. + + There are three flavors (defined below): BadDownstream, BadUpstream, NoUpstream. + + Should be constructed with a human-friendly, localized, PII-free message, suitable for API responses and UI display. + For now, at least, the message can assume that upstreams are Content Library blocks and downstreams are Course + blocks, although that may need to change (see module docstring). + """ + + +class BadDownstream(UpstreamLinkException): + """ + Downstream content does not support sync. + """ + + +class BadUpstream(UpstreamLinkException): + """ + Reference to upstream content is malformed, invalid, and/or inaccessible. + """ + + +class NoUpstream(UpstreamLinkException): + """ + The downstream content does not have an upstream link at all (...as is the case for most XBlocks usages). + + (This isn't so much an "error" like the other two-- it's just a case that needs to be handled exceptionally, + usually by logging a message and then doing nothing.) + """ + def __init__(self): + super().__init__(_("Content is not linked to a Content Library.")) + + +@dataclass(frozen=True) +class UpstreamLink: + """ + Metadata about some downstream content's relationship with its linked upstream content. + """ + upstream_ref: str | None # Reference to the upstream content, e.g., a serialized library block usage key. + 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. + + @property + def ready_to_sync(self) -> bool: + """ + Should we invite the downstream's authors to sync the latest upstream updates? + """ + return bool( + self.upstream_ref and + self.version_available and + self.version_available > (self.version_synced or 0) and + self.version_available > (self.version_declined or 0) + ) + + def to_json(self) -> dict[str, t.Any]: + """ + Get an JSON-API-friendly representation of this upstream link. + """ + return { + **asdict(self), + "ready_to_sync": self.ready_to_sync, + } + + @classmethod + def try_get_for_block(cls, downstream: XBlock) -> t.Self: + """ + Same as `get_for_block`, but upon failure, sets `.error_message` instead of raising an exception. + """ + try: + return cls.get_for_block(downstream) + except UpstreamLinkException as exc: + logger.exception( + "Tried to inspect an unsupported, broken, or missing downstream->upstream link: '%s'->'%s'", + downstream.usage_key, + downstream.upstream, + ) + return cls( + upstream_ref=downstream.upstream, + version_synced=downstream.upstream_version, + version_available=None, + version_declined=None, + error_message=str(exc), + ) + + @classmethod + def get_for_block(cls, downstream: XBlock) -> t.Self: + """ + Get info on a block's relationship with its linked upstream content (without actually loading the content). + + Currently, the only supported upstreams are LC-backed Library Components. This may change in the future (see + module docstring). + + If link exists, is supported, and is followable, returns UpstreamLink. + Otherwise, raises an UpstreamLinkException. + """ + if not downstream.upstream: + raise NoUpstream() + if not isinstance(downstream.usage_key.context_key, CourseKey): + raise BadDownstream(_("Cannot update content because it does not belong to a course.")) + if downstream.has_children: + raise BadDownstream(_("Updating content with children is not yet supported.")) + try: + upstream_key = LibraryUsageLocatorV2.from_string(downstream.upstream) + except InvalidKeyError as exc: + raise BadUpstream(_("Reference to linked library item is malformed")) from exc + downstream_type = downstream.usage_key.block_type + if upstream_key.block_type != downstream_type: + # Note: Currently, we strictly enforce that the downstream and upstream block_types must exactly match. + # It could be reasonable to relax this requirement in the future if there's product need for it. + # For example, there's no reason that a StaticTabBlock couldn't take updates from an HtmlBlock. + raise BadUpstream( + _("Content type mismatch: {downstream_type} cannot be linked to {upstream_type}.").format( + downstream_type=downstream_type, upstream_type=upstream_key.block_type + ) + ) from TypeError( + f"downstream block '{downstream.usage_key}' is linked to " + f"upstream block of different type '{upstream_key}'" + ) + # We import this here b/c UpstreamSyncMixin is used by cms/envs, which loads before the djangoapps are ready. + from openedx.core.djangoapps.content_libraries.api import ( + get_library_block # pylint: disable=wrong-import-order + ) + try: + lib_meta = get_library_block(upstream_key) + except XBlockNotFoundError as exc: + raise BadUpstream(_("Linked library item was not found in the system")) from exc + return cls( + upstream_ref=downstream.upstream, + version_synced=downstream.upstream_version, + version_available=(lib_meta.draft_version_num if lib_meta else None), + # TODO: Previous line is wrong. It should use the published version instead, but the + # LearningCoreXBlockRuntime APIs do not yet support published content yet. + # Will be fixed in a follow-up task: https://github.com/openedx/edx-platform/issues/35582 + # version_available=(lib_meta.published_version_num if lib_meta else None), + version_declined=downstream.upstream_version_declined, + error_message=None, + ) + + +def sync_from_upstream(downstream: XBlock, user: User) -> None: + """ + Update `downstream` with content+settings from the latest available version of its linked upstream content. + + Preserves overrides to customizable fields; overwrites overrides to other fields. + Does not save `downstream` to the store. That is left up to the caller. + + If `downstream` lacks a valid+supported upstream link, this raises an UpstreamLinkException. + """ + link, upstream = _load_upstream_link_and_block(downstream, user) + _update_customizable_fields(upstream=upstream, downstream=downstream, only_fetch=False) + _update_non_customizable_fields(upstream=upstream, downstream=downstream) + downstream.upstream_version = link.version_available + + +def fetch_customizable_fields(*, downstream: XBlock, user: User, upstream: XBlock | None = None) -> None: + """ + Fetch upstream-defined value of customizable fields and save them on the downstream. + + If `upstream` is provided, use that block as the upstream. + Otherwise, load the block specified by `downstream.upstream`, which may raise an UpstreamLinkException. + """ + if not upstream: + _link, upstream = _load_upstream_link_and_block(downstream, user) + _update_customizable_fields(upstream=upstream, downstream=downstream, only_fetch=True) + + +def _load_upstream_link_and_block(downstream: XBlock, user: User) -> tuple[UpstreamLink, XBlock]: + """ + Load the upstream metadata and content for a downstream block. + + Assumes that the upstream content is an XBlock in an LC-backed content libraries. This assumption may need to be + relaxed in the future (see module docstring). + + If `downstream` lacks a valid+supported upstream link, this raises an UpstreamLinkException. + """ + link = UpstreamLink.get_for_block(downstream) # can raise UpstreamLinkException + # We import load_block here b/c UpstreamSyncMixin is used by cms/envs, which loads before the djangoapps are ready. + from openedx.core.djangoapps.xblock.api import load_block # pylint: disable=wrong-import-order + try: + lib_block: XBlock = load_block(LibraryUsageLocatorV2.from_string(downstream.upstream), user) + except (NotFound, PermissionDenied) as exc: + raise BadUpstream(_("Linked library item could not be loaded: {}").format(downstream.upstream)) from exc + return link, lib_block + + +def _update_customizable_fields(*, upstream: XBlock, downstream: XBlock, only_fetch: bool) -> None: + """ + For each customizable field: + * Save the upstream value to a hidden field on the downstream ("FETCH"). + * If `not only_fetch`, and if the field *isn't* customized on the downstream, then: + * Update it the downstream field's value from the upstream field ("SYNC"). + + Concrete example: Imagine `lib_problem` is our upstream and `course_problem` is our downstream. + + * Say that the customizable fields are [display_name, max_attempts]. + + * Set `course_problem.upstream_display_name = lib_problem.display_name` ("fetch"). + * If `not only_fetch`, and `course_problem.display_name` wasn't customized, then: + * Set `course_problem.display_name = lib_problem.display_name` ("sync"). + + * Set `course_problem.upstream_max_attempts = lib_problem.max_attempts` ("fetch"). + * If `not only_fetch`, and `course_problem.max_attempts` wasn't customized, then: + * Set `course_problem.max_attempts = lib_problem.max_attempts` ("sync"). + """ + syncable_field_names = _get_synchronizable_fields(upstream, downstream) + + for field_name, fetch_field_name in downstream.get_customizable_fields().items(): + + if field_name not in syncable_field_names: + continue + + # FETCH the upstream's value and save it on the downstream (ie, `downstream.upstream_$FIELD`). + old_upstream_value = getattr(downstream, fetch_field_name) + new_upstream_value = getattr(upstream, field_name) + setattr(downstream, fetch_field_name, new_upstream_value) + + if only_fetch: + continue + + # Okay, now for the nuanced part... + # We need to update the downstream field *iff it has not been customized**. + # Determining whether a field has been customized will differ in Beta vs Future release. + # (See "PRESERVING DOWNSTREAM CUSTOMIZATIONS" comment below for details.) + + ## FUTURE BEHAVIOR: field is "customized" iff we have noticed that the user edited it. + # if field_name in downstream.downstream_customized: + # continue + + ## BETA BEHAVIOR: field is "customized" iff we have the prev upstream value, but field doesn't match it. + downstream_value = getattr(downstream, field_name) + if old_upstream_value and downstream_value != old_upstream_value: + continue # Field has been customized. Don't touch it. Move on. + + # Field isn't customized -- SYNC it! + setattr(downstream, field_name, new_upstream_value) + + +def _update_non_customizable_fields(*, upstream: XBlock, downstream: XBlock) -> None: + """ + For each field `downstream.blah` that isn't customizable: set it to `upstream.blah`. + """ + syncable_fields = _get_synchronizable_fields(upstream, downstream) + customizable_fields = set(downstream.get_customizable_fields().keys()) + for field_name in syncable_fields - customizable_fields: + new_upstream_value = getattr(upstream, field_name) + setattr(downstream, field_name, new_upstream_value) + + +def _get_synchronizable_fields(upstream: XBlock, downstream: XBlock) -> set[str]: + """ + The syncable fields are the ones which are content- or settings-scoped AND are defined on both (up,down)stream. + """ + return set.intersection(*[ + set( + field_name + for (field_name, field) in block.__class__.fields.items() + if field.scope in [Scope.settings, Scope.content] + ) + for block in [upstream, downstream] + ]) + + +def decline_sync(downstream: XBlock) -> 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. + + 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 + + +def sever_upstream_link(downstream: XBlock) -> None: + """ + Given an XBlock that is linked to upstream content, disconnect the link, such that authors are never again prompted + to sync upstream updates. Erase all `.upstream*` fields from the downtream block. + + However, before nulling out the `.upstream` field, we copy its value over to `.copied_from_block`. This makes sense, + because once a downstream block has been de-linked from source (e.g., a Content Library block), it is no different + than if the block had just been copy-pasted in the first place. + + Does not save `downstream` to the store. That is left up to the caller. + + If `downstream` lacks a link, then this raises NoUpstream (though it is reasonable for callers to handle such + exception and ignore it, as the end result is the same: `downstream.upstream is None`). + """ + if not downstream.upstream: + raise NoUpstream() + downstream.copied_from_block = downstream.upstream + downstream.upstream = None + downstream.upstream_version = None + for _, fetched_upstream_field in downstream.get_customizable_fields().items(): + setattr(downstream, fetched_upstream_field, None) # Null out upstream_display_name, et al. + + +class UpstreamSyncMixin(XBlockMixin): + """ + Allows an XBlock in the CMS to be associated & synced with an upstream. + + Mixed into CMS's XBLOCK_MIXINS, but not LMS's. + """ + + # Upstream synchronization metadata fields + upstream = String( + help=( + "The usage key of a block (generally within a content library) which serves as a source of upstream " + "updates for this block, or None if there is no such upstream. Please note: It is valid for this " + "field to hold a usage key for an upstream block that does not exist (or does not *yet* exist) on " + "this instance, particularly if this downstream block was imported from a different instance." + ), + default=None, scope=Scope.settings, hidden=True, enforce_type=True + ) + upstream_version = Integer( + help=( + "Record of the upstream block's version number at the time this block was created from it. If this " + "upstream_version is smaller than the upstream block's latest published version, then the author will be " + "invited to sync updates into this downstream block, presuming that they have not already declined to sync " + "said version." + ), + default=None, scope=Scope.settings, hidden=True, enforce_type=True, + ) + upstream_version_declined = Integer( + help=( + "Record of the latest upstream version for which the author declined to sync updates, or None if they have " + "never declined an update." + ), + default=None, scope=Scope.settings, hidden=True, enforce_type=True, + ) + + # Store the fetched upstream values for customizable fields. + upstream_display_name = String( + help=("The value of display_name on the linked upstream block."), + default=None, scope=Scope.settings, hidden=True, enforce_type=True, + ) + upstream_max_attempts = Integer( + help=("The value of max_attempts on the linked upstream block."), + default=None, scope=Scope.settings, hidden=True, enforce_type=True, + ) + + @classmethod + def get_customizable_fields(cls) -> dict[str, str]: + """ + Mapping from each customizable field to the field which can be used to restore its upstream value. + + XBlocks outside of edx-platform can override this in order to set up their own customizable fields. + """ + return { + "display_name": "upstream_display_name", + "max_attempts": "upstream_max_attempts", + } + + # PRESERVING DOWNSTREAM CUSTOMIZATIONS and RESTORING UPSTREAM VALUES + # + # For the full Content Libraries Relaunch, we would like to keep track of which customizable fields the user has + # actually customized. The idea is: once an author has customized a customizable field.... + # + # - future upstream syncs will NOT blow away the customization, + # - but future upstream syncs WILL fetch the upstream values and tuck them away in a hidden field, + # - and the author can can revert back to said fetched upstream value at any point. + # + # Now, whether field is "customized" (and thus "revertible") is dependent on whether they have ever edited it. + # To instrument this, we need to keep track of which customizable fields have been edited using a new XBlock field: + # `downstream_customized` + # + # Implementing `downstream_customized` has proven difficult, because there is no simple way to keep it up-to-date + # with the many different ways XBlock fields can change. The `.save()` and `.editor_saved()` methods are promising, + # but we need to do more due diligence to be sure that they cover all cases, including API edits, import/export, + # copy/paste, etc. We will figure this out in time for the full Content Libraries Relaunch (related ticket: + # https://github.com/openedx/frontend-app-authoring/issues/1317). But, for the Beta realease, we're going to + # implement something simpler: + # + # - We fetch upstream values for customizable fields and tuck them away in a hidden field (same as above). + # - If a customizable field DOES match the fetched upstream value, then future upstream syncs DO update it. + # - If a customizable field does NOT the fetched upstream value, then future upstream syncs DO NOT update it. + # - There is no UI option for explicitly reverting back to the fetched upstream value. + # + # For future reference, here is a partial implementation of what we are thinking for the full Content Libraries + # Relaunch:: + # + # downstream_customized = List( + # help=( + # "Names of the fields which have values set on the upstream block yet have been explicitly " + # "overridden on this downstream block. Unless explicitly cleared by the user, these customizations " + # "will persist even when updates are synced from the upstream." + # ), + # default=[], scope=Scope.settings, hidden=True, enforce_type=True, + # ) + # + # def save(self, *args, **kwargs): + # """ + # Update `downstream_customized` when a customizable field is modified. + # + # NOTE: This does not work, because save() isn't actually called in all the cases that we'd want it to be. + # """ + # super().save(*args, **kwargs) + # customizable_fields = self.get_customizable_fields() + # + # # Loop through all the fields that are potentially cutomizable. + # for field_name, restore_field_name in self.get_customizable_fields(): + # + # # If the field is already marked as customized, then move on so that we don't + # # unneccessarily query the block for its current value. + # if field_name in self.downstream_customized: + # continue + # + # # If this field's value doesn't match the synced upstream value, then mark the field + # # as customized so that we don't clobber it later when syncing. + # # NOTE: Need to consider the performance impact of all these field lookups. + # if getattr(self, field_name) != getattr(self, restore_field_name): + # self.downstream_customized.append(field_name) diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index 282629456633..0ee1099ca982 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -8,6 +8,7 @@ dump_js_escaped_json, js_escaped_string ) from cms.djangoapps.contentstore.toggles import use_new_text_editor, use_new_problem_editor, use_new_video_editor, use_video_gallery_flow +from cms.lib.xblock.upstream_sync import UpstreamLink from openedx.core.djangoapps.content_tagging.toggles import is_tagging_feature_disabled %> <% @@ -23,6 +24,8 @@ messages = xblock.validate().to_json() has_not_configured_message = messages.get('summary',{}).get('type', None) == 'not-configured' block_is_unit = is_unit(xblock) + +upstream_info = UpstreamLink.try_get_for_block(xblock) %> <%namespace name='static' file='static_content.html'/> @@ -103,7 +106,22 @@ ${label} % else: - ${label} + % if upstream_info.upstream_ref: + % if upstream_info.error_message: + + + ${_("Sourced from a library - but the upstream link is broken/invalid.")} + % else: + + + ${_("Sourced from a library.")} + % endif + % endif + ${label} % endif % if selected_groups_label:

${selected_groups_label}

@@ -114,6 +132,18 @@