Skip to content

Commit c545556

Browse files
rtibblesbotclaude
andauthored
Remap Unit extra_fields.options node IDs when copying via copy_node (#5860)
* Remap Unit extra_fields.options node IDs when copying via copy_node When a Unit topic (UNIT modality) is cloned, lesson node IDs and pre/post test node IDs in extra_fields.options are rewritten to the IDs of their copies. Entries whose source nodes were not part of the copy operation (e.g. excluded via excluded_descendants) are dropped. Adds _remap_unit_options() static helper on CustomContentNodeTreeManager, calls it in _recurse_to_create_tree() for the deep-copy path and in _copy() for the shallow-copy path. _deep_copy() and _copy() now return (nodes, source_copy_id_map) so the map is available to callers; copy_node() unwraps the tuple and returns only the node list. * Test Unit extra_fields.options remapping across copy paths Adds UnitCopyExtraFieldsTestCase covering: - lesson_objectives keys remapped to cloned lesson PKs (deep and shallow) - pre_test/post_test values remapped to cloned node PKs (deep and shallow) - excluded lesson entry dropped from cloned Unit's lesson_objectives - excluded Unit: copy succeeds with no remap error (deep and shallow) - resource excluded inside lesson: lesson entry preserved (deep and shallow) - assessment_objectives, learning_objectives, completion_criteria unchanged - standalone Unit copy remaps its lesson children correctly - extra_fields on Course and Lesson topics is copied verbatim * Remove pre/post test remapping — Unit options contain no such node-ID keys The pre_test/post_test entries in a Unit's options are assessment items on the Unit node itself, not separate ContentNode IDs. Remove the dead code in _remap_unit_options() and the fantasy test coverage that accompanied it. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * test: drop test_standalone_unit_copy_remaps_lesson_children Reviewer noted this test is redundant — the same behaviour is already exercised by the deep/shallow copy integration tests for Unit nodes. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> --------- Co-authored-by: Claude Sonnet 4.6 <[email protected]>
1 parent d87d43f commit c545556

2 files changed

Lines changed: 427 additions & 6 deletions

File tree

contentcuration/contentcuration/db/models/manager.py

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from django.db.utils import OperationalError
1010
from django_cte import CTEQuerySet
1111
from le_utils.constants import content_kinds
12+
from le_utils.constants import modalities
1213
from mptt.managers import TreeManager
1314
from mptt.signals import node_moved
1415

@@ -359,6 +360,15 @@ def _recurse_to_create_tree(
359360
)
360361
)
361362
source_copy_id_map[source.id] = copy["id"]
363+
364+
if self._is_unit_topic(source.kind_id, copy.get("extra_fields")):
365+
copy["extra_fields"] = {
366+
**copy["extra_fields"],
367+
"options": self._remap_unit_options(
368+
copy["extra_fields"]["options"], source_copy_id_map
369+
),
370+
}
371+
362372
return copy
363373

364374
def _all_nodes_to_copy(self, node, excluded_descendants):
@@ -394,7 +404,7 @@ def copy_node(
394404
if progress_tracker:
395405
progress_tracker.set_total(total_nodes)
396406

397-
return self._copy(
407+
nodes, _ = self._copy(
398408
node,
399409
target,
400410
position,
@@ -406,6 +416,7 @@ def copy_node(
406416
batch_size,
407417
progress_tracker=progress_tracker,
408418
)
419+
return nodes
409420

410421
def _copy(
411422
self,
@@ -424,7 +435,7 @@ def _copy(
424435
:type progress_tracker: contentcuration.utils.celery.ProgressTracker|None
425436
"""
426437
if node.rght - node.lft < batch_size:
427-
copied_nodes = self._deep_copy(
438+
copied_nodes, node_map = self._deep_copy(
428439
node,
429440
target,
430441
position,
@@ -436,7 +447,7 @@ def _copy(
436447
)
437448
if progress_tracker:
438449
progress_tracker.increment(len(copied_nodes))
439-
return copied_nodes
450+
return copied_nodes, node_map
440451
node_copy = self._shallow_copy(
441452
node,
442453
target,
@@ -451,8 +462,9 @@ def _copy(
451462
children = node.get_children().order_by("lft")
452463
if excluded_descendants:
453464
children = children.exclude(node_id__in=excluded_descendants.keys())
465+
combined_map = {node.id: node_copy.id}
454466
for child in children:
455-
self._copy(
467+
_, child_map = self._copy(
456468
child,
457469
node_copy,
458470
"last-child",
@@ -464,7 +476,19 @@ def _copy(
464476
batch_size,
465477
progress_tracker=progress_tracker,
466478
)
467-
return [node_copy]
479+
combined_map.update(child_map)
480+
481+
if self._is_unit_topic(node.kind_id, node.extra_fields):
482+
remapped_extra_fields = {
483+
**node_copy.extra_fields,
484+
"options": self._remap_unit_options(
485+
node.extra_fields["options"], combined_map
486+
),
487+
}
488+
self.filter(pk=node_copy.pk).update(extra_fields=remapped_extra_fields)
489+
node_copy.extra_fields = remapped_extra_fields
490+
491+
return [node_copy], combined_map
468492

469493
def _copy_tags(self, source_copy_id_map):
470494
from contentcuration.models import ContentTag
@@ -582,6 +606,27 @@ def _copy_associated_objects(self, source_copy_id_map):
582606

583607
self._copy_tags(source_copy_id_map)
584608

609+
@staticmethod
610+
def _is_unit_topic(kind_id, extra_fields):
611+
return (
612+
kind_id == content_kinds.TOPIC
613+
and isinstance(extra_fields, dict)
614+
and extra_fields.get("options", {}).get("modality") == modalities.UNIT
615+
)
616+
617+
@staticmethod
618+
def _remap_unit_options(options, source_copy_id_map):
619+
remapped = dict(options)
620+
621+
if "lesson_objectives" in remapped:
622+
remapped["lesson_objectives"] = {
623+
source_copy_id_map[old_id]: objectives
624+
for old_id, objectives in remapped["lesson_objectives"].items()
625+
if old_id in source_copy_id_map
626+
}
627+
628+
return remapped
629+
585630
def _shallow_copy(
586631
self,
587632
node,
@@ -665,4 +710,4 @@ def _deep_copy(
665710

666711
self._copy_associated_objects(source_copy_id_map)
667712

668-
return new_nodes
713+
return new_nodes, source_copy_id_map

0 commit comments

Comments
 (0)