Automattic / woocommerce-services

WooCommerce Services is a feature plugin that integrates hosted services into WooCommerce (3.0+), and currently includes automated tax rates and the ability to purchase and print USPS shipping labels.
GNU General Public License v2.0
107 stars 20 forks source link

No way to override rates from TaxJar if they are incorrect #2249

Closed jessepearson closed 2 years ago

jessepearson commented 3 years ago

Following up to the thread here: p1604238751225700-slack-C7U3Y3VMY

In this particular instance the customer would like to override tax rates for Tenerife to 0%. This is due to although Tenerife is located within the EU, they do not get charged VAT. TaxJar is retuning VAT rates and applying them to customers with addresses in Tenerife.

This means there are two problems:

Ticket is here: 3428621-zen

I tagged this as a High priority Bug due to I believe it's on us to look into the incorrect rates we're providing our customers, and to provide a work around if possible.

thelogicwizards commented 2 years ago

Here's a case of this, also: 4784459-zd-woothemes

This is a merchant based in Milan, Italy. They are returning rates for Berlin, Germany zip codes 10178 & 10015 at 22% instead of the 19% suggested by the TaxJar calculator.

iyut commented 2 years ago

Hi @jessepearson @thelogicwizards , I did some digging and testing for this issue. And it seems the plugin does using whatever value that TaxJar returning, and the plugin doesn't manipulate any tax rate from TaxJar. And in order for the user to be able manipulating the tax rate value, we will need to add a filter around this line.

Perhaps we can do something like this :

$tax_rate = apply_filters( 'taxjar_tax_rate', array(
    'tax_rate_country'  => $location['to_country'],
    'tax_rate_state'    => $to_state,
    // For the US, we're going to modify the name of the tax rate to simplify the reporting and distinguish between the tax rates at the counties level.
    // I would love to do this for other locations, but it looks like that would create issues.
    // For example, for the UK it would continuously rename the rate name with an updated `state` "piece", each time a request is made
    'tax_rate_name'     => sprintf( '%s Tax', self::generate_tax_rate_name( $taxjar_response, $location['to_country'], $to_state ) ),
    'tax_rate_priority' => 1,
    'tax_rate_compound' => false,
    'tax_rate_shipping' => $freight_taxable,
    'tax_rate'          => $rate,
    'tax_rate_class'    => $tax_class,      
), $taxjar_response, $location );

Once it's implemented, the user can add this code in their theme/functions.php to manipulate the tax rate value :

function change_tax_rate( $tax_rate, $response, $location ) {
    if ( 'DE-BE' === $location['to_state'] && '10178' === $location['to_zip'] ) {
        $tax_rate['tax_rate'] = 19; // 22%
    }
    return $tax_rate;
}
add_filter( 'taxjar_tax_rate', 'change_tax_rate', 10, 3 );

What do you guys think? Will this be helpful for the user? Or if you guys have any other thought, let me know.

waclawjacek commented 2 years ago

That is definitely a good solution! However, IIRC as I was digging into that on a different occasion, WCS&T will always save the rates from TaxJar using the same, predictable format for each data column, including priority.

So if I create a custom rate for a specific row and give it a higher priority (in this example: 100; this is the opposite of what is considered "higher priority" in WP filters where lower = earlier):

Markup on 2022-06-09 at 11:07:41

My custom rate from the second row will be applied.

Is this something that we should add to the documentation?

iyut commented 2 years ago

So if I create a custom rate for a specific row and give it a higher priority (in this example: 100; this is the opposite of what is considered "higher priority" in WP filters where lower = earlier):

Thanks, it's good to know this! I did some test by adding new row of tax with different priority to confirm this : image

The custom rate is adding the tax value to TaxJar rate value. So if I have TaxJar rate 22% and I add new custom rate 22%, the total tax will be 44%.

The tax value only from TaxJar ( without custom rate ) : image

The tax with custom rate and TaxJar : image

Is this something that we should add to the documentation?

