[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