[Done 1.3.6] Group discount tax calculation error

[swguy]

l. 71 is

$od_amount[$key] = $tod_amount = round((($od_amount['total'] * $tax_rate)) /100, 2) ;

I believe this is incorrect, since od_amount['total'] would include shipping and or tax if
these respective settings are true. Rather, I believe the following algorithm should be used:

above the switch statement (l.65)

$od_amount['taxable'] = $od_amount['total'];
if ($this->include_shipping == 'true')
$od_amount['taxable'] -= $order->info['shipping_cost'];
if ($this->include_tax == 'true')
$od_amount['taxable'] -= $order->info['tax'];

then l. 71 becomes

$od_amount[$key] = $tod_amount = round((($od_amount['taxable'] * $tax_rate)) /100, 2) ;

Test case: create a discounting group that gives 100% off. Install group discount with default settings, then set include tax = false, include shipping = true. Check out, and you will see the total is less than 0. With this fix, it is zero (and you can permute the include tax and include shipping settings and see that the total is also correct, with the caveat that stores that use include_tax = true should use re-calculate tax = none.

Scott

[DrByte]

You're right -- something's awry there.
It's possible that a more thorough solution would be to recalculate the tax based on the ratio of the deduction to the adjusted order total.

How about this:

	function calculate_deductions($order_total) {
		global $db, $order;
		$od_amount = array();
		$tax_address = zen_get_tax_locations();
		$group_query = $db->Execute("select customers_group_pricing from " . TABLE_CUSTOMERS . " where customers_id = '" . (int)$_SESSION['customer_id'] . "'");
		if ($group_query->fields['customers_group_pricing'] != '0') {
			$group_discount = $db->Execute("select group_name, group_percentage from " . TABLE_GROUP_PRICING . " where
                                        group_id = '" . (int)$group_query->fields['customers_group_pricing'] . "'");
			$gift_vouchers = $_SESSION['cart']->gv_only();
			$discount = ($order_total - $gift_vouchers) * $group_discount->fields['group_percentage'] / 100;
			$od_amount['total'] = round($discount, 2);
      /**
       * when calculating the ratio add some insignificant values to stop divide by zero errors
       */
      $ratio = ($od_amount['total'] + .000001)/($order_total - $gift_vouchers + .000001);
			switch ($this->calculate_tax) {
				case 'Standard':
          reset($order->info['tax_groups']);
          while (list($key, $value) = each($order->info['tax_groups'])) {
					  $tax_rate = zen_get_tax_rate_from_desc($key);
					  if ($tax_rate > 0) {
					    $od_amount[$key] = $tod_amount = round(($order->info['tax_groups'][$key]) * $ratio, 2) ;
					    $od_amount['tax'] += $tod_amount;
					  }
          }
				break;
				case 'Credit Note':
          reset($order->info['tax_groups']);
          while (list($key, $value) = each($order->info['tax_groups'])) {
  					$tax_rate = zen_get_tax_rate_from_desc($order->info['tax_groups']);
					  if ($tax_rate > 0) {
					    $od_amount[$key] = $tod_amount = round(($order->info['tax_groups'][$key]) * $ratio, 2) ;
					    $od_amount['tax'] += $tod_amount;
					  }
          }
				break;
			}
		}
		return $od_amount;
	}

[swguy]

... procesing ... this is an interesting idea.

Do you see any possibility of making this library logic that other order total modules could use
instead of having it in every order total module?

Right now, it's re-implemented (with subtle differences - and bugs) by coupons, group discounts,
and my contribs (better together, quantity discounts).

Thanks,
Scott

[swguy]

What if we were to store the computed tax in the cart data structure? That way we could just take percentages off that, and the numbers would be correct for mixed taxable/non-taxable goods. (I know this would be a painful change, but I wanted to throw it out there.)

The way the coupon module works is to spin through the items and determine if they are taxable. This is also a nice idea.

Thanks for looking at this.
Scott

[DrByte]

What if we were to store the computed tax in the cart data structure?

The whole taxation infrastructure will have an overhaul, most likely in conjunction with the checkout-rewrite, as there exists the need to allow greater control over taxation methods and precision, not to mention discounting and other features.

You're right -- it's a huge task, with a lot of implications, not to mention potential for bugs. Plus, there's a need to consider every possible taxation approach used at different levels in different regions of different countries.

[swguy]

The mind reels. I was completely blown away by how difficult it was to figure out the order total modules (and especially their tax implications). Thanks for your good work!

Scott

[swagmani]

I was having difficulty with getting the Discount Order Totals Module to work correctly especially when the shopping cart had a mixture of Tax included & Tax excluded products. I worked on the code of the file ot_group_pricing.php . I note that this file has been updated in ZC v1.3.6 with the code above. However, I still found it not to work as i would like so i continued to work on a solution.

See attachment for my modified version of ot_group_pricing.php

Basically, I modified & added new code into the function 'calculate_deductions' ie:

        /*
        * calculate the ratio between orig subtotal & new subtotal,
        * then use this ratio to calculate the new tax component value of the new subtotal compared to the orig tax component of the orig subtotal
         * also, when calculating the ratio add some insignificant values to stop divide by zero errors
         */
        $orig_sub_total = $order_total - $gift_vouchers;
        $subTotalDiscRatio = 1 - (($orig_sub_total - $od_amount['total'] + .000001) / ($orig_sub_total + .000001));

            switch ($this->calculate_tax) {
                case 'Standard':
                    reset($order->info['tax_groups']);
                    while (list($key, $value) = each($order->info['tax_groups'])) {
                        $tax_rate = zen_get_tax_rate_from_desc($key);
                        if ($tax_rate > 0) {
                            $od_amount[$key] = $tod_amount = round($order->info['tax'] * $subTotalDiscRatio,4);
                            $od_amount['tax']+=$tod_amount;
                        }
                    }
                break; 

The idea behind this was that ZenCart already knows the tax component of the 'sub_total', so why not just simply apply this calculated ratio to the Tax component.

Also, within the file ot_group_pricing I commented out a few lines to do with the variable '$tax' and also the new if statement, 'if (DISPLAY_PRICE_WITH_TAX', in function 'process', commenting out these lines of code also helped me to get the correct returned Totals figures of my test shopping basket.

function process() {
        global $db, $order, $currencies;
        $od_amount = $this->calculate_deductions($this->get_order_total());
        if ($od_amount['total'] > 0) {
      reset($order->info['tax_groups']);
                     // $tax = 0;
            while (list($key, $value) = each($order->info['tax_groups'])) {
                $tax_rate = zen_get_tax_rate_from_desc($key);
                if ($od_amount[$key]) {
                    $order->info['tax_groups'][$key] -= $od_amount[$key];
                    // $order->info['total'] -=  $od_amount[$key];
                    //                    $tax += $od_amount[$key];
                }
            }
    /*
      if (DISPLAY_PRICE_WITH_TAX == 'true') {
        $od_amount['total'] += zen_calculate_tax($od_amount['total'], $tax);
      }
    */ 

DrByte, a question: I am wondering if the call to function 'zen_calculate_tax' may not work as expected in file ot_group_pricing.php , did you mean to pass value of '$tax' from the global tax set in the 'tax rates' table or its actual value set from the returned value of '$od_amount['tax']' in function 'calculate_deductions' ?

regards, Matt

[swguy]

swagmani - see the fix I posted on 2 Nov in
http://www.zen-cart.com/forum/showth...t=50014&page=2

Scott