-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
Cancel payment action #33
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Answear\Payum\PayU\Action; | ||
|
||
use Answear\Payum\PayU\Enum\OrderStatus; | ||
use Answear\Payum\PayU\Model\Model; | ||
use Answear\Payum\PayU\Request\OrderRequestService; | ||
use Answear\Payum\PayU\Util\PaymentHelper; | ||
use Payum\Core\Action\ActionInterface; | ||
use Payum\Core\Exception\RequestNotSupportedException; | ||
use Payum\Core\Model\PaymentInterface; | ||
use Payum\Core\Request\Cancel; | ||
use Psr\Log\LoggerInterface; | ||
use Webmozart\Assert\Assert; | ||
|
||
class CancelAction implements ActionInterface | ||
{ | ||
private const FINAL_STATUSES = [ | ||
OrderStatus::Completed, | ||
OrderStatus::Canceled, | ||
]; | ||
|
||
public function __construct( | ||
private OrderRequestService $orderRequestService, | ||
private LoggerInterface $logger | ||
) { | ||
} | ||
|
||
/** | ||
* @param Cancel $request | ||
*/ | ||
public function execute($request): void | ||
{ | ||
RequestNotSupportedException::assertSupports($this, $request); | ||
|
||
$model = Model::ensureArrayObject($request->getModel()); | ||
|
||
$payment = PaymentHelper::ensurePayment($request->getFirstModel()); | ||
Assert::notNull($payment, 'Payment must be set on cancel action.'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, PHPStan didn't report anything :( |
||
$orderId = PaymentHelper::getOrderId($model, $payment); | ||
Assert::notEmpty($orderId, 'OrderId must be set on cancel action.'); | ||
|
||
try { | ||
if (!$this->canCancelPayment($model, $payment)) { | ||
return; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we can't cancel Payment we nned to know about it. Throw custom exception here like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
} | ||
|
||
$this->orderRequestService->cancel($model->orderId(), PaymentHelper::getConfigKey($model, $payment)); | ||
} catch (\Throwable $throwable) { | ||
$this->logger->critical('Cannot cancel payment.', ['exception' => $throwable]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can't catch whole exceptions and only log it, we need to know if something happened. Throw custom exception There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed the |
||
} | ||
} | ||
|
||
public function supports($request): bool | ||
{ | ||
return | ||
$request instanceof Cancel | ||
&& $request->getModel() instanceof \ArrayAccess | ||
&& $request->getFirstModel() instanceof PaymentInterface; | ||
} | ||
|
||
private function canCancelPayment(Model $model, PaymentInterface $payment): bool | ||
{ | ||
$response = $this->orderRequestService->retrieve($model->orderId(), PaymentHelper::getConfigKey($model, $payment)); | ||
$status = $response->orders[0]->status; | ||
|
||
return !in_array($status, self::FINAL_STATUSES, true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. create There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have some doubts about checking this on CancelAction method. Maybe we need to check it before run CancelAction but this action should proceed only cancel request 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Api always returns a status code of 200, regardless of whether the payment was successfully canceled or not. For example, when you send a transaction cancellation request for a transaction already paid, you get a status of 200, but after checking the status it is still paid. For this reason, I would prefer to check the status before sending the cancellation and throw a custom exception. |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Answear\Payum\PayU\ValueObject\Response; | ||
|
||
class OrderCanceledResponse | ||
{ | ||
public function __construct( | ||
public readonly ResponseStatus $status, | ||
public readonly string $orderId, | ||
public readonly ?string $extOrderId = null | ||
) { | ||
} | ||
|
||
public static function fromResponse(array $response): self | ||
{ | ||
return new self( | ||
ResponseStatus::fromResponse($response['status']), | ||
$response['orderId'], | ||
$response['extOrderId'] ?? null | ||
); | ||
} | ||
|
||
public function toArray(): array | ||
{ | ||
return [ | ||
'status' => $this->status->toArray(), | ||
'orderId' => $this->orderId, | ||
'extOrderId' => $this->extOrderId, | ||
]; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Answear\Payum\PayU\Tests\Integration\Action; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this test seems like the unit test, not integration |
||
|
||
use Answear\Payum\PayU\Action\CancelAction; | ||
use Answear\Payum\PayU\Request\OrderRequestService; | ||
use Answear\Payum\PayU\Tests\Payment; | ||
use Answear\Payum\PayU\Tests\Util\FileTestUtil; | ||
use Answear\Payum\PayU\ValueObject\Response\OrderCanceledResponse; | ||
use Answear\Payum\PayU\ValueObject\Response\OrderRetrieveResponse; | ||
use Payum\Core\Request\Cancel; | ||
use PHPUnit\Framework\TestCase; | ||
use Psr\Log\LoggerInterface; | ||
|
||
class CancelActionTest extends TestCase | ||
{ | ||
/** | ||
* @test | ||
*/ | ||
public function successTest(): void | ||
{ | ||
$action = $this->getCancelAction( | ||
OrderCanceledResponse::fromResponse( | ||
FileTestUtil::decodeJsonFromFile( | ||
__DIR__ . '/../Request/data/orderCanceledResponse.json' | ||
) | ||
), | ||
OrderRetrieveResponse::fromResponse( | ||
FileTestUtil::decodeJsonFromFile( | ||
__DIR__ . '/../Request/data/retrieveOrderResponse.json' | ||
) | ||
) | ||
); | ||
|
||
$payment = new Payment(); | ||
$payment->setDetails(FileTestUtil::decodeJsonFromFile(__DIR__ . '/data/detailsWithOrderId.json')); | ||
$expectedBaseDetails = $this->getExpectedBaseDetails(); | ||
self::assertSame($expectedBaseDetails, $payment->getDetails()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you just set the details. We need to check it again? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is not needed, removed |
||
|
||
$request = new Cancel($payment); | ||
$request->setModel($payment->getDetails()); | ||
|
||
$action->execute($request); | ||
} | ||
|
||
private function getCancelAction( | ||
OrderCanceledResponse $orderCanceledResponse, | ||
OrderRetrieveResponse $retrieveOrderResponse | ||
): CancelAction { | ||
$orderRequestService = $this->createMock(OrderRequestService::class); | ||
$orderRequestService->method('retrieve') | ||
->willReturn($retrieveOrderResponse); | ||
|
||
$orderRequestService->method('cancel') | ||
->willReturn($orderCanceledResponse); | ||
|
||
$logger = $this->createMock(LoggerInterface::class); | ||
$logger->expects(self::never()) | ||
->method('critical'); | ||
|
||
return new CancelAction($orderRequestService, $logger); | ||
} | ||
|
||
private function getExpectedBaseDetails(): array | ||
{ | ||
return [ | ||
'totalAmount' => 95500, | ||
'firstName' => 'Testy', | ||
'lastName' => 'Mjzykdwmh', | ||
'description' => 'Platnost za objednávku č.: 221214-0026UJ-CZ', | ||
'currencyCode' => 'CZK', | ||
'language' => 'cs', | ||
'validityTime' => 259200, | ||
'buyer' => [ | ||
'email' => '[email protected]', | ||
'firstName' => 'Testy', | ||
'lastName' => 'Mjzykdwmh', | ||
'phone' => '+420733999019', | ||
'language' => 'cs', | ||
], | ||
'extOrderId' => '221214-0026UJ-CZ', | ||
'client_email' => '[email protected]', | ||
'client_id' => '124077', | ||
'customerIp' => '10.0.13.152', | ||
'creditCardMaskedNumber' => null, | ||
'status' => 'PENDING', | ||
'payUResponse' => [ | ||
'status' => [ | ||
'statusCode' => 'SUCCESS', | ||
], | ||
'redirectUri' => 'https://merch-prod.snd.payu.com/pay/?orderId=3MRW8ST2Z6221214GUEST000P01&token=eyJhbGciOiJIUzI1NiJ9.eyJvcmRlcklkIjoiM01SVzhTVDJaNjIyMTIxNEdVRVNUMDAwUDAxIiwicG9zSWQiOiI5eWJZVWFZOSIsImF1dGhvcml0aWVzIjpbIlJPTEVfQ0xJRU5UIl0sInBheWVyRW1haWwiOiJ0ZXN0eS5hdXRvbWF0eWN6bmUrZGVjMTQyMDIyMjMyODUzNDgwMDIzQGFuc3dlYXIuY29tIiwiZXhwIjoxNjcxMzE2Mjk1LCJpc3MiOiJQQVlVIiwiYXVkIjoiYXBpLWdhdGV3YXkiLCJzdWIiOiJQYXlVIHN1YmplY3QiLCJqdGkiOiI3NmYyOGZkMi1jOWIxLTRiYzAtOTM5Zi0xNjQ5NjY0ZWNlZDMifQ.NpmBZw0vQP7WEWQEd-ZhXoyg8oo_eKy8gEyfAjri21g', | ||
'orderId' => '3MRW8ST2Z6221214GUEST000P01', | ||
'extOrderId' => '221214-0026UJ-CZ', | ||
], | ||
'orderId' => '3MRW8ST2Z6221214GUEST000P01', | ||
]; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
"status": { | ||
"statusCode": "SUCCESS", | ||
"statusDesc": "Request processing successful" | ||
}, | ||
"orderId": "WZHF5FFDRJ140731GUEST000P01", | ||
"extOrderId": "extOrderId123" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
{ | ||
"orders": [ | ||
{ | ||
"orderId": "WZHF5FFDRJ140731GUEST000P01", | ||
"extOrderId": "358766", | ||
"orderCreateDate": "2014-10-27T14:58:17.443+01:00", | ||
"notifyUrl": "http://localhost/OrderNotify/", | ||
"customerIp": "127.0.0.1", | ||
"merchantPosId": "145227", | ||
"description": "New order", | ||
"currencyCode": "PLN", | ||
"totalAmount": "3200", | ||
"buyer":{ | ||
"email":"[email protected]", | ||
"phone":"111111111", | ||
"firstName":"John", | ||
"lastName":"Doe", | ||
"language":"pl" | ||
}, | ||
"status": "NEW", | ||
"products": [ | ||
{ | ||
"name": "Product1", | ||
"unitPrice": "1000", | ||
"quantity": "1" | ||
}, | ||
{ | ||
"name": "Product2", | ||
"unitPrice": "2200", | ||
"quantity": "1" | ||
} | ||
] | ||
} | ||
], | ||
"status": { | ||
"statusCode": "SUCCESS", | ||
"statusDesc": "Request processing successful" | ||
}, | ||
"properties": [ | ||
{ | ||
"name": "PAYMENT_ID", | ||
"value": "1234567890" | ||
} | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Answear\Payum\PayU\Tests\Integration\Request; | ||
|
||
use Answear\Payum\PayU\Enum\ResponseStatusCode; | ||
use Answear\Payum\PayU\Exception\PayUAuthorizationException; | ||
use Answear\Payum\PayU\Request\OrderRequestService; | ||
use Answear\Payum\PayU\Tests\Util\FileTestUtil; | ||
use Answear\Payum\PayU\ValueObject\Response\ResponseStatus; | ||
use GuzzleHttp\Psr7\Response; | ||
use Psr\Log\NullLogger; | ||
|
||
class CancelOrderTest extends AbstractRequestTestCase | ||
{ | ||
/** | ||
* @test | ||
*/ | ||
public function createTest(): void | ||
{ | ||
$this->mockGuzzleResponse( | ||
new Response(200, [], FileTestUtil::getFileContents(__DIR__ . '/data/orderCanceledResponse.json')) | ||
); | ||
|
||
$orderId = 'WZHF5FFDRJ140731GUEST000P01'; | ||
$response = $this->getOrderRequestService()->cancel($orderId, null); | ||
|
||
self::assertEquals( | ||
new ResponseStatus( | ||
ResponseStatusCode::Success, | ||
'Request processing successful' | ||
), | ||
$response->status | ||
); | ||
self::assertSame($orderId, $response->orderId); | ||
self::assertSame('extOrderId123', $response->extOrderId); | ||
} | ||
|
||
/** | ||
* @test | ||
*/ | ||
public function createUnauthorizedTest(): void | ||
{ | ||
$this->mockGuzzleResponse( | ||
new Response(401, [], FileTestUtil::getFileContents(__DIR__ . '/data/orderUnauthorizedResponse.json')) | ||
); | ||
|
||
$this->expectException(PayUAuthorizationException::class); | ||
$this->expectExceptionCode(401); | ||
$this->getOrderRequestService()->cancel('WZHF5FFDRJ140731GUEST000P01', null); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. test with 400-500 code from PayU is missing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added missing tests |
||
|
||
private function getOrderRequestService(): OrderRequestService | ||
{ | ||
return new OrderRequestService( | ||
$this->getConfigProvider(), | ||
$this->getClient(), | ||
new NullLogger() | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
"status": { | ||
"statusCode": "SUCCESS", | ||
"statusDesc": "Request processing successful" | ||
}, | ||
"orderId": "WZHF5FFDRJ140731GUEST000P01", | ||
"extOrderId": "extOrderId123" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move it to OrderStatus class,
or create a public method like
OrderStatus->isFinal()