woocommerce / woocommerce

A customizable, open-source ecommerce platform built on WordPress. Build any commerce solution you can imagine.
https://woocommerce.com
9.42k stars 10.77k forks source link

Bug in get_option function in WC_Admin_Settings #48254

Open mryoubou opened 5 months ago

mryoubou commented 5 months ago

Prerequisites

Describe the bug

Here Line 256: https://github.com/woocommerce/woocommerce/blob/5151546159276c72a2b3db2c76d79af0f137091f/plugins/woocommerce/includes/admin/class-wc-admin-settings.php#L256 There is a bug in the code where get_option function is using field id instead name to retrieve the option value. This bug cause issue when getting a field value from an array option.

Fix : use $value['field_name'] instead of $value['id'] as the first parameter: $value['value'] = self::get_option( $value['field_name'], $value['default'] );

Expected behavior

Get a 'example' field value from an array option. eg: 'field_name'=> 'option_name[example]'

Actual behavior

Not getting 'example' field value mentioned above from an array option.

Steps to reproduce

N/A

WordPress Environment

N/A

Isolating the problem

barryhughes commented 5 months ago

@mryoubou could you provide some steps to replicate? This could contain custom code snippets we can invoke via WP CLI, if that is the only way to see the issue. This will help us to arrive at a better understanding of things—thank you!

mryoubou commented 5 months ago

I just made this function that add this extra options to woocommerce general settings as an example:

add_filter( 'woocommerce_general_settings', 'extra_woocommerce_general_settings' );

function extra_woocommerce_general_settings( $settings = array() ) {

    $extra_settings =
        array(

            array(
                'title' => __( 'Extra options', 'woocommerce' ),
                'type'  => 'title',
                'desc'  => '',
                'id'    => 'extra_options',
            ),

            array(
                'title'         => __( 'Enable options', 'woocommerce' ),
                'desc'          => __( 'Enable options one', 'woocommerce' ),
                'id'            => 'woocommerce_option_one',
                'field_name'    => 'woocommerce_enabled_options[one]',
                'default'       => 'yes',
                'type'          => 'checkbox',
                'checkboxgroup' => 'start',
            ),

            array(
                'desc'          => __( 'Enable options two', 'woocommerce' ),
                'id'            => 'woocommerce_option_two',
                'field_name'    => 'woocommerce_enabled_options[two]',
                'default'       => 'no',
                'type'          => 'checkbox',
                'checkboxgroup' => 'end',
            ),

            array(
                'type' => 'sectionend',
                'id'   => 'extra_options',
            ),
        );

        return array_merge( $settings, $extra_settings );
}

Screenshot 2024-06-13 135500

Since self::get_option here in line 256 use id( woocommerce_option_one and woocommerce_option_two) to get the option value, it doesn't get the correct value after we save the settings, it only return the default value because it use the wrong option name instead of the saved one as shown bellow :

Screenshot 2024-06-13 135243

https://github.com/woocommerce/woocommerce/blob/5151546159276c72a2b3db2c76d79af0f137091f/plugins/woocommerce/includes/admin/class-wc-admin-settings.php#L256

to fix this issue which I think its just a typo, change it to field_name( woocommerce_enabled_options[one], woocommerce_enabled_options[two] , etc..) instead of id and it will work fine : $value['value'] = self::get_option( $value['field_name'], $value['default'] );

I hope this example help you understand this issue, if you need any more details let me know. Thank you!

barryhughes commented 5 months ago

Thank you, @mryoubou: great explanation, and I can now see what you're describing directly in my local environment :+1:

Given you have a pretty good handle on this, are you interested in submitting a pull request, or prefer that we do so? No pressure if you prefer not to, but we'd certainly be receptive to a PR if you want to go ahead and create one.

mryoubou commented 5 months ago

please just go ahead and submit it on my behalf, that would be great :)