-
-
Notifications
You must be signed in to change notification settings - Fork 227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lost penny management #6391
Lost penny management #6391
Conversation
i will attempt to be brief. as i stated here, the problem resides in the need for the lets go over why i do not like your code:
and i would solve this problem completely differently. i would look at this method: zencart/includes/classes/order_total.php Line 63 in 945701e
and potentially change it to something like this: function process()
{
global $order, $currencies;
$order_total_array = [];
if (is_array($this->modules)) {
$this->notify('NOTIFY_ORDER_TOTAL_PROCESS_STARTS', ['order_info' => $order->info]);
foreach ($this->modules as $value) {
$class = substr($value, 0, strrpos($value, '.'));
if (!isset($GLOBALS[$class])) continue;
$GLOBALS[$class]->process();
$this->notify('NOTIFY_ORDER_TOTAL_PROCESS_NEXT', ['class' => $class, 'order_info' => $order->info, 'ot_output' => $GLOBALS[$class]->output]);
if (empty($GLOBALS[$class]->output)) {
continue;
}
for ($i = 0, $n = sizeof($GLOBALS[$class]->output); $i < $n; $i++) {
if (zen_not_null($GLOBALS[$class]->output[$i]['title']) && zen_not_null($GLOBALS[$class]->output[$i]['text'])) {
$order_total_array[] = [
'code' => $GLOBALS[$class]->code,
'title' => $GLOBALS[$class]->output[$i]['title'],
'text' => $GLOBALS[$class]->output[$i]['text'],
'value' => $GLOBALS[$class]->output[$i]['value'],
'sort_order' => $GLOBALS[$class]->sort_order,
];
}
}
}
}
$orderTotal = 0;
$notCalc = ['ot_total',];
foreach ($order_total_array as $key => $totalItem) {
if ($totalItem['code'] === 'ot_total') {
$totalKey = $key;
}
if (in_array($totalItem['code'], $notCalc)) {
continue;
}
$orderTotal += $currencies->value($totalItem['value'], false, $order->info['currency']);
}
if (empty($totalKey)) {
return $order_total_array;
}
$order_total_array[$totalKey]['value'] = $orderTotal;
$order_total_array[$totalKey]['text'] = $currencies->format($orderTotal, false, $order->info['currency'], $order->info['currency_value']);
$order->info['total'] = $orderTotal;
return $order_total_array;
} we are now recalculating the order total based on the addition of the all of the order total modules; have the ability to exclude certain modules if their total is included elsewhere and hopefully make address your the issues i see in my code are:
perhaps you can solve those 2 issues. and let us know if this solution works for you. again, while i applaud your effort, i would hope we do not see code this confusing merged into the repo. |
I must say I have a hard time understanding your comments:
This said your approach is interesting, although in the way you show it here it can't work as all modules have already done their calculations and displayed results. It happens on the line with this code: $GLOBALS[$class]->process(); just after the beginning of the foreach loop.
It does the job although with actual ZC code, I don't think it was meant to be done here. Solution looks nicer in simpler too. But it is just an appearance. There are two potential problems that can occur: |
I found a solution to the second problem of this code when using a different currency. By replacing |
Well, if the new code is preferred by most, I could change this PR. But I would appreciate some other opinions first. I think only the code to exclude a module should not be included. As long as calculations are not made in the ot_total class, it is not a good idea. |
Saving results of each module allows adding them in order total to check penny loss. This total could be used as the final total instead of the actual one, eliminating penny loss and making invoice numbers always add correctly to give the final total.
Removed discount coupon and GV modification which belongs to another branch.
640ded1
to
ea7b77f
Compare
Well, it seems ZC user must write their own ot_total module if they want to have invoices that with numbers that add together... |
In case of a penny difference between invoice total and sum of numbers on the same invoice, this modification will adjust the total to equal the sum of other numbers.
The one penny difference is still visible between invoice with prices including tax and invoice with prices without tax, but one by one, they are coherent, all numbers add correctly.
See forum thread 'The Lost Penny of ZC'.