The tax calculation for an order during the checkout form is based on an AJAX callback that doesn't alter prices first. I've attached a patch that fixes things, but I'm not sure it's a complete fix. Basically, if a product was discounted 10% before (I used UC Discount in testing), the tax would still be calculated on the full price. After the patch, it's taxing the discounted price as expected.

I checked into the broader line item functionality but couldn't see any related places to make the change.

Comments

rszrama’s picture

Status: Needs review » Needs work

Bah, stupid patch is stupid. It only works for the checkout form. Apparently other places in tax calculation aren't considering discounts either. :-/

rszrama’s picture

Status: Needs work » Needs review
StatusFileSize
new2.09 KB

Ok, I duplicated the product price setting, but I'm not convinced this will be the best way to go. In any event, it seems to provide a uniform fix, I'm just a little worried about it recalculating product prices when it shouldn't as is... however, I just tested and realized that's an existing bug. If a product price changes, when an order having that product on it is saved the tax gets re-calculated based on the new product price, and this patch does not affect that behavior.

Oh, and I did remove two old debug messages from uc_taxes.module in this.

timmillwood’s picture

Status: Needs review » Needs work

This is not working for me

Item one (tax free) £29
Item two (taxable) £12

subtotal £41
20% discount -£8.20
shipping (tax free) £4
subtotal £36.80
tax (17.5%) £2.47
total £39.27

The tax should be £1.68, without the discount the tax should be £2.10, so I'm not sure where £2.47 is coming from.

Island Usurper’s picture

I think it's (£12 + £2.10) * 17.5% = £2.47, which means the discount isn't being applied, and the tax is being applied to itself.

Altering the product prices during uc_taxes_order('save') as cart items seems semantically wrong to me since that hook is also invoked when orders are edited. They aren't really cart items any more.

I kind of think the right place to alter the prices is right before they are serialized into the checkout form. This takes care of the Javascript callback and locks them into those prices when they are added to the order.

rszrama’s picture

Yeah, might partly be the UC VAT module Tim is using including VAT in the product price for display.

As for where altering prices should occur - yeah, it shouldn't have to be there, but I'm not sure sales taxes should be getting recalculated there at all. i.e. either way the tax is going to change when it probably shouldn't. : P

timmillwood’s picture

I have disabled the VAT module and have my 17.5% tax added just using uc_taxes.

I have been able to get the patch to work when using uc_discount when applying a fixed product discount, although this doesn't work with coupon codes through uc_discount or uc_coupon.

Rob B’s picture

Is there any way to get this patch to work with uc_vat and uc_coupon? For example, I have created a €100 voucher, but the discount only applies to the non-VAT part of the price, so you still get charged the tax component - even when the product is well below €100 inc. VAT.

It seems to work fine without the uc_vat module as Tim mentioned, but I need to show and input prices as tax inclusive.

rszrama’s picture

Yeah, I came up with the patch to try and fix exactly your scenario, but Tim had to hack Ubercart further to make it work. This tells me there are some other places where price alteration isn't happening when necessary, and until those places are found this should be an open bug report.

cangeceiro’s picture

Status: Needs work » Needs review
StatusFileSize
new820 bytes

I've also ran into this, here is a slightly different patch everyone may want to try out, and should in theory work with uc_discount and uc_coupon and any other module that is using a a price handler to alter the price of an item. This patch goes into the function that calculates the taxable value of an item and runs thru uc_price to get that value.

yktdan’s picture

Status: Needs review » Reviewed & tested by the community

This is a review.

It works for us and it works correctly on both uc_discounts_alt and on uc_discount (framework). Both were failing for us and the commonality of the failure led us to this patch. Both of those still have other issues for us, but at least the taxes are correct. Please commit promptly as I hate running with patched core modules. We don't use VAT so I have not checked those issues.

longwave’s picture

I have a feeling this will break uc_vat, as it implements its own price handler which adds tax, so taxes will be added twice. Perhaps we need to specify a flag in $context to say this is a tax calculation, so uc_vat can ignore it.

yktdan’s picture

I don't know anything about VAT taxes. Would one every have both VAT and other taxes? I don't think that the patch adds any additional taxes, it just insures that ones done by the tax handler are used.

longwave’s picture

The function that's being changed here affects the tax line item (I think, not having actually installed it yet). When VAT is in use, all prices are displayed inclusive of tax, and uc_vat's price alterer usually takes care of this. However, when you're calculating the tax line item, you want the untaxed prices for the calculation, but this patch calls uc_price() to get altered prices - so it'll be calculating tax based on item prices that already include tax.

(not setting back to needs work, as it's not Ubercart's fault directly, but now I have posted here I will at least notice if it get committed, and can fix uc_vat and/or provide a followup patch if needed)

Island Usurper’s picture

Status: Reviewed & tested by the community » Fixed

