From 1b2b3228ad6b863947a1150c289d57accbc157f1 Mon Sep 17 00:00:00 2001 From: Anshu Mishra Date: Tue, 3 Jul 2018 17:56:39 +0530 Subject: [PATCH 1/4] admin checkout agreement controllers refactor --- .../Controller/Adminhtml/Agreement.php | 27 +++++++++++++++++-- .../Controller/Adminhtml/Agreement/Delete.php | 6 ++--- .../Controller/Adminhtml/Agreement/Edit.php | 4 +-- .../Controller/Adminhtml/Agreement/Save.php | 4 +-- 4 files changed, 32 insertions(+), 9 deletions(-) diff --git a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement.php b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement.php index 13130a4491eb2..92829feecace9 100644 --- a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement.php +++ b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement.php @@ -5,6 +5,9 @@ */ namespace Magento\CheckoutAgreements\Controller\Adminhtml; +use Magento\Framework\App\ObjectManager; +use Magento\CheckoutAgreements\Api\CheckoutAgreementsRepositoryInterface; + abstract class Agreement extends \Magento\Backend\App\Action { /** @@ -20,15 +23,35 @@ abstract class Agreement extends \Magento\Backend\App\Action * @var \Magento\Framework\Registry */ protected $_coreRegistry = null; + + /** + * @var CheckoutAgreementsRepositoryInterface + */ + protected $_agreementRepository; + + /** + * @var \Magento\CheckoutAgreements\Model\AgreementFactory + */ + protected $_agreementFactory; /** * @param \Magento\Backend\App\Action\Context $context * @param \Magento\Framework\Registry $coreRegistry + * @param CheckoutAgreementsRepositoryInterface $agreementRepository + * @param \Magento\CheckoutAgreements\Model\AgreementFactory $agreementFactory * @codeCoverageIgnore */ - public function __construct(\Magento\Backend\App\Action\Context $context, \Magento\Framework\Registry $coreRegistry) - { + public function __construct( + \Magento\Backend\App\Action\Context $context, + \Magento\Framework\Registry $coreRegistry, + CheckoutAgreementsRepositoryInterface $agreementRepository = null, + \Magento\CheckoutAgreements\Model\AgreementFactory $agreementFactory = null + ) { $this->_coreRegistry = $coreRegistry; + $this->_agreementRepository = $agreementRepository ?: + ObjectManager::getInstance()->get(CheckoutAgreementsRepositoryInterface::class); + $this->_agreementFactory = $agreementFactory ?: + ObjectManager::getInstance()->get(\Magento\CheckoutAgreements\Model\AgreementFactory::class); parent::__construct($context); } diff --git a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Delete.php b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Delete.php index 65aca6205caa4..169246e359b1e 100644 --- a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Delete.php +++ b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Delete.php @@ -14,15 +14,15 @@ class Delete extends \Magento\CheckoutAgreements\Controller\Adminhtml\Agreement public function execute() { $id = (int)$this->getRequest()->getParam('id'); - $model = $this->_objectManager->get(\Magento\CheckoutAgreements\Model\Agreement::class)->load($id); - if (!$model->getId()) { + $repository = $this->_agreementRepository->get($id); + if (!$repository->getAgreementId()) { $this->messageManager->addError(__('This condition no longer exists.')); $this->_redirect('checkout/*/'); return; } try { - $model->delete(); + $repository->delete(); $this->messageManager->addSuccess(__('You deleted the condition.')); $this->_redirect('checkout/*/'); return; diff --git a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Edit.php b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Edit.php index 73ac129bc993c..fb160f450f0dd 100644 --- a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Edit.php +++ b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Edit.php @@ -15,7 +15,7 @@ class Edit extends \Magento\CheckoutAgreements\Controller\Adminhtml\Agreement public function execute() { $id = $this->getRequest()->getParam('id'); - $agreementModel = $this->_objectManager->create(\Magento\CheckoutAgreements\Model\Agreement::class); + $agreementModel = $this->_agreementFactory->create(); if ($id) { $agreementModel->load($id); @@ -26,7 +26,7 @@ public function execute() } } - $data = $this->_objectManager->get(\Magento\Backend\Model\Session::class)->getAgreementData(true); + $data = $this->_session->getAgreementData(true); if (!empty($data)) { $agreementModel->setData($data); } diff --git a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Save.php b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Save.php index 25c034203620b..c8c2b7d0a40bf 100644 --- a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Save.php +++ b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Save.php @@ -15,7 +15,7 @@ public function execute() { $postData = $this->getRequest()->getPostValue(); if ($postData) { - $model = $this->_objectManager->get(\Magento\CheckoutAgreements\Model\Agreement::class); + $model = $this->_agreementFactory->create(); $model->setData($postData); try { @@ -36,7 +36,7 @@ public function execute() $this->messageManager->addError(__('Something went wrong while saving this condition.')); } - $this->_objectManager->get(\Magento\Backend\Model\Session::class)->setAgreementData($postData); + $this->_session->setAgreementData($postData); $this->getResponse()->setRedirect($this->_redirect->getRedirectUrl($this->getUrl('*'))); } } From 538e6816b097d75c47763ef58714b33ef2b56766 Mon Sep 17 00:00:00 2001 From: Anshu Mishra Date: Fri, 13 Jul 2018 15:01:41 +0530 Subject: [PATCH 2/4] refactor code --- .../Controller/Adminhtml/Agreement.php | 33 +++++------------ .../Controller/Adminhtml/Agreement/Delete.php | 32 +++++++++++++++-- .../Controller/Adminhtml/Agreement/Edit.php | 33 +++++++++++++++-- .../Controller/Adminhtml/Agreement/Save.php | 36 ++++++++++++++++--- 4 files changed, 99 insertions(+), 35 deletions(-) diff --git a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement.php b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement.php index 92829feecace9..aa6f461fc5ee2 100644 --- a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement.php +++ b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement.php @@ -5,10 +5,11 @@ */ namespace Magento\CheckoutAgreements\Controller\Adminhtml; -use Magento\Framework\App\ObjectManager; -use Magento\CheckoutAgreements\Api\CheckoutAgreementsRepositoryInterface; +use Magento\Backend\App\Action; +use Magento\Backend\App\Action\Context; +use Magento\Framework\Registry; -abstract class Agreement extends \Magento\Backend\App\Action +abstract class Agreement extends Action { /** * Authorization level of a basic admin session @@ -23,35 +24,17 @@ abstract class Agreement extends \Magento\Backend\App\Action * @var \Magento\Framework\Registry */ protected $_coreRegistry = null; - - /** - * @var CheckoutAgreementsRepositoryInterface - */ - protected $_agreementRepository; - - /** - * @var \Magento\CheckoutAgreements\Model\AgreementFactory - */ - protected $_agreementFactory; /** - * @param \Magento\Backend\App\Action\Context $context - * @param \Magento\Framework\Registry $coreRegistry - * @param CheckoutAgreementsRepositoryInterface $agreementRepository - * @param \Magento\CheckoutAgreements\Model\AgreementFactory $agreementFactory + * @param Context $context + * @param Registry $coreRegistry * @codeCoverageIgnore */ public function __construct( - \Magento\Backend\App\Action\Context $context, - \Magento\Framework\Registry $coreRegistry, - CheckoutAgreementsRepositoryInterface $agreementRepository = null, - \Magento\CheckoutAgreements\Model\AgreementFactory $agreementFactory = null + Context $context, + Registry $coreRegistry ) { $this->_coreRegistry = $coreRegistry; - $this->_agreementRepository = $agreementRepository ?: - ObjectManager::getInstance()->get(CheckoutAgreementsRepositoryInterface::class); - $this->_agreementFactory = $agreementFactory ?: - ObjectManager::getInstance()->get(\Magento\CheckoutAgreements\Model\AgreementFactory::class); parent::__construct($context); } diff --git a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Delete.php b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Delete.php index 169246e359b1e..144ce2b2f6bc5 100644 --- a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Delete.php +++ b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Delete.php @@ -6,15 +6,41 @@ */ namespace Magento\CheckoutAgreements\Controller\Adminhtml\Agreement; -class Delete extends \Magento\CheckoutAgreements\Controller\Adminhtml\Agreement +use Magento\CheckoutAgreements\Api\CheckoutAgreementsRepositoryInterface; +use Magento\CheckoutAgreements\Controller\Adminhtml\Agreement; +use Magento\Backend\App\Action\Context; +use Magento\Framework\Registry; +use Magento\Framework\App\ObjectManager; +use Magento\Framework\Exception\LocalizedException; + +class Delete extends Agreement { + /** + * @var CheckoutAgreementsRepositoryInterface + */ + private $agreementRepository; + + /** + * @param Context $context + * @param Registry $coreRegistry + * @param CheckoutAgreementsRepositoryInterface $agreementRepository + */ + public function __construct( + Context $context, + Registry $coreRegistry, + CheckoutAgreementsRepositoryInterface $agreementRepository = null + ) { + $this->agreementRepository = $agreementRepository ?: + ObjectManager::getInstance()->get(CheckoutAgreementsRepositoryInterface::class); + parent::__construct($context, $coreRegistry); + } /** * @return void */ public function execute() { $id = (int)$this->getRequest()->getParam('id'); - $repository = $this->_agreementRepository->get($id); + $repository = $this->agreementRepository->get($id); if (!$repository->getAgreementId()) { $this->messageManager->addError(__('This condition no longer exists.')); $this->_redirect('checkout/*/'); @@ -26,7 +52,7 @@ public function execute() $this->messageManager->addSuccess(__('You deleted the condition.')); $this->_redirect('checkout/*/'); return; - } catch (\Magento\Framework\Exception\LocalizedException $e) { + } catch (LocalizedException $e) { $this->messageManager->addError($e->getMessage()); } catch (\Exception $e) { $this->messageManager->addError(__('Something went wrong while deleting this condition.')); diff --git a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Edit.php b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Edit.php index fb160f450f0dd..768ac6ee273f0 100644 --- a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Edit.php +++ b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Edit.php @@ -6,8 +6,35 @@ */ namespace Magento\CheckoutAgreements\Controller\Adminhtml\Agreement; -class Edit extends \Magento\CheckoutAgreements\Controller\Adminhtml\Agreement +use Magento\CheckoutAgreements\Controller\Adminhtml\Agreement; +use Magento\CheckoutAgreements\Model\AgreementFactory; +use Magento\Backend\App\Action\Context; +use Magento\Framework\Registry; +use Magento\Framework\App\ObjectManager; +use Magento\CheckoutAgreements\Controller\Adminhtml\Agreement; +use Magento\CheckoutAgreements\Block\Adminhtml\Agreement\Edit; + +class Edit extends Agreement { + /** + * @var AgreementFactory + */ + private $agreementFactory; + + /** + * @param Context $context + * @param Registry $coreRegistry + * @param AgreementFactory $agreementFactory + */ + public function __construct( + Context $context, + Registry $coreRegistry, + AgreementFactory $agreementFactory = null + ) { + $this->agreementFactory = $agreementFactory ?: + ObjectManager::getInstance()->get(AgreementFactory::class); + parent::__construct($context, $coreRegistry); + } /** * @return void * @SuppressWarnings(PHPMD.NPathComplexity) @@ -15,7 +42,7 @@ class Edit extends \Magento\CheckoutAgreements\Controller\Adminhtml\Agreement public function execute() { $id = $this->getRequest()->getParam('id'); - $agreementModel = $this->_agreementFactory->create(); + $agreementModel = $this->agreementFactory->create(); if ($id) { $agreementModel->load($id); @@ -38,7 +65,7 @@ public function execute() $id ? __('Edit Condition') : __('New Condition') )->_addContent( $this->_view->getLayout()->createBlock( - \Magento\CheckoutAgreements\Block\Adminhtml\Agreement\Edit::class + Edit::class )->setData( 'action', $this->getUrl('checkout/*/save') diff --git a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Save.php b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Save.php index c8c2b7d0a40bf..c0eef133a2a9a 100644 --- a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Save.php +++ b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Save.php @@ -6,8 +6,36 @@ */ namespace Magento\CheckoutAgreements\Controller\Adminhtml\Agreement; -class Save extends \Magento\CheckoutAgreements\Controller\Adminhtml\Agreement +use Magento\CheckoutAgreements\Controller\Adminhtml\Agreement; +use Magento\CheckoutAgreements\Model\AgreementFactory; +use Magento\Backend\App\Action\Context; +use Magento\Framework\Registry; +use Magento\Framework\App\ObjectManager; +use Magento\CheckoutAgreements\Controller\Adminhtml\Agreement; +use Magento\Framework\DataObject; +use Magento\Framework\Exception\LocalizedException; + +class Save extends Agreement { + /** + * @var AgreementFactory + */ + private $agreementFactory; + + /** + * @param Context $context + * @param Registry $coreRegistry + * @param AgreementFactory $agreementFactory + */ + public function __construct( + Context $context, + Registry $coreRegistry, + AgreementFactory $agreementFactory = null + ) { + $this->agreementFactory = $agreementFactory ?: + ObjectManager::getInstance()->get(AgreementFactory::class); + parent::__construct($context, $coreRegistry); + } /** * @return void */ @@ -15,11 +43,11 @@ public function execute() { $postData = $this->getRequest()->getPostValue(); if ($postData) { - $model = $this->_agreementFactory->create(); + $model = $this->agreementFactory->create(); $model->setData($postData); try { - $validationResult = $model->validateData(new \Magento\Framework\DataObject($postData)); + $validationResult = $model->validateData(new DataObject($postData)); if ($validationResult !== true) { foreach ($validationResult as $message) { $this->messageManager->addError($message); @@ -30,7 +58,7 @@ public function execute() $this->_redirect('checkout/*/'); return; } - } catch (\Magento\Framework\Exception\LocalizedException $e) { + } catch (LocalizedException $e) { $this->messageManager->addError($e->getMessage()); } catch (\Exception $e) { $this->messageManager->addError(__('Something went wrong while saving this condition.')); From f4d837d1548a2f8d964419f35e9572c7f31a36b0 Mon Sep 17 00:00:00 2001 From: Anshu Mishra Date: Tue, 17 Jul 2018 17:31:13 +0530 Subject: [PATCH 3/4] fixes as CI issues --- .../Controller/Adminhtml/Agreement/Edit.php | 5 ++--- .../Controller/Adminhtml/Agreement/Save.php | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Edit.php b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Edit.php index 768ac6ee273f0..8bec3b581cd54 100644 --- a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Edit.php +++ b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Edit.php @@ -11,8 +11,7 @@ use Magento\Backend\App\Action\Context; use Magento\Framework\Registry; use Magento\Framework\App\ObjectManager; -use Magento\CheckoutAgreements\Controller\Adminhtml\Agreement; -use Magento\CheckoutAgreements\Block\Adminhtml\Agreement\Edit; +use Magento\CheckoutAgreements\Block\Adminhtml\Agreement\Edit as BlockEdit; class Edit extends Agreement { @@ -65,7 +64,7 @@ public function execute() $id ? __('Edit Condition') : __('New Condition') )->_addContent( $this->_view->getLayout()->createBlock( - Edit::class + BlockEdit::class )->setData( 'action', $this->getUrl('checkout/*/save') diff --git a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Save.php b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Save.php index c0eef133a2a9a..05a16d3dd4264 100644 --- a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Save.php +++ b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Save.php @@ -11,7 +11,6 @@ use Magento\Backend\App\Action\Context; use Magento\Framework\Registry; use Magento\Framework\App\ObjectManager; -use Magento\CheckoutAgreements\Controller\Adminhtml\Agreement; use Magento\Framework\DataObject; use Magento\Framework\Exception\LocalizedException; From d49227401db8f926c14b8ef88845a34bb6f08367 Mon Sep 17 00:00:00 2001 From: Stanislav Idolov Date: Tue, 31 Jul 2018 09:41:47 +0300 Subject: [PATCH 4/4] Fixed variable name & repository usage --- .../Controller/Adminhtml/Agreement/Delete.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Delete.php b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Delete.php index 144ce2b2f6bc5..f7b178df99624 100644 --- a/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Delete.php +++ b/app/code/Magento/CheckoutAgreements/Controller/Adminhtml/Agreement/Delete.php @@ -40,15 +40,15 @@ public function __construct( public function execute() { $id = (int)$this->getRequest()->getParam('id'); - $repository = $this->agreementRepository->get($id); - if (!$repository->getAgreementId()) { + $agreement = $this->agreementRepository->get($id); + if (!$agreement->getAgreementId()) { $this->messageManager->addError(__('This condition no longer exists.')); $this->_redirect('checkout/*/'); return; } try { - $repository->delete(); + $this->agreementRepository->delete($agreement); $this->messageManager->addSuccess(__('You deleted the condition.')); $this->_redirect('checkout/*/'); return;