Transition event for published entity not triggered

Created on 29 January 2024, 5 months ago
Updated 21 February 2024, 4 months ago

Problem/Motivation

If a moderated entity has a published default revision with moderation state 'published', the workflow transition event is not triggered if the revision being saved receives the moderation state 'published' again.

Steps to reproduce

  • Drupal standard installation with content moderation module enabled
  • Configure editorial workflow for basic page
  • Add an ECA model for event Workflow: state transition that displays a message to the user
  • Create and save a basic page with moderation state 'draft': the message is displayed
  • Under tab 'latest version' of that basic page change moderation state to 'published' and click 'Apply': the message is displayed
  • Edit and save the same basic page with moderation state 'draft': the message is displayed
  • Under tab 'latest version' of that basic page change moderation state to 'published' and click 'Apply': the message is NOT displayed

Proposed resolution

Instead of retrieving $from_state in \Drupal\eca_workflow\HookHandler::transition() from $entity->original (which contains the latest default revision instead of latest revision in case of published entity), use \Drupal\content_moderation\moderationInformation::getOriginalState().

Remaining tasks

Additional test case.

πŸ’¬ Support request
Status

Postponed

Version

2.0

Component

Documentation

Created by

πŸ‡¨πŸ‡­Switzerland boromino

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Pipeline finished with Skipped
    7 months ago
    #52123
  • 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.

  • Merge request !400Load original entity revision β†’ (Open) created by boromino
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.2.x + Environment: PHP 8.1 & MySQL 8
    last update 5 months ago
    275 pass, 2 fail
  • Pipeline finished with Failed
    5 months ago
    Total: 414s
    #84756
  • 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 4 months ago
  • πŸ‡©πŸ‡ͺ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.

  • Pipeline finished with Failed
    14 days ago
    Total: 405s
    #197245
Production build 0.69.0 2024