-
-
Notifications
You must be signed in to change notification settings - Fork 225
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
base: master
Are you sure you want to change the base?
Lost penny management #6391
Conversation
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.
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. |
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'.