acf-extended / ACF-Extended

🚀 All-in-one enhancement suite that improves WordPress & Advanced Custom Fields
https://www.acf-extended.com
238 stars 27 forks source link

Fail to access a Woocommerce order edit screen #122

Closed LucasDemea closed 4 months ago

LucasDemea commented 10 months ago

Describe the bug When trying to access a Woocommerce order. I get this error:

Function ID was called incorrectly. Order properties should not be accessed directly. Backtrace: do_action('load-woocommerce_page_wc-orders'), WP_Hook->do_action, WP_Hook->apply_filters, Automattic\WooCommerce\Internal\Admin\Orders\PageController->call, call_user_func_array, Automattic\WooCommerce\Internal\Admin\Orders\PageController->handle_load_page_action, Automattic\WooCommerce\Internal\Admin\Orders\PageController->setup_action_new_order, Automattic\WooCommerce\Internal\Admin\Orders\PageController->prepare_order_edit_form, Automattic\WooCommerce\Internal\Admin\Orders\Edit->setup, do_action('add_meta_boxes'), WP_Hook->do_action, WP_Hook->apply_filters, acfe_module_post->add_meta_boxes, WC_Abstract_Legacy_Order->get, wc_doing_it_wrong

This has to do with this line: https://github.com/acf-extended/ACF-Extended/blob/318659f6f6fa646383cb049601fabf330d0b34ca/includes/module-post.php#L44C1-L44C1

Which gets the $post->ID, which woocommerce doesn't allow.:

If I replace $post->ID with $post->get_id(), it fixes the issue in this case.

To Reproduce Access a woocommerce order edit screen with acf, acf extended & woocommerce enabled.

Expected behavior I can access order edit screen.

Logs See details and full log here : https://flareapp.io/share/V7jW4ao5

WordPress & ACF WordPress version: 6.4.1 ACF Pro version: 6.2.3 ACF add-ons: ACF extended 0.8.9.5

Add any other context about the problem here.

acf-extended commented 10 months ago

Hello,

Thanks for the feedback and the detailed report.

The issue come from this line added in a recent update of Woocommerce:

do_action( 'add_meta_boxes', $this->screen_id, $this->order );

In the file /woocommerce/src/Internal/Admin/Orders/Edit.php:159

Woocommerce fires the native add_meta_boxes hook, but instead of filling arguments with the WP_Post, it uses a custom WC_Order object.

The WP documentation explicitly requires a $post_type as first argument and a WP_Post as second argument. So it looks like a case doing it wrong here.

This code will produce the same notice on any third party plugin that use this hook and expect a WP_Post, as it's the common hook to add native meta boxes. I'll open a ticket on their repo to let them know.

The notice can be reproduced using this code in the functions.php file without ACF or ACF Extended:

add_action('add_meta_boxes', 'my_add_meta_boxes', 10, 2);
function my_add_meta_boxes($post_type, $post){
    $post_id = $post->ID;
}

In the meantime, I wouldn't recommend to use your solution $post->get_id(), as it will break the normal WP logic (when using a WP_Post). Instead use the following quick fix:

// validate post
if(!isset($post)){
    return;
}

// validate WP_Post
if(!is_a($post, 'WP_Post')){
    return;
}

// get module
$module = acfe_get_module_by_item($post->ID);

I'll add this safe guard logic in the next patch just in case, but Woocommerce should fix that part in their code IMO, to avoid breaking other plugins too.

Hope it helps!

Have a nice day!

Regards.

acf-extended commented 10 months ago

Linked WooCommerce repo ticket: https://github.com/woocommerce/woocommerce/issues/41579

LucasDemea commented 10 months ago

Thanks for the quick and detailed answer. Sure, I wasn't going to use my quick fix. It was only to demonstrate the cause of the issue. I'll use your quick patch instead until you fix it, thanks.

acf-extended commented 4 months ago

Hello,

Just a heads up to let you know that I included an additional safe guard logic in the latest 0.9 patch, to avoid this issue in the future. I think (but not 100% sure) Woocommerce fixed the issue on their side in the meantime.

Thanks again for the report.

Have a nice day! Regards.