OK, longwave, good luck fixing uc_vat or figuring out a follow-up patch.

Committed #9 with some whitespace changes, and one fewer node_load() than necessary.

smscotten’s picture

Priority: Normal » Major
Status: Fixed » Needs work

Applied fix in #9, but still seeing the problem.

Steps to reproduce:

1) Put 12 items in cart to satisfy the "10% off 12 items or more" discount. (total $100, total after discount is $90)
2) go to checkout.
3) Observe that at checkout that tax (10%) is calculated based on discounted amount correctly ($9 on $90 discounted total).
4) Review order
5) Observe that on review page, tax is calculated based on non-discounted amount ($10 on 100 non-discounted total).

Additionally, there is a red herring that made me think it was fixed:

6) return to checkout page
7) Observe that at checkout that tax (10%) is calculated based on discounted amount correctly ($9 on $90 discounted total).
8) Review order
9) Observe that at checkout that tax (10%) is calculated based on discounted amount correctly ($9 on $90 discounted total).

*returning* to the review page the tax is calculated correctly. But on initial review, tax is calculated incorrectly Can duplicate this:

10) return to shopping cart page
11) proceed to checkout page
12) Observe that at checkout that tax (10%) is calculated based on discounted amount correctly ($9 on $90 discounted total).
13) Review order
14) Observe that on review page, tax is calculated based on non-discounted amount ($10 on 100 non-discounted total).

if I submit order after steps 5 or 14 the order is charged at $100 (incorrect tax) rather than $99. If order is submitted after step 9, the customer is charged the correct $99.

This indicates that the order is not getting updated between review and processing, which would be fine if review always had the correct tax.

We can't instruct our customers to do the workaround (return to checkout page after reviewing) obviously, so this means issuing a lot of refunds for the difference between the correct tax and the (higher) tax which has been charged.

NOT using uc_vat.

We are using ubercart (and uc_taxes) 6.x-2.4, uc_discount (framework) 6x-1.x-dev. Discount IS set as a taxed line item.

smscotten’s picture

PS I went back to "stock" uc_taxes and applied patch from #2 and got some really strange numbers from an order:

Subtotal: $323.88
Local pick-up: $0.00
10% off orders of 12 bottles or more: -$388.66
Subtotal excluding taxes: -$64.78
New York State Sales Tax: $310.44
Total: $245.66

longwave’s picture

@smscotten: It's possible that your scenario in #15 is being caused by #603356: uc_taxes module's weight should be set heavier on install - try the patch in the original post and see if that helps things.

andymantell’s picture

Ok, here's my take on the issue. First, just an overview of my situation:

* I am using the uc_taxes module set up with a 20% rate to represent VAT. I am *not* using uc_vat at this point.
* I have products who's price varys using the following price handler (I include this here in case any spots any contexts that I have missed. Does my price handler cover all the bases it needs to?):

function _foo_phonenumbers_price_handler_alter(&$price, &$context, &$options) {
  // Get the phone number attribute out of the context
  if(isset($context['subject']['cart_item'])) {    
    $number = $context['subject']['cart_item']->data['attributes'][1];    
  } else if(isset($context['subject']['product']->data['attributes']['Phone number'][0])) {      
    $number = $context['subject']['product']->data['attributes']['Phone number'][0];    
  }
  
  // Score the number according to memorableness and then alter the price accordingly
  $score = _foo_phonenumbers_get_score($number);
  $price['price'] = $price['price'] + ($score * 100);

  // If the destination number is a mobile number, add £50 to the cost
  if(strpos($number, '07') === 0) {
    $price['price'] += 50;
  }
}

I have tried the following combinations and uc_taxes is still calculating the tax based on the price *before* alteration. I cleared the cache between each test and started from the /cart page each time.

