Hi. Discounts are not reflected in sales tax calculations on the checkout or review order screens. Even if the "Discount" line item is checked in the sales tax config. Using latest Drupal 6 and Ubercart Beta5 (also a problem on Beta4).

This was happening on uc_coupon module and the developer moved where the discount line items were being added to the order to solve the problem. See comment #4 here:

http://drupal.org/node/350359

Thanks.

CommentFileSizeAuthor
#1 uc_discounts.test.zip10.27 KBryangroe

Comments

ryangroe’s picture

ryangroe commented
StatusFileSize
new uc_discounts.test.zip10.27 KB

I did a quick test and I believe the tax shown on the checkout page (cart/checkout) is wrong but the tax shown on the "review order" page is correct. Please see if this is the case on your system too.

The reason for this is a bit convoluted. To add line items to checkout page they must have a unique key. So discounts have keys like uc_discounts0, uc_discounts1, etc. This is different than how it works on the server side where you can have multiple line items with the same key (just uc_discounts).

uc_taxes looks at the line item key value to determine if it should include it in the taxed subtotal. On the checkout page the keys don't match ("uc_discounts0" != "uc_discounts"), though on the sever it works fine.

The easiest way to fix this is to change from a line item for each discount on the checkout page to a single a "total discounts" line item on the checkout page. Of course this isn't quite as nice of behavior since the discounts aren't itemized in the payment method total but I think it's probably the best solution because it should reduce the impact changes to the uc_taxes module has on this module's behavior. Please note that the review page will not be effected. The review page will still have each discount as a separate line item.

Please try dropping in the files in the attached zip for a test.

ryangroe’s picture

ryangroe commented
Assigned: Unassigned » ryangroe
Status: Active » Needs review

Please try beta 5 to see if this problem has been fixed.

chrisschaub’s picture

chrisschaub commented

Ok, I just tried your beta5 with the latest taxes code from bzr (uc_taxes). They had a bug where dupe taxes were getting added to the order. A few things I saw:

1. On the checkout screen, the "Order Total" does not reflect the discount. Shows the pre-discount amount. The discount total seems right.

2. The tax thing is still buggy, the discounts are not reflected in the tax calculation on the review order screen, but they are correct on the checkout screen! Weird.

Using Latest Drupal, Ubercart (with uc_taxes from bzr per Ryan's suggestion) and discounts_alt beta5.

ryangroe’s picture

ryangroe commented

I have the duplicate tax line item problem myself. Can you post a link to information about that?

Anyway, the checkout page bug you found is caused by a dumb mistake I made. Here's the fix:

In uc_discounts.js change line 162 from:
  parseFloat(Drupal.settings.uc_discounts.line_item_weight) + 0.5, 0, false);
To:
  parseFloat(Drupal.settings.uc_discounts.line_item_weight) + 0.5, 1, false);

This parameter tells Ubercart whether or not to include the [Discount Total] line item amount in the total (notice how uc_taxes does not respect this flag. That's why the "Subtotal excluding taxes" line item is wrong in this module's beta 5 version). I was playing around with having a non-summed line item for display purposes only but uc_taxes didn't like that.

I will create a beta 6 release with the above fix shortly.

chrisschaub’s picture

chrisschaub commented

Thanks, that did the trick!

chrisschaub’s picture

chrisschaub commented

This seems to work, so I'm marking as fixed since I posted the original issue.

chrisschaub’s picture

chrisschaub commented
Status: Needs review » Fixed

Works.

chrisschaub’s picture

chrisschaub commented
Title: Tax calculation and discounts » Tax calculation and discounts - bug is back
Version: 6.x-1.0-beta4 » 6.x-1.0-beta9
Status: Fixed » Active

Ok, I spoke too fast! This bug seems to be open. The taxes are correct on the checkout screen thanks to the fix in #4. But, the tax amount does not reflect the discount on the review order screen -- unless you go back to checkout and then forward again to review order. I'm using the latest bzr head of uc_taxes, latest ubercart and uc_discount_alt. There was an issue with calling order_save in the ubercart forums about this with other discount modules. Basically, you have to add discount line items in hook_pane, not hook_order. You set a flag in hook_pane, but hook_order is too late.

