ot_coupon -> potential undefined index and other errors....

[proseLA]

https://github.com/zencart/zencart/blob/bf85f1ea4a2ab56701203ca8669736b8d42482a4/includes/modules/order_total/ot_coupon.php#L597

i'm thinking there should be a check here prior to subtracting. i'm definitely getting an undefined index error when playing around on a customized site. whether the sort order of the order_total modules is related, i can not say.

i'm currently doing this with no exhibited ill effect:

               $tax_description = zen_get_tax_description($product['tax_class_id']);
               if (empty($orderTaxGroups[$tax_description])) {
                    $orderTaxGroups[$tax_description] = 0 - $productsTaxAmount;
                } else {
                   $orderTaxGroups[$tax_description] -= $productsTaxAmount;
                }

[proseLA]

some more issues i see here in ot_coupon:

https://github.com/zencart/zencart/blob/bf85f1ea4a2ab56701203ca8669736b8d42482a4/includes/modules/order_total/ot_coupon.php#L429

here if we look at the comment, the orderTotal is really only the total for the qualifying products. the comment is correct. however, when we look at the part of the math associated with that calculation here:

https://github.com/zencart/zencart/blob/bf85f1ea4a2ab56701203ca8669736b8d42482a4/includes/modules/order_total/ot_coupon.php#L608-L610

we can see that the orderTotal amount has the $order->info['tax'] subtracted from it. why would that work? if one had a coupon that was only good for certain items, ie items on sale are not valid, we would be subtracting the tax for whole order when in fact, we should only be subtracting the tax for the qualifying products.

in my tests, however, ZC calculates this amount correctly..... very odd... but again, i am NOT a fan of the order total modules construction and how it relates to the $order->info array... because when calling this method, it is dependent on what is in the info array.

HOWEVER, if one makes use of the edit orders plugin, curious things happen. the bug that i pointed above now manifests itself... the processing of a coupon on the admin side gives different results v the processing of the coupon on the store side. personally i do not think this situation is ideal. calling ot_coupon->process() should yield the same results on the admin and on the storefront.

in my limited testing, the following code addresses the issue, but again, what a hack!

      if (DISPLAY_PRICE_WITH_TAX != 'true') {
            if (IS_ADMIN_FLAG) {
                $orderTotal -= ($productsTaxAmount ?? 0);
            } else {
                $orderTotal -= $order->info['tax'];
            }
        }

[proseLA]

there is a problem here: https://github.com/zencart/zencart/blob/bf85f1ea4a2ab56701203ca8669736b8d42482a4/includes/modules/order_total/ot_coupon.php#L666 as seen here: https://github.com/zencart/zencart/blob/bf85f1ea4a2ab56701203ca8669736b8d42482a4/includes/modules/order_total/ot_coupon.php#L643

keys is now a function, not a var. i believe line 666 would work as expected if changed to:

$keys = implode("','", $this->keys());

[scottcwilson]

this was broken in 1.5.8; correctly called as a function in 1.5.7.

[proseLA]

as previously stated, while this section of code works on the storefront, it does NOT work on the admin for edit_orders:

https://github.com/zencart/zencart/blob/bf85f1ea4a2ab56701203ca8669736b8d42482a4/includes/modules/order_total/ot_coupon.php#L608-L610

further testing suggests this correction will process the coupon the same on the store front as well as on the admin with edit_orders:

       if (DISPLAY_PRICE_WITH_TAX != 'true') {
            $orderTotal -= $orderTotalTax;
       }

$orderTotalTax gets initialized with $order->info['tax']:

https://github.com/zencart/zencart/blob/bf85f1ea4a2ab56701203ca8669736b8d42482a4/includes/modules/order_total/ot_coupon.php#L571

then we look at not valid products for the coupon:

https://github.com/zencart/zencart/blob/bf85f1ea4a2ab56701203ca8669736b8d42482a4/includes/modules/order_total/ot_coupon.php#L584

and subtract the $productsTaxAmount for the invalid product from the $orderTotalTax:

https://github.com/zencart/zencart/blob/bf85f1ea4a2ab56701203ca8669736b8d42482a4/includes/modules/order_total/ot_coupon.php#L596

so this is the amount that needs to be subtracted, when display price with tax is false.

why this works on the storefront in its current status is a question for another day. but, to me, my correction above provides consistent results on the storefront and on the admin.

when i look at this module, i see improvement in variable naming in many places. i think the vars can be better named here, as $orderTotal and $orderTotalTax refer only to qualifying products for the coupon.

[webspacecreations]

If coupon amounts exceeds order total at checkout, a 503 error is generated upon proceeding to payment. Definitely don't want to give customer impression site is broken (v1.5.7c).

[proseLA]

If coupon amounts exceeds order total at checkout, a 503 error is generated upon proceeding to payment. Definitely don't want to give customer impression site is broken (v1.5.7c).

i have tested having a coupon worth more than the value of the order on a vanilla install of v158 and v157c, and have not been able to reproduce the error. which suggests to me your site has been modified.

on v157c, i did get this (mis-leading at best) error in the message stack when the coupon was already used:

Warning Coupon "test503" has already been used the maximum permitted times (0).

that error looks to be removed in v158. bravo!

in both tests i did, the order completed, but ZC generated debug logs due to PHP notices of undefined vars, that started here (on v158):

PHP Notice: Undefined variable: payment_modules in /var/www/zcdev/includes/modules/pages/checkout_confirmation/header_p hp.php on line 96.

while a definite problem, this is not an issue related to ot_coupon as the coupon correctly applied the value to be the value of the order. so whether someone wants to tackle that problem is a different issue.

in addition, why you are getting a 503 response, i can not see in the base code. i would suggest more trouble shooting on your end to pin point where the problem is, as i am not seeing it in a base ZC install.

[drbyte]

The 503 error is probably a PHP Fatal Error, in which case the exact details should be recorded in a /logs/myDebug-xxxxxxxxx.log file.

[webspacecreations]

I'm guessing they were in some .log file, but since every single error (fatal or not) is being logged and in separate files, it wasn't obvious which file it might be. FWIW free shipping was also selected for the coupon. "Modifications" were minimal (SEO friendly URLs add-on, UPS shipping add-on, bootstrap template). The problem immediately disappeared when the coupon amount was corrected to be lower than the item purchase price. Since coupons aren't regularly used on this store and it was solely for testing payment, I really don't have any more information to supply. It's entirely possible the source of the fatal error lies in something else (like passing a negative amount to a payment module), but the feature associated with the error seemed pretty cut and dry.