diff --git a/CHANGELOG.md b/CHANGELOG.md index 114a9a8d..d0f66825 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## [1.1.3] - 2023-??-?? + +- [0007526](https://bugs.oxid-esales.com/view.php?id=7526) Order would be saved only, if everything is correct. In all other cases redirect to checkout +- [0007509](https://bugs.oxid-esales.com/view.php?id=7509) Order would be saved only, if everything is correct. In all other cases redirect to checkout +- [0007524](https://bugs.oxid-esales.com/view.php?id=7524) catch Error if Unzer-API not working and redirect to Checkout +- [0007527](https://bugs.oxid-esales.com/view.php?id=7527) prevent clicking the buy-now-button several times + ## [1.1.2] - 2023-08-18 ### FIXED diff --git a/metadata.php b/metadata.php index c6a3e7e2..c65b894e 100644 --- a/metadata.php +++ b/metadata.php @@ -51,7 +51,7 @@ ', ], 'thumbnail' => 'logo.svg', - 'version' => '1.1.2', + 'version' => '1.1.3-rc.1', 'author' => 'OXID eSales AG', 'url' => 'https://www.oxid-esales.com', 'email' => 'info@oxid-esales.com', @@ -61,7 +61,6 @@ \OxidEsales\Eshop\Core\Config::class => Config::class, \OxidEsales\Eshop\Application\Model\Payment::class => Payment::class, \OxidEsales\Eshop\Application\Controller\OrderController::class => OrderController::class, - \OxidEsales\Eshop\Application\Model\PaymentGateway::class => PaymentGateway::class, \OxidEsales\Eshop\Application\Model\Order::class => Order::class, \OxidEsales\Eshop\Core\ShopControl::class => ShopControl::class, \OxidEsales\Eshop\Application\Controller\Admin\ModuleConfiguration::class => ModuleConfiguration::class, diff --git a/src/Controller/DispatcherController.php b/src/Controller/DispatcherController.php index 8ff52acf..4374bf8d 100644 --- a/src/Controller/DispatcherController.php +++ b/src/Controller/DispatcherController.php @@ -8,6 +8,7 @@ namespace OxidSolutionCatalysts\Unzer\Controller; use Exception; +use JsonException; use OxidEsales\Eshop\Application\Controller\FrontendController; use OxidEsales\Eshop\Application\Model\Order; use OxidEsales\Eshop\Core\Exception\DatabaseConnectionException; @@ -36,6 +37,7 @@ class DispatcherController extends FrontendController * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) * @SuppressWarnings(PHPMD.ElseExpression) + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function updatePaymentTransStatus(): void { @@ -44,9 +46,18 @@ public function updatePaymentTransStatus(): void /** @var string $jsonRequest */ $jsonRequest = file_get_contents('php://input'); - /** @var array $aJson */ - $aJson = json_decode($jsonRequest, true, 512, JSON_THROW_ON_ERROR); - /** @var array $url */ + $aJson = []; + + try { + /** @var array $aJson */ + $aJson = json_decode($jsonRequest, true, 512, JSON_THROW_ON_ERROR); + if (!count($aJson)) { + throw new JsonException('Invalid Json'); + } + } catch (JsonException $e) { + Registry::getUtils()->showMessageAndExit("Invalid Json"); + } + $url = parse_url($aJson['retrieveUrl']); /** @var Transaction $transaction */ $transaction = $this->getServiceFromContainer(Transaction::class); @@ -64,7 +75,11 @@ public function updatePaymentTransStatus(): void } if ( - ($url['scheme'] !== "https" || ($url['host'] !== "api.unzer.com" && $url['host'] !== "sbx-api.heidelpay.com")) + $url['scheme'] !== "https" || + ( + $url['host'] !== "api.unzer.com" && + $url['host'] !== "sbx-api.heidelpay.com" + ) ) { Registry::getUtils()->showMessageAndExit("No valid retrieveUrl"); } diff --git a/src/Controller/InstallmentController.php b/src/Controller/InstallmentController.php index 267d06ba..a1588c1f 100644 --- a/src/Controller/InstallmentController.php +++ b/src/Controller/InstallmentController.php @@ -37,7 +37,7 @@ class InstallmentController extends FrontendController /** @var Payment $uzrPayment */ protected $uzrPayment; - /** @var \OxidSolutionCatalysts\Unzer\Model\Payment $oxPayment */ + /** @var PaymentModel $oxPayment */ protected $oxPayment; /** diff --git a/src/Controller/OrderController.php b/src/Controller/OrderController.php index 0d136711..2583b716 100644 --- a/src/Controller/OrderController.php +++ b/src/Controller/OrderController.php @@ -9,13 +9,14 @@ use OxidEsales\Eshop\Application\Model\Country; use OxidEsales\Eshop\Application\Model\Order; -use OxidEsales\Eshop\Core\Exception\ArticleInputException; -use OxidEsales\Eshop\Core\Exception\NoArticleException; -use OxidEsales\Eshop\Core\Exception\OutOfStockException; +use OxidEsales\Eshop\Core\DatabaseProvider; +use OxidEsales\Eshop\Core\Exception\DatabaseErrorException; use OxidEsales\Eshop\Core\Registry; use OxidSolutionCatalysts\Unzer\Exception\Redirect; +use OxidSolutionCatalysts\Unzer\Exception\RedirectWithMessage; use OxidSolutionCatalysts\Unzer\Model\Payment; use OxidSolutionCatalysts\Unzer\Service\ModuleSettings; +use OxidSolutionCatalysts\Unzer\Service\Payment as PaymentService; use OxidSolutionCatalysts\Unzer\Service\ResponseHandler; use OxidSolutionCatalysts\Unzer\Service\Translator; use OxidSolutionCatalysts\Unzer\Service\Unzer; @@ -67,10 +68,6 @@ public function render() */ public function execute() { - if (!$this->isSepaConfirmed()) { - return ''; - } - $ret = parent::execute(); if ($ret && str_starts_with($ret, 'thankyou')) { @@ -87,10 +84,6 @@ public function execute() $response->setData([ 'redirectUrl' => $unzer->prepareRedirectUrl('thankyou') ])->sendJson(); - } elseif ($this->isSepaPayment()) { - /** @var \OxidSolutionCatalysts\Unzer\Model\Order $order */ - $order = $this->getActualOrder(); - $order->markUnzerOrderAsPaid(); } return $ret; @@ -98,6 +91,9 @@ public function execute() /** * @throws Redirect + * @throws DatabaseErrorException + * @SuppressWarnings(PHPMD.StaticAccess) + * @SuppressWarnings(PHPMD.ElseExpression) */ public function unzerExecuteAfterRedirect(): void { @@ -105,27 +101,35 @@ public function unzerExecuteAfterRedirect(): void $oUser = $this->getUser(); $oBasket = $this->getSession()->getBasket(); if ($oBasket->getProductsCount()) { - try { - /** @var \OxidSolutionCatalysts\Unzer\Model\Order $oOrder */ - $oOrder = $this->getActualOrder(); + $oDB = DatabaseProvider::getDb(); - //finalizing ordering process (validating, storing order into DB, executing payment, setting status ...) - $iSuccess = (int)$oOrder->finalizeUnzerOrderAfterRedirect($oBasket, $oUser); + /** @var \OxidSolutionCatalysts\Unzer\Model\Order $oOrder */ + $oOrder = $this->getActualOrder(); - // performing special actions after user finishes order (assignment to special user groups) - $oUser->onOrderExecute($oBasket, $iSuccess); + $oDB->startTransaction(); - $nextStep = $this->_getNextStep($iSuccess); + //finalizing ordering process (validating, storing order into DB, executing payment, setting status ...) + $iSuccess = (int)$oOrder->finalizeUnzerOrderAfterRedirect($oBasket, $oUser); - // proceeding to next view - $unzerService = $this->getServiceFromContainer(Unzer::class); + // performing special actions after user finishes order (assignment to special user groups) + $oUser->onOrderExecute($oBasket, $iSuccess); + + $nextStep = $this->_getNextStep($iSuccess); + + // commit transaction and proceeding to next view + $unzerService = $this->getServiceFromContainer(Unzer::class); + + if ('thankyou' === $nextStep) { + $oDB->commitTransaction(); throw new Redirect($unzerService->prepareRedirectUrl($nextStep)); - } catch (OutOfStockException $oEx) { - $oEx->setDestination('basket'); - Registry::getUtilsView()->addErrorToDisplay($oEx, false, true, 'basket'); - } catch (NoArticleException | ArticleInputException $oEx) { - Registry::getUtilsView()->addErrorToDisplay($oEx); } + + $oDB->rollbackTransaction(); + $translator = $this->getServiceFromContainer(Translator::class); + throw new RedirectWithMessage( + $unzerService->prepareRedirectUrl($nextStep), + $translator->translate('OSCUNZER_ERROR_DURING_CHECKOUT') + ); } } @@ -247,4 +251,47 @@ public function getUnzerCompanyTypes(): array } return $this->companyTypes; } + + /** + * execute Unzer defined via getExecuteFnc + */ + public function executeoscunzer(): ?string + { + if (!$this->isSepaConfirmed()) { + return null; + } + + if (!$this->_validateTermsAndConditions()) { + $this->_blConfirmAGBError = 1; + return null; + } + + $paymentService = $this->getServiceFromContainer(PaymentService::class); + /** @var \OxidEsales\Eshop\Application\Model\Payment $payment */ + $payment = $this->getPayment(); + $paymentOk = $paymentService->executeUnzerPayment($payment); + + // all orders without redirect would be finalized now + if ($paymentOk) { + $this->unzerExecuteAfterRedirect(); + } + + return null; + } + + /** + * OXID-Core + * @inheritDoc + */ + public function getExecuteFnc() + { + /** @var Payment $payment */ + $payment = $this->getPayment(); + if ( + $payment->isUnzerPayment() + ) { + return 'executeoscunzer'; + } + return parent::getExecuteFnc(); + } } diff --git a/src/Model/Order.php b/src/Model/Order.php index d6a6b310..a4bd9f09 100644 --- a/src/Model/Order.php +++ b/src/Model/Order.php @@ -23,9 +23,6 @@ class Order extends Order_parent { use ServiceContainer; - /** @var bool $isRedirectOrder */ - protected $isRedirectOrder = false; - /** * @param Basket $oBasket * @param User $oUser @@ -38,27 +35,44 @@ public function finalizeUnzerOrderAfterRedirect( Basket $oBasket, User $oUser ) { - $this->isRedirectOrder = true; + $orderId = Registry::getSession()->getVariable('sess_challenge'); + $orderId = is_string($orderId) ? $orderId : ''; + $iRet = self::ORDER_STATE_PAYMENTERROR; + + if (!$orderId) { + return $iRet; + } + $this->setId($orderId); $unzerPaymentStatus = $this->getServiceFromContainer(PaymentService::class)->getUnzerPaymentStatus(); - if ($unzerPaymentStatus != "ERROR") { + if ($unzerPaymentStatus !== "ERROR") { + // copies user info + $this->_setUser($oUser); + + // copies basket info + $this->_loadFromBasket($oBasket); + + $oUserPayment = $this->_setPayment($oBasket->getPaymentId()); + + // set folder information, order is new + $this->_setFolder(); + + //saving all order data to DB + $this->save(); + if (!$this->getFieldData('oxordernr')) { $this->_setNumber(); } - // else { - // oxNew(\OxidEsales\Eshop\Core\Counter::class) - // ->update($this->_getCounterIdent(), $this->oxorder__oxordernr->value); - //} // deleting remark info only when order is finished - \OxidEsales\Eshop\Core\Registry::getSession()->deleteVariable('ordrem'); + Registry::getSession()->deleteVariable('ordrem'); //#4005: Order creation time is not updated when order processing is complete $this->_updateOrderDate(); // store orderid - $oBasket->setOrderId($this->getId()); + $oBasket->setOrderId($orderId); // updating wish lists $this->_updateWishlist($oBasket->getContents(), $oUser); @@ -69,9 +83,7 @@ public function finalizeUnzerOrderAfterRedirect( // marking vouchers as used and sets them to $this->_aVoucherList (will be used in order email) $this->_markVouchers($oBasket, $oUser); - $oUserPayment = $this->_setPayment($oBasket->getPaymentId()); // send order by email to shop owner and current user - // don't let order fail due to stock check while sending out the order mail Registry::getSession()->setVariable('blDontCheckProductStockForUnzerMails', true); $iRet = $this->_sendOrderByEmail($oUser, $oBasket, $oUserPayment); @@ -79,7 +91,7 @@ public function finalizeUnzerOrderAfterRedirect( $this->_setOrderStatus($unzerPaymentStatus); - if ($unzerPaymentStatus == 'OK') { + if ($unzerPaymentStatus === 'OK') { $this->markUnzerOrderAsPaid(); } @@ -87,7 +99,7 @@ public function finalizeUnzerOrderAfterRedirect( } else { // payment is canceled $this->delete(); - $iRet = self::ORDER_STATE_PAYMENTERROR; + } return $iRet; @@ -101,7 +113,7 @@ public function markUnzerOrderAsPaid(): void { /** @var string $oxpaid */ $oxpaid = $this->getFieldData('oxpaid'); - if ($oxpaid == '0000-00-00 00:00:00') { + if ($oxpaid === '0000-00-00 00:00:00' || is_null($oxpaid)) { $utilsDate = Registry::getUtilsDate(); $date = date('Y-m-d H:i:s', $utilsDate->getTime()); $this->setFieldData('oxpaid', $date); @@ -118,18 +130,6 @@ public function setUnzerTransId(string $sTransId): void $this->save(); } - /** - * @inheritDoc - */ - protected function _checkOrderExist($sOxId = null): bool // phpcs:ignore PSR2.Methods.MethodDeclaration.Underscore - { - if ($this->isRedirectOrder) { - return false; - } - - return parent::_checkOrderExist($sOxId); - } - /** * @param \UnzerSDK\Resources\Payment|null $unzerPayment * @return bool diff --git a/src/Model/PaymentGateway.php b/src/Model/PaymentGateway.php deleted file mode 100644 index be30e7ad..00000000 --- a/src/Model/PaymentGateway.php +++ /dev/null @@ -1,36 +0,0 @@ -getFieldData('oxpaymenttype'); - $oPayment = oxNew(Payment::class); - if ($oPayment->load($oxpaymenttype)) { - if ($oPayment->isUnzerPayment()) { - $paymentService = $this->getServiceFromContainer(PaymentService::class); - $paymentService->executeUnzerPayment($oPayment); - } - } - - return parent::executePayment($dAmount, $oOrder); - } -} diff --git a/src/Service/Payment.php b/src/Service/Payment.php index a8e2e64a..4d4053de 100644 --- a/src/Service/Payment.php +++ b/src/Service/Payment.php @@ -99,6 +99,7 @@ public function __construct( */ public function executeUnzerPayment(PaymentModel $paymentModel): bool { + $paymentExtension = null; try { /** @var string $customerType */ $customerType = Registry::getRequest()->getRequestParameter('unzer_customer_type', ''); @@ -117,14 +118,6 @@ public function executeUnzerPayment(PaymentModel $paymentModel): bool $basket ); - /** @var string $sess_challenge */ - $sess_challenge = $this->session->getVariable('sess_challenge'); - $this->transactionService->writeTransactionToDB( - $sess_challenge, - $this->session->getUser()->getId(), - $this->getSessionUnzerPayment() - ); - $paymentStatus = $this->getUnzerPaymentStatus() !== self::STATUS_ERROR; if ($this->redirectUrl) { @@ -137,8 +130,6 @@ public function executeUnzerPayment(PaymentModel $paymentModel): bool } catch (Redirect $e) { throw $e; } catch (UnzerApiException $e) { - $this->removeTemporaryOrder(); - throw new RedirectWithMessage( $this->unzerService->prepareOrderRedirectUrl( $paymentExtension instanceof AbstractUnzerPayment && $paymentExtension->redirectUrlNeedPending() @@ -146,8 +137,6 @@ public function executeUnzerPayment(PaymentModel $paymentModel): bool $this->translator->translateCode($e->getErrorId(), $e->getClientMessage()) ); } catch (Exception $e) { - $this->removeTemporaryOrder(); - throw new RedirectWithMessage( $this->unzerService->prepareOrderRedirectUrl( $paymentExtension instanceof AbstractUnzerPayment && $paymentExtension->redirectUrlNeedPending() @@ -178,8 +167,12 @@ public function getUnzerPaymentStatus(): string { $result = self::STATUS_ERROR; - /** @var \UnzerSDK\Resources\Payment $sessionUnzerPayment */ $sessionUnzerPayment = $this->getSessionUnzerPayment(); + if (is_null($sessionUnzerPayment)) { + return $result; + } + + /** @var \UnzerSDK\Resources\Payment $sessionUnzerPayment */ $transaction = $sessionUnzerPayment->getInitialTransaction(); if ($sessionUnzerPayment->isCompleted()) { diff --git a/src/Service/Transaction.php b/src/Service/Transaction.php index 5e2aac09..b52f10c6 100644 --- a/src/Service/Transaction.php +++ b/src/Service/Transaction.php @@ -116,7 +116,8 @@ public function writeTransactionToDB( $this->deleteInitOrder($params); // Fallback: set ShortID as OXTRANSID - $oOrder->setUnzerTransId($params['shortid']); + $shortId = $params['shortid'] ?? ''; + $oOrder->setUnzerTransId($shortId); return true; } @@ -349,7 +350,7 @@ protected function getUnzerPaymentData(Payment $unzerPayment): array /** @var AbstractTransactionType $initialTransaction */ $initialTransaction = $unzerPayment->getInitialTransaction(); - $params['shortid'] = $initialTransaction->getShortId() !== null ? + $params['shortid'] = !is_null($initialTransaction) && !is_null($initialTransaction->getShortId()) ? $initialTransaction->getShortId() : Registry::getSession()->getVariable('ShortId'); diff --git a/translations/de/oscunzer_lang.php b/translations/de/oscunzer_lang.php index 8cc40105..8792c9ef 100644 --- a/translations/de/oscunzer_lang.php +++ b/translations/de/oscunzer_lang.php @@ -13,6 +13,9 @@ $aLang = [ 'charset' => 'UTF-8', + // Error + 'OSCUNZER_ERROR_DURING_CHECKOUT' => 'Bei der Abwicklung der Zahlung ist ein Fehler aufgetreten. Der Prozess wurde rückgängig gemacht. Bitte wählen Sie alternativ eine andere Zahlart aus.', + // Invoice 'OSCUNZER_BANK_DETAILS_AMOUNT' => 'Bitte überweisen sie den Betrag von %s %s auf folgendes Bankkonto:

', 'OSCUNZER_BANK_DETAILS_HOLDER' => 'Kontoinhaber: %s
', diff --git a/translations/en/oscunzer_lang.php b/translations/en/oscunzer_lang.php index d4b022c6..8bc19894 100644 --- a/translations/en/oscunzer_lang.php +++ b/translations/en/oscunzer_lang.php @@ -13,6 +13,9 @@ $aLang = [ 'charset' => 'UTF-8', + // Error + 'OSCUNZER_ERROR_DURING_CHECKOUT' => 'An error occurred while processing the payment. The process was reversed. Alternatively, please select another payment method.', + //Invoice 'OSCUNZER_BANK_DETAILS_AMOUNT' => 'Please transfer the amount of %s %s to the following account:

', 'OSCUNZER_BANK_DETAILS_HOLDER' => 'Holder: %s
',