New filter to compare package rates and standard package rates

ecairol commented 1 year ago

Creates a new filter that will return the boolean comparison between the package rates and the standard_package rates.

This is to allow a theme to include shipping taxes on the comparison between package rates and standard package rates. More context on issue 430

Fixes #430

Description

There seems to be a data gap between Avatax, WooCommerce Subscriptions when creating a Subscription with Google Pay (WooCommerce Payments)

When Avatax sets the taxes for the shipping methods, it modifies the standard package rates but for some reason that change is not getting into the recurring cart rates.

While we aim to find and fix the root cause, the following filter will allow a theme to manually include the shipping taxes from the standard package rates to the actual package rates, and then return the == comparison from the hook function:

$package_rates_match = apply_filters(
    'wcs_recurring_shipping_package_rates_match_standard_rates', 
    $package['rates'] == $standard_packages[ $package_index ]['rates'], 
    $package['rates'], $standard_packages[ $package_index ]['rates'], 
    $recurring_cart_key
);

How to test this PR

Set up a site with the Avatax plugin and configure a flat rate
Configure the site to have Google Pay or Apple Pay enabled, via WooCommerce Payments
Try to create a Subscriptions directly from the Product page
The user will get the error "Invalid recurring shipping method"

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

thenbrent commented 1 year ago

Adding a filter here looks fine to me (previous concerns about finding a more robust fix notwithstanding).

@james-allan or @mattallan could you review this when you get a chance?

It's for p9MPsk-2dA-p2#comment-24685

I also notice there is an error on codesniffer due to use of weak comparison, but that's carried over from the existing code. If that's updated to use strict comparison to pass linting, the patch risks breaking backward compatibility, so will need more thorough review and testing.

james-allan commented 1 year ago

Given the urgency around finding a solution to this issue, I'm ok with moving forward with this solution if Team 51 are.

I'll have some time this afternoon to test this just to make sure this isn't introducing unexpected consequences. I'll come back later today my time and leave an official review.

james-allan commented 1 year ago

@ecairol I've just approved this PR. Feel free to merge, otherwise I'll come through and merge it tomorrow in preparation for a release. :)

Automattic / woocommerce-subscriptions-core