I have installed the Content Moderation module and I have applied the default workflow to one node type.
If I create a new node (without moderation), I got the following error when trying to save it:

PHP Fatal error:  Call to a member function getTypePlugin() on a non-object in /var/www/html/drupal8/core/modules/content_moderation/src/Plugin/Field/ModerationStateFieldItemList.php on line 156,

If I reopen the page and if I fill everything except the image field, then everything is fine.
But if I upload an image into the image field (with the image_crop_widget) then I get the fatal error described upper. (really strange isn't it)

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

DuneBL created an issue. See original summary.

DuneBL’s picture

Here is a small patch that solve/fix this issue... But I think we should dig more deeply because I don't understand why updateModeratedEntity($moderation_state_id) is called fromwithin a content type that doesn't play with moderation.

tacituseu’s picture

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.

tacituseu’s picture

Issue tags: +Workflow Initiative
timmillwood’s picture

Good find!

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

timmillwood’s picture

Status: Active » Needs review
FileSize
918 bytes

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.

larowlan’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

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

timmillwood’s picture

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. 😢

The last submitted patch, 8: 2900687-8-test-only.patch, failed testing. View results

timmillwood’s picture

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

timmillwood’s picture

Status: Needs review » Needs work

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?

Sam152’s picture

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.

Sam152’s picture

Status: Needs work » Needs review
FileSize
1.76 KB

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.

Sam152’s picture

The last submitted patch, 14: 2900687-14--test-only.patch, failed testing. View results

timmillwood’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Great investigating @Sam152!

Simple patch, and nice test.

catch’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs re-roll

Needs a re-roll.

timmillwood’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs re-roll
FileSize
4.18 KB

  • catch committed 8c9c8d3 on 8.5.x
    Issue #2900687 by timmillwood, Sam152, DuneBL: Fatal error when saving a...

  • catch committed 1484dbf on 8.4.x
    Issue #2900687 by timmillwood, Sam152, DuneBL: Fatal error when saving a...
catch’s picture

Version: 8.5.x-dev » 8.4.x-dev
Status: Reviewed & tested by the community » Fixed

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

prestonso’s picture

Issue tags: +8.4.0 release notes

Tagging for inclusion in 8.4.0-beta1 release notes.

Sam152’s picture

Issue tags: -8.4.0 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.