Yoast / wordpress-seo

Yoast SEO for WordPress
https://yoast.com/wordpress/plugins/seo/
Other
1.77k stars 893 forks source link

WPSEO_Post_Watcher - Trying to get property 'ID' of non-object #18719

Open alevgal opened 2 years ago

alevgal commented 2 years ago

Please give us a description of what happened.

After woocommerce product update I got an error Trying to get property 'ID' of non-object at .../plugins/wordpress-seo-premium/classes/post-watcher.php:234 After research I found that before product update WooCommerce Multilingual & Multicurrency plugin starts its synchronization process, during which it calls the function wp_delete_post but it passes the post_id argument as a string. After this, method WPSEO_Post_Watcher->check_public_post_status throws an error because post_id doesn't pass the condition

if ( is_int( $post ) ) {
    $post_id = $post;
}

Technical info

Used versions

mmikhan commented 2 years ago

Are you referring to the plugin here https://wordpress.org/plugins/woocommerce-multilingual/? Can you please share a step-by-step guide on reproducing the issue? Additionally, can you please share the exact file name from where you shared the relevant code snippet so we can dive into this further?

alevgal commented 2 years ago

To reproduce this issue:

  1. Install and activate Woocommerce, WPML, WooCommerce Multilingual & Multicurrency plugin
  2. Create variable product with variations and translations.
  3. Click Update.

As I wrote above, I have go deep to all srack trace of this error and found that during a product update WooCommerce Multilingual & Multicurrency calls a wordpress function wp_delete_post to remove unnecessary copies of variations. But passes the $post_id argument not as a number, but as a string. After that, the detect_post_delete method of the WPSEO_Post_Watcher class is called, which in turn calls several more methods of this class, including check_public_post_status which will cause an error. This method is located in the file .../plugins/wordpress-seo-premium/classes/post-watcher.php near line 222. This check gives an error:

if ( is_int( $post ) ) {
  $post_id = $post;
}
else {
 $post_id = $post->ID;
}

Since the $post variable in this case is a string, not an integer or an object

By reading the documentation for the function wp_delete_post I see that the $postid variable should be integer, but if you look at the function itself, you can see that the string will not cause an error and will work correctly. Therefore, in fact, this is not a bug of your plugin, but rather a bug of WooCommerce Multilingual & Multicurrency plugin. But to avoid such errors, you could change the check, for example:

if ( is_a( $post, 'WP_Post' ) ) {
  $post_id = $post->ID;
}
else {
 $post_id = absint($post);
}

But it's up to you. I solved this problem by disabling post watcher during this process, because it is not required there.

mmikhan commented 2 years ago

Thank you so much for the detailed information. As you have already said, it's not an issue on how we handle the condition but the WooCommerce Multilingual & Multicurrency with WPML plugin. So, it would be much better if the relevant 3rd party plugin developer was also aware of the issue.

That said, we'll see what we can do about it from our end. In the meantime, if you can submit this bug to the relevant plugin support, that would be fantastic!

mmikhan commented 2 years ago

Issue opened internally: IM-2051