Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[payum][paypal] add suppor of instant notifactions. #725

Merged
merged 6 commits into from
Jan 19, 2014
Merged

[payum][paypal] add suppor of instant notifactions. #725

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c7e6838
[payum][paypal] add suppor of instant notifactions.
makasim Dec 18, 2013
9f2d1cd
fix comments.
makasim Jan 14, 2014
c60fd42
fix notify order action class.
makasim Jan 16, 2014
3b2750d
fix sync request not supported.
makasim Jan 17, 2014
5e28b37
[paypal] add specs for notify action.
makasim Jan 18, 2014
41d9d92
fix spec fails on php 5.3
makasim Jan 19, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/config/payum.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ payum:
sandbox: %paypal.express_checkout.sandbox%
actions:
- sylius.payum.paypal.action.capture_order_using_express_checkout
- sylius.payum.paypal.action.notify_order
- sylius.payum.action.order_status
- sylius.payum.action.execute_same_request_with_payment_details

Expand Down
3 changes: 3 additions & 0 deletions app/config/routing.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ sylius_web:
payum_capture:
resource: "@PayumBundle/Resources/config/routing/capture.xml"

payum_notify:
resource: "@PayumBundle/Resources/config/routing/notify.xml"

fos_user_security:
resource: "@FOSUserBundle/Resources/config/routing/security.xml"

Expand Down
18 changes: 18 additions & 0 deletions src/Sylius/Bundle/PayumBundle/Payum/Paypal/Action/CaptureOrderUsingExpressCheckoutAction.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,27 @@

namespace Sylius\Bundle\PayumBundle\Payum\Paypal\Action;

use Payum\Bundle\PayumBundle\Security\TokenFactory;
use Payum\Core\Action\PaymentAwareAction;
use Payum\Core\Exception\RequestNotSupportedException;
use Payum\Core\Request\SecuredCaptureRequest;
use Sylius\Bundle\CoreBundle\Model\OrderInterface;