See this commit for an approach that works:

http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/uc_coupon/u...

This problem still exists in beta 9.

Thanks.

gooddesignusa’s picture

gooddesignusa commented

New version of ubercart today to help fix some tax issues. There was this bug where if someone pressed back on the review order page and then submit it again the taxes would be added again. Entering the wrong Credit Card Number a couple times and I had like 4 lines of taxes being applied from going back and forth trying to correct it. The new version that came out today seemed to fix that bug so not sure if that maxes a difference with this module.

ryangroe’s picture

ryangroe commented

I think lutegrass and myself applied a patch to correct this duplicate tax line item issue. However, I think there is an additional problem here that is specific to uc_discounts_alt. I've gotten around this on my site because I don't include the tax line item in my discount but I see why you would want to include it. I will review the way uc_coupon handles this and make the necessary changes.

chrisschaub’s picture

chrisschaub commented

Yes, the ubercart update fixed the dupe tax problem. But ryangroe is correct, this is a separate problem with uc_discounts_alt. It has to do with when the line items are added to the order and taxes. Thanks ryangroe.

chrisschaub’s picture

chrisschaub commented

Hey ryangroe, any progress or thoughts on this? Thanks in advance.

ryangroe’s picture

ryangroe commented

I'll start working on fixes/upgrades tonight. I only have the weekend to devote to this module right now.

chrisschaub’s picture

chrisschaub commented

Revised. I think we need to do something like ...

// Save order with new discount lines
uc_discounts_order('save', $arg1, '');

When the javascript callback runs so that the order gets updated.

ryangroe’s picture

ryangroe commented

If that's all there is to it I'll get to this one first and release another beta for you two to test. Thanks for the input.

chrisschaub’s picture

chrisschaub commented

I was wrong, read my updated #14. The problem is that the order needs to get save at the calculate discount stage on the checkout screen. Ignore my previous solution, too hasty! :) The only problem is that the discounts get added twice on back / forth. Still investigating. Any ideas are greatly appreciated. The "save" code maybe should be in a discount_order_save function that can be called by the order hook and the js panel update.

chrisschaub’s picture

chrisschaub commented

Hey ryangroe, is there any chance you could take a look at this bug? I'm really having a tough time on this, and the sales tax thing is pretty important. Sorry to ask, but I'm under the gun. Any help is greatly appreciated.

ryangroe’s picture

ryangroe commented

This weekend I plan to get all my issues closed or ready for review. It may spill until about Tuesday but I'll get it done.

chrisschaub’s picture

chrisschaub commented

I'm wondering about hook_order. Many people have solved this by moving the save/submit stuff into hook_checkout_pane. What do you think? I could take a stab at doing that.

ryangroe’s picture

ryangroe commented

I don't think hook_order is reliable. I've personally seen that Ubercart does not call submit for PayPal. As a result I now check for "Payment received" in another module I've written. I think it still needs to be used to stop checkout if the discounts don't match what the user saw in the review page. The rest of the processing can be done in a different place if it makes sense.

ryangroe’s picture

ryangroe commented

OK, I think I have this one fixed. Here is how I reproduced it:
add item to cart
click "Checkout"
(the checkout page has the correct tax)
enter information without errors to reach review page
(the review page has the wrong tax)
click "Back"
(the checkout page has the correct tax)
enter information without errors to reach review page
(the review page has the correct tax)

I needed to update the order's line_item property in uc_discounts_order when $op == "save".

Please grab beta 10 I am about to release and test it out.

ryangroe’s picture

ryangroe commented
Status: Active » Needs review

Forgot to mark "needs review"

chrisschaub’s picture

chrisschaub commented

Thanks for this, it works against ubercart BETA6. I tested a couponless qty discount and coded discount. Back and forth works too.

Just a reminder about the rounding and applicable products bug here -- both affect the order total and are pretty serious. I think my patch will work to fix it.

http://drupal.org/node/405394#comment-1470382

Thanks!

ezra-g’s picture

ezra-g commented
Status: Needs review » Fixed

This has been committed, so I'm marking it as fixed ;).

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.