degica / spree_komoju

Spree extension for using the Komoju Payments API.
https://komoju.com/
BSD 3-Clause "New" or "Revised" License
6 stars 11 forks source link

Add source views and forms to spree admin #32

Closed rramsden closed 8 years ago

rramsden commented 8 years ago

This pull requests adds missing source views for CreditCard, Konbini, BankTransfer, WebMoney, and PayEasy. I've managed to recycle the checkout forms used when creating payments in the spree admin dashboard.

A few bugs I found and fixed when adding these forms:

rramsden commented 8 years ago

@shioyama Think you could take a look? This just fixes the views in spree admin. There are a few other bugs related to "capturing" and "voiding" payments which I'll look into now.

shioyama commented 8 years ago

@camelmasa's going to have a look at it.

camelmasa commented 8 years ago

@rramsden > This just fixes the views in spree admin

I confirmed that. Yes, that was fixed now. Please continue to investigate for fix other bugs.

camelmasa commented 8 years ago

@rramsden I checked that failed payment's prepaid number will be blank.

spree_ __

Related those bugs :question: I don't know that.

rramsden commented 8 years ago

@camelmasa is that a bug or a feature? haha

shioyama commented 8 years ago

I'm getting an error about a missing method with_public_profile in Spree::Konbini on the new payments page, maybe my setup is different, just posting here for now:

latest-screenshot

shioyama commented 8 years ago

I'm not really sure maybe our setups are different, but when I started working on this I had to mess around with the reusable_sources method which is called in the payment form here. I'm not sure how it would render without doing something about that because reusable_sources calls sources_by_order which assumes there is a scope with_payment_profile on the source (which does not exist for alternative payment methods). I had to add something there to get this view to render correctly for me.

Maybe my setup was different, are you testing with enable_customer_profiles on or off? I'll keep digging.

shioyama commented 8 years ago

So I find that if I add these scopes + method to all Komoju payment methods (other than credit card) the issue is gone:

scope :with_payment_profile, -> {}
scope :default, -> {}

def reusable_sources
  []
end

And actually about the latter method Spree mentions in a comment in the code that this should be overridden for custom gateways, so this makes sense, but I still don't understand how you guys didn't see it. Maybe our versions of spree or something are different? I'm running 3-0-stable (from the github branch), maybe that's the difference.

shioyama commented 8 years ago

Confirmed that I see the error with 3.0.4 as well, fixed with the method added.

shioyama commented 8 years ago

I also find that clicking "capture" on say a konbini payment does try to go to the gateway to capture it, but since there is no capture method on the activemerchant gateway it errors and falls through to capture(stream) in activesupport.

But actually I also see that when I create the konbini payment from the admin it does create it on Komoju, see below:

latest-screenshot

I can't capture it because of the issue above but that should just be a matter of aliasing capture to purchase or something in the konbini gateway. Not sure why we're seeing different things...

shioyama commented 8 years ago

So adding a method capture to the am gateway allows you to capture a konbini payment that has been authorized on komoju:

def capture(money, payment, options = {})
  Response.new(true, "force captured")
end

Of course this does not mark the payment as captured on Komoju because we have no endpoint for doing that (normally it would only be captured when the customer actually pays, and in this case nobody has actually paid). So not sure if we actually want to allow this, but it does seem to work for me as expected.

shioyama commented 8 years ago

Actually I think we can just return false from can_capture? for all asynchronous methods (konbini, bank transfer, payeasy), since you should not be able to capture them manually from spree anywhere (just through the callback, which uses payment.complete! which is not affected by can_capture?). That way the capture buttons will not show for those payment methods and we don't have to worry about it.

We can also get rid of the void buttons for these payment methods by changing can_void? to just return false always.

rramsden commented 8 years ago

@shioyama thanks I was also thinking of just returning can_capture? since we can't capture directly from spree (they should be captured in Komoju). I will also set can_void? to false since out gateway doesn't support authorization/voiding.

shioyama commented 8 years ago

@rramsden actually also just redefining actions to remove those should do it I think.

rramsden commented 8 years ago

@shioyama I've removed void and capture actions from alternative payments

shioyama commented 8 years ago

Thanks!