- Issue created by @boromino
- 🇩🇪Germany jurgenhaas Gottmadingen
This sounds like the core bug described in 🐛 Wrong revision can be loaded into original in \Drupal\Core\Entity\EntityStorageBase::doPreSave Needs work . The patch from MR!3599 did solve this for some users in the past, and we're using that patch in our deployment as well. The official solution in Drupal core will be different, though, as @berdir pointed out in that other issue. Until that'll be available, that MR should do the trick and I would not like to change anything in ECA for that, since it is confirmed, that core is loading the wrong revision into
original
under those circumstances. - last update
about 1 year ago 275 pass, 2 fail - Issue was unassigned.
- 🇨🇭Switzerland boromino
It is related to that core issue. However, the patch does not fix it, because the entity is the default revision when \Drupal\eca_workflow\HookHandler::transition() is run.
\Drupal\content_moderation\moderationInformation::getOriginalState() isn't appropriate, either, because it returns the initial state for new entities, which would prevent triggering the event.
Therefore, I have provided a patch. It may become obsolete, once the core issue is fixed.
- 🇩🇪Germany jurgenhaas Gottmadingen
Hmm, I'm not sure yet if we should really do this in ECA. As it appears, the issue is to load
$entity->original
correctly in all circumstances. We should do that in core, where the problem comes from. Otherwise, that issue is being addressed at 2 different places, which doesn't fee right. Also, introducing yet another database query for an entity at this point is a potential performance issue. During event handling, we need to be careful with things like that, as events get dispatched so much, that this could easily cause a significant number of extra db queries for almost all requests in that context.Therefore, as a quick fix we could compare this MR with the patch in the core issue 🐛 Wrong revision can be loaded into original in \Drupal\Core\Entity\EntityStorageBase::doPreSave Needs work , they both want to achieve the same and if the proposed patch in core doesn't do that correctly yet, it should be improved there.
And for the long term solution, testing the official patch from @berdir and giving feedback there, would be helpful too.
While being on it, just saw that the MR in this issue was done for the 1.1.x branch, which should be 2.0.x as we fix bugs always in the latest release and then back port to previous supported branches. But we don't have to change that right now, only if my proposal above really doesn't work.
- Status changed to Postponed
about 1 year ago 4:34pm 21 February 2024 - 🇩🇪Germany jurgenhaas Gottmadingen
I'm changing the status to documentation for now, since this issue may probably help others who run into the same issue. It provides the necessary links to solutions and we keep the issue open for potentially more discussions around this topic.
- Status changed to Fixed
7 months ago 9:45am 1 August 2024 Automatically closed - issue fixed for 2 weeks with no activity.