[Done v1.3.9] Problem with "credit_covers"

[Tick]

Hello all,

I think I've stumbled across a bug in Zen where credit order total modules do not get handled correctly if they apply a credit that is near the order total.

Steps to reproduce (on a fresh install):

1. Enable the low order fee module to charge $5 for orders under $50.
2. Enable the Gift Certificates order total module.
3. Ensure a cheap shipping module is enabled. I used per item shipping at $2.50.
4. Enable PayPal WPP in sandbox mode.
5. Create a customer account
6. Give the customer account a GV balance of $40
7. Add a $35 dollar item to your cart. I selected "A Bug's Life"
8. Proceed to checkout. For step 1, select per-item shipping at $2.50.
9. In step 2, your order total should be $43.49. Apply a GV balance of $40.00 (or any amount that reduces the grand total to less than the low order fee). Select PayPal for your payment method.
10. Step 3, Confirm.

You will be immediately redirected to checkout_success rather than PayPal. If you examine the order in the admin section it will show the payment method as "Gift Certificate / Coupon."

As far as I can tell, the issue is that order_total->pre_confirmation_check() is called before order_total->process(). Since process() is the function that actually modifies the global order object, pre_confirmation_check() uses incorrect $order->info['total'] amounts to calculate the deductions versus the order total. As a consequence credit_covers is set true and $_SESSION['payment'] is cleared so the customer is never redirected to PayPal.

I don't think this is a bug in the low order fee module per se, but a design issue (or at least an issue in the order_total class). Using the other modules as an example it looks like modifying the global order object in process() is the "right thing to do," but that breaks pre_confirmation_check(). I came across this while working on my own order total class, but when I realized it was reproducible with a stock cart I decided to post here.

When I am writing an order total module, is process() the correct place to modify the global order object?

Thanks for any and all help!

[wilt]

Hi,

The problem here is we cannot just run the process function before the pre_confirmation as that would then set in stone all discounts, and then calculations would take place based on the reduced order totals.

I did however come up with a fix, based solely on changing the pre_confirmation_check routine in the order_total class

  function pre_confirmation_check() {
    global $order, $credit_covers;
    if (MODULE_ORDER_TOTAL_INSTALLED) {
      $total_deductions  = 0;
      reset($this->modules);
      $orderInfoSaved = $order->info;
      while (list(, $value) = each($this->modules)) {
        $class = substr($value, 0, strrpos($value, '.'));
        if ( $GLOBALS[$class]->credit_class ) {
          $order_total = $GLOBALS[$class]->get_order_total();
          if (is_array($order_total)) $order_total = $order_total['total'];
          $total_deductions = $total_deductions + $GLOBALS[$class]->pre_confirmation_check($order_total);
        }
        else 
        {
          $GLOBALS[$class]->process();
          $GLOBALS[$class]->output = array();
        }
      }
      $difference = $order->info['total'] - $total_deductions;
      if ( $difference <= 0.009 ) {
        $credit_covers = true;
      }
      $order->info = $orderInfoSaved;
    }
  } 

So although I have run the process function, i've restricted it to non Credit Class modules, and reset order totals after pre_confirmation_check finishes.

This works on my up to date v1.3.8. Would appreciate some testing/feedback before I commit that as a bug fix.

[Tick]

Hi,

The problem here is we cannot just run the process function before the pre_confirmation as that would then set in stone all discounts, and then calculations would take place based on the reduced order totals.

I did however come up with a fix, based solely on changing the pre_confirmation_check routine in the order_total class

(snip)

This works on my up to date v1.3.8. Would appreciate some testing/feedback before I commit that as a bug fix.

Thanks, Wilt! I will apply this to my development site tonight and do some test runs

Can I make a suggestion for 1.4? Perhaps the (returned) output from the process() function should be eliminated. Then you could break it into two functions: process() and output(). Then, process() could be called from the header_php file to set the discounts/additions in stone and it wouldn't matter when (or if) output() was called. In practice the new flow would go: process(), pre_confirmation_check(), output(). I know that would break existing order total modules, but hey, it's a major release!

As an added bonus, it would also make it easier to display messages to the user using the global messageStack object. I worked around it by calling order_total->process() at the top of the template so that I could use messageStack to warn customers about low order fees, but I imagine there would be other uses as well.

Thanks again, and I'll let you know how the testing goes....

[Tick]

This works on my up to date v1.3.8. Would appreciate some testing/feedback before I commit that as a bug fix.

Hi Wilt,

I've also done some preliminary testing and this works for me as well. I plan to add some logging statements to it and move it to production this week. I'll let you know if I run into any issues.

Thanks again for your help!