Skip to content
Merged
9 changes: 9 additions & 0 deletions Block/Checkout/Onepage/Success/AdditionalInformation.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,15 @@ public function getPaymentType()
{
return $this->paymentType;
}

/**
* @return bool
*/
protected function isUpaPayment()
{
return !empty($this->getPaymentAdditionalInformation('session_id'));
}

/**
* returns payment additional information depending on $key.
* @param string $key
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,16 @@
*
* @return string
*/
protected function _toHtml()

Check warning on line 31 in Block/Checkout/Onepage/Success/ConveniencestoreAdditionalInformation.php

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This method has 4 returns, which is more than the 3 allowed.

See more on https://sonarcloud.io/project/issues?id=omise_omise-magento&issues=AaDU7mZBjipSud772J8a&open=AaDU7mZBjipSud772J8a&pullRequest=549
{
$order = $this->_checkoutSession->getLastRealOrder();
$paymentData = $order->getPayment()->getData();
$paymentAdditionalInfo = $paymentData['additional_information'];

if (array_key_exists('session_id', $paymentAdditionalInfo) && !empty($paymentAdditionalInfo['session_id'])) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

question: ‏Can we use $this->isUpaPayment() here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No. Because this class is extending Magento\Framework\View\Element\Template. and other block class like propmptpay, tesco extending Omise\Payment\Block\Checkout\Onepage\Success\AdditionalInformation.
so not possible to use the same method here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems a bit weird that only this class that does not extend from AdditionalInformation like other classes do. However since this is the existing implementation. I'm ok to keep it like this.

return;
}

