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

Jump to bottom

make Orderprocess more stable #194

Merged
merged 14 commits into from
Sep 12, 2023
Merged

make Orderprocess more stable #194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
2f2059b
WIP: use getExecuteFnc
mariolorenz Sep 5, 2023
78e48a1
use more functions from finalizeOrder
mariolorenz Sep 7, 2023
6e69fe9
remove unecessary try catch
mariolorenz Sep 8, 2023
836a61e
set folder before save the order
mariolorenz Sep 8, 2023
c406b08
glue executeUnzerPayment2 and executeUnzerPayment togehter
mariolorenz Sep 8, 2023
ad552ba
fix codestyle, PHPMD, PHPStan
mariolorenz Sep 8, 2023
dedfc16
fix Unit-Error
mariolorenz Sep 8, 2023
136914d
fix missing Payment-IDs that run into Maintenance
mariolorenz Sep 8, 2023
1884a7d
CHANGELOG
mariolorenz Sep 8, 2023
2b95a28
move SepaValidation the executeoscunzer
mariolorenz Sep 8, 2023
6d9beba
null is now also possible
mariolorenz Sep 8, 2023
04308c9
add error-message if rollbackTransaction
mariolorenz Sep 12, 2023
981dfd4
Merge remote-tracking branch 'origin/b-6.3.x' into UNZER-225_preventZ…
mariolorenz Sep 12, 2023
f8a9aa1
CHANGELOG
mariolorenz Sep 12, 2023
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
7 changes: 7 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions metadata.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
</ul>',
],
'thumbnail' => 'logo.svg',
'version' => '1.1.2',
'version' => '1.1.3-rc.1',
'author' => 'OXID eSales AG',
'url' => 'https://www.oxid-esales.com',
'email' => '[email protected]',
Expand All @@ -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,
Expand Down
23 changes: 19 additions & 4 deletions src/Controller/DispatcherController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -36,6 +37,7 @@
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
* @SuppressWarnings(PHPMD.NPathComplexity)
* @SuppressWarnings(PHPMD.ElseExpression)
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*/
public function updatePaymentTransStatus(): void
{
Expand All @@ -44,13 +46,22 @@
/** @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);
$aPath = explode("/", $url['path']);

Check failure on line 64 in src/Controller/DispatcherController.php

View workflow job for this annotation

GitHub Actions / styles (8.0) 

Cannot access offset 'path' on array{scheme?: string, host?: string, port?: int<0, 65535>, user?: string, pass?: string, path?: string, query?: string, fragment?: string}|false.

Check failure on line 64 in src/Controller/DispatcherController.php

View workflow job for this annotation

GitHub Actions / styles (8.0) 

Cannot access offset 'path' on array{scheme?: string, host?: string, port?: int<0, 65535>, user?: string, pass?: string, path?: string, query?: string, fragment?: string}|false.
$typeid = end($aPath);

/** @var Request $request */
Expand All @@ -64,7 +75,11 @@
}

if (
($url['scheme'] !== "https" || ($url['host'] !== "api.unzer.com" && $url['host'] !== "sbx-api.heidelpay.com"))
$url['scheme'] !== "https" ||

Check failure on line 78 in src/Controller/DispatcherController.php

View workflow job for this annotation

GitHub Actions / styles (8.0) 

Cannot access offset 'scheme' on array{scheme?: string, host?: string, port?: int<0, 65535>, user?: string, pass?: string, path?: string, query?: string, fragment?: string}|false.

Check failure on line 78 in src/Controller/DispatcherController.php

View workflow job for this annotation

GitHub Actions / styles (8.0) 

Cannot access offset 'scheme' on array{scheme?: string, host?: string, port?: int<0, 65535>, user?: string, pass?: string, path?: string, query?: string, fragment?: string}|false.
(
$url['host'] !== "api.unzer.com" &&

Check failure on line 80 in src/Controller/DispatcherController.php

View workflow job for this annotation

GitHub Actions / styles (8.0) 

Offset 'host' does not exist on array{scheme: 'https', host?: string, port?: int<0, 65535>, user?: string, pass?: string, path?: string, query?: string, fragment?: string}.

Check failure on line 80 in src/Controller/DispatcherController.php

View workflow job for this annotation

GitHub Actions / styles (8.0) 

Offset 'host' does not exist on array{scheme: 'https', host?: string, port?: int<0, 65535>, user?: string, pass?: string, path?: string, query?: string, fragment?: string}.
$url['host'] !== "sbx-api.heidelpay.com"
)
) {
Registry::getUtils()->showMessageAndExit("No valid retrieveUrl");
}
Expand Down
2 changes: 1 addition & 1 deletion src/Controller/InstallmentController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class InstallmentController extends FrontendController
/** @var Payment $uzrPayment */
protected $uzrPayment;

