Order left as unsubmitted after PayPal payment has been made

[GoogleCodeExporter]

eCommerce version affected: SSU branch, r2302 (NOTE: problem introduced at 
r2180)

SilverStripe version: 2.4.6
Browser/Operating System used: Windows + Firefox

What steps will reproduce the problem?
1. Create an order
2. Pay via PayPal (NOTE: Use the sandbox
3.

What is the expected output? What do you see instead?
It should send you through to the order confirmation page. Instead, it sends 
you back to the cart page, and the order's status is left as "created." 

Even worse, the user can then edit the order, and when they go to pay again, it 
will pick up the original payment, and mark the modified order as submitted. It 
will also log a failed PayPal payment.** 

The problem was first created in r2302, which doesn't attempt to mark the order 
as submitted until after the payment has been processed. Unfortunately, the 
PayPal payment has to redirect to the PayPal website, thus preventing this from 
taking place. No doubt any payment type that relies on a redirect to process 
the payment will no longer work.

Attached is a patch that reinserts the code to mark the order as submitted 
prior to payment processing.

** NOTE: I'm using the PaypalExpressCheckoutPayment class from the 
payment_paypal module.

Original issue reported on code.google.com by hjr29s...@gmail.com on 29 Mar 2012 at 4:37

Attachments:

• OrderForm-PayPal-Fix.patch

[GoogleCodeExporter]

thank you for reporting this bug.... I will fix and let you know.

Original comment by nfranc...@gmail.com on 29 Mar 2012 at 6:43

• Changed state: Accepted

[GoogleCodeExporter]

Please check commit 2402

Original comment by nfranc...@gmail.com on 6 Apr 2012 at 9:12

• Changed state: PeerReview

[GoogleCodeExporter]

I'm afraid that commit 2402 doesn't work. I updated just OerderForm.php, 
attempted to make a test purchase, and got the attached error.

Original comment by hjr29s...@gmail.com on 9 Apr 2012 at 9:52

Attachments:

• [POST checkout OrderForm.htm](https://storage.googleapis.com/google-code-attachments/silverstripe-ecommerce/issue-220/comment-3/POST checkout OrderForm.htm)

[GoogleCodeExporter]

Okay, it fails at line 81 of EcommercePayment.php:

        if($result->isProcessing()) {
>>>>        return $result->getValue();
        }

The Payment_Processing object that PayPalExpressCheckoutPayment returns leaves 
$value at its default, which is null. The PayPalPayment class that comes with 
the Payment module sets $value to be a form. It looks like it's using JQuery to 
get the client to auto-submit the form, and move to the next step. The 
WorldPayPayment class does something similar. Rather weird code. I think that a 
simple redirect is more reliable (will work even when Javascript is switched 
off).

It looks like the payment_paypal module needs a small update. I'm not sure what 
$value in the returned Payment_Processing object should be set too, but it 
looks like a simple string will do.

Original comment by hjr29s...@gmail.com on 9 Apr 2012 at 10:19

[GoogleCodeExporter]

Okay, next problem. Having updated PayPalExpressCheckoutPayment to set 
Payment_Processing's $value to " ", it now fails at the next error. See the 
attached error message.

Line 150 of OrderForm.php needs to be changed from:
            ShoppingCart::singleton()->tryToFinaliseOrder();
to
            $order->tryToFinaliseOrder();

After these two changes, it works as it should.

Original comment by hjr29s...@gmail.com on 9 Apr 2012 at 11:11

Attachments:

• [POST checkout OrderForm.htm](https://storage.googleapis.com/google-code-attachments/silverstripe-ecommerce/issue-220/comment-5/POST checkout OrderForm.htm)

[GoogleCodeExporter]

COMMENT:
"The Payment_Processing object that PayPalExpressCheckoutPayment returns leaves 
$value at its default, which is null. The PayPalPayment class that comes with 
the Payment module sets $value to be a form. It looks like it's using JQuery to 
get the client to auto-submit the form, and move to the next step. The 
WorldPayPayment class does something similar. Rather weird code. I think that a 
simple redirect is more reliable (will work even when Javascript is switched 
off)."

REPLY:
The form works in such a way that it can be submitted if JS is turned off and 
it auto-submits if JS is turned on.  This is a good, because it creates a 
simple POST that looks like a post from the user.  it is a little strange at 
first, but it works. 

COMMENT:
"It looks like the payment_paypal module needs a small update. I'm not sure 
what $value in the returned Payment_Processing object should be set too, but it 
looks like a simple string will do."

REPLY:
The isProcessing Method should return TRUE - returning " " is dangerous!  Yes, 
it is a true value, but it looks like false!!!

FIXED

THANK YOU for looking into all this. 

Original comment by nfranc...@gmail.com on 10 Apr 2012 at 1:54

• Changed state: Fixed

[GoogleCodeExporter]

QUOTE:
The isProcessing Method should return TRUE - returning " " is dangerous!  Yes, 
it is a true value, but it looks like false!!!

REPLY:
I was NOT talking about the isProcessing() method, but the value returned by 
getValue() (i.e., the value of the $value field). Look again at the code that I 
quoted (line 81 of EcommercePayment.php):

        if($result->isProcessing()) {
>>>>        return $result->getValue();
        }

Note the ">>>>" pointing to getValue().

The getValue() method returns the value passed to the constructor. I suggested 
a dummy string simply because other payment classes return strings, and all 
that was required was not returning null. While hackish, this solution works (I 
tested it).

I hope that you fixed the value returned by getValue(), and not the 
isProcessing() method (which wasn't broken in the first place). If not, replace 
line 95 of PayPalExpressCheckoutPayment.php:
            return new Payment_Processing();
with:
            return new Payment_Processing(true);

Original comment by hjr29s...@gmail.com on 10 Apr 2012 at 3:19