Fixes #22251: Re-parent child ModuleBays when a Module is moved to a new bay

[bctiemann]

Summary

Closes: #22251

When Module.module_bay_id is updated (the module is moved to a different bay), the child ModuleBay records belonging to that module were not having their MPTT parent pointers updated. They stayed anchored to the old bay in the tree even though the module had moved, producing the display anomaly reported in the issue.

Root cause: ModuleBay.save() derives self.parent from self.module.module_bay. On initial creation this runs automatically, but when only the module's module_bay_id changes there is nothing to trigger a re-save of the child bays. Their stored parent_id goes stale.

Fix: In Module.save(), snapshot the current module_bay_id from the DB before saving. If it changed, iterate and re-save each child ModuleBay. The existing ModuleBay.save() logic then recalculates parent from the now-updated module.module_bay and MPTT moves each node to the correct position in the tree.

Test plan

• New regression test ModuleBayTestCase.test_moving_module_reparents_child_module_bays creates a module with two child bays, moves the module to a different top-level bay, and asserts both parent_id and tree_id on each child reflect the new location.
• Full ModuleBayTestCase (20 tests) passes.

[bctiemann]

@claude /review and report only on areas of potential concern.

[github-actions]

Claude finished @bctiemann's task in 2m 10s —— View job

---

Code Review

The fix is correct and the tests are solid. Two areas of potential concern:

---

1. Change-log noise and missing pre-change snapshots

child_bay.save() fires the normal post_save → handle_changed_object signal path, which means every child bay receives its own ObjectChange ("Updated") record whenever the parent module is moved. For a module with many child bays this pollutes the changelog with entries the user didn't directly cause.

More subtly, _prechange_snapshot is never set on the child bay instances before save() is called, so the "before" state of each generated change record will be whatever to_objectchange() infers from scratch — the parent_id / tree_id fields that actually changed will likely show an empty or incorrect diff in the UI.

The straightforward fix is to call .snapshot() on each child bay before re-saving it:

# modules.py:338-340
if old_module_bay_id is not None and old_module_bay_id != self.module_bay_id:
    for child_bay in self.modulebays.all():
        child_bay.snapshot()   # ← record pre-change state for changelog
        child_bay.save()

Whether the changelog entries themselves are desirable at all is a separate design decision — if they're considered noise, a flag like _no_log (or suppressing signals around these saves) could be used — but at minimum the snapshot should be present for correctness.

---

2. N+1 queries inside the loop

self.modulebays.all() fetches child bay rows, but each subsequent child_bay.save() calls self.module.module_bay (inside ModuleBay.save()) which issues two more queries per iteration: one to load the Module, one for its module_bay. For a module with many child bays this is O(3N) queries.

Adding select_related on the queryset would cut it to a single prefetch:

for child_bay in self.modulebays.select_related('module__module_bay').all():

---

Minor / non-blocking

The old_module_bay_id is not None guard is technically redundant — module_bay is a non-nullable OneToOneField, so for any saved module the DB value will never be None. The guard is harmless but could be replaced with a plain if old_module_bay_id != self.module_bay_id: to make the intent clearer (the real guard is already not is_new).

Module.objects.update() bypass — not introduced here, but worth documenting in a comment: calling Module.objects.filter(...).update(module_bay_id=new_id) bypasses save() entirely and would leave child bays stale. The fix does not need to address this, but noting it as a known limitation would help future maintainers.

[bctiemann]

Both actionable items are addressed.