Skip to content

Commit

Permalink
Merged PR 47850: Remove getRequest() in controllers
Browse files Browse the repository at this point in the history
## What's being changed

Our controllers.

## Why it's being changed

- Using `$this->getRequest()` in controllers is not allowed - we have to use `$context->getRequest()`.
- All controllers apart from admin ones no longer extend `\Magento\Framework\App\Action\Action`
- I've resolved all PHPStan and PHPCS errors in our controllers.

## How to review / test this change

- Test creating a new address book in Sync Settings
- Test creating a new data field in Data Mapping
- Test log viewer
- Test mass deleting in any of our report views
- Test exclusion rules (CRUD operations plus condition UI)
- Run Automap Data Fields from Developer Settings
- Run any reset button (include a date range) from Developer Settings
- Run set up integration
- Test email capture from checkout email field (POST on blur)
- Test OAuth connection
- Test microsite trial signup
- Test subscriptions management via account
- Test getbasket controller from abandoned cart EDC

Related work items: #223644
  • Loading branch information
sta1r committed Sep 19, 2023
1 parent 411b969 commit 804d23b
Show file tree
Hide file tree
Showing 22 changed files with 463 additions and 227 deletions.
23 changes: 16 additions & 7 deletions Controller/Adminhtml/Addressbook/Save.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

use Dotdigitalgroup\Email\Helper\Data;
use Magento\Backend\App\Action;
use Magento\Backend\App\Action\Context;
use Magento\Framework\App\Action\HttpPostActionInterface;
use Magento\Framework\App\ResponseInterface;
use Magento\Framework\Controller\ResultInterface;
use Magento\Framework\App\RequestInterface;

