Always pass the object ID as the second param to the 'get_edit_post_link' filter

[james-allan]

Fixes #822

Description

The wcs_get_edit_post_link() helper function is designed to get the URL for the edit screen of an order/subscription.

It's designed to take an order or subscription ID as the parameter, however there are cases where we pass the full order/subscription object.

If you're a 3rd party hooking onto the 'get_edit_post_link' filter, this means you'd sometimes get a whole order/subscription object - not just an ID.

This PR fixes that by making sure we always pass the ID as the second parameter to the get_edit_post_link filter in line with it's intended design.

This also appears to fix warnings like this:

PHP Deprecated:  Creation of dynamic property WC_Subscription::$ID is deprecated in /plugins/woocommerce-subscriptions-core/includes/class-wc-subscription.php on line 211
PHP Deprecated:  Creation of dynamic property WC_Subscription::$filter is deprecated in /plugins/woocommerce-subscriptions-core/includes/class-wc-subscription.php on line 211

🗒️ I was curious where these dynamic property errors were coming from so I logged the stack trace. I've included it below for completeness.

Details

#0 app/public/wp-includes/post.php(2830): WC_Subscription->__set('ID', 0)
#1 app/public/wp-includes/post.php(1101): sanitize_post(Object(WC_Subscription), 'raw')
#2 app/public/wp-includes/post.php(1571): get_post(Object(WC_Subscription))
#3 app/public/wp-content/plugins/woocommerce/src/Internal/DataStores/Orders/CustomOrdersTableController.php(798): get_post_type(Object(WC_Subscription))
#4 app/public/wp-includes/class-wp-hook.php(326): Automattic\WooCommerce\Internal\DataStores\Orders\CustomOrdersTableController->maybe_rewrite_order_edit_link('http://wcsubscr...', Object(WC_Subscription))
#5 app/public/wp-includes/plugin.php(205): WP_Hook->apply_filters('http://wcsubscr...', Array)
#6 app/public/wp-content/plugins/woocommerce-subscriptions-core/includes/wcs-helper-functions.php(71): apply_filters('get_edit_post_l...', 'http://wcsubscr...', Object(WC_Subscription), '')
#7 app/public/wp-content/plugins/woocommerce-subscriptions-core/includes/admin/class-wc-subscriptions-admin.php(1595): wcs_get_edit_post_link(Object(WC_Subscription))
#8 app/public/wp-includes/class-wp-hook.php(324): WC_Subscriptions_Admin::display_renewal_filter_notice('')
#9 app/public/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters(NULL, Array)
#10 app/public/wp-includes/plugin.php(517): WP_Hook->do_action(Array)
#11 app/public/wp-admin/admin-header.php(303): do_action('admin_notices')
#12 app/public/wp-admin/admin.php(239): require_once('/Users/...')

Essentially:

1. we're doing this: apply_filters( 'get_edit_post_l...', 'http://...', Object(WC_Subscription), '' )
2. WooCommerce core hooks onto that filter and attempts to rewrite it to the new HPOS link.
3. In the process of doing that they call get_post_type(Object(WC_Subscription)) which WP eventually tries to set the ID. 😨

As far as I can tell this doesn't actually lead to any issues.

How to test this PR

1. Make sure HPOS is enabled.
2. Go to WooCommerce → Subscriptions
3. Click on one of the Orders columns to filter the order list by that subscriptions orders.

6. Observe the warnings I mentioned above about dynamic variables.
7. You can also use the example code snippet below to illustrate the inconsistent second param type.

add_filter( 'get_edit_post_link', function( $url, $post_id ) {
	error_log( '$post_id type: ' . gettype( $post_id ) );
	return $url;
}, 10, 2 );

Product impact

• Added changelog entry (or does not apply)
• Will this PR affect WooCommerce Subscriptions? yes/no/tbc, add issue ref
• Will this PR affect WooCommerce Payments? yes/no/tbc, add issue ref
• Added deprecated functions, hooks or classes to the spreadsheet

[leonardola]

Completely out of scope but I noticed that we're calling wcs_get_objects_property in some places which is a deprecated function. Feel free to fix that in another PR.

Other than that the solution works great. Thanks for the fix.