Skip to content
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

"Save and continue" does nothing useful when PayPal method is selected #32

Open
eevee opened this issue Oct 6, 2013 · 12 comments
Open

Comments

@eevee
Copy link

eevee commented Oct 6, 2013

On /checkout/payment, pick a PayPal Express payment method and click "Save and continue". You get the same page again with a red banner that helpfully proclaims,

Payment could not be processed, please check the details you entered

Granted the button is supposed to be hidden by JS, but not everyone has JS enabled, and also someone might hypothetically have forgotten to finish the install process (but what are the chances of that?).

Would be nice if this error actually said something like, say, "please click the paypal logo that doesn't look entirely like a button".

@radar
Copy link
Contributor

radar commented Oct 6, 2013

Would you please be able to submit a patch to fix this issue?

On Sun, Oct 6, 2013 at 11:45 AM, Eevee notifications@github.com wrote:

On /checkout/payment, pick a PayPal Express payment method and click "Save and continue". You get the same page again with a red banner that helpfully proclaims,

Payment could not be processed, please check the details you entered
Granted the button is supposed to be hidden by JS, but not everyone has JS enabled, and also someone might hypothetically have forgotten to finish the install process (but what are the chances of that?).

Would be nice if this error actually said something like, say, "please click the paypal logo that doesn't look entirely like a button".

Reply to this email directly or view it on GitHub:
#32

@FineLineAutomation
Copy link
Contributor

I agree that the standard flow should be followed in this case and that the Save and Continue" button should redirect to the payment screen. I was very confused when it did this functionality because I have removed the continue class.There is an additional consequence to this that in that if a person puts in coupon code in and then presses the checkout with PayPal button the coupon never gets applied.

@radar
Copy link
Contributor

radar commented Mar 11, 2014

This is not possible at this point in time because the form submits to a route that is internal to Spree's frontend. To customize that would require JavaScript to point the form somewhere else, and that's pretty hacky. Clicking the PayPal button is the way to do it and the way that PayPal recommends.

@radar radar closed this as completed Mar 11, 2014
@FineLineAutomation
Copy link
Contributor

Why couldn't you put a before_filter on the CheckoutControllers update method and redirect from there? Also note that the original paypal express extension handle the PayPal redirection on both the Checkout with PayPal button and the Save and Continue button.

@radar
Copy link
Contributor

radar commented Mar 11, 2014

Because it's suicide to hack CheckoutController, as the previous PayPal extension shows. It should be possible to keep the functionality of this extension completely inclusive and to not hack anything within Spree. Hacking Spree should be a last resort option.

The "Save and Continue" button will be hidden automatically by JavaScript if you run the installer.

@eevee
Copy link
Author

eevee commented Mar 11, 2014

Can the error message at least be made more useful? Not everyone has JavaScript enabled, and shuffling a dead simple form around with JavaScript sounds far more hacky to me.

The PayPal button barely even looks like a button, relative to the rest of the app.

Alternatively, why can Spree core not have a non-hacky hook for redirecting to an external processor? I imagine e.g. Google Checkout would benefit as well.

@FineLineAutomation
Copy link
Contributor

I can buy not wanting to pollute the checkout controller model, especially given how it looked in the old extension. I thought that since we hacked CheckoutController in Spree Gateway with Skrill (https://github.com/spree/spree_gateway/blob/master/app/controllers/spree/checkout_controller_decorator.rb) that it would be an acceptable solution for PayPal.

I coded up this solution real quick that works with minimal code: https://gist.github.com/FineLineAutomation/9479801

All it does is redirect to the paypal/ url that the PayPal checkout buttons directs to. I personally think that consistency in how the payment methods function to the user (i.e. all payment methods must use the Save and Continue button) trumps a little code in Checkout Controller.

@radar
Copy link
Contributor

radar commented Mar 11, 2014

I don't know of anyone who is either a) using Skrill or b) who is reporting bugs in that Skrill code. So either the code is beautifully flawless or, more reasonably, nobody is using it. Hacks on CheckoutController is a path down which lies only madness. We shouldn't do it.

@radar radar reopened this Mar 11, 2014
@radar
Copy link
Contributor

radar commented Mar 11, 2014

There are other external payment gateways that need to do this kind of thing, and @FineLineAutomation and I were talking in #spree about having an "external checkout" hook in CheckoutController that would enable this kind of thing. Something that would check if the payment method selected is supposed to redirect out and then, if it is, actually do the redirection. Amazon Payments, Google Checkout and the Skrill hacks mentioned earlier would probably benefit from this kind of work.

FineLineAutomation added a commit to FineLineAutomation/better_spree_paypal_express that referenced this issue Mar 11, 2014
…roper post hook is put in place.

spree-contrib#32 has the details.
Signed-off-by: Nathan Lowrie <nate@finelineautomation.com>
@FineLineAutomation
Copy link
Contributor

Note that my commit that references the issue is just a temporary one for my own use. I don't expect to to be merged upstream. I really like the idea of the post checkout hook.

@alepore
Copy link
Member

alepore commented Jul 22, 2014

just my 2c...
i "solved" this issue for me just moving payment methods html inside the "save and continue" button container and adding a button-like style, so users always see the same button, but under the hood sometimes is a form submit, sometimes is a link to the payment method.

@seangreen
Copy link

While I agree, that checkout hacks are not beatiful... in this case it's so little code and so much UI improvement for the user, that I really think we should do it this way - until there is a proper solution within the spree extension.

So just adding this here in my custom frontend Javascript solved the issue for me:

$('#checkout_form_payment button[type=submit]').click(function() {
if ($('#order_payments_attributes__payment_method_id_3').is(':checked')) {
event.preventDefault();
$('a#paypal_button').click();
}
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants