search

spree-contrib / spree_paypal_express

allowing use of paypal express in spree, using (a modified version of) active merchant
BSD 3-Clause "New" or "Revised" License
113 stars 141 forks source link

Branch 2-0-stable - cannot access order#show #151

Closed MatthieuBarthel closed 11 years ago

MatthieuBarthel commented 11 years ago

Hi,

I just move on Spree 2.0.3, but it looks like the spree_paypal_express 2-0-0-stable branch isn't ready yet, right ?

If I try to access to any order show action (event not paid with paypal), I got the following error :

NoMethodError in Spree/orders#show

Showing /Users/Matt/.rbenv/versions/1.9.3-p392/lib/ruby/gems/1.9.1/gems/spree_frontend-2.0.3/app/views/spree/shared/_order_details.html.erb where line #42 raised:

undefined method `payment' for #<Spree::Order:0x007f832f985530>
Extracted source (around line #42):

39:       <% order.payments.valid.each do |payment| %>
40:         <%= render payment%><br><% end %>
41:     
42: <% if order.payment && order.payment.source.class.to_s == 'Spree::PaypalAccount' %>
43:   <span class="cc-type">
44:     <%= image_tag "paypal.png" %>
45:     <%= t("paypal_payer_statuses.#{order.payment.source.payer_status.to_s}").capitalize %>
Trace of template inclusion: /Users/Matt/.rbenv/versions/1.9.3-p392/lib/ruby/gems/1.9.1/gems/spree_frontend-2.0.3/app/views/spree/orders/show.html.erb

If I remove the paypal gem, I can access to this page without error.

Thanks for your help, Matt

pavelnikolov commented 11 years ago

I have the same problem

undefined method `payment' for #<Spree::Order:

It seems that now there can be more than one payment. So we have to replace payment with payments

@order.payments.first...
pavelnikolov commented 11 years ago

The problem is with this file spree_paypal_express / app / overrides / spree / shared / _order_details / add_paypal_details.html.erb.deface instead the changes should override - spree / core / app / views / spree / payments / _payment.html.erb

MatthieuBarthel commented 11 years ago

Awesome, thanks @pavelnikolov :+1:

It would be cool if someone responsible of this repository can check your PR and merge it, some superhero like @radar maybe ? :)

JDutil commented 11 years ago

Fix has been merged.

Also Note: Speaking for myself (this does not reflect the Spree teams opinions) I'm pretty sure this extension is no longer maintained by anyone, and I have no interest in maintaining it. I don't like paypal and this extension is badly needing a refactoring or rewrite to clean it up. Members of the community are going to need to pick up the torch to keep this extension going. It probably wouldn't hurt for someone to make their own canonical fork of the project.

radar commented 11 years ago

I'm also speaking for myself when I say I actively avoid working on this project. As much as I would absolutely love there to be a working gateway for PayPal, I don't want to work on this project because it's too much of a mess. It is in desperate need of a refactoring.

radar commented 11 years ago

I've just spent 20 minutes seeing if the PayPal developer experience is as bad as I thought it was.

TL;DR: yes.

I challenge anyone here to attempt to find out how you can get login credentials to even begin to test the express checkout feature. My favourite part was this PDF document. Go ahead, click the link. Then click the link in the next PDF document. Then read through the complete lack of useful documentation.

Whenever do you eventually find that it's at developer.paypal.com (underneath "Accounts" then "Sandbox Accounts", not "My Apps", as I originally thought), your first account creation will fail with "We're sorry but something went wrong. Please delete the account." Why was the account even created in the first place? Does reality even exist any more?

Creating another account succeeds fine. Click the tiny little arrow and then click "Profile" and wait approximately three quarters of an age for the dialog box to load. I don't know why they couldn't have this box loaded in the background already. Click "API Credentials" and then realise that the "Password" and "Signature" fields are blank. Reload the page, go through the whole process again and now MAGICALLY they're there.

Yay!

Now all you have to do is integrate with PayPal. LOL.

Go back and re-visit the Profile dialog box 5 mins later. Find that API credentials have gone missing again.

How can a company so large get things so so so wrong?

Good luck to whoever wants to take this on. It's HARD.

MatthieuBarthel commented 11 years ago

Ok, thanks to both of you to be clear and giving your opinion. I don't like paypal very much but for european countries there is very few gateways available.

cbrunsdon commented 11 years ago

We use paypal express in... THREE spree stores. We're going to have to deal with this eventually, but I don't have a timeline on it. Is there a better option out there for taking paypal payments than this extension? Or have people just been ignoring paypal because this extension is a pain?

msevestre commented 11 years ago

Hi all,

I am also using Paypal Express in various stores. While the code is really hard to understand and definitely needs some TLC , it does the job in store using spree 1-2 and 1-3 Removing PayPal support in spree is in my opinion not an option, especially for smaller shops

I am seriously considering rewriting the spree_paypal_extension from scratch. I won't have time to deal with that before September however. But then, I am definitely committed to take time and deep dive into PayPal API (Classic and/or REST)

I would really appreciate any feedback from the spree core team that could help me understand, where the design issues with the current extension are and why is this so much of a pain to work with. How would you do things differently?

