Users can't confirm orders #34

Closed
AlainPilon opened this Issue Oct 9, 2013 · 18 comments

Comments

Projects
None yet
4 participants

When clicking on the Paypal express button, the user has to login, review the order then click on the "continue" button.

review_your_information

The text next to the button says "You're almost done. You will confirm your payment on Alain Pilon's Test Store."

This is faulty since clicking on the button will redirect to the order#show page:

order_r382023264_-_spree_demo_site

There are 2 possibles fixes:

1- Have the Paypal redirect to the Spree order confirmation page.
2- Change the Paypal behaviour so the confirmation is done on Paypal.

Related to #2:
According to this page (at the bottom):

https://developer.paypal.com/webapps/developer/docs/classic/express-checkout/integration-guide/ECCustomizing/

the call to Paypal should add &useraction=commit in the param string.

Contributor

radar commented Oct 11, 2013

I can't find a way to pass the useraction parameter along in the redirect url using PayPal's own library. The method that generates that URL doesn't even take more parameters:

[1] pry(#<Spree::PaypalController>)> provider.express_checkout_url(pp_response, :test => true)
ArgumentError: wrong number of arguments (2 for 1)

This is slightly annoying. I have two options here: I can either chuck it into URI.parse and do some magical trickery around the parameters on the end, or I can patch PayPal's code and fix this problem. I'm going with the latter.

radar referenced this issue in paypal/merchant-sdk-ruby Oct 11, 2013

Merged

Allow express_checkout_url to accept extra parameters #6

Contributor

radar commented Oct 11, 2013

Opened that PR on merchant-sdk-ruby to address this problem. Once that's been solved, we'll be able to pass through this parameter and have the flow be correct.

Thanks @AlainPilon!

Contributor

sbounmy commented Oct 24, 2013

hi @radar,

merchant-sdk-ruby issue is fixed now paypal/merchant-sdk-ruby@3960abf

Any chance to see a quick fix for this please ?

Otherwise would be glad to give some help.
Thanks!

Contributor

radar commented Oct 25, 2013

Have now bumped the version. Thanks @sbounmy.

Contributor

azinazadi commented Nov 29, 2013

+1 for this. we also have the same problem :(

Contributor

azinazadi commented Nov 29, 2013

The problem is that, in here: https://github.com/radar/better_spree_paypal_express/blob/master/app/controllers/spree/paypal_controller.rb#L73
we shouldnt simply go to the next step without actual confirmation of user, but should let user push the confirm button first.

Contributor

azinazadi commented Dec 2, 2013

We investigated more into the issue. In PaypalController#confirm we create a payment in the order. Then we call order.next.
But in Order#confirmation_required? we check that there are no payments before we decide to go to the confirmation page.
Thus no final confirmation can ever happen with paypal.

PaypalController:

    def confirm
      order = current_order
      order.payments.create!({
        :source => Spree::PaypalExpressCheckout.create({
          :token => params[:token],
          :payer_id => params[:PayerID]
        }),
        :amount => order.total,
        :payment_method => payment_method
      })
      order.next
      if order.complete?
        flash.notice = Spree.t(:order_processed_successfully)
        flash[:commerce_tracking] = "nothing special"
        redirect_to order_path(order, :token => order.token)
      else
        redirect_to checkout_state_path(order.state)
      end
    end

Order:

# If true, causes the confirmation step to happen during the checkout process
    def confirmation_required?
      if payments.empty? && Spree::Config[:always_include_confirm_step]
        true
      else
        payments.valid.map(&:payment_method).compact.any?(&:payment_profiles_supported?)
      end
    end
Contributor

radar commented Dec 3, 2013

@azinazadi Does #48 fix this?

@azinazadi azinazadi added a commit to keevee/better_spree_paypal_express that referenced this issue Dec 3, 2013

@azinazadi azinazadi Customer confirms the payment inside paypal.
according to the [paypal doc page](https://developer.paypal.com/webapps/developer/docs/classic/express-checkout/integration-guide/ECCustomizing/)
" If you collect no additional information after buyers return from PayPal, you can skip the confirm-order page on your website. " , which is the case for spree. so they can finish the payment
in paypal, cuz we already gathered all the required information.

[Fixes #34]
fe19e3f
Contributor

azinazadi commented Dec 3, 2013

I think yes, but it does several extra stuff. I made a simple pull request which just address this issue. tested it and it works fine:

review_your_information_-_paypal-11

Contributor

sbounmy commented Dec 3, 2013

@azinazadi I am not sure this is enough to have the confirmation step displayed on spree.
there are methods like confirmation_required? https://github.com/spree/spree/blob/master/core/app/models/spree/order.rb#L165 which needs to return true to display the final confirmation step.

radar closed this in 8af7300 Dec 4, 2013

@radar radar added a commit that referenced this issue Dec 4, 2013

@azinazadi @radar azinazadi + radar Customer confirms the payment inside paypal.
according to the [paypal doc page](https://developer.paypal.com/webapps/developer/docs/classic/express-checkout/integration-guide/ECCustomizing/)
" If you collect no additional information after buyers return from PayPal, you can skip the confirm-order page on your website. " , which is the case for spree. so they can finish the payment
in paypal, cuz we already gathered all the required information.

Fixes #34
Fixes #49
7bf25d5

@radar radar added a commit that referenced this issue Dec 4, 2013

@azinazadi @radar azinazadi + radar Customer confirms the payment inside paypal.
according to the [paypal doc page](https://developer.paypal.com/webapps/developer/docs/classic/express-checkout/integration-guide/ECCustomizing/)
" If you collect no additional information after buyers return from PayPal, you can skip the confirm-order page on your website. " , which is the case for spree. so they can finish the payment
in paypal, cuz we already gathered all the required information.

Fixes #34
Fixes #49
0ec25a2
Contributor

radar commented Dec 4, 2013

There will be no confirm step shown in Spree. The confirmation is all done on the PayPal side and then the user submits from there to Spree. Spree then accepts the order and marks it as complete. Does this sound OK to you @sbounmy?

Contributor

sbounmy commented Dec 4, 2013

@radar, we have a built-in checkout flow with confirmation step in spree that basically let users review their order before placing their order. I don't see any valid reason to not use this, the code added in spree_paypal_express are methods that are required by spree to determine if the payment requires a final confirmation step or not

Contributor

radar commented Dec 4, 2013

@sbounmy I don't understand any of that. Spree has a confirmation step, sure. Confirmation can also happen on the PayPal side, which is now how this extension works. What's wrong with that?

Contributor

sbounmy commented Dec 4, 2013

nothing wrong. I just like consistency with other spree payment checkout flow that requires confirmation.

Contributor

azinazadi commented Dec 4, 2013

I think as well, that the current payment flow is not quite fine, as described on my other comment. I asked my colleague and looks like , at least , for german low, this flow is not quite fine and can make problems for German shops...

also I think the fix by @sbounmy is too complicated and not compatible with spree 2.1 I tried for an hour to run it, but couldnt make it. so I am gonna make a simple pull request which approaches just this problem on spree 2.1

Contributor

radar commented Dec 12, 2013

@azinazadi I don't understand what you mean. Can you please explain what is broken here?

Contributor

azinazadi commented Dec 12, 2013

@radar I double checked the issue , considering the recent changes on Order#confirmation_required? , and it works perfectly fine. thanks

Contributor

radar commented Dec 13, 2013

Excellent, thanks for confirming!

On Thu, Dec 12, 2013 at 8:45 PM, azinazadi notifications@github.com
wrote:

@radar I double checked the issue , considering the recent changes on Order#confirmation_required? , and it works perfectly fine. thanks

Reply to this email directly or view it on GitHub:
radar#34 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment