AdvancedCustomFields / acf

Advanced Custom Fields
http://advancedcustomfields.com/
851 stars 174 forks source link

Checkbox field gets a PHP E_NOTICE for a missing 'multiple' index in the $field array. #300

Open rumur opened 4 years ago

rumur commented 4 years ago

Hey @elliotcondon

Recently I've updated the ACF PRO from 5.8.9 to 5.8.11 and started to get the E_NOTICE when I'm trying to create a checkbox.

The $field is missing a multiple index

Specs:

The stack trace of the error is the following:

ErrorException thrown with message "Undefined index: multiple"

Stacktrace:
#17 ErrorException in /pathToProject/htdocs/content/mu-plugins/advanced-custom-fields-pro/includes/fields/class-acf-field-select.php:492
#16 Themosis\Core\Bootstrap\ExceptionHandler:handleError in /pathToProject/htdocs/content/mu-plugins/advanced-custom-fields-pro/includes/fields/class-acf-field-select.php:492
#15 acf_field_select:update_field in /pathToProject/htdocs/content/mu-plugins/advanced-custom-fields-pro/includes/fields/class-acf-field-checkbox.php:445
#14 acf_field_checkbox:update_field in /pathToProject/htdocs/cms/wp-includes/class-wp-hook.php:287
#13 WP_Hook:apply_filters in /pathToProject/htdocs/cms/wp-includes/plugin.php:249
#12 apply_filters_ref_array in /pathToProject/htdocs/content/mu-plugins/advanced-custom-fields-pro/includes/acf-hook-functions.php:101
#11 _acf_apply_hook_variations in /pathToProject/htdocs/cms/wp-includes/class-wp-hook.php:287
#10 WP_Hook:apply_filters in /pathToProject/htdocs/cms/wp-includes/plugin.php:206
#9 apply_filters in /pathToProject/htdocs/content/mu-plugins/advanced-custom-fields-pro/includes/acf-field-functions.php:971
#8 acf_update_field in /pathToProject/htdocs/content/mu-plugins/advanced-custom-fields-pro/includes/admin/admin-field-group.php:488
#7 acf_admin_field_group:save_post in /pathToProject/htdocs/cms/wp-includes/class-wp-hook.php:289
#6 WP_Hook:apply_filters in /pathToProject/htdocs/cms/wp-includes/class-wp-hook.php:311
#5 WP_Hook:do_action in /pathToProject/htdocs/cms/wp-includes/plugin.php:478
#4 do_action in /pathToProject/htdocs/cms/wp-includes/post.php:4153
#3 wp_insert_post in /pathToProject/htdocs/cms/wp-includes/post.php:4244
#2 wp_update_post in /pathToProject/htdocs/cms/wp-admin/includes/post.php:409
#1 edit_post in /pathToProject/htdocs/cms/wp-admin/post.php:227
#0 require in ~/.composer/vendor/laravel/valet/server.php:158

The Content of that $field:

array:1 [▼
  "field" => array:24 [▼
    "ID" => 13026
    "key" => "field_5ecb95ae4f017"
    "label" => "Checkbox Test"
    "name" => "testing"
    "prefix" => ""
    "type" => "checkbox"
    "value" => null
    "menu_order" => 0
    "instructions" => ""
    "required" => 0
    "id" => ""
    "class" => ""
    "conditional_logic" => 0
    "parent" => 13025
    "wrapper" => array:3 [▶]
    "choices" => array:2 [▼
      1 => "CheckBox"
      2 => "Select"
    ]
    "allow_custom" => 0
    "default_value" => []
    "layout" => "vertical"
    "toggle" => 0
    "return_format" => "value"
    "_name" => "testing"
    "_valid" => 1
    "save_custom" => 0
  ]
]
elliotcondon commented 4 years ago

Hi @rumur

Thanks for the bug report. Can you please test the following fix? Please edit the file "includes/fields/class-acf-field-checkbox.php" and change line 445 from:

return acf_get_field_type('select')->update_field( $field );

To:

// Decode choices (convert to array).
$field['choices'] = acf_decode_choices( $field['choices'] );
$field['default_value'] = acf_decode_choices( $field['default_value'], true );
return $field;
rumur commented 4 years ago