class CaptureOrderUsingExpressCheckoutAction extends PaymentAwareAction
{
/**
* @var TokenFactory
*/
protected $tokenFactory;

/**
* @param TokenFactory $tokenFactory
*/
public function __construct(TokenFactory $tokenFactory)
{
$this->tokenFactory = $tokenFactory;
}

/**
* {@inheritDoc}
*/
Expand All @@ -36,6 +50,10 @@ public function execute($request)
if (empty($details)) {
$details['RETURNURL'] = $request->getToken()->getTargetUrl();
$details['CANCELURL'] = $request->getToken()->getTargetUrl();
$details['NOTIFYURL'] = $this->tokenFactory->createNotifyToken(
$request->getToken()->getPaymentName(),
$order
);
$details['INVNUM'] = $order->getNumber();

$details['PAYMENTREQUEST_0_CURRENCYCODE'] = $order->getCurrency();
Expand Down
83 changes: 83 additions & 0 deletions src/Sylius/Bundle/PayumBundle/Payum/Paypal/Action/NotifyOrderAction.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
<?php

/*
* This file is part of the Sylius package.
*
* (c) Paweł Jędrzejewski
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Sylius\Bundle\PayumBundle\Payum\Paypal\Action;

use Payum\Core\Action\PaymentAwareAction;
use Payum\Core\Exception\RequestNotSupportedException;
use Payum\Core\Request\SecuredNotifyRequest;
use Payum\Core\Request\SyncRequest;
use Sylius\Bundle\CoreBundle\Model\OrderInterface;
use Sylius\Bundle\PaymentsBundle\SyliusPaymentEvents;
use Sylius\Bundle\PayumBundle\Payum\Request\StatusRequest;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\EventDispatcher\GenericEvent;

class NotifyOrderAction extends PaymentAwareAction
{
/**
* @var EventDispatcherInterface
*/
protected $eventDispatcher;

/**
* @param EventDispatcherInterface $eventDispatcher
*/
public function __construct(EventDispatcherInterface $eventDispatcher)
{
$this->eventDispatcher = $eventDispatcher;
}

/**
* {@inheritDoc}
*/
public function execute($request)
{
/** @var $request SecuredNotifyRequest */
if (!$this->supports($request)) {
throw RequestNotSupportedException::createActionNotSupported($this, $request);
}

/** @var OrderInterface $order */
$order = $request->getModel();
$payment = $order->getPayment();
$previousState = $payment->getState();

$this->payment->execute(new SyncRequest($order));

$status = new StatusRequest($order);
$this->payment->execute($status);
$payment->setState($status->getStatus());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be done between the 2 dispatched events (pre and post), right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually not, The pre and post events expect to get new state from payment object where the previous one passed as an event parameter

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant: what is the point of dispatching 2 events with exact same parameters? (the $order and $previousState variables don't vary)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here it mean 'just to be compatible". Pre event means before we actually saved anything to db, so you can change state to another one, post event means we cannot change anything in payment object.

Since the save will be done later (when we return from this action) we cannot do save between pre and post without injecting extra dependency. To keep things simple I decided to dispatch events so you can change state in post event too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So maybe just dispatch the "pre" event then, to avoid any confusion
Le 15 janv. 2014 15:58, "Maksim Kotlyar" notifications@github.com a écrit
:

In src/Sylius/Bundle/PayumBundle/Payum/Paypal/Action/NotifyOrderAction.php:

  • {
  •    /*\* @var $request SecuredNotifyRequest */

  •    if (!$this->supports($request)) {

  •        throw RequestNotSupportedException::createActionNotSupported($this, $request);

  •    }

  •    /*\* @var OrderInterface $order */

  •    $order = $request->getModel();

  •    $payment = $order->getPayment();

  •    $previousState = $payment->getState();

  •    $this->payment->execute(new SyncRequest($order));

  •    $status = new StatusRequest($order);

  •    $this->payment->execute($status);

  •    $payment->setState($status->getStatus());

here it mean 'just to be compatible". Pre event means before we actually
saved anything to db, so you can change state to another one, post event
means we cannot change anything in payment object.

Since the save will be done later (when we return from this action) we
cannot do save between pre and post without injecting extra dependency. To
keep things simple I decided to dispatch events so you can change state in
post event too.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/725/files#r8888660
.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@winzou I dont think its a good idea, One can have a listener subscribed to post event (let's say it sends an email to admin) and he would be surprised to see its not working. IMO it is better to dispatch both.


if ($previousState !== $payment->getState()) {
$this->eventDispatcher->dispatch(
SyliusPaymentEvents::PRE_STATE_CHANGE,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question: is there already a listener listening this event and which actually confirms the order (send shipments, etc.)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont aware of any, @pjedrzejewski do you?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@winzou There is none. I assume you mean inventory updates only when order is paid, hm?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pjedrzejewski Yes. We really need to clean all these events (between checkout_*, order_create*, etc.) and make it clear of which one is doing what, etc. It's a bit messy right now!

new GenericEvent($order->getPayment(), array('previous_state' => $previousState))
);

$this->eventDispatcher->dispatch(
SyliusPaymentEvents::POST_STATE_CHANGE,
new GenericEvent($order->getPayment(), array('previous_state' => $previousState))
);
}
}

/**
* {@inheritDoc}
*/
public function supports($request)
{
return
$request instanceof SecuredNotifyRequest &&
$request->getModel() instanceof OrderInterface
;
}
}
8 changes: 7 additions & 1 deletion src/Sylius/Bundle/PayumBundle/Resources/config/services.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
<parameter key="sylius.payum.checkout_step.purchase.class">Sylius\Bundle\PayumBundle\Checkout\Step\PurchaseStep</parameter>
<parameter key="sylius.payum.paypal.action.capture_order_using_express_checkout.class">Sylius\Bundle\PayumBundle\Payum\Paypal\Action\CaptureOrderUsingExpressCheckoutAction</parameter>
<parameter key="sylius.payum.stripe.action.capture_order_using_credit_card.class">Sylius\Bundle\PayumBundle\Payum\Stripe\Action\CaptureOrderUsingCreditCardAction</parameter>
<parameter key="sylius.payum.paypal.action.notify_order.class">Sylius\Bundle\PayumBundle\Payum\Stripe\Action\NotifyOrderAction</parameter>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@makasim Should be Paypal\Action\NotifyOrderAction, not Stripe :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kayue indeed!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

<parameter key="sylius.payum.be2bill.action.capture_order_using_credit_card.class">Sylius\Bundle\PayumBundle\Payum\Be2bill\Action\CaptureOrderUsingCreditCardAction</parameter>
<parameter key="sylius.payum.dummy.action.capture_order.class">Sylius\Bundle\PayumBundle\Payum\Dummy\Action\CaptureOrderAction</parameter>
<parameter key="sylius.payum.dummy.action.order_status.class">Sylius\Bundle\PayumBundle\Payum\Dummy\Action\OrderStatusAction</parameter>
Expand All @@ -35,7 +36,12 @@
</call>
<tag name="sylius.process.step" alias="sylius_checkout_purchase" />
</service>
<service id="sylius.payum.paypal.action.capture_order_using_express_checkout" class="%sylius.payum.paypal.action.capture_order_using_express_checkout.class%" />
<service id="sylius.payum.paypal.action.capture_order_using_express_checkout" class="%sylius.payum.paypal.action.capture_order_using_express_checkout.class%">
<argument type="service" id="payum.security.token_factory" />
</service>
<service id="sylius.payum.paypal.action.notify_order" class="%sylius.payum.paypal.action.notify_order.class%">
<argument type="service" id="event_dispatcher" />
</service>
<service id="sylius.payum.stripe.action.capture_order_using_credit_card" class="%sylius.payum.stripe.action.capture_order_using_credit_card.class%" />
<service id="sylius.payum.be2bill.action.capture_order_using_credit_card" class="%sylius.payum.be2bill.action.capture_order_using_credit_card.class%">
<call method="setRequest">
Expand Down