Problem/Motivation

An entity reference field with the Complex widget is returning a vague error ("This value should not be null.") without indicating what field or value is incorrect. Sometimes nothing is incorrect, and it's the fact that inline_entity_form isn't adding values to the main entity before it's validated.

In some cases this is happening when all the fields are filled in, such as on clicking the "preview" button when adding a new node.

Steps to reproduce:

1) go to https://simplytest.me/ and select "Inline Entity Form" as the project

2) Once site launches, add an entity reference field on the article content type to reference the page content type

3) Edit the form display to use the Complex Widget widget.

use complex widget

4) Create a test page node (/node/add/page)

5) Go to create an article node (/node/add/article), fill in all the fields (including the reference to the page node you created

6) Click Preview

7) Observe error.

Expected: You should see a preview of your node with the image displaying

Actual: An error message "This value should not be null." and the form reloads. I traced it back to validation on the image field (see stack trace below).

Proposed resolution

I'm not sure of the best approach yet. I have a patch, but it needs review.

Next Steps

- test coverage to demonstrate the error
- needs review
- needs test coverage for the fix

CommentFileSizeAuthor
#38 inline_entity_form-2932492-38.patch2.24 KBquadrexdev
#37 interdiff-36-37.txt820 bytesdavid.muffley
#37 inline_entity_form-2932492-37.patch2.19 KBdavid.muffley
#36 inline_entity_form-value_should_not_be_null_reroll_24-2932492-36.patch2.75 KBa.sotirov
#33 inline_entity_form-value_should_not_be_null_reroll_24-2932492-33.patch3.11 KBguypaddock
#29 iniline-entity-form-vague-error-preview-2932492-29.patch690 bytesalbertosilva
#25 inline-entity-form-vague-error-2932492-25.patch523 byteslarvymortera
#24 inline-entity-form-2932492-24.patch2.79 KBoknate
#24 interdiff-12-24.txt902 bytesoknate
#16 inline_entity_form complex.png59.85 KBoknate
#12 inline-entity-form-2932492-12.patch2.66 KBoknate
#3 inline-entity-form-vague-error-2932492-3.patch1.82 KBoknate
#2 inline-entity-form-vague-error-2932492-2.patch1.36 KBoknate

Comments

oknate created an issue. See original summary.

oknate’s picture

oknate
Location Greater New York City Area
commented
StatusFileSize
new inline-entity-form-vague-error-2932492-2.patch1.36 KB
oknate’s picture

oknate
Location Greater New York City Area
commented
StatusFileSize
new inline-entity-form-vague-error-2932492-3.patch1.82 KB

Made some changes after seeing it fail tests.

oknate’s picture

oknate
Location Greater New York City Area
commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
commented
Status: Active » Needs review
darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
commented

Can you provide steps to reproduce for review?

bojanz’s picture

bojanz commented
Status: Needs review » Needs work

Feels hacky to add entity_browser specific code. A better fix is probably needed, along with more instructions for reproducing.

oknate’s picture

oknate
Location Greater New York City Area
commented

I'm experiencing the vague "This value should not be null." error again.

This time with this set up:

node with field_image (entity reference to media entity of type bundle), using the complex widget. On the node/add/mynodetype form, clicking preview triggers the error, even though the field is filled in.

