[WIP] Sylius payum integration #275

makasim · 2013-08-19T19:52:43Z

The PR attempt to integration payum into sylius e-commerce. Currently it contains two examples. First is offsite payment (paypal express checkout was used) and second credit card payment (stripe via omnipay bridge was used).

Integration

I briefly describe integration steps required. Check Files Changes for more details.

Add payum's lib to composer.json and install them
Update PaymentMethods. Remove all exist and add Stripe and Payal EC.
Add PurchaseStep to checkout scenario.
Configure payum

Order <-> Payment details relation problem

We should definitely discuss the order <-> payment details relation problem.

Clarification:

Payum does not provide unified payment model in any way. All of them payment specific, do not share any interfaces or parent classes. For paypal rest situation even better: we reuse their official model that comes with paypal/rest-api-sdk. The bridge for jms plugings works with its PaymentInstruction model. Omnipay does not provide any model yet.

How I solve it in customers projects:

We have Order to PaymentDetails realtion (Order is owning side). Let's say one-to-one. PaymentDetails model implements class table inheritance pattern. There should be a way to dynamiclly add new classes to the map. I think it is possible with metadata doctrine event. I used this strategy in several projects: works fine but I doubt it is the best solution for the e-comerrace platform.

Solution:

I added details property to Paymnet model (and its iterface). The details contains simple array. This way we do not need any extra models and can reuse current code. Payum native and omnipay's gateways have to work fine with this solution where jms plugins not.

Moving payum related code to a bundle

What about moving all payum related stuff to payment bundle? or even better create one SyliusPayumBundle. It will contains all integration details.

Solution:

Moved.

Credit card processing questions?

Where to ask for credit card (if it is required)? It could be asked on payment method selection method (after user choose one) or inside CaptureOrderWithXXXAction.

Note: Since it is proof of concept everybody are welcome to discuss\criticize\ask about this PR. I am looking forward to get your feedback.

Solution:

It is asked on the separate page only if payment require it.

