Problem/Motivation
The Media Widget module, which defines media reference field widgets mimicking core's File upload and Link widgets, includes an ugly hack (see here and here) to work around a problem happening when submitting the entity form with an existing media item.
When creating a new media item, the submission will work just fine, since the entity reference field supports automatically saving new referenced entities, however it does not currently support re-saving existing referenced entities. As a consequence, when submitting the entity form, if any media field value was updated (e.g. if the "alt" media field value was changed), there is no way for the save to happen as part of referencing entity save transaction. This in turn means that, if either the referencing entity save or the media entity save fail, an inconsistent state can result.
Proposed resolution
Add support to optionally re-save a referenced entity when saving the referencing entity.
Remaining tasks
- Validate the proposed solution
Write code- Perform reviews
User interface changes
None
Introduced terminology
None
API changes
None, only API additions
Data model changes
None
Release notes snippet
TBD
| Comment | File | Size | Author |
|---|
Issue fork drupal-3490274
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
plachComment #3
quietone commentedComment #4
plachComment #6
plachThe MR adds configurable functionality to automatically save referenced entities when the referencing entity is saved. I heavily relied on AI to generate tests, fix test failures, and draft the initial solution. See the MR description for more details.
Comment #7
plachComment #8
amateescu commentedFound an older issue with the same purpose: #3203913: Make EntityReferenceItem autosave and another one that would help greatly: #2862574: Add ability to track an entity object's dirty fields (and see if it has changed)
Started reviewing the current MR and I have an initial architectural question: what's the purpose of making this mechanism optional? Is it only for BC purposes (i.e. we could get into double-save situations in various contrib modules that already handle this themselves)
If that's the only reason, I think the burden of this decision shouldn't be placed on the user (site builder) and it would be worthwhile updating those contrib modules (Inline Entity Form, Entity Reference Revisions, Media Widget) to stop auto-saving existing entities when the core functionality is detected instead.
Comment #9
plachYep, I have a @todo in the code about that one :)
BC was one aspect, the other main one was performance: depending on the widget you are using, it may or may not make sense to ever re-save the referenced entity, especially until we have proper change-detection in place. Even a simple entity may have several entity references, so I was trying to reduce the performance impact as much as possible.
You definitely have a point about admin UX, though: I was imagining that modules providing widgets able to leverage this logic would automatically switch the save mode when their widgets are activated, however this may be problematic if multiple entity form displays activate widgets with different save mode requirements. Maybe we could do something similar to what @Berdir was suggesting in #3203913: Make EntityReferenceItem autosave but on the field item itself? E.g. a
::setNeedsEntitySavemethod setting aneedsEntitySaveproperty, defaulting to the::hasNewEntityreturn value?Comment #10
amateescu commentedThis is a popular feature request, I just closed another duplicate: #2657716: Entity reference field to save referenced entities on parent entity presave hook :)
I think that could work, but we should ask @Berdir why it wasn't implemented like that in ERR, maybe there are some hidden gotchas around dealing with entity reference items in a separate request, like a modal or something...
Comment #11
amateescu commentedThis issue is also very closely related: #2831727: [PP-1] Prepare-view of entity references silently drops changes contained in referenced entities
Comment #12
plachI posted an implementation of this approach here:
Comment #14
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #15
plachThe alternative approach above is being discussed in Slack.