Skip to content

Commit e2526cf

Browse files
authored
feat: intra library container copy [FC-0097] (#37483)
Allows pasting sections and subsections in libraries, adding support for intra-library copy and paste
1 parent 0c214db commit e2526cf

File tree

2 files changed

+193
-43
lines changed

2 files changed

+193
-43
lines changed

openedx/core/djangoapps/content_libraries/api/blocks.py

Lines changed: 117 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@
1919
from django.utils.text import slugify
2020
from django.utils.translation import gettext as _
2121
from lxml import etree
22-
from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2
22+
from opaque_keys import InvalidKeyError
23+
from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryUsageLocatorV2
2324
from opaque_keys.edx.keys import LearningContextKey, UsageKeyV2
2425
from openedx_events.content_authoring.data import (
2526
ContentObjectChangedData,
@@ -469,22 +470,36 @@ def _import_staged_block(
469470
return get_library_block(usage_key)
470471

471472

473+
def _is_container(block_type: str) -> bool:
474+
"""
475+
Return True if the block type is a container.
476+
"""
477+
return block_type in ["vertical", "sequential", "chapter"]
478+
479+
472480
def _import_staged_block_as_container(
473-
olx_str: str,
474481
library_key: LibraryLocatorV2,
475482
source_context_key: LearningContextKey,
476483
user,
477484
staged_content_id: int,
478485
staged_content_files: list[StagedContentFileData],
479486
now: datetime,
487+
*,
488+
olx_str: str | None = None,
489+
olx_node: etree.Element | None = None,
490+
copied_from_map: dict[str, LibraryUsageLocatorV2 | LibraryContainerLocator] | None = None,
480491
) -> ContainerMetadata:
481492
"""
482493
Convert the given XBlock (e.g. "vertical") to a Container (e.g. Unit) and
483494
import it into the library, along with all its child XBlocks.
484495
"""
485-
olx_node = etree.fromstring(olx_str)
486-
if olx_node.tag != "vertical":
487-
raise ValueError("This method is only designed to work with <vertical> XBlocks (units).")
496+
if olx_node is None:
497+
if olx_str is None:
498+
raise ValueError("Either olx_str or olx_node must be provided")
499+
olx_node = etree.fromstring(olx_str)
500+
501+
assert olx_node is not None # This assert to make sure olx_node has the correct type
502+
488503
# The olx_str looks like this:
489504
# <vertical><block1>...[XML]...</block1><block2>...[XML]...</block2>...</vertical>
490505
# Ideally we could split it up and preserve the strings, but that is difficult to do correctly, so we'll split
@@ -493,34 +508,91 @@ def _import_staged_block_as_container(
493508

494509
title = _title_from_olx_node(olx_node)
495510

496-
# Start an atomic section so the whole paste succeeds or fails together:
497-
with transaction.atomic():
498-
container = create_container(
499-
library_key=library_key,
500-
container_type=ContainerType.Unit,
501-
slug=None, # auto-generate slug from title
502-
title=title,
503-
user_id=user.id,
504-
)
505-
new_child_keys: list[LibraryUsageLocatorV2] = []
506-
for child_node in olx_node:
511+
container = create_container(
512+
library_key=library_key,
513+
container_type=ContainerType.from_source_olx_tag(olx_node.tag),
514+
slug=None, # auto-generate slug from title
515+
title=title,
516+
user_id=user.id,
517+
)
518+
519+
# Keep track of which blocks were copied from the library, so we don't duplicate them
520+
if copied_from_map is None:
521+
copied_from_map = {}
522+
523+
# Handle children
524+
new_child_keys: list[LibraryUsageLocatorV2 | LibraryContainerLocator] = []
525+
for child_node in olx_node:
526+
child_is_container = _is_container(child_node.tag)
527+
copied_from_block = child_node.attrib.get('copied_from_block', None)
528+
if copied_from_block:
529+
# Get the key of the child block
507530
try:
508-
child_metadata = _import_staged_block(
509-
block_type=child_node.tag,
510-
olx_str=etree.tostring(child_node, encoding='unicode'),
511-
library_key=library_key,
512-
source_context_key=source_context_key,
513-
user=user,
514-
staged_content_id=staged_content_id,
515-
staged_content_files=staged_content_files,
516-
now=now,
517-
)
518-
new_child_keys.append(child_metadata.usage_key)
519-
except IncompatibleTypesError:
520-
continue # Skip blocks that won't work in libraries
521-
update_container_children(container.container_key, new_child_keys, user_id=user.id)
522-
# Re-fetch the container because the 'last_draft_created' will have changed when we added children
523-
container = get_container(container.container_key)
531+
child_key: LibraryContainerLocator | LibraryUsageLocatorV2
532+
if child_is_container:
533+
child_key = LibraryContainerLocator.from_string(copied_from_block)
534+
else:
535+
child_key = LibraryUsageLocatorV2.from_string(copied_from_block)
536+
537+
if child_key.context_key == library_key:
538+
# This is a block that was copied from the library, so we just link it to the container
539+
new_child_keys.append(child_key)
540+
continue
541+
542+
except InvalidKeyError:
543+
# This is a XBlock copied from a course, so we need to create a new copy of it.
544+
pass
545+
546+
# This block is not copied from a course, or it was copied from a different library.
547+
# We need to create a new copy of it.
548+
if child_is_container:
549+
if copied_from_block in copied_from_map:
550+
# This container was already copied from the library, so we just link it to the container
551+
new_child_keys.append(copied_from_map[copied_from_block])
552+
continue
553+
554+
child_container = _import_staged_block_as_container(
555+
library_key=library_key,
556+
source_context_key=source_context_key,
557+
user=user,
558+
staged_content_id=staged_content_id,
559+
staged_content_files=staged_content_files,
560+
now=now,
561+
olx_node=child_node,
562+
copied_from_map=copied_from_map,
563+
)
564+
if copied_from_block:
565+
copied_from_map[copied_from_block] = child_container.container_key
566+
new_child_keys.append(child_container.container_key)
567+
continue
568+
569+
# This is not a container, so we import it as a standalone block
570+
try:
571+
if copied_from_block in copied_from_map:
572+
# This block was already copied from the library, so we just link it to the container
573+
new_child_keys.append(copied_from_map[copied_from_block])
574+
continue
575+
576+
child_metadata = _import_staged_block(
577+
block_type=child_node.tag,
578+
olx_str=etree.tostring(child_node, encoding='unicode'),
579+
library_key=library_key,
580+
source_context_key=source_context_key,
581+
user=user,
582+
staged_content_id=staged_content_id,
583+
staged_content_files=staged_content_files,
584+
now=now,
585+
)
586+
if copied_from_block:
587+
copied_from_map[copied_from_block] = child_metadata.usage_key
588+
new_child_keys.append(child_metadata.usage_key)
589+
except IncompatibleTypesError:
590+
continue # Skip blocks that won't work in libraries
591+
592+
update_container_children(container.container_key, new_child_keys, user_id=user.id) # type: ignore[arg-type]
593+
# Re-fetch the container because the 'last_draft_created' will have changed when we added children
594+
container = get_container(container.container_key)
595+
524596
return container
525597

526598

@@ -548,17 +620,19 @@ def import_staged_content_from_user_clipboard(library_key: LibraryLocatorV2, use
548620

549621
now = datetime.now(tz=timezone.utc)
550622

551-
if user_clipboard.content.block_type == "vertical":
552-
# This is a Unit. To import it into a library, we have to create it as a container.
553-
return _import_staged_block_as_container(
554-
olx_str,
555-
library_key,
556-
source_context_key,
557-
user,
558-
staged_content_id,
559-
staged_content_files,
560-
now,
561-
)
623+
if _is_container(user_clipboard.content.block_type):
624+
# This is a container and we can import it as such.
625+
# Start an atomic section so the whole paste succeeds or fails together:
626+
with transaction.atomic():
627+
return _import_staged_block_as_container(
628+
library_key,
629+
source_context_key,
630+
user,
631+
staged_content_id,
632+
staged_content_files,
633+
now,
634+
olx_str=olx_str,
635+
)
562636
else:
563637
return _import_staged_block(
564638
user_clipboard.content.block_type,

openedx/core/djangoapps/content_libraries/tests/test_api.py

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1312,6 +1312,82 @@ def test_delete_component_and_revert(self) -> None:
13121312
},
13131313
)
13141314

1315+
def test_copy_and_paste_container_same_library(self) -> None:
1316+
# Copy a section with children
1317+
api.copy_container(self.section1.container_key, self.user.id)
1318+
# Paste the container
1319+
new_container: api.ContainerMetadata = (
1320+
api.import_staged_content_from_user_clipboard(self.lib1.library_key, self.user) # type: ignore[assignment]
1321+
)
1322+
1323+
# Verify that the container is copied
1324+
assert new_container.container_type == self.section1.container_type
1325+
assert new_container.display_name == self.section1.display_name
1326+
1327+
# Verify that the children are linked
1328+
subsections = api.get_container_children(new_container.container_key)
1329+
assert len(subsections) == 2
1330+
assert isinstance(subsections[0], api.ContainerMetadata)
1331+
assert subsections[0].container_key == self.subsection1.container_key
1332+
assert isinstance(subsections[1], api.ContainerMetadata)
1333+
assert subsections[1].container_key == self.subsection2.container_key
1334+
1335+
def test_copy_and_paste_container_another_library(self) -> None:
1336+
# Copy a section with children
1337+
api.copy_container(self.section1.container_key, self.user.id)
1338+
1339+
self._create_library("test-lib-cont-2", "Test Library 2")
1340+
lib2 = ContentLibrary.objects.get(slug="test-lib-cont-2")
1341+
# Paste the container
1342+
new_container: api.ContainerMetadata = (
1343+
api.import_staged_content_from_user_clipboard(lib2.library_key, self.user) # type: ignore[assignment]
1344+
)
1345+
1346+
# Verify that the container is copied
1347+
assert new_container.container_type == self.section1.container_type
1348+
assert new_container.display_name == self.section1.display_name
1349+
1350+
# Verify that the children are copied
1351+
subsections = api.get_container_children(new_container.container_key)
1352+
assert len(subsections) == 2
1353+
assert isinstance(subsections[0], api.ContainerMetadata)
1354+
assert subsections[0].container_key != self.subsection1.container_key # This subsection was copied
1355+
assert subsections[0].display_name == self.subsection1.display_name
1356+
units_subsection1 = api.get_container_children(subsections[0].container_key)
1357+
assert len(units_subsection1) == 2
1358+
assert isinstance(units_subsection1[0], api.ContainerMetadata)
1359+
assert units_subsection1[0].container_key != self.unit1.container_key # This unit was copied
1360+
assert units_subsection1[0].display_name == self.unit1.display_name == "Unit 1"
1361+
unit1_components = api.get_container_children(units_subsection1[0].container_key)
1362+
assert len(unit1_components) == 2
1363+
assert isinstance(unit1_components[0], api.LibraryXBlockMetadata)
1364+
assert unit1_components[0].usage_key != self.problem_block_usage_key # This component was copied
1365+
assert isinstance(unit1_components[1], api.LibraryXBlockMetadata)
1366+
assert unit1_components[1].usage_key != self.html_block_usage_key # This component was copied
1367+
1368+
assert isinstance(units_subsection1[1], api.ContainerMetadata)
1369+
assert units_subsection1[1].container_key != self.unit2.container_key # This unit was copied
1370+
assert units_subsection1[1].display_name == self.unit2.display_name == "Unit 2"
1371+
unit2_components = api.get_container_children(units_subsection1[1].container_key)
1372+
assert len(unit2_components) == 1
1373+
assert isinstance(unit2_components[0], api.LibraryXBlockMetadata)
1374+
assert unit2_components[0].usage_key != self.html_block_usage_key
1375+
1376+
# This is the same component, so it should not be duplicated
1377+
assert unit1_components[1].usage_key == unit2_components[0].usage_key
1378+
1379+
assert isinstance(subsections[1], api.ContainerMetadata)
1380+
assert subsections[1].container_key != self.subsection2.container_key # This subsection was copied
1381+
assert subsections[1].display_name == self.subsection2.display_name
1382+
units_subsection2 = api.get_container_children(subsections[1].container_key)
1383+
assert len(units_subsection2) == 1
1384+
assert isinstance(units_subsection2[0], api.ContainerMetadata)
1385+
assert units_subsection2[0].container_key != self.unit1.container_key # This unit was copied
1386+
assert units_subsection2[0].display_name == self.unit1.display_name
1387+
1388+
# This is the same unit, so it should not be duplicated
1389+
assert units_subsection1[0].container_key == units_subsection2[0].container_key
1390+
13151391

13161392
class ContentLibraryExportTest(ContentLibrariesRestApiTest):
13171393
"""

0 commit comments

Comments
 (0)