Cheers, Michael

On Fri, Jul 12, 2013 at 12:35 PM, Clarke Brunsdon notifications@github.comwrote:

We use paypal express in... THREE spree stores. We're going to have to deal with this eventually, but I don't have a timeline on it. Is there a better option out there for taking paypal payments than this extension? Or have people just been ignoring paypal because this extension is a pain?

— Reply to this email directly or view it on GitHubhttps://github.com/spree/spree_paypal_express/issues/151#issuecomment-20888093 .

radar commented 11 years ago

Removing PayPal support in spree is in my opinion not an option, especially for smaller shops

I agree. In the same way, I think having an official poorly written extension that is used by a whole bunch of Spreeple is not good and that needs to change.

I am seriously considering rewriting the spree_paypal_extension from scratch. I won't have time to deal with that before September however. But then, I am definitely committed to take time and deep dive into PayPal API (Classic and/or REST)

I have already begun this process in my extension called better_spree_paypal_express, however it's pretty rough around the edges right now. The Free Running Technology guys have volunteered to test it out, and I hope I can get some more people on board for that too. Patches very welcome.

I would really appreciate any feedback from the spree core team that could help me understand, where the design issues with the current extension are and why is this so much of a pain to work with. How would you do things differently?

The main pain point imo is that there are no tests around the functionality at all and that it hacks CheckoutController. This is a terrible idea (remember the good ol' days where spree_promo used to do this too?) and I think having a completely separate controller (as is done in better_spree_paypal_express) is the correct way to go about doing things.

Writing some solid integration tests that actually walk through a PayPal checkout flow using PayPal's sandbox is the way to go here. Once we have that framework in place, we will have a nice safety net that we can leverage when things break.

cbrunsdon commented 11 years ago

Yea, honestly we don't give a crap what we use, we only care about the functionality. I'm very happy with better_spree_paypal_express having almost no code in it, relaying on activemerchant for the heavy lifting.

Its got some work to do for sure, but at this point even if Radar never makes another commit we'll look after it eventually.

That said, I haven't looked at what it would take to maintain or migrate existing payments using spree_paypal_express when we do finally switch production sites over to better_spree_paypal_express.

radar commented 11 years ago

I've switched better_spree_paypal_express to use PayPal's Merchant SDK. Can I get @cbrunsdon @gmacdougall @msevestre @MatthieuBarthel and so on to test this please?

JDutil commented 11 years ago

Thanks @radar @cbrunsdon and @msevestre for looking to pick this up from scratch. I occasionally have clients working with paypal express and it's the only time I touch this project with dread. Having a fresh solution next time around will be welcome.

MatthieuBarthel commented 11 years ago

Thank you @radar and team :) I tested in development env (Rails 3.2.13, Spree 2.0.3).

On checkout, when I am at stage 3, I select PayPal and click "save and continue", then I get the following flash alert :

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

Everything looks correct in my checkout, I have an address and a delivery method. I tried as guest and registered user, same result. Also I tried with another method (Spree::PaymentMethod::Check) which works.

Do not hesitate if I can do something else.

radar commented 11 years ago

Matthieu: click the PayPal button itself. I need to devise a way of disabling the save and continue button still.

On Fri, Jul 26, 2013 at 2:12 AM, Matthieu Barthel notifications@github.com wrote:

Thank you @radar and team :) I tested in development env (Rails 3.2.13, Spree 2.0.3). On checkout, when I am at stage 3, I select PayPal and click "save and continue", then I get the following flash alert :

Payment could not be processed, please check the details you entered Everything looks correct in my checkout, I have an address and a delivery method. I tried as guest and registered user, same result. Also I tried with another method (Spree::PaymentMethod::Check) which works.

Do not hesitate if I can do something else.

Reply to this email directly or view it on GitHub: https://github.com/spree/spree_paypal_express/issues/151#issuecomment-21565412

MatthieuBarthel commented 11 years ago

I just tried, I get the following error : NameError (uninitialized constant Spree::Gateway::PayPal::SDK): .../better_spree_paypal_express-67513f9e1704/app/models/spree/gateway/pay_pal_express.rb:16:inprovider' .../gems/better_spree_paypal_express-67513f9e1704/app/controllers/spree/paypal_controller.rb:87:in provider' .../gems/better_spree_paypal_express-67513f9e1704/app/controllers/spree/paypal_controller.rb:26:inexpress'`

radar commented 11 years ago

Please see radar/better_spree_paypal_express#7

On Fri, Jul 26, 2013 at 7:24 AM, Matthieu Barthel notifications@github.com wrote:

I just tried, I get the following error : 


.../better_spree_paypal_express-67513f9e1704/app/models/spree/gateway/pay_pal_express.rb:16:in`provider'
.../gems/better_spree_paypal_express-67513f9e1704/app/controllers/spree/paypal_controller.rb:87:in `provider'

## .../gems/better_spree_paypal_express-67513f9e1704/app/controllers/spree/paypal_controller.rb:26:in `express'```

Reply to this email directly or view it on GitHub:
https://github.com/spree/spree_paypal_express/issues/151#issuecomment-21585788