Fatal error when saving a node type without content moderation

core/modules/content_moderation/src/EntityTypeInfo::entityBaseFieldInfo() is adding the computed field per entity type (not per bundle/content type), so all node bundles (content types) have it.
Makes sense to check ModerationInformation::getWorkflowForEntity() return value.

Good find!

I've just been able to replicate this, and as you say, it only happens when adding a file.

I think I'd fix it like this.

Although it's weird it only happens when a file is added, I think that needs further investigation.

We need a test here, but I'm guessing it is something to do with ajax/serialized form state

I wrote a test for this, but it passes even without the fix.
When trying to find out why that is I noticed the issue is only reproducible with JavaScript enabled. So looks like we need a JavaScript test to cover this. 😢

Looks like I deleted too much in the test-only patch, let's try that again.
Although I'm expecting both patches to pass.

ok, as I thought, both are passing. So need a JS test, should be able to copy a lot of \Drupal\Tests\image\FunctionalJavascript\QuickEditImageTest but with Content Moderation.

Not sure it's worth keeping the extra testing in #9/#10?

Fix looks good, but I was keen to see what the root cause of this was. Why do we have a FieldItem on our custom FieldItemList for an entity that isn't being moderated? Findings below:

• During the image uploading the entity is serialized and stored somewhere and all of the entity values are copied over.
• We attach to all entity types that are being moderated through a computed base field, then our widget checks if the entity is being moderated and decides if the widget should be displayed or not:
  

      if (!$this->moderationInformation->isModeratedEntity($entity)) {
        return $element + ['#access' => FALSE];
      }

• Even though in our widget, we've set #access => false it's still considered part of the form and all of the wrapping form elements still exist. The line of code which trips things up is in \Drupal\Core\Field\WidgetBase::formMultipleElements:
  

        // Add a new empty item if it doesn't exist yet at this delta.
        if (!isset($items[$delta])) {
          $items->appendItem();
        }
  

  It creates a field item for us, so that the widget element can be rendered (remember writing widgets you are given $items and $delta and you always have some FieldItem for $items[$delta]?

• WidgetBase is usually super helpful and tries to filter out empty items in ::extractFormValues:
  

        // Assign the values and remove the empty ones.
        $items->setValue($values);
        $items->filterEmptyItems();
  

  ...BUT since this code is guarded by checking if the actual item is in $form_state (which it's not cause the widget is hidden), it gets skipped:

      $values = NestedArray::getValue($form_state->getValues(), $path, $key_exists);
      if ($key_exists) {
      ...
      // Call to filter empty items.
  

Hence I think fixing the root cause would involve making sure our widget calls filterEmptyItems in such a way that isn't dependent on the form_state having a value for the field.

I wonder if it would be better to avoid this situation with some tweaks in our widget. Not starting the process of building the widget form seems to fix this for me. Lets see what testbot says.

I'd say things are going to be more stable if we never have the situation of $not_moderated_entity->moderation_state having a value.

Great investigating @Sam152!

Simple patch, and nice test.

Needs a re-roll.

Committed/pushed to 8.5.x and cherry-picked to 8.4.x. Thanks!

Tagging for inclusion in 8.4.0-beta1 release notes.

Does this belong in the release notes if it didn't exist in 8.3? I believe it was introduced in #2753717: Add select field to choose moderation state on entity forms which was committed to 8.4.