Content Moderation decides to set a new revision as the default one way too late in the entity update process

We would also need to make sure this is done on Entity forms and on forms such as \Drupal\content_moderation\Form\EntityModerationForm.

This could also cause issues if the moderation state is changed programmatically and not via a form. The main place I can think of that this is done is tests.

Yeah, we can't purely rely on forms to do this for us, it has to work at an API level for changes made programmatically as well as for REST support. Given we want the same validation to apply for REST, is there another concept which can accomodate this earlier in the lifecycle?

I wonder if this will also address #2803717: Allow 'syncing' content to be updated in content moderation without forcing the creation of a new revision.

Looks good and works well, tested in my local env.

It would be great to get this in ASAP to unblock the following issues:

• #2858434: Menu changes from node form leak into live site when creating draft revision
• #2858431: Book storage and UI is not revision aware, changes to drafts leak into live site
• #2856363: Path alias changes for draft revisions immediately leak into live site

Committed/pushed to 8.4.x, thanks!

Knocking back to get some clarification on the first point, although it would be nice to quickly fix 2./3. as well, but they are really minor, so feel free to knock back to RTBC otherwise.

1. +++ b/core/modules/content_moderation/src/EntityOperations.php
   @@ -110,7 +110,7 @@ public function entityPresave(EntityInterface $entity) {
          $update_default_revision = $entity->isNew()
            || $entity->isNewTranslation()
            || $current_state->isDefaultRevisionState()
   -        || !$this->isDefaultRevisionPublished($entity, $workflow);
   +        || !$this->moderationInfo->isDefaultRevisionPublished($entity);
   

   Do we still need this given that ModerationStateFieldItemList::onChange() seems to to the same thing? At the very least it seems the $entity->isDefaultRevision() call can be removed from ModerationHandler? #7 / #10 seem to reference this, but it's not entirely clear to me what's going on.

2. +++ b/core/modules/content_moderation/src/ModerationInformation.php
   @@ -140,6 +141,29 @@ public function isLiveRevision(ContentEntityInterface $entity) {
   +    $default_revision = \Drupal::entityTypeManager()->getStorage($entity->getEntityTypeId())->load($entity->id());
   

   As far as I can tell $this->entityTypeManager is already available in this class.

3. +++ b/core/modules/content_moderation/src/Plugin/Field/ModerationStateFieldItemList.php
   @@ -117,4 +118,41 @@ protected function computeModerationFieldItemList() {
   +    $current_state_id = $this->list[0]->value;
   

   I get that the current workflow state field will always have only a single item, but since we are being passed $delta anyway, I think it would make sense to use it here, instead of hardcoding 0.

Edit: X-posted with the commit. Thus, leaving at fixed, but would still like to get some clarification on point 1.

And RTBC again, thanks @tstoeckler for spotting these!

Yep, we still need to do the same thing in preSave() because ModerationStateFieldItemList::onChange() does not handle the case when an entity is created with an upfront value for moderation_state.

Ahh, that kind of (?) makes sense, as I guess nothing is "changing" in that case. Very interesting, in any case, hadn't thought of that. That definitely makes things clear, thanks!

And thanks for the quick follow-up!

Opened a proper follow-up for this at #2897483: Use injected entity type manager in ModerationInformation, moving back to fixed.

Are we not missing a $entity->setNewRevision(TRUE); here?