Problem/Motivation
In #2474075: Fix Node::preSave() and document that preSave() and postSave() are not working with ContentEntity translations we agreed that we have an inconsistent API. While FieldItemListInterface::preSave()
is called on every translation of a field, EntityInterface::preSave()
is only called once on an entity before save, regardless of it's translations.
Proposed resolution
There're already exiting implementations of EntityInterface::preSave()
that should not run multiple times per save. If we simply call EntityInterface::preSave()
on every translation of en entity, these implementations need a switch to just be active on the default translation.
Therefor @catch proposed to consider to split this method into two. One that will be executed once and another that will be executed on every translation of the entity before save.
Remaining tasks
Decide about the best API.
User interface changes
None.
API changes
Sure ;-)
Data model changes
None.
Comment | File | Size | Author |
---|---|---|---|
#8 | et-presave-2577609-8.patch | 6.88 KB | plach |
Issue fork drupal-2577609
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
catchI'd consider adding a hook invocation during RC, we may have contributed project blockers that require that, so moving back to 8.0.x for now.
Comment #3
plachComment #4
plach#2474075: Fix Node::preSave() and document that preSave() and postSave() are not working with ContentEntity translations is in.
Comment #5
xjmSee https://groups.drupal.org/node/484788 for more information on the rc target tag. This issue should possibly be retagged as rc deadline instead, since it sounds disruptive?
Comment #6
plach@xjm:
@catch said he'd consider this, at least in some form, during RC.
Comment #7
plachComment #8
plachFirst attempt
Comment #9
plachComment #11
Gábor HojtsyNot sure how to implement this in a backwards compatible way? Also would be 8.1.x due to the API change. Removing from sprint due to inactivity.
Comment #13
xjmComment #14
plachHappy to get back to this but for now I have to unassign :(
Comment #20
effulgentsia CreditAttribution: effulgentsia at Acquia commentedChanging this from bug to task, but I still think it's a great idea to do in whatever Drupal minor version it ends up making it into.
Comment #29
Bhanu951 CreditAttribution: Bhanu951 as a volunteer commentedComment #31
Bhanu951 CreditAttribution: Bhanu951 as a volunteer commentedComment #32
smustgrave CreditAttribution: smustgrave at Mobomo commentedMR has a bunch of failures.
And was previously tagged for tests which still needs to happen.