TODO (maybe not in this PR (: )

Fallback payments
Simplify storages configurations
Offline payments
Allow set custom status for dummy payments.
Design for credit card page.

cordoval · 2013-08-19T21:29:59Z

src/Sylius/Bundle/CoreBundle/Checkout/Step/FinalizeStep.php

@@ -37,15 +38,19 @@ public function displayAction(ProcessContextInterface $context)
     */
    public function forwardAction(ProcessContextInterface $context)
    {
+        /** @var Order $order */


you can do this on the docblock of createOrder and not here

I agree with you that createOrder is a better place for that.

cordoval · 2013-09-04T13:01:39Z

src/Sylius/Bundle/CoreBundle/Checkout/CheckoutProcessScenario.php

@@ -52,6 +52,7 @@ public function build(ProcessBuilderInterface $builder)
            ->add('shipping', 'sylius_checkout_shipping')
            ->add('payment', 'sylius_checkout_payment')
            ->add('finalize', 'sylius_checkout_finalize')
+            ->add('do_payment', 'sylius_checkout_do_payment')


i want to like the name however there must be some better name maybe

@cordoval you are welcome to propose one.

something with capture or complete purchase?

weird to have something after finalize... maybe the others names should also be renamed

@jjanvier yes I am completely agree with you. I am thinking of such renaming

finalize to approve

payment to choose_payment

do_payment to payment or purchase.

@makasim perfect for me, cc/ @pjedrzejewski

chugas · 2013-09-17T16:08:35Z

I am testing the integration of payum with paypal but I am having some troubles. I am going to tell you the changes I had to make and the issue that have me stucked.

First of all, I have changed the following lines of the DoPaymentStep.php file:

1- Line 23, function displayAction
$captureToken = $this->getPayumTokenManager()->createTokenForCaptureRoute
by
$captureToken = $this->getTokenFactory()->createCaptureToken

2- Line 38, function forwardAction
$token = $this->getPayumTokenManager()->getTokenFromRequest($context->getRequest());
by
$token = $this->getTokenFactory()->getTokenFromRequest($context->getRequest());

3- Change the function
protected function getPayumTokenManager()
{
return $this->get('payum.token_manager');
}
by
protected function getTokenFactory()
{
return $this->get('payum.security.token_factory');
}

When the process finishes and when I create the order I get this error for the relative url "/payment/capture/{token}":
Request Payum\Request\SecuredCaptureRequest is not supported.

I am attaching an image.

Thank you very much

makasim · 2013-09-17T18:48:55Z

@chugas I did some BC breaks in 0.6 version, you know it is not stable so I can break it (:

I am going to fix it right now.

makasim · 2013-09-17T20:00:18Z

@chugas Should work fine except one place: Currently relation between payment details and order is not establish. So the payment always endup with unknown status. I suppose you change it to doctrine relation or something similar. In this case I expect everything should work fine.

Thanks for your feedback

makasim · 2013-09-17T20:00:52Z

Also I am thinking of moving some code from CoreBundle to PaymentBundle. IMO it would be a better place.

jjanvier · 2013-09-17T20:22:51Z

@makasim which part of the CoreBundle would you like to move ?

makasim · 2013-09-17T20:38:42Z

@makasim which part of the CoreBundle would you like to move ?

the one I added and which related to payum integration. Sylius\CoreBundle\Payum\Action\OrderStatusAction for example.

jeremyFreeAgent · 2013-09-27T12:17:54Z

Any news about that?

makasim · 2013-09-27T12:24:39Z

@jeremyFreeAgent I have lots of ideas and plans how to move it forward but I decided to wait for some feedback from sylius team side. Unfortunately any news so far.

pjedrzejewski · 2013-10-15T09:32:31Z

@stloyd Yeah, or maybe we just move it outside main app. (like it was before) At some point maybe someone/other org would like to adopt it, because we're not going to use it in Sylius apparently. One repo less to maintain is always good.

pjedrzejewski · 2013-10-15T11:03:06Z

PR requires a rebase, @maksim need a hand? :)

makasim · 2013-10-15T11:13:42Z

@pjedrzejewski hands are always welcome, I can do something only tomorrow, no time left for today.

makasim · 2013-10-15T13:44:05Z

rebased&squashed

makasim · 2013-10-17T08:27:36Z

@pjedrzejewski

I'm not sure I like saving stuff to app/Resources/payments - I'm pretty sure Payum provides db storage. Possible?

fixed - formapro-forks@aa8e12c

makasim · 2013-10-17T08:57:25Z

@pjedrzejewski what is left? I am gonna write some specs today\tomorrow what else should I do?

makasim · 2013-10-17T10:56:47Z

rebased&squashed

pjedrzejewski · 2013-10-18T12:26:24Z

My new scenarios for inventory update during checkout are failing, but I'll fix this with another PR. Thank you for your amazing work Maskim, I think it's great start and big step forward. There are some little issues, but we'll iterate on the payments just like with everything else. More in the blog post today, I'm writing it right now. Also I'll register the payum bundle on packagist and update my subtree split script.

Thanks to everyone involved in this PR! Awesome work. 👍

[WIP] Sylius payum integration

makasim · 2013-10-18T12:28:57Z

@pjedrzejewski oh my... good news before weekends

jjanvier · 2013-10-18T12:31:01Z

congratulations @makasim :)

norberttech · 2013-10-18T12:33:38Z

Time for a 🍺 ! Congratz @makasim

jeremyFreeAgent · 2013-10-18T12:34:48Z

👍

arnolanglade · 2013-10-18T12:35:13Z

👍 @makasim !

amenophis · 2013-10-18T12:43:07Z

@makasim 👍 too ! Great work !!

Jérémy LEHERPEUR
jeremy@leherpeur.net

2013/10/18 Arnaud Langlade notifications@github.com

[image: 👍]@makasim https://github.com/makasim !

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/275#issuecomment-26592030
.

pentarim · 2013-10-18T13:07:12Z

thanks & congratz

antonioperic · 2013-10-18T13:12:56Z

Good job guys

pjedrzejewski · 2013-10-18T13:36:16Z

SyliusPayumBundle is available as standalone package on packagist.org - https://packagist.org/packages/sylius/payum-bundle.

winzou · 2013-10-18T13:51:48Z

Awesome, great job guys!

Richtermeister · 2013-10-18T15:55:43Z

Awesome! Congratz & great job!

lughino · 2013-10-27T16:29:37Z

Hello,
excellent job of integration!
But I have a problem:
I state that I installed a complete project Sylvius.
When I make a payment with PayPal, the payment remains in "pending".
Then it is normal paypal, once payment has been made, redirects you to the homepage?

I can customize this behavior?

makasim · 2013-10-27T18:26:10Z

When I make a payment with PayPal, the payment remains in "pending".

could you check pending_reason in the payment details (details column in the sylius_payments table). It most probably set to multi-currency. If so you have different currencies set for sale account and buyer. Paypal does not convert currencies automatically and waiting for sales to make a decision.

Let me know if you have other then this reason.

Then it is normal paypal, once payment has been made, redirects you to the homepage?

yes it is expected behavior. You have to see a flash message on the home page. Right now you can customize it only extending PurchaseStep.

I have plans to add an event there to make such tasks easier to do.

lughino · 2013-10-27T19:09:12Z

Thanks, that was exactly what the problem!

2013/10/27 Maksim Kotlyar notifications@github.com

When I make a payment with PayPal, the payment remains in "pending".

could you check pending_reason in the payment details (details column in
the sylius_payments table). It most probably set to multi-currency. If so
you have different currencies set for sale account and buyer. Let me know
if you have other then this reason.

Then it is normal paypal, once payment has been made, redirects you to the
homepage?

yes it is expected behavior. You have to see a flash message on the home
page. Right now you can customize it only extending PurchaseStephttps://github.com/Sylius/Sylius/blob/master/src/Sylius/Bundle/PayumBundle/Checkout/Step/PurchaseStep.php
.

I have plans to add an event there to make such tasks easier to do.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/275#issuecomment-27175600
.

Update taxable_interface.rst

cordoval reviewed Aug 19, 2013
View reviewed changes

This was referenced Sep 4, 2013

Payment Integration #210

Closed

[RFC] Payment Integration #112

Closed

cordoval reviewed Sep 4, 2013
View reviewed changes

stloyd mentioned this pull request Oct 16, 2013

[RFC] Drop OmnipayBundle from main repository and use it as standalone #422

Closed

[PoC] Payum integration with paypal and stripe examples

5540995

pjedrzejewski pushed a commit that referenced this pull request Oct 18, 2013

Merge pull request #275 from formapro/sylius-payum-integration

73726b9

[WIP] Sylius payum integration

pjedrzejewski merged commit 73726b9 into Sylius:master Oct 18, 2013

This was referenced Oct 19, 2013

Issue showing the description on paypal checkout Payum/PayumBundle#52

Closed

Installing omnipay-bundle #283

Closed

makasim referenced this pull request in Payum/Payum Nov 13, 2013

[security] introduce sensitve value object.

3691fe5

pamil pushed a commit to pamil/Sylius that referenced this pull request Mar 21, 2016

Merge pull request Sylius#275 from uNknownMark/patch-1

3435d7d

Update taxable_interface.rst

[WIP] Sylius payum integration #275

[WIP] Sylius payum integration #275

Conversation

makasim commented Aug 19, 2013

Integration

Order <-> Payment details relation problem

Moving payum related code to a bundle

Credit card processing questions?

TODO (maybe not in this PR (: )

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chugas commented Sep 17, 2013

makasim commented Sep 17, 2013

makasim commented Sep 17, 2013

makasim commented Sep 17, 2013

jjanvier commented Sep 17, 2013

makasim commented Sep 17, 2013

jeremyFreeAgent commented Sep 27, 2013

makasim commented Sep 27, 2013

pjedrzejewski commented Oct 15, 2013

pjedrzejewski commented Oct 15, 2013

makasim commented Oct 15, 2013

makasim commented Oct 15, 2013

makasim commented Oct 17, 2013

makasim commented Oct 17, 2013

makasim commented Oct 17, 2013

pjedrzejewski commented Oct 18, 2013

makasim commented Oct 18, 2013

jjanvier commented Oct 18, 2013

norberttech commented Oct 18, 2013

jeremyFreeAgent commented Oct 18, 2013

arnolanglade commented Oct 18, 2013

amenophis commented Oct 18, 2013

pentarim commented Oct 18, 2013

antonioperic commented Oct 18, 2013

pjedrzejewski commented Oct 18, 2013

winzou commented Oct 18, 2013

Richtermeister commented Oct 18, 2013

lughino commented Oct 27, 2013

makasim commented Oct 27, 2013

lughino commented Oct 27, 2013