Problem/Motivation
During each phase of entity CRUD, hooks are invoked.
For example, when saving a new entity, hook_entity_presave()
and then hook_entity_insert()
are invoked.
When saving an existing entity, hook_entity_presave()
and then hook_entity_update()
are invoked.
create, presave, predelete, delete, load: these are all clear.
postsave is divided into insert and update. This is not clear.
Proposed resolution
Add hook_entity_postsave(), deprecate hook_entity_insert() and hook_entity_update()
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#11 | 2798783-presave-11.patch | 15.46 KB | tim.plunkett |
#11 | 2798783-presave-11-interdiff.txt | 10.53 KB | tim.plunkett |
#2 | 2798783-entity-2.patch | 4.93 KB | tim.plunkett |
Comments
Comment #2
tim.plunkettComment #3
claudiu.cristeaI think hook_entity_insert() still make sense. There are specific tasks that are applying only to entity insertion.
Comment #4
fago>I think hook_entity_insert() still make sense. There are specific tasks that are applying only to entity insertion.
Well for that postSave has the $update boolean. I'm +1 for adding postSave as it helps with consistency also, I'm not so sure about deprecating insert/update. Seems to be a too large change to do between to minor versions to me (for everyone who wants to be a good citizen and follow deprecations).
Comment #5
fagoNote: slightly related: #1729812: Separate storage operations from reactions
Comment #6
tim.plunkettJust had someone else hit this today. It continues to be very confusing.
Adding postsave is nice, but that won't solve the problem. Deprecating (and eventually removing) the hooks that don't do what you think is the fix here.
Comment #8
tim.plunkettThat is an extremely brittle test. I think I'll just deal with it here, but #2824165: Remove brittleness from ConfigEntityStorageTest would help if it happens first
Comment #9
BerdirShould we include some conversions, to see how that looks?
The deprecation message isn't following the standard yet :)
Comment #10
BerdirI guess the only argument against this is the long-term goal of removing hooks completely in favor of events? So adding new hooks is going a bit against that. Personally, I'm in no hurry to go there, though :)
Comment #11
tim.plunkettIn order to keep the "events for hooks" issue clean, I was hoping to keep it 1:1. Therefore to have the right events, we'd want to do this first.
Did a couple conversions and fixed the tests.
Comment #12
Berdirthis doesn't work AFAIK, isNew() is already updated at this point and it is no longer new. That's why ::postSave() has an $update argument.
Comment #15
dpiThis issue is a dupe of #2221347: Add hook_entity_postsave hook. Although this issue has progress...
Comment #16
tim.plunkett