Skip to content

Commit

Permalink
fix: error pasting a unit containing a completely blank problem
Browse files Browse the repository at this point in the history
  • Loading branch information
bradenmacdonald committed Dec 7, 2023
1 parent dbdf335 commit 5c7cf44
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 8 deletions.
18 changes: 10 additions & 8 deletions cms/djangoapps/contentstore/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,16 @@ def _import_xml_node_to_parent(
# and VAL will thus make the transcript available.

child_nodes = []

if issubclass(xblock_class, XmlMixin):
# Hack: XBlocks that use "XmlMixin" have their own XML parsing behavior, and in particular if they encounter
# an XML node that has no children and has only a "url_name" attribute, they'll try to load the XML data
# from an XML file in runtime.resources_fs. But that file doesn't exist here. So we set at least one
# additional attribute here to make sure that url_name is not the only attribute; otherwise in some cases,
# XmlMixin.parse_xml will try to load an XML file that doesn't exist, giving an error. The name and value
# of this attribute don't matter and should be ignored.
node.attrib["x-is-pointer-node"] = "no"

if not xblock_class.has_children:
# No children to worry about. The XML may contain child nodes, but they're not XBlocks.
temp_xblock = xblock_class.parse_xml(node, runtime, keys, id_generator)
Expand All @@ -314,14 +324,6 @@ def _import_xml_node_to_parent(
# serialization of a child block, in order. For blocks that don't support children, their XML content/nodes
# could be anything (e.g. HTML, capa)
node_without_children = etree.Element(node.tag, **node.attrib)
if issubclass(xblock_class, XmlMixin):
# Hack: XBlocks that use "XmlMixin" have their own XML parsing behavior, and in particular if they encounter
# an XML node that has no children and has only a "url_name" attribute, they'll try to load the XML data
# from an XML file in runtime.resources_fs. But that file doesn't exist here. So we set at least one
# additional attribute here to make sure that url_name is not the only attribute; otherwise in some cases,
# XmlMixin.parse_xml will try to load an XML file that doesn't exist, giving an error. The name and value
# of this attribute don't matter and should be ignored.
node_without_children.attrib["x-is-pointer-node"] = "no"
temp_xblock = xblock_class.parse_xml(node_without_children, runtime, keys, id_generator)
child_nodes = list(node)
if xblock_class.has_children and temp_xblock.children:
Expand Down
37 changes: 37 additions & 0 deletions cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
allow users to paste XBlocks that were copied using the staged_content/clipboard
APIs.
"""
import ddt
from opaque_keys.edx.keys import UsageKey
from rest_framework.test import APIClient
from xmodule.modulestore.django import contentstore
Expand All @@ -13,6 +14,7 @@
XBLOCK_ENDPOINT = "/xblock/"


@ddt.ddt
class ClipboardPasteTestCase(ModuleStoreTestCase):
"""
Test Clipboard Paste functionality
Expand Down Expand Up @@ -99,6 +101,41 @@ def test_copy_and_paste_unit(self):
# The new block should store a reference to where it was copied from
assert dest_unit.copied_from_block == str(unit_key)

@ddt.data(
# A problem with absolutely no fields set. A previous version of copy-paste had an error when pasting this.
{"category": "problem", "display_name": None, "data": ""},
)
def test_copy_and_paste_component(self, block_args):
"""
Test copying a component (XBlock) from one course into another
"""
source_course = CourseFactory.create(display_name='Destination Course')
source_block = BlockFactory.create(parent_location=source_course.location, **block_args)

dest_course = CourseFactory.create(display_name='Destination Course')
with self.store.bulk_operations(dest_course.id):
dest_chapter = BlockFactory.create(parent=dest_course, category='chapter', display_name='Section')
dest_sequential = BlockFactory.create(parent=dest_chapter, category='sequential', display_name='Subsection')

# Copy the block
client = APIClient()
client.login(username=self.user.username, password=self.user_password)
copy_response = client.post(CLIPBOARD_ENDPOINT, {"usage_key": str(source_block.location)}, format="json")
assert copy_response.status_code == 200

# Paste the unit
paste_response = client.post(XBLOCK_ENDPOINT, {
"parent_locator": str(dest_sequential.location),
"staged_content": "clipboard",
}, format="json")
assert paste_response.status_code == 200
dest_block_key = UsageKey.from_string(paste_response.json()["locator"])

dest_block = self.store.get_item(dest_block_key)
assert dest_block.display_name == source_block.display_name
# The new block should store a reference to where it was copied from
assert dest_block.copied_from_block == str(source_block.location)

def test_paste_with_assets(self):
"""
When pasting into a different course, any required static assets should
Expand Down

0 comments on commit 5c7cf44

Please sign in to comment.