Reverting a revision doesn't keep moderation state

Uploading a test only patch, because it's a little puzzling why the condition right above it would ever be FALSE while the new one would be TRUE.

edit: figured out above comment after reviewing the patch in more detail.

1. +++ b/core/modules/content_moderation/src/Plugin/Field/ModerationStateFieldItemList.php
   @@ -57,6 +57,14 @@ protected function getModerationState() {
   +    if (!empty($entity->getLoadedRevisionId()) && $entity->getLoadedRevisionId() != $entity->getRevisionId()) {
   

   Based on the condition right above this one I think the whole second part of this condition is superfluous. You only enter this line if getRevisionId is null.

   Maybe this method could do with some more commenting.

2. +++ b/core/modules/content_moderation/tests/src/Kernel/ContentModerationStateTest.php
   @@ -101,6 +101,21 @@ public function testBasicModeration() {
   +    $node->set('moderation_state', 'archived');
   

   Rest of the test uses $node->moderation_state->value =, not ->set

3. +++ b/core/modules/content_moderation/tests/src/Kernel/ContentModerationStateTest.php
   @@ -101,6 +101,21 @@ public function testBasicModeration() {
   +    $prev_rev = \Drupal::entityTypeManager()->getStorage('node')->loadRevision(4);
   

   nit, should be full form, $previous_revision.

4. +++ b/core/modules/content_moderation/tests/src/Kernel/ContentModerationStateTest.php
   @@ -101,6 +101,21 @@ public function testBasicModeration() {
   +    // Get the default revision
   

   nit, full stop

5. +++ b/core/modules/content_moderation/tests/src/Kernel/ContentModerationStateTest.php
   @@ -101,6 +101,21 @@ public function testBasicModeration() {
   +
   

   nit, extra whitespace

Could this implementation also work?

Agreed. NW based on that.

Added some comments and addressed feedback in #12. The extra methods are a bit more self documenting, add some extra comments and reduce the complexity of the big getModerationStateId method.

+++ b/core/modules/content_moderation/src/Plugin/Field/ModerationStateFieldItemList.php
@@ -27,43 +29,67 @@ protected function getModerationState() {
+  protected function loadCorrespondingContentModerationStateLanguage(ContentEntityInterface $entity, ContentModerationStateInterface $content_moderation_state) {
...
+  protected function loadContentModerationStateRevision(ContentEntityInterface $entity) {

Let's not have two additional methods here - I think this is unnecessary. You're always going to want the correct language. I think all we need to do is merge loadContentModerationRevision and loadCorrespondingContentModerationStateLanguage together and just call it loadContentModerationState.

1. +++ b/core/modules/content_moderation/src/Plugin/Field/ModerationStateFieldItemList.php
   @@ -27,43 +29,56 @@ protected function getModerationState() {
   +      return null;
   

   The early return here is fine but lets capitalise null as per standards.

2. +++ b/core/modules/content_moderation/src/Plugin/Field/ModerationStateFieldItemList.php
   @@ -27,43 +29,56 @@ protected function getModerationState() {
   +    $content_moderation_state = $content_moderation_storage->loadRevision(key($revisions));
   +    if (!$entity->getEntityType()->hasKey('langcode')) {
   +      return $content_moderation_state;
   +    }
   +    ¶
   +    $langcode = $entity->language()->getId();
   +    if (!$content_moderation_state->hasTranslation($langcode)) {
   +      $content_moderation_state->addTranslation($langcode);
        }
   +    return $content_moderation_state->language()->getId() !== $langcode ? $content_moderation_state->getTranslation($langcode) : $content_moderation_state;
   

   Let's turn this logic around to avoid the early return. Basically as this logic was before. And then the final line can be return $content_moderation_state;

interdiff LGTM

Committed 6b34291 and pushed to 8.3.x. Thanks!