/** @var \OxidSolutionCatalysts\Unzer\Model\Payment $oxPayment */
/** @var PaymentModel $oxPayment */
protected $oxPayment;

/**
Expand Down
99 changes: 73 additions & 26 deletions src/Controller/OrderController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -67,10 +68,6 @@
*/
public function execute()
{
if (!$this->isSepaConfirmed()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

warum entfernst du diesen code, im ticket kann ich auf den ersten blick nicht über sepa finden

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alle Unzer-Bestellungen gehen über die neue "executeoscunzer". Da ist auch der SEPA-Check mit drin.

Einzig Apple-Pay nutzt aus meiner Sicht noch die "execute". Das kann ich leider nur nicht richtig testen ...

return '';
}

$ret = parent::execute();

if ($ret && str_starts_with($ret, 'thankyou')) {
Expand All @@ -87,45 +84,52 @@
$response->setData([
'redirectUrl' => $unzer->prepareRedirectUrl('thankyou')
])->sendJson();
} elseif ($this->isSepaPayment()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

siehe oben:
warum entfernst du diesen code, im ticket kann ich auf den ersten blick nicht über sepa finden

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

s.o.

/** @var \OxidSolutionCatalysts\Unzer\Model\Order $order */
$order = $this->getActualOrder();
$order->markUnzerOrderAsPaid();
}

return $ret;
}

/**
* @throws Redirect
* @throws DatabaseErrorException
* @SuppressWarnings(PHPMD.StaticAccess)
* @SuppressWarnings(PHPMD.ElseExpression)
*/
public function unzerExecuteAfterRedirect(): void
{
// get basket contents
$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')
);
}
}

Expand Down Expand Up @@ -247,4 +251,47 @@
}
return $this->companyTypes;
}

/**
* execute Unzer defined via getExecuteFnc
*/
public function executeoscunzer(): ?string
Copy link
Contributor

Choose a reason for hiding this comment

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

fände camel case lesbarer

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm. Das ist eine Methode die als fnc gestartet wird. Im Core werden diese Methoden alle in "klein" geschrieben. Möglicherweise ist es egal. Ich habe mich hier an den Core gehalten. Könnte ich aber ausprobieren ...

{
if (!$this->isSepaConfirmed()) {
return null;
}

if (!$this->_validateTermsAndConditions()) {
$this->_blConfirmAGBError = 1;

Check failure on line 265 in src/Controller/OrderController.php

View workflow job for this annotation

GitHub Actions / styles (8.0) 

Property OxidEsales\EshopCommunity\Application\Controller\OrderController::$_blConfirmAGBError (bool) does not accept int.

Check failure on line 265 in src/Controller/OrderController.php

View workflow job for this annotation

GitHub Actions / styles (8.0) 

Property OxidEsales\EshopCommunity\Application\Controller\OrderController::$_blConfirmAGBError (bool) does not accept int.
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();
}
}
56 changes: 28 additions & 28 deletions src/Model/Order.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@
{
use ServiceContainer;

/** @var bool $isRedirectOrder */
protected $isRedirectOrder = false;

/**
* @param Basket $oBasket
* @param User $oUser
Expand All @@ -38,27 +35,44 @@
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);
Expand All @@ -69,25 +83,23 @@
// 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);
Registry::getSession()->deleteVariable('blDontCheckProductStockForUnzerMails');

$this->_setOrderStatus($unzerPaymentStatus);

if ($unzerPaymentStatus == 'OK') {
if ($unzerPaymentStatus === 'OK') {
$this->markUnzerOrderAsPaid();
}

$this->initWriteTransactionToDB();
} else {
// payment is canceled
$this->delete();
$iRet = self::ORDER_STATE_PAYMENTERROR;

}

return $iRet;
Expand All @@ -101,7 +113,7 @@
{
/** @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)) {

Check failure on line 116 in src/Model/Order.php

View workflow job for this annotation

GitHub Actions / styles (8.0) 

Call to function is_null() with string will always evaluate to false.

Check failure on line 116 in src/Model/Order.php

View workflow job for this annotation

GitHub Actions / styles (8.0) 

Call to function is_null() with string will always evaluate to false.
$utilsDate = Registry::getUtilsDate();
$date = date('Y-m-d H:i:s', $utilsDate->getTime());
$this->setFieldData('oxpaid', $date);
Expand All @@ -118,18 +130,6 @@
$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
Expand Down
36 changes: 0 additions & 36 deletions src/Model/PaymentGateway.php
View file
Open in desktop

This file was deleted.

Loading
Loading