1) unmodified uc_taxes module. Default module weight of zero
2) unmodified uc_taxes module. Increased module weight of 10
3) Patched uc_taxes module (Patch #9). Default module weight of zero
4) Patched uc_taxes module (Patch #9). Increased module weight of 10

Edit:
A couple more:

5) Patched uc_taxes module (Patch #2). Default module weight of zero
6) Patched uc_taxes module (Patch #2). Increased module weight of 10

Should have mentioned this before, but the place I am looking at the taxes is at the bottom of the summary table on /cart/checkout/review and also the amount that is being passed to uc_sagepayserver

andymantell’s picture

Ok, things have changed a bit over here. Part of my problem seems to have been that my price alteration hook wasn't handling all context types. In addition to that, the way in which attributes were represented on the context subject array is *very* inconsistent - resulting in my price handler sometimes failing to pick up and act on the attribute values.

To summarise:
* when using ubercart-6.x-2.4 taxes appear to be calculated using the pre alteration prices
* when using ubercart-6.x-2.x-dev the issue is now fixed for me - taxes display correctly using the altered prices

For reference, here is my price alteration hook:

function _tamar_phonenumbers_price_handler_alter(&$price, &$context, &$options) {
  switch($context['type']) {
    case 'cart_item':
      if(isset($context['subject']['cart_item']->data['attributes'][1])) {
        $number = $context['subject']['cart_item']->data['attributes'][1];
      } else if(isset($context['subject']['cart_item']->data['attributes']['Phone number'][0])) {
        $number = $context['subject']['cart_item']->data['attributes']['Phone number'][0];
      }

      break;

    case 'order_product':
      if(isset($context['subject']['product']->data['attributes'][1])) {
        $number = $context['subject']['product']->data['attributes'][1];
      } else if(isset($context['subject']['product']->data['attributes']['Phone number'][0])) {
        $number = $context['subject']['product']->data['attributes']['Phone number'][0];
      }

      break;

    case 'amount':
    case 'attribute_option':
    case 'payment':
    case 'line_item':
    case 'product':
    case 'price':

      break;


    default:
      watchdog('unhandled_pricehandler', 'Unhandled price handler context: %context', array('%context' => $context['type']));
  }

  if(isset($number)) {
    $score = _tamar_phonenumbers_get_score($number);
    $price['price'] = $price['price'] + ($score * 100);

    // If the destination number is a mobile number, add £50 to the cost
    if(strpos($number, '07') === 0) {
      $price['price'] += 50;
    }
  }
}

If you look at the way I'm having to test for attributes, there are two ways in which they appear, either indexed by attribute id or indexed by their label.

earthangelconsulting’s picture

subscribe

andyf’s picture

subscribe

kirtimansharma’s picture

StatusFileSize
new14.66 KB

Same problem.
Tax cahanges is the product price is changed. But if a line item is added or a line item discount is offered it show 2 lables -> subtotal and subtotal excluding all taxes. Why?
and the taxes do not change. See the attachment.

andymantell’s picture

@kirtimansharma - What version of Ubercart are you using? Does the problem go away for you when using the dev version?

kirtimansharma’s picture

I m using 6x.2.4 and I have not tries the dev version as the site is a production site.

Anonymous’s picture

subscribe

andymantell’s picture

This ticket is raised against 6.x-2.4 - could some of the people subscribing to this issue try the 6.x-2.x-dev version and confirm whether or not the issue is fixed for them? The problem went away when I tried it on the dev version - it would be good to know if other people find this too.

If you are having this problem on a live site, try the 6.x-2.x-dev version on your development server instead...

Anonymous’s picture

Priority: Major » Critical

Not sure if it is related to this problem but on 6.x-2.x-dev july 2nd there is a problem that the VAT is not correctly calculated when there are line items like a fee. The VAT on the fee is just ignored.

Anonymous’s picture

Version: 6.x-2.4 » 6.x-2.x-dev
andymantell’s picture

@compubase Have you set your "fee" line item to be taxable in the tax settings? E.g:

/admin/store/settings/taxes/1/edit (1 might not be the right id for you).

This issue is regarding taxes not being applied to the altered prices of items (with a price alteration hook). If you aren't getting any taxes calculated *at all* then it probably warrants a new ticket rather than discussing it here.

Anonymous’s picture

The problem is that taxes are calculated but the line items (in my case a fee), are not taken into account.
For example:
Cart subtotal: 100,- inc VAT 19%
Fee: -15,-
Calculated tax 15,97

The calculated tax is not correct. This should have been 13,57

In Ubercart 2.4 this works as expected. In the current dev version, the line item correction is not applied.

handsofaten’s picture

Has this issue ever been resolved? I'm still seeing it with 6.x-2.7, when using UC Store Credit for discounts. I'm not sure if this is a problem with UC Store Credit or UC itself, but it seems very similar to this issue. When the customer goes through the checkout process, store credit is applied on the checkout page.

On the checkout/review page, the taxes are calculated before discounts are applied. However, if the customer clicks the "back" button at the bottom of the review page, and then resubmits the order, it will be calculated correctly.

timmillwood’s picture

I think it finally came down to an issue with UC

handsofaten’s picture

Was it resolved? As I mentioned, I'm on 6.x-2.7 and having the problem mentioned above. I've been digging through module code (mostly looking at uc_taxes), and made some progress, but still haven't fully resolved it. To be clear, my problem is not with discounts to individual items, but a general discount on the entire order (like a gift certificate, but in this case a rewards program driven by userpoints).

tr’s picture

Issue summary: View changes
Status: Needs work » Closed (outdated)

The whole uc_price() mechanism was removed in 7.x-3.x, so this issue only ever applied to 6.x-2.x. Drupal 6 is now obsolete and unsupported.

If you have a similar problem with 7.x-3.x or 8.x-4.x, please open a new issue.