Yeah, I think it will be good to add this information to the documentation.

That is definitely a good solution!

Thanks, let me try to implement this solution.

waclawjacek commented 2 years ago

To automatically provide a user interface for users, what do you think about doing something a bit different and adding a custom tax class that WCS&T could be writing to instead of the "Standard" tax class, and then if a matching tax is found in the Standard class, use that but otherwise, use the rate from the "Automated" class?

The opposite could also be done - keep WCS&T writing to "Standard" but add a custom "Automated taxes override" tax class.

Would that be feasible? It might lead to some complications that I didn't think about, including regarding compatibility. 🤔

Markup on 2022-06-09 at 14:11:00

iyut commented 2 years ago

I think this is a good idea. The user will have easier way to manipulate the tax rate with this feature. But I will need to check the feasibility of this feature first. And since this feature might require big changes and more testing, what do you think if we create separate enhancement task?

I could add the TaxJar filter first as it's faster to implement, and so another user can have benefit of the TaxJar filter if they want to manipulate the tax value.

Then we could continue to add the enhancement.

waclawjacek commented 2 years ago

Sounds good! Please make sure that with this overriding logic, the filter is added late enough to override rates all of the time (or most of the time), and early enough so unnecessary calculations aren't performed. The reason why I'm saying this is I think you are currently filtering the address that will be sent to TaxJar.

Maybe it's more beneficial to exit the method early with a predefined return value?

iyut commented 2 years ago

The filter that I suggested in this comment is located in WC_Connect_TaxJar_Integration::create_or_update_tax_rate() method. The filter will be called after TaxJar response but before the plugin updating the tax rate using WC_Tax::_update_tax_rate() or WC_Tax::insert_tax_rate().

I also plan to implement a couple of filter around WC_Connect_TaxJar_Integration in this PR. so the user could have more than one alternative to override the TaxJar result.

Maybe it's more beneficial to exit the method early with a predefined return value?

I am still not sure on how we do this, to be honest. Do you have some specific idea that we can put into the PR?

waclawjacek commented 2 years ago

The filter that I suggested in this comment is located in WC_Connect_TaxJar_Integration::create_or_update_tax_rate() method. The filter will be called after TaxJar response but before the plugin updating the tax rate using WC_Tax::_update_tax_rate() or WC_Tax::insert_tax_rate().

Gotcha! I took the time to understand the code there and it looks good! How about naming the filter something like woocommerce_services_pre_taxjar_rate_save to namespace it and to distinguish it from other filters that might be doing things with the tax rate in WCS&T?

I also plan to implement a couple of filter around WC_Connect_TaxJar_Integration in https://github.com/Automattic/woocommerce-services/pull/2561. so the user could have more than one alternative to override the TaxJar result.

Love it! :100: 🚀

Maybe it's more beneficial to exit the method early with a predefined return value?

I am still not sure on how we do this, to be honest. Do you have some specific idea that we can put into the PR?

Sorry for not being clear enough here!

So in this case, the merchant wants to override tax rates for Tenerife to 0%. We know that we want the rate to always be 0%. To speed things up a bit, we could avoid making the API request to get the tax rate altogether.

We could do this by adding code such as below around here in WC_Connect_TaxJar_Integration::calculate_tax():

$pre_taxjar_request = apply_filters( 'wcship_pre_taxjar_request', false, $some_params_we_might_want_to_pass );

if ( $pre_taxjar_request ) {
    return $pre_taxjar_request;
}

Then if anything truthy is returned from the filter, the TaxJar request doesn't get executed at all and we can bail early based on what the request location is.

iyut commented 2 years ago

So in this case, the merchant wants to override tax rates for Tenerife to 0%. We know that we want the rate to always be 0%. To speed things up a bit, we could avoid making the API request to get the tax rate altogether.

We could do this by adding code such as below around here in WC_Connect_TaxJar_Integration::calculate_tax():

Thanks, this is good idea! I will surely implement this.