array:37 [▼
  0 => array:7 [▼
    "file" => "/home/vagrant/docroot/web/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/NotNullConstraintValidator.php"
    "line" => 28
    "function" => "validate"
    "class" => "Symfony\Component\Validator\Constraints\NotNullValidator"
    "object" => NotNullConstraintValidator {#2669 ▶}
    "type" => "->"
    "args" => array:2 [▶]
  ]
  1 => array:7 [▼
    "file" => "/home/vagrant/docroot/web/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php"
    "line" => 191
    "function" => "validate"
    "class" => "Drupal\Core\Validation\Plugin\Validation\Constraint\NotNullConstraintValidator"
    "object" => NotNullConstraintValidator {#2669 ▶}
    "type" => "->"
    "args" => array:2 [▶]
  ]
 
... etc ...

  6 => array:7 [▼
    "file" => "/home/vagrant/docroot/web/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php"
    "line" => 99
    "function" => "validateNode"
    "class" => "Drupal\Core\TypedData\Validation\RecursiveContextualValidator"
    "object" => RecursiveContextualValidator {#2575 ▶}
    "type" => "->"
    "args" => array:3 [▶]
  ]
  7 => array:7 [▼
    "file" => "/home/vagrant/docroot/web/core/lib/Drupal/Core/TypedData/Validation/RecursiveValidator.php"
    "line" => 90
    "function" => "validate"
    "class" => "Drupal\Core\TypedData\Validation\RecursiveContextualValidator"
    "object" => RecursiveContextualValidator {#2575 ▶}
    "type" => "->"
    "args" => array:3 [▶]
  ]
  8 => array:7 [▼
    "file" => "/home/vagrant/docroot/web/core/lib/Drupal/Core/TypedData/TypedData.php"
    "line" => 131
    "function" => "validate"
    "class" => "Drupal\Core\TypedData\Validation\RecursiveValidator"
    "object" => RecursiveValidator {#2572 ▶}
    "type" => "->"
    "args" => array:1 [▶]
  ]
  9 => array:7 [▼
    "file" => "/home/vagrant/docroot/web/core/lib/Drupal/Core/Entity/ContentEntityBase.php"
    "line" => 488
    "function" => "validate"
    "class" => "Drupal\Core\TypedData\TypedData"
    "object" => EntityAdapter {#2396 ▶}
    "type" => "->"
    "args" => []
  ]
  10 => array:7 [▼
    "file" => "/home/vagrant/docroot/web/core/lib/Drupal/Core/Entity/ContentEntityForm.php"
    "line" => 192
    "function" => "validate"
    "class" => "Drupal\Core\Entity\ContentEntityBase"
    "object" => Node {#2385 ▶}
    "type" => "->"
    "args" => []
  ]
  11 => array:5 [▼
    "function" => "validateForm"
    "class" => "Drupal\Core\Entity\ContentEntityForm"
    "object" => NodeForm {#2218 ▶}
    "type" => "->"
    "args" => array:2 [▶]
  ]
  12 => array:4 [▼
    "file" => "/home/vagrant/docroot/web/core/lib/Drupal/Core/Form/FormValidator.php"
    "line" => 82
    "function" => "call_user_func_array"
    "args" => array:2 [▼
      0 => array:2 [▼
        0 => NodeForm {#2218 ▶}
        1 => "validateForm"
      ]
      1 => array:2 [▼
        0 => &29 array:60 [▶]
        1 => &45 FormState {#2170 ▶}
      ]
    ]
  ]
oknate’s picture

oknate
Location Greater New York City Area
commented
Title: Error message from InlineEntityFormComplex is vague » Vague "This value should not be null." error.
oknate’s picture

oknate
Location Greater New York City Area
commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
commented

Note: As a quick fix / hack for this issue with node preview this is one idea, just prevent validation errors.

/**
 * Implements hook_form_alter().
 */
function mymodule_form_alter(&$form, FormStateInterface $form_state, $form_id) {

  $formObject = $form_state->getFormObject();

  if (!$formObject instanceof NodeForm || empty($form['actions']['preview'])) {
    return;
  }

  $form['actions']['preview']['#limit_validation_errors'] = [];
}

Another quick fix / hack is to not make the field required.

oknate’s picture

oknate
Location Greater New York City Area
commented
StatusFileSize
new inline-entity-form-2932492-12.patch2.66 KB

Here's a patch that takes care of the vague message in the context of node preview, and fixes the node preview of inline entity form elements, at least with the one context I tested. Will need to see if it breaks anything else.

It also needs test coverage to demonstrate the error and the fix.

oknate’s picture

oknate
Location Greater New York City Area
commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
commented
Status: Needs work » Active
oknate’s picture

oknate
Location Greater New York City Area
commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
commented
Issue summary: View changes
StatusFileSize
new inline_entity_form complex.png59.85 KB
oknate’s picture

oknate
Location Greater New York City Area
commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
commented
Title: Vague "This value should not be null." error. » Vague "This value should not be null." error when clicking node preview
oknate’s picture

oknate
Location Greater New York City Area
commented
Title: Vague "This value should not be null." error when clicking node preview » Vague error when clicking node preview
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
commented
Issue summary: View changes
Status: Active » Needs review
oknate’s picture

oknate
Location Greater New York City Area
commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
commented
Status: Needs review » Needs work

The patch needs work I'm seeing this error on another content type:

Warning: Invalid argument supplied for foreach() in inline_entity_form_node_preview_builder() (line 70 of modules/contrib/inline_entity_form/inline_entity_form.module).
inline_entity_form_node_preview_builder('node', Object, Array, Object)

This line is where the bug is:
foreach ($form_state->get('inline_entity_form') as &$widget_state) {

oknate’s picture

oknate
Location Greater New York City Area
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-12-24.txt902 bytes
new inline-entity-form-2932492-24.patch2.79 KB

Fixing bug in #23

larvymortera’s picture

larvymortera commented
StatusFileSize
new inline-entity-form-vague-error-2932492-25.patch523 bytes

This patch makes sure that IEFs submit callbacks are called when clicking "preview".

bojanz’s picture

bojanz commented

This is a dangerous change. It means that clicking Preview will save all of the inline entities. So if you click Cancel, or navigate away from the page, your data model will be left in an inconsistent state.

guypaddock’s picture

guypaddock commented

IMHO It's difficult to get an accurate node preview without committing the inline entities.

The only solution I can think of that would produce an accurate preview while also preventing persistence of the nested entities would be a database transaction that gets created during validation and rolled back after the preview is rendered, but I doubt there's a clean injection point for something like that and it would likely cause issues for the form cache.

guypaddock’s picture

guypaddock commented
Status: Needs review » Needs work

I take it back... with the patch from #24 applied:

  • Preview works.
  • Preview seems accurate.
  • The inline entities do not get committed before preview.
  • The node and all inline entities got saved properly after Preview.

However, it would be good for there to be a test included for this issue since this is not the first time this error has appeared in this module. See:

albertosilva’s picture

albertosilva
Primary language Basque
Location Basque Country
commented
Related issues: +#2830829: Entities are not updated during buildEntity() phase
StatusFileSize
new iniline-entity-form-vague-error-preview-2932492-29.patch690 bytes

I've found a really simple way to make complex widgets work, even with nested entities (inside paragraphs, like my use case). During InlineEntityFormComplex.php::extractFormValues(), which is called twice for each widget, that function is returning nothing the second time it is executed. Making it to be completely executed when the preview button is clicked seems to fix each and every one of the problems with IEF and previews:

  • No more "This value should not be null" errors
  • Entities are properly built without any need for patch #24 and #entity_builder
  • Preview works as expected, even for complex widgets inside paragraphs

It seems to me that this is a too much simple solution to be true, but it simply works, at least for my use case, with nested and non-nested complex widgets.

I would like some feedback from other developers, because maybe this is the solution we where looking for to solve all problems between IEF and previews.

albertosilva’s picture

albertosilva
Primary language Basque
Location Basque Country
commented
Status: Needs work » Needs review
geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Title: Vague error when clicking node preview » "This value should not be null." on node preview
Priority: Minor » Normal
guypaddock’s picture

guypaddock commented
Status: Needs review » Needs work

@alberto_silva: Won't that lead to the IEF changes getting committed on Preview? With the patch from #29, what happens if you make a change in IEF, Preview, then cancel? My understanding of the prior approach was that we did not want to actually _commit_ or save the IEF unless the real submit button that saved changes was clicked.

Also, your approach hard-codes this to just work for a English-language button named 'Preview', which means that it won't work as a fix for other languages AND it may not work if there are other preview-style buttons in sites that aren't actually titled 'Preview'.

My preference is for the patch in #24 (which now needs a re-roll) because it looks more reliable. Either way, we need a test to cover this issue that ideally confirms that previewing does not save changes and does not display an error.

guypaddock’s picture

guypaddock commented
StatusFileSize
new inline_entity_form-value_should_not_be_null_reroll_24-2932492-33.patch3.11 KB

Attached is a re-roll of #24 against 8.x-1.x. Still needs tests.

khaldoon_masud’s picture

khaldoon_masud commented

I spent two days trying to figure this out, and below did the trick for me:

$form['actions']['preview']['#limit_validation_errors'] = [];

Seems like inline_entity_form doesn't like if any other element is using AJAX on the form.

attheshow’s picture

attheshow commented

Patch #33 is working well for us.

a.sotirov’s picture

a.sotirov commented
StatusFileSize
new inline_entity_form-value_should_not_be_null_reroll_24-2932492-36.patch2.75 KB

@GuyPaddock I just added an additional t() function, because if the button "op" value for the preview button is translated the code will not be executed.

if (\Drupal::request()->request->get('op') == 'Preview') {
....

Now with the latest patch it should works on the translated button value.

david.muffley’s picture

david.muffley commented
StatusFileSize
new inline_entity_form-2932492-37.patch2.19 KB
new interdiff-36-37.txt820 bytes

Rerolled for rc-12. The previous patch included several unrelated code style changes that were covered by #3135178: Fix Coding Standards .

quadrexdev’s picture

quadrexdev
Location Lutsk
commented
StatusFileSize
new inline_entity_form-2932492-38.patch2.24 KB

Attaching a new patch with a changed check in inline_entity_form_node_preview_builder.

Comparing the "op" title didn't work for me cuz there could be a project-specific case with a changed title of the button. That will lead to a failed check 

if (\Drupal::request()->request->get('op') == t('Preview'))

So, added a check by triggered element ID. Not sure this is the best way but it works for any button title (Preview/Live preview/etc).