class Save extends Action implements HttpPostActionInterface
{
Expand All @@ -27,30 +27,39 @@ class Save extends Action implements HttpPostActionInterface
*/
private $helperData;

/**
* @var RequestInterface
*/
private $request;

/**
* Save constructor.
*
* @param Data $data
* @param Action\Context $context
* @param Context $context
*/
public function __construct(
Data $data,
Action\Context $context
Context $context
) {
$this->helperData = $data;
$this->messageManager = $context->getMessageManager();
$this->request = $context->getRequest();

parent::__construct($context);
}

/**
* Execute method.
*
* @return void
* @throws \Exception
*/
public function execute()
{
$addressBookName = $this->getRequest()->getParam('name');
$visibility = $this->getRequest()->getParam('visibility');
$website = (int) $this->getRequest()->getParam('website', 0);
$addressBookName = $this->request->getParam('name');
$visibility = $this->request->getParam('visibility');
$website = (int) $this->request->getParam('website', 0);

if ($this->helperData->isEnabled($website)) {
$client = $this->helperData->getWebsiteApiClient($website);
Expand Down
10 changes: 9 additions & 1 deletion Controller/Adminhtml/Connector/Ajaxlogcontent.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Magento\Backend\App\Action\Context;
use Magento\Framework\Escaper;
use Magento\Framework\App\Action\HttpPostActionInterface;
use Magento\Framework\App\RequestInterface;
use Magento\Framework\Controller\Result\Json;
use Magento\Framework\Controller\Result\JsonFactory;

Expand All @@ -29,6 +30,11 @@ class Ajaxlogcontent extends Action implements HttpPostActionInterface
*/
private $escaper;

/**
* @var RequestInterface
*/
private $request;

/**
* @var JsonFactory
*/
Expand All @@ -50,7 +56,9 @@ public function __construct(
) {
$this->file = $file;
$this->escaper = $escaper;
$this->request = $context->getRequest();
$this->resultJsonFactory = $resultJsonFactory;

parent::__construct($context);
}

Expand All @@ -61,7 +69,7 @@ public function __construct(
*/
public function execute()
{
$logFile = $this->getRequest()->getParam('log');
$logFile = $this->request->getParam('log');
switch ($logFile) {
case "connector":
$header = 'Marketing Automation Log';
Expand Down
18 changes: 13 additions & 5 deletions Controller/Adminhtml/Datafield/Save.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Magento\Backend\App\Action;
use Magento\Backend\App\Action\Context;
use Magento\Framework\App\Action\HttpPostActionInterface;
use Magento\Framework\App\RequestInterface;
use Magento\Framework\Message\ManagerInterface as MessageManager;

class Save extends Action implements HttpPostActionInterface
Expand All @@ -27,6 +28,11 @@ class Save extends Action implements HttpPostActionInterface
*/
private $datafieldHandler;

/**
* @var RequestInterface
*/
private $request;

/**
* Save constructor.
*
Expand All @@ -39,6 +45,8 @@ public function __construct(
) {
$this->datafieldHandler = $datafieldHandler;
$this->messageManager = $context->getMessageManager();
$this->request = $context->getRequest();

parent::__construct($context);
}

Expand All @@ -49,19 +57,19 @@ public function __construct(
*/
public function execute()
{
$datafield = $this->getRequest()->getParam('name');
$datafield = $this->request->getParam('name');

if (!empty($datafield)) {
if (!$this->datafieldHandler->hasValidLength($datafield)) {
$this->messageManager->addErrorMessage(__('Please limit Data Field Name to 20 characters.'));
return;
}
$response = $this->datafieldHandler->createDatafield(
(int) $this->getRequest()->getParam('website_id'),
(int) $this->request->getParam('website_id'),
$datafield,
$this->getRequest()->getParam('type'),
$this->getRequest()->getParam('visibility'),
$this->getRequest()->getParam('default')
$this->request->getParam('type'),
$this->request->getParam('visibility'),
$this->request->getParam('default')
);

if (isset($response->message)) {
Expand Down
8 changes: 6 additions & 2 deletions Controller/Adminhtml/MassDeleteCsrf.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@
use Magento\Backend\Model\View\Result\Redirect;
use Magento\Framework\App\Action\HttpPostActionInterface;
use Magento\Framework\Controller\ResultFactory;
use Magento\Framework\Model\ResourceModel\Db\AbstractDb;
use Magento\Framework\Model\ResourceModel\Db\Collection\AbstractCollectionFactory;
use Magento\Ui\Component\MassAction\Filter;

abstract class MassDeleteCsrf extends Action implements HttpPostActionInterface
{
/**
* Inherited
* @var AbstractDb
*/
protected $collectionResource;

Expand All @@ -21,11 +23,13 @@ abstract class MassDeleteCsrf extends Action implements HttpPostActionInterface
protected $filter;

/**
* Inherited
* @var AbstractCollectionFactory
*/
protected $collectionFactory;

/**
* Execute.
*
* @return Redirect
* @throws \Magento\Framework\Exception\NotFoundException|\Magento\Framework\Exception\LocalizedException
*/
Expand Down
14 changes: 11 additions & 3 deletions Controller/Adminhtml/Rules/Ajax.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Magento\Backend\App\Action\Context;
use Dotdigitalgroup\Email\Model\ExclusionRule\RuleValidator;
use Magento\Framework\App\Action\HttpPostActionInterface;
use Magento\Framework\App\RequestInterface;
use Magento\Framework\Controller\Result\Json;
use Magento\Framework\Controller\Result\JsonFactory;
use Magento\Framework\View\Element\Html\Select;
Expand Down Expand Up @@ -44,6 +45,11 @@ class Ajax extends Action implements HttpPostActionInterface
*/
private $ruleValidator;

/**
* @var RequestInterface
*/
private $request;

/**
* @var JsonFactory
*/
Expand Down Expand Up @@ -71,7 +77,9 @@ public function __construct(
$this->ruleCondition = $ruleCondition;
$this->ruleValue = $ruleValue;
$this->ruleValidator = $ruleValidator;
$this->request = $context->getRequest();
$this->resultJsonFactory = $resultJsonFactory;

parent::__construct($context);
}

Expand All @@ -82,9 +90,9 @@ public function __construct(
*/
public function execute()
{
$attribute = $this->getRequest()->getParam('attribute');
$conditionName = $this->getRequest()->getParam('condition');
$valueName = $this->getRequest()->getParam('value');
$attribute = $this->request->getParam('attribute');
$conditionName = $this->request->getParam('condition');
$valueName = $this->request->getParam('value');
$response = [];

if ($attribute && $conditionName && $valueName) {
Expand Down
29 changes: 19 additions & 10 deletions Controller/Adminhtml/Rules/Delete.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@

namespace Dotdigitalgroup\Email\Controller\Adminhtml\Rules;

use Dotdigitalgroup\Email\Model\ResourceModel\Rules as RulesResource;
use Dotdigitalgroup\Email\Model\Rules;
use Magento\Backend\App\Action;
use Magento\Backend\App\Action\Context;
use Magento\Framework\App\Action\HttpPostActionInterface;
use Magento\Framework\App\ResponseInterface;
use Magento\Framework\Controller\ResultInterface;
use Magento\Framework\App\RequestInterface;

class Delete extends Action implements HttpPostActionInterface
{
Expand All @@ -17,30 +19,37 @@ class Delete extends Action implements HttpPostActionInterface
public const ADMIN_RESOURCE = 'Dotdigitalgroup_Email::exclusion_rules';

/**
* @var \Dotdigitalgroup\Email\Model\Rules
* @var Rules
*/
private $rules;

/**
* @var \Dotdigitalgroup\Email\Model\ResourceModel\Rules
* @var RulesResource
*/
private $rulesResource;

/**
* @var RequestInterface
*/
private $request;

/**
* Delete constructor.
*
* @param \Dotdigitalgroup\Email\Model\ResourceModel\Rules $rulesResource
* @param \Magento\Backend\App\Action\Context $context
* @param \Dotdigitalgroup\Email\Model\Rules $rules
* @param \Magento\Backend\App\Action\Context $context
*/
public function __construct(
\Dotdigitalgroup\Email\Model\ResourceModel\Rules $rulesResource,
\Magento\Backend\App\Action\Context $context,
\Dotdigitalgroup\Email\Model\Rules $rules
RulesResource $rulesResource,
Rules $rules,
Context $context
) {
parent::__construct($context);
$this->rules = $rules;
$this->rulesResource = $rulesResource;
$this->request = $context->getRequest();

parent::__construct($context);
}

/**
Expand All @@ -50,7 +59,7 @@ public function __construct(
*/
public function execute()
{
$id = $this->getRequest()->getParam('id');
$id = $this->request->getParam('id');
if ($id) {
try {
$model = $this->rules;
Expand Down
29 changes: 16 additions & 13 deletions Controller/Adminhtml/Rules/Edit.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@

namespace Dotdigitalgroup\Email\Controller\Adminhtml\Rules;

use Dotdigitalgroup\Email\Model\ResourceModel\Rules;
use Magento\Backend\App\Action;
use Magento\Backend\App\Action\Context;
use Magento\Framework\App\Action\HttpGetActionInterface;
use Magento\Framework\App\RequestInterface;
use Magento\Framework\Registry;

class Edit extends Action implements HttpGetActionInterface
{
Expand All @@ -25,36 +29,35 @@ class Edit extends Action implements HttpGetActionInterface
private $rules;

/**
* @var \Magento\Framework\Escaper
* @var \Dotdigitalgroup\Email\Model\ResourceModel\Rules
*/
private $escaper;
private $rulesResource;

/**
* @var \Dotdigitalgroup\Email\Model\ResourceModel\Rules
* @var RequestInterface
*/
private $rulesResource;
private $request;

/**
* Edit constructor.
*
* @param \Dotdigitalgroup\Email\Model\ResourceModel\Rules $rulesResource
* @param \Magento\Backend\App\Action\Context $context
* @param Rules $rulesResource
* @param Context $context
* @param \Dotdigitalgroup\Email\Model\Rules $rules
* @param \Magento\Framework\Registry $registry
* @param \Magento\Framework\Escaper $escaper
* @param Registry $registry
*/
public function __construct(
\Dotdigitalgroup\Email\Model\ResourceModel\Rules $rulesResource,
\Magento\Backend\App\Action\Context $context,
\Dotdigitalgroup\Email\Model\Rules $rules,
\Magento\Framework\Registry $registry,
\Magento\Framework\Escaper $escaper
\Magento\Framework\Registry $registry
) {
parent::__construct($context);
$this->rules = $rules;
$this->registry = $registry;
$this->escaper = $escaper;
$this->rulesResource = $rulesResource;
$this->request = $context->getRequest();

parent::__construct($context);
}

/**
Expand All @@ -64,7 +67,7 @@ public function __construct(
*/
public function execute()
{
$id = $this->getRequest()->getParam('id');
$id = $this->request->getParam('id');

$this->_view->loadLayout();
$this->_setActiveMenu(
Expand Down
Loading

0 comments on commit 804d23b

Please sign in to comment.