Problem/Motivation

We're getting the following notices after updating to the latest webform version:

Notice: Undefined index: primer_event_signup_form in Drupal\webform\Plugin\Field\FieldWidget\WebformEntityReferenceSelectWidget->formElement() (line 106 of .../web/modules/contrib/webform/src/Plugin/Field/FieldWidget/WebformEntityReferenceWidgetTrait.php)"

The code blindly assumes that it can access $user_input[$field_name][$delta]['target_id'];, but there are multiple reasons why that is wrong:

a) Users might not have access to that field
b) The widget might be inside paragraphs, IEF or other nested structures and not on the root-level of the form.

I'm also not sure why it has to be unvalidated raw user input and not regular values.

Proposed resolution

Rely on the element parents to get the id.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#5 webform-target-id-notice-3134351-5.patch1.15 KBberdir

Comments

Berdir created an issue. See original summary.

berdir’s picture

Will create a patch later, both these seem to work for me in first tests:

$target_id = NestedArray::getValue($user_input, array_merge($element['target_id']['#field_parents'], [$field_name, $delta, 'target_id']));
$target_id = $form_state->getValue(array_merge($element['target_id']['#field_parents'], [$field_name, $delta, 'target_id']))
jrockowitz’s picture

berdir’s picture

I saw that has been committed already since then. It does indeed fix the notice, but I don't think it's a proper fix. It will simply not find the value in the user input values, which means it will _not_ update to use the correct webform information then. Also, there's no reason for multiple issets, one would have been enough, it's smart enough to check all the keys.

My fix based on the parents tree should find it in case it is nested as well and not fail if a key isn't defined in case of access.

berdir’s picture

Status: Active » Needs review
StatusFileSize
new1.15 KB

Here's a patch that uses $form_state->getValue(), I think that should work, we'll see if tests disagree.

Also reverted the isset stuff, not necessary anymore in the first case and never was in the second. we already access $delta earlier, so that has to exist and ->target_id is a field property, that also can't not exist if the delta exists. And the function will not be called with an on-existing delta.

Status: Needs review » Needs work

The last submitted patch, 5: webform-target-id-notice-3134351-5.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jrockowitz’s picture

Status: Needs work » Needs review

@Berdir Thanks for the patch.

LOL. The broken test is just an annoying timezone issue that I have not been able to fix.

The $form_state->isRebuilding() code is only called via Ajax and there is no test coverage. I will do some manual testing and document my steps here.

jrockowitz’s picture

Steps to test

  • Close the 'Contact' webform. -- /admin/structure/webform/manage/contact/settings/form

Webform Node

  • Create a 'Contact' webform node -- /node/add/webform?webform_id=contact
  • Confirm "The Contact webform is closed. The below status will be ignored." warning is displayed
  • Toggle between the closed 'Contact' form and another webform. Confirm warning is only visible for the 'Contact' form

Paragraphs

  • Enable webform_test_paragraphs.module.
  • Create a Paragraph node with a webform. -- /node/add/paragraphs
  • Toggle between the closed 'Contact' form and another webform. Confirm warning is only visible for the 'Contact' form

The Paragraph node is failing to show the warning without the patch. The patch remove the notice and fixes the issue. I did not know that \Drupal\Core\Form\FormStateValuesTrait::getValue supports an array of keys.

  • Berdir authored f709fcd on 8.x-5.x
    Issue #3134351 by Berdir: Notice: Undefined index: fieldname in...
jrockowitz’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.