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

Fixed #26345 Reorder in Admin panel leads to report page in case of changed product custom option max characters #26348

Merged
merged 7 commits into from
Mar 17, 2020
Merged

Fixed #26345 Reorder in Admin panel leads to report page in case of changed product custom option max characters #26348

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
16079d9
Update Reorder.php
cedmudit Jan 10, 2020
86dc12d
Merge branch '2.4-develop' of http://github.com/magento/magento2 into…
engcom-Echo Feb 6, 2020
a21b246
Cover unit test
engcom-Echo Feb 6, 2020
43f9e2b
Add implement HttpPostActionInterface for renderer controller
engcom-Echo Feb 6, 2020
7ba11b8
Merge branch '2.4-develop' of http://github.com/magento/magento2 into…
engcom-Echo Feb 20, 2020
6c5fbcb
Changed implements class
engcom-Echo Feb 20, 2020
f7819a6
Added an error logging
engcom-Echo Mar 12, 2020
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
48 changes: 35 additions & 13 deletions app/code/Magento/Sales/Controller/Adminhtml/Order/Create/Reorder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,28 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\Sales\Controller\Adminhtml\Order\Create;

use Magento\Backend\App\Action;
use Magento\Backend\Model\View\Result\Forward;
use Magento\Backend\Model\View\Result\ForwardFactory;
use Magento\Backend\Model\View\Result\Redirect;
use Magento\Catalog\Helper\Product;
use Magento\Framework\Escaper;
use Magento\Framework\View\Result\PageFactory;
use Magento\Sales\Model\Order\Reorder\UnavailableProductsProvider;
use Magento\Sales\Api\OrderRepositoryInterface;
use Magento\Sales\Controller\Adminhtml\Order\Create;
use Magento\Sales\Helper\Reorder as ReorderHelper;
use Magento\Sales\Model\Order;
use Magento\Sales\Model\Order\Reorder\UnavailableProductsProvider;
use Magento\Framework\App\Action\HttpPostActionInterface;

class Reorder extends \Magento\Sales\Controller\Adminhtml\Order\Create
/**
* Controller create order.
*/
class Reorder extends Create implements HttpPostActionInterface
{
/**
* @var UnavailableProductsProvider
Expand All @@ -31,8 +43,8 @@ class Reorder extends \Magento\Sales\Controller\Adminhtml\Order\Create

/**
* @param Action\Context $context
* @param \Magento\Catalog\Helper\Product $productHelper
* @param \Magento\Framework\Escaper $escaper
* @param Product $productHelper
* @param Escaper $escaper
* @param PageFactory $resultPageFactory
* @param ForwardFactory $resultForwardFactory
* @param UnavailableProductsProvider $unavailableProductsProvider
Expand All @@ -41,8 +53,8 @@ class Reorder extends \Magento\Sales\Controller\Adminhtml\Order\Create
*/
public function __construct(
Action\Context $context,
\Magento\Catalog\Helper\Product $productHelper,
\Magento\Framework\Escaper $escaper,
Product $productHelper,
Escaper $escaper,
PageFactory $resultPageFactory,
ForwardFactory $resultForwardFactory,
UnavailableProductsProvider $unavailableProductsProvider,
Expand All @@ -62,19 +74,21 @@ public function __construct(
}

/**
* @return \Magento\Backend\Model\View\Result\Forward|\Magento\Backend\Model\View\Result\Redirect
* Adminhtml controller create order.
*
* @return Forward|Redirect
*/
public function execute()
{
$this->_getSession()->clearStorage();
$orderId = $this->getRequest()->getParam('order_id');
/** @var \Magento\Sales\Model\Order $order */
/** @var Order $order */
$order = $this->orderRepository->get($orderId);
if (!$this->reorderHelper->canReorder($order->getEntityId())) {
return $this->resultForwardFactory->create()->forward('noroute');
}

/** @var \Magento\Backend\Model\View\Result\Redirect $resultRedirect */
/** @var Redirect $resultRedirect */
$resultRedirect = $this->resultRedirectFactory->create();
if (!$order->getId()) {
$resultRedirect->setPath('sales/order/');
Expand All @@ -90,10 +104,18 @@ public function execute()
}
$resultRedirect->setPath('sales/order/view', ['order_id' => $orderId]);
} else {
$order->setReordered(true);
$this->_getSession()->setUseOldShippingMethod(true);
$this->_getOrderCreateModel()->initFromOrder($order);
$resultRedirect->setPath('sales/*');
try {
$order->setReordered(true);
$this->_getSession()->setUseOldShippingMethod(true);
$this->_getOrderCreateModel()->initFromOrder($order);
$resultRedirect->setPath('sales/*');
} catch (\Magento\Framework\Exception\LocalizedException $e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @cedmudit. I've noticed that we don't log the exception. It's not that critical in case of LocalizedException since we show the message using the messageManager. However, it's still recommended to log the exception.

In the case of catching \Exception it's very important to have the details in the error log. Otherwise, we just catch the exception and hide important error details.

Please, add error logging for both cases. Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, @rogyar. I will add an error logging.

$this->messageManager->addErrorMessage($e->getMessage());
return $resultRedirect->setPath('sales/*');
} catch (\Exception $e) {
$this->messageManager->addException($e, __('Error while processing order.'));
return $resultRedirect->setPath('sales/*');
}
}

return $resultRedirect;
Expand Down
Loading