Skip to content

Fix orphaned records being dropped when promoted to the tree root - #221

Open
payal-canyon wants to merge 1 commit into
CodeWithDennis:4.xfrom
payal-canyon:fix/orphaned-children-dropped-from-tree
Open

payal-canyon wants to merge 1 commit into
CodeWithDennis:4.xfrom
payal-canyon:fix/orphaned-children-dropped-from-tree

Conversation

@payal-canyon

Copy link
Copy Markdown

Problem

When a record's parent is not in the query results (for example, the parent is soft-deleted or filtered out) and strictNullParentRootNodes is disabled, buildTreeFromResults() promotes the orphan to the root of the tree.

It merges each group of children into the root using the array union operator:

$resultMap[$parent] += $orphanedResult;

Every group is a list numbered from 0, and the actual root records go through this same loop. The array union keeps the existing entry when an index already exists, so any orphan whose index is already taken is silently discarded:

[] + ['A', 'B'] + ['C']; // ['A', 'B']

Example

  • Software: root record
  • Hardware: root record, excluded from the results (for example, soft-deleted)
  • Printers: child of Hardware

The groups [0 => Software] and [0 => Printers] are merged into [0 => Software], so Printers is missing from the tree even though it is in the results.

Fix

Append the orphaned records to the root instead of union-merging them.

Tests

Added tests/OrphanedRecordsTest.php (with a small tests/Fixtures/Category.php model). It builds a tree through the public query() / getTree() API with one root record and one record whose parent is excluded from the results.

  • Without this change, the test fails: only Software is returned.
  • With this change, the full suite passes locally (vendor/bin/pest, 3 tests).

Note: run-tests.yml currently only triggers for main, so the tests won't run automatically on this PR against 4.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant