Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Any price alterations to the order products are not included in the subtotal without taxes. VAT uses the tax_subtotal line for its own purposes, so it'd be a very good idea for someone to review this patch with uc_vat enabled once I post it.
Comment | File | Size | Author |
---|---|---|---|
#18 | uc_taxes_uc_price_600478.patch | 2.33 KB | stockliasteroid |
#17 | uc_taxes_uc_price_600478.patch | 2.35 KB | stockliasteroid |
#11 | uc_taxes.module.patch | 2.45 KB | cangeceiro |
#7 | 600478_tax_subtotal.patch | 2.01 KB | Island Usurper |
#1 | 600478_tax_subtotal.patch | 803 bytes | Island Usurper |
Comments
Comment #1
Island Usurper CreditAttribution: Island Usurper commentedPatch.
Comment #2
xibun CreditAttribution: xibun commentedI have tested this patch and my use cases work ok.
But I came across a rounding error in Subtotal excluding taxes. The following is in checkout - Order total preview.
Applied VAT is 16%. The correct value for subtotal excl. VAT would be €99.09.
After this page the value is correctly €99.09, (i.e. checkout/review, admin/store/orders/x & invoices online and emailed - all correct)
I believe this rounding error is not related to this patch. But it shows an inconsistency in subtotal excl. tax which is worked on in this patch.
Comment #3
longwaveuc_vat overrides the tax_subtotal line item callback, so this code in uc_taxes is never called if uc_vat is enabled. This is done so the subtotal without taxes line item is displayed in more cases (the $different check is removed) and so it can replace the word "taxes" with "VAT" or another tax name.
However, applying a similar patch to uc_vat_line_item_tax_subtotal() doesn't calculate the subtotal excluding taxes correctly; it includes any tax applied to the products, as the subtotal line item already includes taxes when uc_vat is enabled. Perhaps the loop through $order->products needs to stay, adding a call to uc_price() with an option to tell uc_vat not to apply VAT to these particular prices?
Comment #4
xibun CreditAttribution: xibun commented@longwave: so the rounding error I saw resides in uc_vat?
Comment #5
longwave@xibun: I am trying to reproduce that issue now, but uc_vat doesn't do any rounding itself, so I'm not sure where the problem is coming from!
Comment #6
longwave@xibun: I tracked your issue down to uc_quote.js, see #602624: uc_quote.js incorrectly rounds shipping costs to 2dp for a patch that changes the subtotal excluding VAT to €99.09 using your example.
Comment #7
Island Usurper CreditAttribution: Island Usurper commentedAs it turns out, the actual tax calculation isn't using the altered product prices either. (See #597992-2: Discount not reflected on total order (FIXED for Trigger: Calculate order discounts).) Applying this updated patch now does some really interesting things on my test site. I've got a couple of discounts enabled, one for products and another for the total order. I also have a tax that applies to products, shipping, and discount line items. When I enable uc_vat, it seems like the order discount is taxed twice, so I removed discounts from the taxable line item list. I'm still not sure the numbers work out, though they may be fixed by rearranging the price alterer order. Currently, discounts are applied before taxes.
Comment #8
GiorgosKis there a need for this patch in 2.2 ?
I tried 2.2 taxes with and without http://drupal.org/project/uc_discount module in use and I noticed no problem whatsoever
if not should be closed because users are wrongly directed to use it from this issue #597992: Discount not reflected on total order (FIXED for Trigger: Calculate order discounts)
-----------------------------------
when this patch is used the subtotal does not appear in line_items array for the product and wrong "subtotal before taxes" appear in checkout/review (its just an appearence glitch but nevertheless)
All else seems to behave OK
noted first by valdez in this comment http://drupal.org/node/597992#comment-2145974
but I have not problems without the patch using uc 6.x 2.2
taxes behave as expected
Comment #9
a_c_m CreditAttribution: a_c_m commentedsubscribe
Comment #10
TR CreditAttribution: TR commentedMarked #765726: uc_line_item_tax_subtotal doesn't honor price alters as a duplicate. That issue also contains a patch, which should be considered here.
Comment #11
cangeceiro CreditAttribution: cangeceiro commenteddisregard the patch i wrote in #10. I worked on this a little longer and came up with a better solution. This patch also pulls in some of the previous patches in this thread.
Comment #12
fp CreditAttribution: fp commentedI haven't looked into the details but it works as expected. Thank you.
Comment #13
fp CreditAttribution: fp commentedWell, almost. :)
It addresses the following instances:
* the review page
* the completed order as seen from the user or admin point of view
* the email sent out to the user
But *not* the actual checkout page.
Comment #14
mcarrera CreditAttribution: mcarrera commentedI am having this problem with Ubercart 2.4 and discount framework 6-1. Discounts applied correctly, but disregarded when calculating taxes.
I am testing a hack on a couple of functions in uc_taxes.module. Basically I call ca_pull trigger to apply the discount, and use $item->sell_price rather than $item->price to calculate the taxable amount.
Here is the code, sorry I am still testing it and have yet to figure how to write a patch.
function uc_line_item_tax_subtotal($op, $order) {
$amount = 0;
if (is_array($order->products)) {
foreach ($order->products as $item) {
$amount += $item->price * $item->qty;
}
}
becomes
function uc_line_item_tax_subtotal($op, $order) {
$amount = 0;
if (is_array($order->products)) {
foreach ($order->products as $item) {
# hack begin
global $user;
$my_item = uc_product_load(node_load($item->nid));
ca_pull_trigger('calculate_product_discounts', $my_item, $user));
$amount += $my_item->sell_price * $item->qty;
}
and, most important
function uc_taxes_apply_item_tax($item, $tax) {
$node = node_load($item->nid);
// Special handling for manually added "Blank line" products.
if (!$node) {
$node = new stdClass();
$node->type = 'blank-line';
$node->shippable = $item->weight > 0;
}
// Tax products if they are of a taxed type and if it is shippable if
// the tax only applies to shippable products.
if (in_array($node->type, $tax->taxed_product_types) && ($tax->shippable == 0 || $node->shippable == 1)) {
return $item->price * $item->qty;
}
}
becomes
function uc_taxes_apply_item_tax($item, $tax) {
$node = node_load($item->nid);
// Special handling for manually added "Blank line" products.
if (!$node) {
$node = new stdClass();
$node->type = 'blank-line';
$node->shippable = $item->weight > 0;
}
// Tax products if they are of a taxed type and if it is shippable if
// the tax only applies to shippable products.
if (in_array($node->type, $tax->taxed_product_types) && ($tax->shippable == 0 || $node->shippable == 1)) {
#hack begins
global $user;
$my_item = uc_product_load(node_load($item->nid));
return $my_item->sell_price * $item->qty;
}
}
Comment #15
mcarrera CreditAttribution: mcarrera commentedDisregard my previous post, I am using as per #11 by cangeceiro and it works better.
Only remaining problem is in checkout, the users see a subtotal excluding taxes which has nothing to do, like is randomly computed.
Comment #16
longwaveOver two years since the last post, not worth changing this now in 6.x-2.x.
Comment #17
stockliasteroid CreditAttribution: stockliasteroid commentedSorry to tag this again, but the patch in #11 isn't quite right and I thought others may benefit from having a working patch even if it won't make it into a release. The patch failed to give a correct context to uc_price, so modules like uc_discount just didn't alter the price at all. This plus another small issue is what caused #15 I think.
Tested, seems to work all right in my application... Perhaps this will help someone else.
Comment #18
stockliasteroid CreditAttribution: stockliasteroid commentedArgh, sorry, I had a dsm in there still... *headdesk*