Fix/select tree modify query subtree

[CodeWithDennis]

Summary

SelectTree builds options from two Eloquent queries: one for root rows (parent_id is null) and one for non-root rows. Previously, modifyQueryUsing only applied to the root query. The non-root query still loaded all related records with a parent, so unrelated branches could appear and buildTreeFromResults could place nodes incorrectly.

This PR, when modifyQueryUsing is present, resolves matching root primary keys, walks descendants via the parent_id graph, and constrains both queries with whereIn on that id set so only the intended subtree is loaded. Behavior without modifyQueryUsing is unchanged from 4.x.

---

Example usage

Limit the picker to the category branch rooted at id = 1, including all descendants:

SelectTree::make('category_id')
    ->label('Category')
    ->clearable(false)
    ->searchable(false)
    ->relationship(
        'category',
        'name',
        'parent_id',
        function ($query) {
            return $query->where('id', 1);
        },
        function ($query) {
            return $query;
        },
    )
    ->enableBranchNode(false)
    ->searchable(),

The fourth argument scopes which roots participate. The fifth runs only on the non-root query; return $query adds no extra constraints.

---

Previous behavior (technical)

Query A — roots

Category::query()->where('parent_id', null);
// + modifyQueryUsing, e.g. ->where('id', 1)

Query B — non-roots

Category::query()->where('parent_id', '!=', null);
// modifyQueryUsing was not applied; only modifyChildQueryUsing, if any

With where('id', 1) on A only, A returned the single root, while B returned every non-root row in the relation. Descendants of that root appeared because they were included in B, but so could any other non-root row.

---

Timeline

1. Pre-7e57336: Two-query layout; modifier on roots only → subtree intent leaked via unscoped B.
2. 7e57336 (Baspa): Single full query with the modifier on the entire builder → where('id', 1) returned one row; descendants required an explicit wider predicate.
3. 31efbc6 (revert): Restored two-query / root-only modifier → leak returned.
4. This PR: Root modifier unchanged in purpose; both queries restricted to ids reachable from matched roots.

---

Risks

• Callers that depended on “filter roots but load every non-root row” will see fewer options; that pattern was inconsistent with subtree selection.
• Constraints applied only in modifyQueryUsing are not automatically re-run on each descendant fetch; use model global scopes or a shared base query on SelectTree when a predicate must apply to every row.

---

No modifyQueryUsing

If the fourth argument is omitted, queries match current 4.x (full relation, split roots / non-roots).

[CodeWithDennis]

Hey @Baspa

I found some weird behaviour with SelectTree and modifyQueryUsing and I think it ties back to how buildTree changed over time—including your commit 7e57336 where it went to a single query, and later when it got reverted to two queries but the modifier only hit the root query.

That second part meant the “children” query could still pull basically everything, so you’d get weird extra categories in the tree even when you thought you filtered it.

I put up a PR that keeps two queries but, when modifyQueryUsing is set, figures out the subtree from the matching roots and sticks a whereIn on both queries so it only loads that branch.

Can you check if that would mess with anything you’re doing on your side?

[Baspa]

@CodeWithDennis Thanks for reaching out! I will try to check tomorrow if it does. Will let you know.

[Baspa]

@CodeWithDennis

One concern: modifyQueryUsing is only applied to the throwaway root-clone, not to collectIdsInSubtrees() or the final nullParentQuery / nonNullParentQuery. Which has the following side-effects:

1. Filters in modifyQueryUsing (tenant scopes, language filters, etc.) don't propagate to the descendant walk.
2. select([...]) or with() tweaks inside modifyQueryUsing silently fall back to select * on the final queries.

Could modifyQueryUsing also be applied to the $childQuery in collectIdsInSubtrees() and to the final queries before the whereIn(...) clamp? For context, in our CMS we use it
for whereBelongsTo(tenant), a language_code filter and a narrowed select. All silently stop working on the non-root path with the current diff.

[CodeWithDennis]

@Baspa

Mmm.. its a pretty complex problem and i don't wanna break other peoples stuff either.. like the original idea was simple right; you have a root with children, you can filter the roots but still see the children, and if you wanna filter out children you use the child query.

But so much code has changed that im honestly not even sure whats the best way to do it without breaking things i just pushed another change, do you think that could work?

Im open to suggestions and changes too its genuinely a hard one.

[gp-lnuff]

I am reminded of #178 and #179

The original issue was that filtering root items would not yield children that were present in the results but did not have a "null" parent_attribute.
Which is why in #182 children in the result set whose parents did not appear in the result set were now being promoted to root items.
This issue seems to be a side effect of that change - if the parent query filters out some roots, the child query would still fetch the filtered roots' children and promote them to root items.
Now, I don't think that this behaviour is wrong or unexpected, necessarily.
The parent and child query are, at their core, "a query for items that don't have a parent (null or otherwise specified)" and "a query for items that have a parent (different from the $parentNullValue)".
The child query was never functionally "a query strictly for children of items yielded by the parent query".
That is exactly why #178 was an issue.
The use case of relationship() just so happened to line up with this view, although in its limited self-referencial way (by default), which is why #173 introduced a way to break free of that.

In the interest of not introducing breaking changes, I have a suggestion:

• Add a Closure|bool $strictNullParentRootNodes = false property to disable child-to-root node promotion in buildTree() and a configuration method to set it.
• If necessary, add that configuration method to relationship() in a major version to change the default behaviour.

I believe it should be introduced as optional as to not regress #178 .
If you're amenable to this suggestion I can whip up an implementation and open a PR.

On a side note, I think that a $scopeRelationshipQueryUsing would be a nice feature to scope both queries at once, but here are my thoughts:

• $scopeRelationshipQueryUsing is actually $modifyQueryUsing
• There should be a $modifyParentQueryUsing to apply only to the parent query

Again, such changes are best saved for a major version, but I think it would make sense to change the relationship() method to use $modifyQueryUsing on both queries and have dedicated $modifyParentQueryUsing and $modifyChildQueryUsing Closures.
This would also be in line with the query() method, where three queries are supplied (one base, which would be the relationship query + $modifyQueryUsing, a $modifyQueryUsing, which would be an hypothetical $modifyParentQueryUsing, and a $modifyChildQueryUsing).

I have other minor suggestions for a future major version, such as a couple poorly named methods, but I'll save those for later with the hope I'll get a chance to voice them before it releases.

[CodeWithDennis]

@gp-lnuff I like this idea!