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

mineshaftgap’s picture

From 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.

longwave’s picture

"git bisect" is very helpful in tracking down the precise commit that caused the problem.

mineshaftgap’s picture

It 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.

  if (is_null($order->delivery_country) || $order->delivery_country == 0) {
    $order->delivery_country = variable_get('uc_store_country', 840);
  }
longwave’s picture

Can you use uc_order_is_shippable() to determine which address to use, instead of testing for delivery country?

jhodgdon’s picture

Title: Taxes not getting saved in final order » Delivery address being partially filled in for final order, for non-shippable orders

longwave: 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.

longwave’s picture

The 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?

longwave’s picture

Actually 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..

longwave’s picture

Status: Active » Postponed (maintainer needs more info)

The 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?

mineshaftgap’s picture

Using 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.