Closed RiaanKnoetze closed 4 years ago
The issue here is actually that the county fields don't accept spaces.
Any county will work, genuine or not (gibberish will also work in this field), but as soon as you add a space it will clear the tax.
A major bug still present in the most current version.
And to clarify/confirm, you’ll notice that the three example addresses used in the original ‘steps to replicate’ above all had spaces in them, this is why he was able to force this error in the examples he used.:
County Armagh South Yorkshire East Sussex
Unfortunately he put this down to them being genuine counties rather than the true cause which is that it doesn’t accept spaces.
Any update on this? I noticed a major update to Jetpack today but still no fix for this major bug?!
Sorry for the delay. We'll expedite this.
Another case here: 14189124-hc
Sorry for the delay. We'll expedite this.
Hi allendav, any updates please? I hear this has now been patched within Taxjar today in it's latest release. Can you tell me when this will be reflected in Jetpack please?
Any updates here guys? How come support for JetPack is so slow, especially for such a major bug?
Any updates please? I notice there have been a few Jetpack updates recently but still non of them correct this tax bug?
Anyone?
Another in 2524915-zen
Also reported in 2537618-zen
@laurendavissmith this is actually a major bug for store owners. Not collecting taxes properly leads to substantial financial loses for them.
Can we revisit the priority of this bug and add it to the next milestone?
@treibalen I completely agree. @laurendavissmith why has this been removed? There needs to be more urgency in patching this, it has been present for almost a year on my store now! It should have been addressed immediately.
I can confirm the bug is still present, for instance if you add 'Devon Devon' as the county the VAT clears to zero. If you add 'Devon', 'Devon ' or ' Devon' (spaces before and after the word) it works fine. So it is only a problem when a word or letter follows a space. There are plenty of counties in the UK which use two words, such as 'Greater Manchester' or 'Greater London'. For these I lose money on every order as the tax isn't collected. Please can someone from Jetpack or WooCommerce Services confirm that this is going to be patched asap.
Hi, this is still an ongoing issue, my newly launched site for a client is experiencing the same issue, the scary thing is that "automatic gurus" have directed me to this board, what scares me is that such a major problem has been ongoing for nearly 8 months and still isn't resolved!! - Is it worth customizing the shipping/cart/address pages to make this field invisible until it's fixed? that is if it ever will be fixed!!??
Hi, this is still an ongoing issue, my newly launched site for a client is experiencing the same issue, the scary thing is that "automatic gurus" have directed me to this board, what scares me is that such a major problem has been ongoing for nearly 8 months and still isn't resolved!! - Is it worth customizing the shipping/cart/address pages to make this field invisible until it's fixed? that is if it ever will be fixed!!??
I’m in the same boat as you @Wassmo1. I thought hiding the county field would be a quick temporary fix too but unfortunately it just causes more problems as customers who have already added a county within their account when registering will hit errors in the cart. The only solution to this problem is for Jetpack to pull their finger out and patch it like they should have done when I reported it back in February. I pay for premium support, yet, like you, have just been sent to this page where no one seems to acknowledge how serious the issue is.
Hi, this is still an ongoing issue, my newly launched site for a client is experiencing the same issue, the scary thing is that "automatic gurus" have directed me to this board, what scares me is that such a major problem has been ongoing for nearly 8 months and still isn't resolved!! - Is it worth customizing the shipping/cart/address pages to make this field invisible until it's fixed? that is if it ever will be fixed!!??
I’m in the same boat as you @Wassmo1. I thought hiding the county field would be a quick temporary fix too but unfortunately it just causes more problems as customers who have already added a county within their account when registering will hit errors in the cart. The only solution to this problem is for Jetpack to pull their finger out and patch it like they should have done when I reported it back in February. I pay for premium support, yet, like you, have just been sent to this page where no one seems to acknowledge how serious the issue is.
Hi mikelanemods, sorry to hear, as we are a trade website we have our own registration form which then needs authorization internally so I can update and remove customer details from here I hope? does it affect any new accounts once the field's hidden/removed? Like you I have a business plan with WordPress and am shocked this issue has been like this for 8 months!!?
@Wassmo1 Yes it’s really bad, JetPack should be ashamed. There’s no sense of urgency even though it’s costing store owners like myself thousands of pounds. If you cleared the county data from your existing customers in your database first, then hid the county field and check your theme hasn’t marked the county field as required you may be able to try and hide the field. However be aware you may face other issues such as with shipping etc, most courier companies for instance require a county to ship. This is another reason why I can’t just simply hide the field unfortunately. I have been relying on JetPak to fix it, considering it’s a major bug I’m suprised they haven’t done so yet. TaxJar, which I believe is used for the tax calculations within JetPack patched the bug a long while back, but only within their standalone plugin. It hasn’t been carried over to JetPack or WooCommerce Services yet. Why? Only JetPack can answer that but they don’t seem willing to. Please escalate and address this ASAP JetPack! Please!!!!
Just wanted to pop in and mention that we'll be taking a look at this one first thing next week
Great. That’s good news @laurendavissmith. Please keep us updated.
I was able to reproduce this problem and get the duplicated entries behaviour described above.
When I use County Armagh
as county, multiple entries are being added to the "Standard Rate" as described above. And VAT 0
is calculated as tax.
When I was using Plympton
as country, rate was returned fine.
After taking a look, the problem seems to be how we insert the tax entry into our database. This code is in our other project woocommerce
.
When we create an entry, we run our tax entry through the below code:
https://github.com/woocommerce/woocommerce/blob/104f40c36e3b6495165c8a7bdae3d396e4c75f39/includes/class-wc-tax.php#L1024
Notice how we pass $tax_rate
into self::prepare_tax_rate()
. Originally, the $tax_rate
is:
> $tax_rate
array(8)
tax_rate_country:"GB"
tax_rate_state:"County Armagh"
tax_rate_name:"County Armagh Tax"
tax_rate_priority:1
tax_rate_compound:false
tax_rate_shipping:1
tax_rate:20
tax_rate_class:""
After, it becomes
> self::prepare_tax_rate( $tax_rate )
array(8)
tax_rate_country:"GB"
tax_rate_state:"COUNTYARMAGH"
tax_rate_name:"County Armagh Tax"
tax_rate_priority:1
tax_rate_compound:false
tax_rate_shipping:1
tax_rate:"20.0000"
tax_rate_class:""
Note how tax_rate_state
has been modified and space is removed. Now our tax entry is COUNTYARMAGH
.
Then, when we perform a rate look up, https://github.com/woocommerce/woocommerce/blob/104f40c36e3b6495165c8a7bdae3d396e4c75f39/includes/class-wc-tax.php#L413, we create our query $criteria_string
with the county without modifying it, the $criteria_string
will be something like
tax_rate_country IN ( 'GB', '' ) AND tax_rate_state IN ( 'COUNTY ARMAGH', '' ) AND tax_rate_class = '' AND...
Note how it is trying to find an entry in column tax_rate_state
where it is equal to'COUNTY ARMAGH'
. This will never return a match since earlier we saved it as COUNTYARMAGH
.
To fix this, we will have to either address the problem in the insert entry part, or the search query part. I think we should create an issue in woocommerce
project and seek their expertise on what is a better fix.
Since "Automated taxes" is powered by WooCommerce Services, and this problem won't happen without the WooCommerce Services extension . I am going to take a look at how we pass tax rates to WooCommerce to see what we should do to fix this.
In https://docs.woocommerce.com/document/setting-up-taxes-in-woocommerce/#section-12, it stated
State Code – 2 digit state code for the rate. See i18n/states/COUNTRYCODE.php for supported states. For the US, use a 2 digit abbreviation e.g. AL. Leave blank (*) to apply to all states.
When the cart is from US, the "state" field became a dropdown This drop down actually maps all states to a 2-letter state code.
For non-US, this is actually a text field
So then woocommerce tax seems to just use this as the "state code" in which it removes all spaces.
More, if we try to manually create a tax rate in "Standard Rates" with space,
For example, if i typed "something something" in the "state code", it will also strip the space.
This is something that we will have chat with the Woocommerce team. I will make an issue there.
Will be resolved by: https://github.com/woocommerce/woocommerce/issues/24354
Hi Harris
Great, you're trying to find a solution to this, and are getting woo-commerce involved.
I'm not a coder, some of this is going over my head, can you advise if woo-commerce are now looking to fix, this?
tks
Wasim
On 10 December 2019 at 21:24 Harris Wong notifications@github.com wrote:
More, if we try to manually create a tax rate in "Standard Rates" with space, For example, if i typed "something something" in the "state code", it will also stripe the space. This is something that we will have chat with the Woocommerce team. I will make an issue there. —You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or unsubscribe.
Kindest Regards Wasim Mann
07949231440 www.theprosperonline.com
Hi @Wassmo1 . Please feel free to skip the tech notes/code comments. They are meant to provide details and references so we don't forget what we investigated and found out.
Regarding the status, I have post a comment in the existing issue here: https://github.com/woocommerce/woocommerce/issues/24354. Please follow the discussion there instead.
Closing this. Please continue to report/post to https://github.com/woocommerce/woocommerce/issues/24354 instead.
@harriswong @allendav we should not change it on Core, in fact it need to be fixed here, WooCommerce Services is not setting those tax rows properly, it's a overkill set "State code", "Postcode / ZIP" and "City" name in all tax rows if the amount of the tax is just the same for most of the states, see more details on https://github.com/woocommerce/woocommerce/issues/24354#issuecomment-578879130
@claudiosanches thanks for the link, I will take a look at this again. Note that this issue has tax setting Enable Automated Taxes
enabled. The last time I checked, the text field was passing County Armagh
into Woocommerce as COUNTYARMAGH
. More details on: https://github.com/Automattic/woocommerce-services/issues/1600#issuecomment-563506712
[edited]
I see that Enable Automated Taxes
is a WCS feature https://docs.woocommerce.com/document/woocommerce-services/. I will take another look into how we can have this feature work with the state code problem.
@harriswong but it not suppose to pass anything like that, that field expects a code, if you don't have a code you shouldn't try to fill it like that, so it's a problem on WooCommerce Services abusing WooCommerce's tax system. Also with two or three lines is already possible to set taxes for GB instead of what this plugin is trying to do setting taxes per state/county even when there's no difference in the tax amount.
if you don't have a code you shouldn't try to fill it like that
I agree. I will check why County Armagh
is allowed in this field during checkout, will check the address validator if any. If there isn't any validation, I will look into how WooCommerce Service can screen this input on the address form so that County Armagh
is not allowed, given that it should only accept state code.
We need something to merge similar taxes for a country, so we include less rows, what is also good for performance. I can help code it, I'll investigate how it works.
After looking at the problem this morning, it seems to me like the easiest/fastest solution would be to include a list of states/counties for GB (and for all countries where WCS calculates taxes automatically, so also CA, AU and all EU countries) in WC core. While this removes some flexibility (customers can't type anything they want into the State/County field), it handles orders with both unset and set county/state in an acceptable way, as far as I can see and does not create multiple tax records (as it can find the matching tax rate records).
While ignoring the state field when storing and searching for the tax rates can help in these cases, I don't think it is conceptually a correct solution, as even tax matching by country + state + zip + city is not always correct (same city and zip code can have different tax rates) and the matching mechanism is still quite fragile.
In addition, it feels like a leaky abstraction/code smell that get_matched_tax_rates
interfaces directly with the tax database, maybe we need to provide API in core to perform these tasks?
Using this WooCommerce patch from https://github.com/woocommerce/woocommerce/pull/25509/files from this issue: https://github.com/woocommerce/woocommerce/issues/24354#issuecomment-579438302
I tested the automated tax again. UK will now have a drop down:
Tax are calculated properly with Jetpack.
England isn't a county. It's a country.
I've included more granular subdivisions for the UK instead of the countries in the PR now. Hope it makes more sense.
Another option would be to replace "County" with "Constituent country", but that seems confusing, although I guess it's true that all people living in the UK are living in 2 countries at the same time--the UK and the respective constituent country inside the country of UK.
Hi Guys, I'm not a techie but it's still not working for me when a 2 worded county is entered, see my screenshots that show this and my current tax settings within WooCommerce.
Thanks for testing this @Wassmo1, and providing screenshots and descriptions, appreciate it! I noticed the first screenshot's county is not a dropdown. In order to test this, you would need to patch the changes from https://github.com/woocommerce/woocommerce/pull/25509/files into your test environment.
Then the county should become a dropdown instead of a text field. The dropdown has values that will allow WooCommerce Service to automate tax.
Hi @harriswong
Where can I locate my test environment?
tks
Wasim
@Wassmo1 you will have to setup your own environment for testing, similar to setting up your production environment but setup another one for testing only.
@peterfabian I just pulled the latest change and the tax populates 👍
Checkout page:
Tax entry:
@Wassmo1 you will have to setup your own environment for testing, similar to setting up your production environment but setup another one for testing only.
Hi @harriswong tks for the direction, I'll set it up and try it, @snooplewand have you had a chance to test it yet? hope it works.
@harriswong how do I go about installing this patch please? Thank you
@snooplewand The best way to test is by setting up a local development environment. You can follow our developer documentation here: https://github.com/woocommerce/woocommerce/wiki.
Hi @peterfabian @harriswong @snooplewand, setting up a test environment seems quite technical to me, I've never done anything like that before, is there a way I can test it on my live site and then remove it if it isn't working, without compromising the existing framework? tks
Hi @Wassmo1, I don't recommend testing patches on the production/live site. Currently, we have a patch under review and once that's completed, the fix will be included in the next WooCommerce Service release.
@harriswong, tks for the update is the patch working well? do you know when the next service release maybe?
@Wassmo1 This has been added to our next release which has a tentative due date at the end of March (https://github.com/Automattic/woocommerce-services/milestone/37?closed=1)
In the mean time, if you want to test out the change, you can add these 3 lines to your classes/class-wc-connect-taxjar-integration.php
file: https://github.com/Automattic/woocommerce-services/pull/1899/files.
Great, thanks @harriswong, where can /classes be found?
Reported in:
1942485-zen 14189124-hc 2524915-zen 2537618-zen
Summary
For all addresses in the UK, the tax rate on the front end will display as 0.00 if a county is entered that actually exists.
Steps to replicate
Here's are some screenshots to illustrate that better:
Front-end Checkout
Full Image: https://cld.wthms.co/4Jes3J+
Tax Settings after checkout
Full Image: https://cld.wthms.co/p9KLqN+
Additional Notes
@mikkamp managed to narrow things down even further to the
county
field. When using "Bogus" as the county, tax rates work as expected. When a legitimate county is used (e.g. "County Armagh"), tax rates are borked and multiple entries are created in the standard rates section.