Paypal Express is broken on spree 1-1-stable

[spree-contrib]

Paypal Express is broken on spree 1-1-stable

[r3ap3r2004]

I get the error:

undefined method `authorize' for #<Spree::BillingIntegration::PaypalExpress:0x007f977d0aba38>

Appears to be part of the same issue.

[GeekOnCoffee]

Sent to the email list To Brian (BDQ) or any other qualified person,

Any chance that you can update the PayPal Express gem to take into
account the latest changes to the payment processing code in the
master branch of Spree?

I'm trying to launch my site this week, but I've hit a show stopper
with the PayPal Express gem and I'm not sure that I fully understand
the proper timing of the calls to authorize/capture and the responses
from PayPal to properly fix it myself.

I get the following error after being redirected back from PayPal to
my Spree store.

START OUTPUT

NoMethodError in Spree::CheckoutController#paypal_confirm

undefined method `authorize' for
#Spree::BillingIntegration::PaypalExpress:0x007f9d8ad3e508

activemodel (3.2.3) lib/active_model/attribute_methods.rb:407:in
method_missing' activerecord (3.2.3) lib/active_record/attribute_methods.rb:148:in method_missing'
spree_master/core/app/models/spree/payment/processing.rb:106:in block in gateway_action' spree_master/core/app/models/spree/payment/processing.rb:146:in protect_from_connection_error'
spree_master/core/app/models/spree/payment/processing.rb:103:in gateway_action' spree_master/core/app/models/spree/payment/processing.rb:22:inauthorize!'
spree_master/core/app/models/spree/payment/processing.rb:11:in `process!'

END OUTPUT

NOTE: The line numbers may be off in the output because of various
debugging output lines that I have in my copy of the PayPal Express
code.

My understanding of the process is that after successfully completing
the steps on PayPal's site that I'm redirected back to the
paypal_confirm method (inside the checkout_controller_decorator.rb
file). This method then calls paypal_finish (assuming the confirm
step isn't activated).

paypal_finish then sends the auth/capture to PayPal and upon
successful response tells the @order to step to the 'complete' state.
At that point the payment processing code calls the 'authorize' method
on the Spree::BillingIntegration::PaypalExpress object.

Just creating the authorize method doesn't seem logical since that
part of the process has already happened before telling the order to
advance steps. But moving the authorization outside of the
paypal_finish into its own 'authorize' method has problems since the
order is only advanced to 'complete' if we successfully authorized.
(chicken and egg, catch 22 problem).

I assume there is a logical process for how this should be changed and
should only take a small amount of time for someone familiar with how
the latest payment processing code in Spree is handled.

Anyway, if someone could take a look at updating it I would REALLY
appreciate it.

Thanks!
Chris

[jumph4x]

Man this whole extension needs a serious rewrite. I am supposed to be the one to rewrite it, but work just swamped me. What to do, what to do...

[r3ap3r2004]

Denis,
Do you have some time to jot down some pseudo code for how the process should work. I'm happy to take a stab at it as long as I understand the right steps. Unfortunately my understanding of the PayPal API is a little lacking, which is why I'm hesitant to make any changes without some direction.

Alternatively, if you have time to make a branch and move the pieces around into their proper order (even if it doesn't fully work properly) I can probably get it working. If you go this route I would appreciate some quick comments in the code saying what each of the major parts is attempting to accomplish.

I'm positive that I can make it work right now, I just question whether I would be approaching the problem correctly. I'm a perfectionist, so I hate just slapping a bandaid on the problem just to get it working.

Thanks again,
Chris

[GeekOnCoffee]

Chris,

https://merchant.paypal.com/us/cgi-bin/?cmd=_render-content&content_ID=merchant/express_checkout details the flow from the paypal side.

From the Spree side, essentially:

• You build your cart and click the paypal button
• that makes the call to Paypal with all the transaction information
• User does the paypal stuff
• Paypal does a callback that completes the transaction.

Chris, Do you use IRC? I'll be online for a bit longer tonight, and at least business hours the rest of the week. I'd be happy to answer any questions I can, and guide you through it.

[r3ap3r2004]

Andrew,
I probably should have been a little more specific. I understand
the basic PayPal process, and the Spree general process. I actually
was the one who made the changes to this gem to initially get it
working with Rails 3.0.

What I'm not confident about is the latest changes to the Spree core
that moved the payment info into the payment/processing.rb file and
the changes regarding the timing.

As I recall, with the prior way of doing things, the process! method
handled everything and then you advanced to the 'completed' step after
the processing was completed. (Although I haven't looked at it in
quite a while). Now it appears that the 'authorize/capture' triggers
have been moved into the before transition to complete step, which
would mean you would need to attempt to move to the next step in order
to initiate the auth/capture process, and then return some sort of
failure to prevent the transition if there was an error.

What I'm looking for is the correct ordering of the actual (redirect
to PayPal, authorize, capture, transition to complete/pending states)
since it appears this ordering has slightly changed. Although this
could simply be a misunderstanding on my part.

Jabber/Gmail: r3ap3r2004 (@gmail.com)
Skype: r3ap3r2004

I'm in Japan right now, so if you are in the U.S. then there is a 13 -
16 hour time difference. It's 9:30 a.m. here right now.

Thanks again,
Chris

On Wed, May 2, 2012 at 9:11 AM, Andrew Hooker
[email protected]
wrote:

Chris,

https://merchant.paypal.com/us/cgi-bin/?cmd=_render-content&content_ID=merchant/express_checkout details the flow from the paypal side.

From the Spree side, essentially:

• You build your cart and click the paypal button
• that makes the call to Paypal with all the transaction information
• User does the paypal stuff
• Paypal does a callback that completes the transaction.

Chris, Do you use IRC? I'll be online for a bit longer tonight, and at least business hours the rest of the week. I'd be happy to answer any questions I can, and guide you through it.

---

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

[radar]

As far as I am aware, the ordering has not changed. That was not the intention behind the changes. The intention was simply to move the code out of the CreditCard model where it did not belong and into the Payment model. The Payment model was too crowded, and so it is now in the Payment::Processing module.

I would love to get to the bottom of why this is now an issue. The messiness of spree_paypal_express is not helping in that regard, as is my impending travel (wherein I go through 5 cities in 28 hours). Perhaps the solution to this problem is to sit down with this extension and spend some quality time giving it the love, attention, good code and documentation that it so deeply needs.

Until then, I do not have an answer for you. I'm sorry.

[radar]

@r3ap3r2004 Given that you're pretty close to my timezone, perhaps we could work this thing out together? Let me know.

[jumph4x]

Me toooooo
On May 1, 2012 9:39 PM, "Ryan Bigg" <
[email protected]>
wrote:

@r3ap3r2004 Given that you're pretty close to my timezone, perhaps we
could work this thing out together? Let me know.

---

Reply to this email directly or view it on GitHub:

#48 (comment)

[r3ap3r2004]

Ryan,
It must just be a misunderstanding on my part then. I guess it wasn't as easy for someone in the know to fix as I originally thought.

I'll spend some time today going through the latest Spree code to make sure I'm understanding the exact flow, then I'll go back and look through this gem and see if I can make it work the way it should. Any help you can give to at least start getting it moving in the right direction will help. I can then keep working on it while you are traveling.

Any idea if there is another gem that works using the same basic process that PayPal does that would be a good reference for how it should be done?

[radar]

The fix isn't simple because the error itself is cryptic. There should be no authorize method on the Spree::BillingIntegration::PaypalExpress object because there's never been one on that class, nor on the Spree::BillingIntegration class it inherits from, or the Spree::PaymentMethod class that that inherits from. For points of reference, check out the 1-0-stable branch.

For instance, here's all the methods in 1-0-stable that are called authorize:

core/app/models/spree/creditcard.rb
76:    def authorize(amount, payment)

core/app/models/spree/gateway/bogus.rb
26:    def authorize(money, creditcard, options = {})

core/app/models/spree/gateway/bogus_simple.rb
9:    def authorize(money, creditcard, options = {})

The last two can be ruled out immediately because we're not using a bogus gateway. The first can be ruled out with less sureity because nothing in the inheritance chain inherits from the Creditcard model... but somehow the change to that model is what has caused this breakage.

I'm going to take a look into it tonight, but I'm not promising any magic.

[r3ap3r2004]

The PayPal Express paypal_finish method calls this:

        #need to force checkout to complete state
        until @order.state == "complete"
          if @order.next!
            @order.update!
            state_callback(:after)
          end
        end

That causes the @order model to execute this:

      before_transition :to => 'complete' do |order|
        begin
          order.process_payments!
        rescue Core::GatewayError
          !!Spree::Config[:allow_checkout_on_gateway_error]
        end
      end

Which calls this:

    def process_payments!
      ret = payments.each(&:process!)
    end

Which calls this:

def process!
        if payment_method && payment_method.source_required?
          if source
            if !processing?
              if Spree::Config[:auto_capture]
                purchase!
              else
                authorize!
              end
            end
          else
            raise Core::GatewayError.new(I18n.t(:payment_processing_failed))
          end
        end
      end

Which in turn calls the gateway_action:

      def authorize!
        started_processing!
        gateway_action(source, :authorize, :pend)
      end

Which looks like this:

    def gateway_action(source, action, success_state)
      protect_from_connection_error do
        check_environment

        response = payment_method.send(action, (amount * 100).round,
                                       source,
                                       gateway_options)
        handle_response(response, success_state, :failure)
      end
    end

That is causing the 'authorize' method to be sent to the gateway, which it promptly fails to find.

It isn't a problem necessarily to create an 'authorize' method for the PayPal express gateway and let that send the information on to the active merchant gem (rather than doing it in the paypal_finish step. But that would require attempting to move the state machine to 'complete' before calling the active merchant code like is happening right now.

That's why I said it looks like the ordering has changed.

[radar]

How is that a change from how it was done previously though?

[r3ap3r2004]

Because the original payment.rb process! method looked like this

def process!
      if payment_method && payment_method.source_required?
        if source
          if !processing? && source.respond_to?(:process!)
            started_processing!
            source.process!(self) # source is responsible for updating the payment state when it's done processing
          end
        else
          raise Core::GatewayError.new(I18n.t(:payment_processing_failed))
        end
      end
    end

But now it looks like this:

      def process!
        if payment_method && payment_method.source_required?
          if source
            if !processing?
              if Spree::Config[:auto_capture]
                purchase!
              else
                authorize!
              end
            end
          else
            raise Core::GatewayError.new(I18n.t(:payment_processing_failed))
          end
        end
      end

Originally it was calling the source's 'process!' method, but now it is calling the 'authorize' or 'purchase!' method.

The problem seems to be that the Payment class has changed the internals of how the process! method works.

But again, I may be misunderstanding the Spree flow.

[radar]

I was able to get it to go all the way through to the paypal_finish step tonight, but when writing up steps to reproduce it from a fresh app I've not been able to replicate it again. Something's up.

[r3ap3r2004]

I can get it to go into the paypal_finish step just fine, but I can't
get it out of that step. :-) (since that's the step that attempts
to move things to the 'completed' stage. I needed to make a few other
changes to the code to deal with the mass assignment issues, but that
was all I've changed so far.

[GeekOnCoffee]

i just pushed a commit that fixed the mass assignment issues.

[r3ap3r2004]

Andrew,
I didn't realize you had committed that. I had already made those
changes to my local copy before you made that commit (although I went
the :with_protection => false method). I'll need to update my code.

Thanks,
Chris

[GeekOnCoffee]

Chris,

i just committed it a few minutes ago. Sorry if it causes any issues.

[radar]

omg you guys.

Look at @5ab0b9d6bea87b741627a914b4d7b5920ae539f0.

The cause of this problem was that the state callback on the order was attempting to process the paypal payment after it had already done it.

To stop this, we set the order state to complete explicitly using an update_attribute and then call finalize! after that so we can be sure that the email is sent out (h/t @GeekOnCoffee for that idea).

I've gone through an order on my local Spree setup on 1-1-stable and it works for me. Please try it on your systems and let me know if it works for you.

Thanks for playing :)

[r3ap3r2004]

Ryan,
THANK YOU!
That worked great for me. I think this extension still needs a lot of love, but that has solved my immediate issues.

Thanks again,
Chris

I

[radar]

Yay! Another happy customer :) still delights me as much as the first one.

Indeed it needs love but it's at a "it'll do" stage. When someone like jumph4x can dedicate some effort to it, the gem will get the love it needs.

Ryan Bigg

On Wednesday, 2 May 2012 at 17:12, r3ap3r2004 wrote:

Ryan,
THANK YOU!
That worked great for me. I think this extension still needs a lot of love, but that has solved my immediate issues.

Thanks again,
Chris

I

---

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