From d82aadab516ed67684e2661fa1d04e5ff4273992 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Thu, 7 Nov 2024 20:24:53 +0530 Subject: [PATCH] fix: set upstream link for re-copied block from course originally from library (#35784) Sets upstream link to library block for blocks that were copied from a course block which were originally copied/imported from a library. --- cms/djangoapps/contentstore/helpers.py | 77 +++++++++++++------ .../views/tests/test_clipboard_paste.py | 48 +++++++----- 2 files changed, 81 insertions(+), 44 deletions(-) diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index e67e337e55fa..8ff1f1aa39cc 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -24,7 +24,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 cms.lib.xblock.upstream_sync import UpstreamLink, UpstreamLinkException, 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 @@ -323,6 +323,56 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) -> return new_xblock, notices +def _fetch_and_set_upstream_link( + copied_from_block: str, + copied_from_version_num: int, + temp_xblock: XBlock, + user: User +): + """ + Fetch and set upstream link for the given xblock. This function handles following cases: + * the xblock is copied from a v2 library; the library block is set as upstream. + * the xblock is copied from a course; no upstream is set, only copied_from_block is set. + * the xblock is copied from a course where the source block was imported from a library; the original libary block + is set as upstream. + """ + # 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 UpstreamLinkException: + # 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). + + # In case if the source block was imported from a library, we need to check its upstream + # and set the same upstream link for the new block. + source_descriptor = modulestore().get_item(UsageKey.from_string(copied_from_block)) + if source_descriptor.upstream: + _fetch_and_set_upstream_link( + source_descriptor.upstream, + source_descriptor.upstream_version, + temp_xblock, + user, + ) + else: + # else 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) + + def _import_xml_node_to_parent( node, parent_xblock: XBlock, @@ -404,28 +454,7 @@ 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: - # 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) - + _fetch_and_set_upstream_link(copied_from_block, copied_from_version_num, temp_xblock, 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) parent_xblock.children.append(new_xblock.location) @@ -436,7 +465,7 @@ def _import_xml_node_to_parent( # Allow an XBlock to do anything fancy it may need to when pasted from the clipboard. # These blocks may handle their own children or parenting if needed. Let them return booleans to # let us know if we need to handle these or not. - children_handed = new_xblock.studio_post_paste(store, node) + children_handled = new_xblock.studio_post_paste(store, node) if not children_handled: for child_node in child_nodes: diff --git a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py index 68771ceaec04..9244ffa989b6 100644 --- a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py +++ b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py @@ -455,26 +455,6 @@ def setUp(self): self.lib_block_tags = ['tag_1', 'tag_5'] tagging_api.tag_object(str(self.lib_block_key), taxonomy_all_org, self.lib_block_tags) - 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 - def test_paste_from_library_read_only_tags(self): """ When we copy a v2 lib block into a course, the dest block should have read-only copied tags. @@ -555,6 +535,34 @@ def test_paste_from_library_copies_asset(self): assert image_asset.name == "1px.webp" assert image_asset.length == len(webp_raw_data) + def test_paste_from_course_block_imported_from_library_creates_link(self): + """ + When we copy a course xblock which was imported or copied from v2 lib block into a course, + the dest block should be linked up to the original lib block. + """ + def _copy_paste_and_assert_link(key_to_copy): + copy_response = self.client.post(CLIPBOARD_ENDPOINT, {"usage_key": str(key_to_copy)}, 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 + return new_block_key + + # first verify link for copied block from library + new_block_key = _copy_paste_and_assert_link(self.lib_block_key) + # next verify link for copied block from the pasted block + _copy_paste_and_assert_link(new_block_key) + class ClipboardPasteFromV1LibraryTestCase(ModuleStoreTestCase): """