if (!array_key_exists('payment_type', $paymentAdditionalInfo)) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ class PaynowAdditionalInformation extends AdditionalInformation
*/
protected function _toHtml()
{
if ($this->isUpaPayment()) {
return;
}
if ($this->getPaymentType() !== 'paynow') {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ class PromptpayAdditionalInformation extends AdditionalInformation
*/
protected function _toHtml()
{
if ($this->isUpaPayment()) {
return;
}
if ($this->getPaymentType() !== 'promptpay') {
return;
}
Expand Down
3 changes: 3 additions & 0 deletions Block/Checkout/Onepage/Success/TescoAdditionalInformation.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ class TescoAdditionalInformation extends AdditionalInformation
*/
protected function _toHtml()
{
if ($this->isUpaPayment()) {
return;
}
if ($this->getPaymentType() !== 'bill_payment_tesco_lotus') {
return;
}
Expand Down
33 changes: 5 additions & 28 deletions Controller/Callback/UPACallback.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
use Magento\Checkout\Model\Session as CheckoutSession;
use Magento\Framework\App\Request\Http;
use Omise\Payment\Model\Api\CheckoutSession as OmiseCheckoutSession;
use Magento\Sales\Model\Order\Payment\Transaction\BuilderInterface as TransactionBuilderInterface;
use Magento\Framework\App\Action\Action;
use Magento\Sales\Model\Order;
use Magento\Sales\Model\Order\Payment\Transaction;
Expand Down Expand Up @@ -71,11 +70,6 @@
*/
protected $omiseCheckoutSession;

/**
* @var TransactionBuilderInterface
*/
protected $transactionBuilder;

/**
* @param Context $context
* @param Session $session
Expand All @@ -87,7 +81,6 @@
* @param CheckoutSession $checkoutSession
* @param Http $request
* @param OmiseCheckoutSession $omiseCheckoutSession
* @param TransactionBuilderInterface $transactionBuilder
*/
public function __construct(
Context $context,
Expand All @@ -99,8 +92,7 @@
Config $config,
CheckoutSession $checkoutSession,
Http $request,
OmiseCheckoutSession $omiseCheckoutSession,
TransactionBuilderInterface $transactionBuilder
OmiseCheckoutSession $omiseCheckoutSession
) {
parent::__construct($context);
$this->session = $session;
Expand All @@ -112,7 +104,6 @@
$this->checkoutSession = $checkoutSession;
$this->request = $request;
$this->omiseCheckoutSession = $omiseCheckoutSession;
$this->transactionBuilder = $transactionBuilder;
$this->omise->defineUserAgent();
$this->omise->defineApiVersion();
$this->omise->defineApiKeys();
Expand Down Expand Up @@ -140,8 +131,8 @@
$checkoutSession = $this->getCheckoutSession($payment);
$sessionPayments = $checkoutSession->payments;

if($checkoutSession && !is_array($sessionPayments) || empty($sessionPayments)) {
if ($checkoutSession && !is_array($sessionPayments) || empty($sessionPayments)) {
$errorMessage = __('The payment session is invalid or no payment information was found. Please contact our support if you have any questions.');

Check warning on line 135 in Controller/Callback/UPACallback.php

View workflow job for this annotation

GitHub Actions / M2 Coding Standard

Line exceeds 120 characters; contains 160 characters
return $this->redirectBackToCart($order, $errorMessage);
}

Expand All @@ -152,8 +143,8 @@
$chargeId = $finalPayment['charge_id'];
$charge = $this->charge->find($chargeId);
} else {
$errorMessage = __('The payment session is invalid or no payment information was found. Please contact our support if you have any questions.');

Check warning on line 146 in Controller/Callback/UPACallback.php

View workflow job for this annotation

GitHub Actions / M2 Coding Standard

Line exceeds 120 characters; contains 160 characters
return $this->redirectBackToCart($order,$errorMessage);
return $this->redirectBackToCart($order, $errorMessage);
}

if (!$charge instanceof \Omise\Payment\Model\Api\BaseObject) {
Expand All @@ -173,27 +164,13 @@

// Do not proceed if webhook is enabled
if ($this->config->isWebhookEnabled()) {
$this->transactionBuilder
->setPayment($payment)
->setOrder($order)
->setTransactionId($charge->id)
->setAdditionalInformation([
Transaction::RAW_DETAILS => [
'omise_charge_id' => $charge->id,
'status' => $charge->status,
'charge_id' => $charge->id
]
])
->setFailSafe(true)
->build(Transaction::TYPE_PAYMENT);
$order->save();
return $this->redirect(self::PATH_SUCCESS);
Comment thread
rosle marked this conversation as resolved.
}

$payment->setTransactionId($charge->id);
$payment->setLastTransId($charge->id);
$payment->setAdditionalInformation('charge_id', $charge->id);

if ($charge->isSuccessful()) {
return $this->handleSuccess($order, $charge, $payment);
}
Expand Down
24 changes: 4 additions & 20 deletions Gateway/Response/UPAPaymentDetailsHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,21 @@

use Magento\Payment\Gateway\Helper\SubjectReader;
use Magento\Payment\Gateway\Response\HandlerInterface;
use Magento\Sales\Model\Order\Payment\Transaction;
use Omise\Payment\Helper\OmiseHelper;

class UPAPaymentDetailsHandler implements HandlerInterface
{
/**
* @var \Magento\Sales\Model\Order\Payment\Transaction\BuilderInterface
*/
protected $transactionBuilder;

/**
* @var OmiseHelper
*/
protected $helper;

/**
* @param Transaction\BuilderInterface $transactionBuilder
* @param OmiseHelper $helper
*/
public function __construct(
\Magento\Sales\Model\Order\Payment\Transaction\BuilderInterface $transactionBuilder,
OmiseHelper $helper
) {
$this->transactionBuilder = $transactionBuilder;
$this->helper = $helper;
}

Expand All @@ -48,19 +39,12 @@ public function handle(array $handlingSubject, array $response)
$payment->setAdditionalInformation('session_id', $response['session']->id);
$payment->setAdditionalInformation('payment_type', $paymentType);

$transaction = $this->transactionBuilder
->setPayment($payment)
->setOrder($order)
->setTransactionId($response['session']->id)
->setAdditionalInformation([Transaction::RAW_DETAILS => (array) $payment])
->setFailSafe(true)
->build(Transaction::TYPE_PAYMENT);
$payment->addTransactionCommentsToOrder(
$transaction,
$order->addStatusHistoryComment(
$payment->prependMessage(
__(
'Processing amount of %1 via Omise Checkout Gateway.',
$order->getBaseCurrency()->formatTxt($order->getTotalDue())
'Processing amount of %1 via Omise Checkout session ID: %2',
$order->getBaseCurrency()->formatTxt($order->getTotalDue()),
$response['session']->id
)
)
);
Expand Down
47 changes: 1 addition & 46 deletions Test/Unit/Controller/Callback/UPACallbackTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
use Magento\Sales\Model\Order;
use Magento\Sales\Model\Order\Payment;
use Magento\Sales\Model\Order\Payment\Transaction;
use Magento\Sales\Model\Order\Payment\Transaction\BuilderInterface;
use Omise\Payment\Controller\Callback\UPACallback;
use Omise\Payment\Helper\OmiseEmailHelper;
use Omise\Payment\Helper\OmiseHelper;
Expand All @@ -35,7 +34,6 @@ class UPACallbackTest extends TestCase
private $checkoutSession;
private $request;
private $omiseCheckoutSession;
private $transactionBuilder;
private $messageManager;
private const ORDER_ID = 1;
private const SESSION_ID = 'session_123';
Expand All @@ -60,7 +58,6 @@ protected function setUp(): void
$this->omiseCheckoutSession = $this->createMock(
\Omise\Payment\Model\Api\CheckoutSession::class
);
$this->transactionBuilder = $this->createMock(BuilderInterface::class);

$this->messageManager = $this->createMock(ManagerInterface::class);

Expand All @@ -86,8 +83,7 @@ private function getController()
$this->config,
$this->checkoutSession,
$this->request,
$this->omiseCheckoutSession,
$this->transactionBuilder
$this->omiseCheckoutSession
]
)
->onlyMethods(['_redirect', 'getRequest'])
Expand Down Expand Up @@ -706,7 +702,6 @@ public function testExecuteWithWebhookEnabledBuildsTransactionAndRedirectsToSucc
{
$payment = $this->createPayment(self::SESSION_ID);
$order = $this->createOrder($payment, self::ORDER_ID, true, Order::STATE_PENDING_PAYMENT);
$transaction = $this->createMock(Transaction::class);

$this->session->method('getLastRealOrder')
->willReturn($order);
Expand Down Expand Up @@ -751,46 +746,6 @@ public function testExecuteWithWebhookEnabledBuildsTransactionAndRedirectsToSucc
$this->config->method('isWebhookEnabled')
->willReturn(true);

$this->transactionBuilder->expects($this->once())
->method('setPayment')
->with($payment)
->willReturnSelf();

$this->transactionBuilder->expects($this->once())
->method('setOrder')
->with($order)
->willReturnSelf();

$this->transactionBuilder->expects($this->once())
->method('setTransactionId')
->with(self::CHARGE_ID)
->willReturnSelf();

$this->transactionBuilder->expects($this->once())
->method('setAdditionalInformation')
->with([
Transaction::RAW_DETAILS => [
'omise_charge_id' => self::CHARGE_ID,
'status' => 'pending',
'charge_id' => self::CHARGE_ID
]
])
->willReturnSelf();

$this->transactionBuilder->expects($this->once())
->method('setFailSafe')
->with(true)
->willReturnSelf();

$this->transactionBuilder->expects($this->once())
->method('build')
->with(Transaction::TYPE_PAYMENT)
->willReturn($transaction);

$order->expects($this->once())
->method('save')
->willReturnSelf();

$controller = $this->getController();

$redirectResult = $this->createMock(Redirect::class);
Expand Down
Loading
Loading