Someone reported this issue on the UC Washington State Sales Tax module:
#1649652: Not applying tax
Basically, what they're seeing is that when they upgraded from UC 6.x-2.4 to 6.x-2.9, although the tax line items show up in preview, once the order is finalized, they go missing.
I am not sure if this is the same as the 7.x UC issue:
#1300608: Tax line items are no longer stored
(which has been marked "fixed", but seemed to be a purely 7.x issue?), and I couldn't find any other related issues in the Ubercart issue queue...
Does the WA sales tax module need to be updated to comply with some kind of API change in the UC module between 6.x-2.4 and 6.x-2.9? Or is it a bug in the underlying UC tax module? I am not sure... Thanks for any illumination (or better yet fixes) you can provide.
Comments
Comment #1
mineshaftgap CreditAttribution: mineshaftgap commentedFrom what I can tell so far, this issue was introduced in 6.x-2.6 in the uc_order module. I am in the process of trying to pin-point the change.
Comment #2
longwave"git bisect" is very helpful in tracking down the precise commit that caused the problem.
Comment #3
mineshaftgap CreditAttribution: mineshaftgap commentedIt seems that uc_order_save() in uc_order.module is now providing a default delivery country even when things are not being delivered. This breaks the current logic in Washington State Sales Tax to use delivered instead of billed customer info. So the delivery country is US, with no other delivery info.
Comment #4
longwaveCan you use uc_order_is_shippable() to determine which address to use, instead of testing for delivery country?
Comment #5
jhodgdonlongwave: The UC Tax WA module was previously checking to see if the delivery address was empty, and if so, it was using the shipping address. UC previously would leave the delivery address completely empty, but as of 6.x-2.6, it is apparently (and only during the final order save!) now leaving it *almost* empty (filling in a default country and leaving everything else on the address out. This breaks the logic that UC Tax WA was using previously.
So I think UC should either leave the delivery address *completely* empty (as it is during order preview and previously was for the whole order), or fill it in completely. It's the partial address that is causing us problems.
Yes, I can probably get around this problem in the WA module, but I still think the UC behavior is a buggy behavior.
Comment #6
longwaveThe commit where this was changed is http://drupalcode.org/project/ubercart.git/commitdiff/6672f85
This is a backport of D7 functionality, which I do not really want to revert entirely, though I guess we could remove the offending code if it does not cause any side effects. Does the 7.x version of WA Sales Tax have the same issue?
Comment #7
longwaveActually it looks like the db_query() statement in the previous code had the same effect:
((is_null($order->delivery_country) || $order->delivery_country == 0) ? variable_get('uc_store_country', 840) : $order->delivery_country)
so either there is a previous commit that caused the problem, or something else..
Comment #8
longwaveThe code quoted in #7 appears to have been around since Drupal 5, are you sure it's this specific statement that's causing the problem? Did you try git bisect to find where the problem started?
Comment #9
mineshaftgap CreditAttribution: mineshaftgap commentedUsing a print_r on the $order you can clearly see that the code from 2.6 adds a 'delivery_country' property to the order object.
Using the same print_r in 2.4 show that this property just doesn't exist. This is what causes the bug.