Hi @elliotcondon

I've tried your snippet and it worked out. I didn't get any E_NOTICE this time.

Beee4life commented 4 years ago

@elliotcondon not getting it on first attempt... but had it on mult. sites I think so will check.

rumur commented 4 years ago

Not relevant to this issue, just a question.

@elliotcondon after you mentioned acf_deceode_choice function, I've looked inside of it and noticed one thing at this point https://github.com/AdvancedCustomFields/acf/blob/ddf8837941c7422968aa8ac10ae7cd176ea36482/includes/api/api-helpers.php#L2110

if the passed string is already an array, you bail right away, but what if you've passed $array_keys as true, you won't be able to get array keys in this case, because the condition that checks it is placed at the very end.

Is it ok like this?

elliotcondon commented 4 years ago

Hi @rumur. Yes, for our use-case, this is perfectly fine :)

Beee4life commented 4 years ago

@elliotcondon is there a hotfix coming for this anytime soon ? We have a ton of sites to manage.

elliotcondon commented 4 years ago

Hi @Beee4life

Yes. A new version will be available in the next few days 👍

Beee4life commented 4 years ago

After applying aforementioned fix, I got this error, when I saved a field group. I just changed a text field (in a repeater) to select (no choices defined, because they'll be added with acf/load_field).

Notice: Undefined index: parent in /var/www/html/web/app/plugins/advanced-custom-fields-pro/includes/admin/admin-field-group.php on line 483

Warning: Cannot modify header information - headers already sent by (output started at /var/www/html/web/app/plugins/advanced-custom-fields-pro/includes/admin/admin-field-group.php:483) in /var/www/html/web/wp/wp-includes/pluggable.php on line 1281

Warning: Cannot modify header information - headers already sent by (output started at /var/www/html/web/app/plugins/advanced-custom-fields-pro/includes/admin/admin-field-group.php:483) in /var/www/html/web/wp/wp-includes/pluggable.php on line 1284
elliotcondon commented 4 years ago

@Beee4life Can you please confirm which fix you have applied? This new error does not seem related at this stage.

Beee4life commented 4 years ago

I replaced

return acf_get_field_type('select')->update_field( $field );

with:

// Decode choices (convert to array).
$field['choices'] = acf_decode_choices( $field['choices'] );
$field['default_value'] = acf_decode_choices( $field['default_value'], true );
return $field;
elliotcondon commented 4 years ago

Thanks @Beee4life

This has me completely stumped. I've attempted to replicate the issue, but can't get any kind of PHP error to occur when changing a sub Text field into a Select field. I also can't see how this change to the Checkbox field would affect a Select field.

If it's not too much to ask, can you please spin up a fresh WP site in "Local by Flywheel" and try to replicate the issue? Please keep the default theme active and edit the functions.php file with any extra logic needed.​

​​If you can replicate the issue, please export the site (from flywheel app, right click the site and click "export") and attach to your next reply. ​I'll import this and get to work replicating / testing the issue :)

Beee4life commented 4 years ago

I encountered it on a personal project, so it's not too big of a deal...

But if it happens again, I might be able to do you a better one... I can give you read access to the repository I'm using to maintain the site, which has everything (if you know how to start a Docker container). I can also provide SQL if needed but not through GH of course.

Will be working on it a lot in few days, so if it happens I'll pick up on it soon (I think). I'll post it here, when I encounter it again. You know where to reach me (if needed) to get some specific info.

Beee4life commented 4 years ago

@elliotcondon I just got the error again... the action didn't complete so had to navigate to the field group again and then I saw I got 2 new empty fields.

Screenshot 2020-07-07 at 22 09 31

I removed the fields and saved it again, no probs whatsoever.

I have to say I haven't had this issue once since my last post.

Beee4life commented 4 years ago

PS running v5.8.12

elliotcondon commented 4 years ago

Hi @Beee4life

Thanks for the update. Are you able to replicate this on a fresh WP install? I wonder if there is something specific with either your theme code, plugins installed or the field group data that is causing the problem. If you are able to narrow down the variables, that would be a great help!

Beee4life commented 4 years ago

Didn't happen again, so not able yet to replicate.