[6.x] Give reference updater fields the item being updated as their parent - #15443
Merged
Conversation
Blueprints hand back a Fields instance cached globally by handle, and nested fields are constructed without a parent at all, so neither can be relied upon to have the right one during a reference update. Clone the fields rather than mutating them in place, so the item doesn't leak into every other consumer of that blueprint for the rest of the request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a term is renamed or an asset is moved,
DataReferenceUpdaterloops over every entry, term, global and user, and hands each item's fields to the fieldtypes that participate in reference updating. Those fields were not reliably carrying the item as their parent, so any fieldtype reading$field->parent()during a reference update would read the wrong thing.Two separate causes:
getTopLevelFields()used$this->item->blueprint()->fields()->all().Blueprint::fields()returns$this->fieldsCachebefore it re-applies the parent, and theFieldsobject underneath comes fromBlink::once($this->fieldsBlinkKey(), ...)— cached globally by blueprint handle, not perBlueprintinstance. In a loop over many items, the parent stuck on whichever item got there first.processNestedFields()called$fields->all()with no parent at all. Replicator, Bard, Grid and Group each build a barenew Fields($fields)for the reference-update path, so there was nothing to inherit.Both call sites now go through one helper. It clones each field rather than calling
setParent()on the sharedFields, becauseFields::setParent()mutates in place and propagates to every childField— setting the parent there would fix the loop but leave the last-updated item pinned to the blueprint's cached fields for the rest of the request, where an unrelated reader would pick it up.Scope
This is a latent bug on
6.x: nothing currently on the reference-update path reads$field->parent(). I checked every implementation —Assets,Link,Markdown,Bard,Replicator,Grid,GroupandTerms— and none of them do. The one asset-side parent read,Assets::dynamicFolder(), is on the publish path, not this one.So this is a contract fix rather than a user-visible bug fix: during a reference update, a field's parent is the item being updated, and now it says so. It's being fixed on its own because the hierarchical taxonomies work (#15192) adds the first fieldtype that would misread it, and the fix doesn't belong in a taxonomy PR.
Tests
tests/Data/DataReferenceUpdaterTest.phpregisters a fieldtype that records the parent it was handed, since no stock fieldtype exercises this yet. Each test was checked to fail for the right reason:setParent()on the shared fieldsnull