Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Options Element settings override should check if allowed_values_function and default_value_function are set. #6654

Closed
herbdool opened this issue Jul 12, 2024 · 9 comments · Fixed by backdrop/backdrop#4829

Comments

@herbdool
Copy link

herbdool commented Jul 12, 2024

Description of the bug

When editing a list item field widget, I get:

Warning: Undefined array key "field_list" in list_form_field_ui_field_edit_form_alter() (line 158 of /app/httpdocs/core/modules/field/modules/list/list.module).
Warning: Trying to access array offset on value of type null in list_form_field_ui_field_edit_form_alter() (line 158 of /app/httpdocs/core/modules/field/modules/list/list.module).

Steps To Reproduce

This happens if creating a list field that uses allowed_values_function and default_value_function instead of the values set in the UI. This was originally added in #1005

Expected behavior

It should either not bother to alter the form or fetch the default value from default_value_function.

@herbdool herbdool changed the title options_element settings override should check if allowed_values_function and default_value_function are set Options Element settings override should check if allowed_values_function and default_value_function are set Jul 12, 2024
@herbdool herbdool added this to the 1.28.3 milestone Jul 12, 2024
@argiepiano
Copy link

argiepiano commented Jul 12, 2024

I'm having a hard time reproducing this error.

  • I created a text list field attached to an entity.
  • I manually set the allowed_values_function to a function that returns an array
Screen Shot 2024-07-12 at 5 21 44 PM

I visited the field UI and I'm not seeing a warning. I'm seeing this instead, which is the normal behavior.

Screen Shot 2024-07-12 at 5 23 19 PM

Will you be able to post more detailed instructions?

@herbdool
Copy link
Author

@argiepiano I suspect you may have to set default_value_function as well.

I'll try to recreate the error on a fresh site too.

@argiepiano
Copy link

argiepiano commented Jul 13, 2024

@herbdool I've been able to reproduce, but there are some caveats.

IF I add BOTH:

  • default_value_function to the field instance info, AND
  • allowed_values_function to the field info

then your PR solves the issue - no more warning. 👍🏽

IF I add ONLY:

  • allowed_values_function to the field info

There is no warning (before or after the PR).

BUT if I add ONLY:

  • default_value_function to the field instance info

then the warning persists, even with the PR. 👎🏽


I think your check is not quite right. if you do this instead:

  $field = $form['#field'];
  $instance = $form['#instance'];

  if (in_array($field['type'], array('list_integer', 'list_float', 'list_text'))) {
    // If the default value is set automatically skip setting the widget.
    if ($instance['default_value_function']) {
      return;
    }
//...

Then I believe the issue is solved.

@herbdool
Copy link
Author

@argiepiano thanks for testing. I was wondering which was better to check.

@argiepiano
Copy link

argiepiano commented Jul 13, 2024

@herbdool there are widespread test failures. Your PR does not define $instance.

In my sample code above I had:

$instance = $form['#instance'];

@herbdool
Copy link
Author

This is what happens when I do it on a phone.

@argiepiano
Copy link

@herbdool sorry, but there are still a lot of failing tests. Please notice my code - you are using $form['instance'], instead of $form['#instance'].

@argiepiano
Copy link

OK, reviewed and tested, LGTM.

@quicksketch
Copy link
Member

Thanks @herbdool, @argiepiano, and @klonos! I made an adjustment to the code formatting to define the $instance variable and then check that, like @argiepiano's code samples above. I merged backdrop/backdrop#4829 into 1.x and 1.28.x. Thanks!

@jenlampton jenlampton changed the title Options Element settings override should check if allowed_values_function and default_value_function are set Options Element settings override should check if allowed_values_function and default_value_function are set. Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment