Skip to content

Use constructor property promotion in module Sitemap #37216

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.

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

Open
wants to merge 1 commit into
base: 2.4-develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 12 additions & 7 deletions app/code/Magento/Sitemap/Block/Adminhtml/Edit.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,33 @@
*/
namespace Magento\Sitemap\Block\Adminhtml;

use Magento\Backend\Block\Widget\Context as WidgetContext;
use Magento\Backend\Block\Widget\Form\Container;
use Magento\Framework\Phrase;
use Magento\Framework\Registry;

/**
* Sitemap edit form container
*
* @author Magento Core Team <core@magentocommerce.com>
*/
class Edit extends \Magento\Backend\Block\Widget\Form\Container
class Edit extends Container
{
/**
* Core registry
*
* @var \Magento\Framework\Registry
* @var Registry
*/
protected $_coreRegistry = null;

/**
* @param \Magento\Backend\Block\Widget\Context $context
* @param \Magento\Framework\Registry $registry
* @param WidgetContext $context
* @param Registry $registry
* @param array $data
*/
public function __construct(
\Magento\Backend\Block\Widget\Context $context,
\Magento\Framework\Registry $registry,
WidgetContext $context,
Registry $registry,
Copy link
Member

Choose a reason for hiding this comment

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

Why not promote this property?

Suggested change
Registry $registry,
protected readonly Registry $_coreRegistry,

array $data = []
) {
$this->_coreRegistry = $registry;
Expand Down Expand Up @@ -67,7 +72,7 @@ protected function _construct()
/**
* Get edit form container header text
*
* @return \Magento\Framework\Phrase
* @return Phrase
*/
public function getHeaderText()
{
Expand Down
32 changes: 20 additions & 12 deletions app/code/Magento/Sitemap/Block/Adminhtml/Edit/Form.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,30 +5,38 @@
*/
namespace Magento\Sitemap\Block\Adminhtml\Edit;

use Magento\Backend\Block\Store\Switcher\Form\Renderer\Fieldset\Element as FieldsetElementRenderer;
use Magento\Backend\Block\Template\Context as TemplateContext;
use Magento\Backend\Block\Widget\Form\Generic;
use Magento\Framework\Data\Form as FormData;
use Magento\Framework\Data\FormFactory;
use Magento\Framework\Registry;
use Magento\Store\Model\System\Store as SystemStore;

/**
* Sitemap edit form
*
* @author Magento Core Team <core@magentocommerce.com>
*/
class Form extends \Magento\Backend\Block\Widget\Form\Generic
class Form extends Generic
{
/**
* @var \Magento\Store\Model\System\Store
* @var SystemStore
*/
protected $_systemStore;

/**
* @param \Magento\Backend\Block\Template\Context $context
* @param \Magento\Framework\Registry $registry
* @param \Magento\Framework\Data\FormFactory $formFactory
* @param \Magento\Store\Model\System\Store $systemStore
* @param TemplateContext $context
* @param Registry $registry
* @param FormFactory $formFactory
* @param SystemStore $systemStore
* @param array $data
*/
public function __construct(
\Magento\Backend\Block\Template\Context $context,
\Magento\Framework\Registry $registry,
\Magento\Framework\Data\FormFactory $formFactory,
\Magento\Store\Model\System\Store $systemStore,
TemplateContext $context,
Registry $registry,
FormFactory $formFactory,
SystemStore $systemStore,
array $data = []
) {
$this->_systemStore = $systemStore;
Expand Down Expand Up @@ -56,7 +64,7 @@ protected function _prepareForm()
{
$model = $this->_coreRegistry->registry('sitemap_sitemap');

/** @var \Magento\Framework\Data\Form $form */
/** @var FormData $form */
$form = $this->_formFactory->create(
['data' => ['id' => 'edit_form', 'action' => $this->getData('action'), 'method' => 'post']]
);
Expand Down Expand Up @@ -106,7 +114,7 @@ protected function _prepareForm()
]
);
$renderer = $this->getLayout()->createBlock(
\Magento\Backend\Block\Store\Switcher\Form\Renderer\Fieldset\Element::class
FieldsetElementRenderer::class
);
$field->setRenderer($renderer);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,19 @@
*/
namespace Magento\Sitemap\Block\Adminhtml\Grid\Renderer;

use Magento\Backend\Block\Widget\Grid\Column\Renderer\Action as ColumnRendererAction;
use Magento\Framework\DataObject;

/**
* Sitemap grid action column renderer
*/
class Action extends \Magento\Backend\Block\Widget\Grid\Column\Renderer\Action
class Action extends ColumnRendererAction
{
/**
* @param \Magento\Framework\DataObject $row
* @param DataObject $row
* @return string
*/
public function render(\Magento\Framework\DataObject $row)
public function render(DataObject $row)
{
$this->getColumn()->setActions(
[
Expand Down
27 changes: 4 additions & 23 deletions app/code/Magento/Sitemap/Block/Adminhtml/Grid/Renderer/Link.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,6 @@
*/
class Link extends AbstractRenderer
{
/**
* @var Filesystem
*/
private $filesystem;

/**
* @var SitemapFactory
*/
private $sitemapFactory;

/**
* @var DocumentRoot
*/
private $documentRoot;

/**
* @param Context $context
* @param SitemapFactory $sitemapFactory
Expand All @@ -44,15 +29,11 @@ class Link extends AbstractRenderer
*/
public function __construct(
Context $context,
SitemapFactory $sitemapFactory,
Filesystem $filesystem,
DocumentRoot $documentRoot,
private readonly SitemapFactory $sitemapFactory,
private readonly Filesystem $filesystem,
private readonly DocumentRoot $documentRoot,
array $data = []
) {
$this->sitemapFactory = $sitemapFactory;
$this->filesystem = $filesystem;
$this->documentRoot = $documentRoot;

parent::__construct($context, $data);
}

Expand All @@ -65,7 +46,7 @@ public function __construct(
*/
public function render(DataObject $row)
{
/** @var $sitemap Sitemap */
/** @var Sitemap $sitemap */
$sitemap = $this->sitemapFactory->create();
$sitemap->setStoreId($row->getStoreId());
$url = $this->_escaper->escapeHtml($sitemap->getSitemapUrl($row->getSitemapPath(), $row->getSitemapFilename()));
Expand Down
21 changes: 13 additions & 8 deletions app/code/Magento/Sitemap/Block/Adminhtml/Grid/Renderer/Time.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,26 @@
*/
namespace Magento\Sitemap\Block\Adminhtml\Grid\Renderer;

class Time extends \Magento\Backend\Block\Widget\Grid\Column\Renderer\AbstractRenderer
use Magento\Backend\Block\Context;
use Magento\Backend\Block\Widget\Grid\Column\Renderer\AbstractRenderer;
use Magento\Framework\DataObject;
use Magento\Framework\Stdlib\DateTime\DateTime;

class Time extends AbstractRenderer
{
/**
* @var \Magento\Framework\Stdlib\DateTime\DateTime
* @var DateTime
*/
protected $_date;

/**
* @param \Magento\Backend\Block\Context $context
* @param \Magento\Framework\Stdlib\DateTime\DateTime $date
* @param Context $context
* @param DateTime $date
* @param array $data
*/
public function __construct(
\Magento\Backend\Block\Context $context,
\Magento\Framework\Stdlib\DateTime\DateTime $date,
Context $context,
DateTime $date,
array $data = []
) {
$this->_date = $date;
Expand All @@ -34,10 +39,10 @@ public function __construct(
/**
* Prepare link to display in grid
*
* @param \Magento\Framework\DataObject $row
* @param DataObject $row
* @return string
*/
public function render(\Magento\Framework\DataObject $row)
public function render(DataObject $row)
{
$time = date('Y-m-d H:i:s', strtotime($row->getSitemapTime()) + $this->_date->getGmtOffset());
return $time;
Expand Down
4 changes: 3 additions & 1 deletion app/code/Magento/Sitemap/Block/Adminhtml/Sitemap.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
*/
namespace Magento\Sitemap\Block\Adminhtml;

use Magento\Backend\Block\Widget\Grid\Container;

/**
* Adminhtml catalog (google) sitemaps block
*
* @api
* @since 100.0.2
*/
class Sitemap extends \Magento\Backend\Block\Widget\Grid\Container
class Sitemap extends Container
{
/**
* Block constructor
Expand Down
36 changes: 5 additions & 31 deletions app/code/Magento/Sitemap/Block/Robots.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,27 +25,6 @@
*/
class Robots extends AbstractBlock implements IdentityInterface
{
/**
* @var CollectionFactory
*/
private $sitemapCollectionFactory;

/**
* @var SitemapHelper
* @deprecated
*/
private $sitemapHelper;

/**
* @var StoreManagerInterface
*/
private $storeManager;

/**
* @var SitemapConfigReader
*/
private $sitemapConfigReader;

/**
* @param Context $context
* @param StoreResolver $storeResolver
Expand All @@ -59,15 +38,12 @@ class Robots extends AbstractBlock implements IdentityInterface
public function __construct(
Context $context,
StoreResolver $storeResolver,
CollectionFactory $sitemapCollectionFactory,
SitemapHelper $sitemapHelper,
StoreManagerInterface $storeManager,
private readonly CollectionFactory $sitemapCollectionFactory,
private readonly SitemapHelper $sitemapHelper,
private readonly StoreManagerInterface $storeManager,
array $data = [],
?SitemapConfigReader $sitemapConfigReader = null
private ?SitemapConfigReader $sitemapConfigReader = null
) {
$this->sitemapCollectionFactory = $sitemapCollectionFactory;
$this->sitemapHelper = $sitemapHelper;
$this->storeManager = $storeManager;
$this->sitemapConfigReader = $sitemapConfigReader
?: ObjectManager::getInstance()->get(SitemapConfigReader::class);

Expand Down Expand Up @@ -116,9 +92,7 @@ protected function getSitemapLinks(array $storeIds)
$collection->addStoreFilter($storeIds);

$sitemapLinks = [];
/**
* @var Sitemap $sitemap
*/
/** @var Sitemap $sitemap */
foreach ($collection as $sitemap) {
$sitemapUrl = $sitemap->getSitemapUrl($sitemap->getSitemapPath(), $sitemap->getSitemapFilename());
$sitemapLinks[$sitemapUrl] = 'Sitemap: ' . $sitemapUrl;
Expand Down
4 changes: 3 additions & 1 deletion app/code/Magento/Sitemap/Controller/Adminhtml/Sitemap.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
*/
namespace Magento\Sitemap\Controller\Adminhtml;

use Magento\Backend\App\Action;

/**
* XML sitemap controller
*/
abstract class Sitemap extends \Magento\Backend\App\Action
abstract class Sitemap extends Action
{
/**
* Authorization level of a basic admin session
Expand Down
22 changes: 6 additions & 16 deletions app/code/Magento/Sitemap/Controller/Adminhtml/Sitemap/Delete.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,41 +7,31 @@

namespace Magento\Sitemap\Controller\Adminhtml\Sitemap;

use Exception;
use Magento\Backend\App\Action\Context;
use Magento\Framework\App\Action\HttpPostActionInterface;
use Magento\Framework\App\Filesystem\DirectoryList;
use Magento\Framework\Filesystem;
use Magento\Sitemap\Controller\Adminhtml\Sitemap;
use Magento\Sitemap\Model\Sitemap as ModelSitemap;
use Magento\Sitemap\Model\SitemapFactory;

/**
* Controller class Delete. Represents adminhtml request flow for a sitemap deletion
*/
class Delete extends Sitemap implements HttpPostActionInterface
{
/**
* @var SitemapFactory
*/
private $sitemapFactory;

/**
* @var Filesystem
*/
private $filesystem;

/**
* @param Context $context
* @param SitemapFactory $sitemapFactory
* @param Filesystem $filesystem
*/
public function __construct(
Context $context,
SitemapFactory $sitemapFactory,
Filesystem $filesystem
private readonly SitemapFactory $sitemapFactory,
private readonly Filesystem $filesystem
) {
parent::__construct($context);
$this->sitemapFactory = $sitemapFactory;
$this->filesystem = $filesystem;
}

/**
Expand All @@ -57,7 +47,7 @@ public function execute()
if ($id) {
try {
// init model and delete
/** @var \Magento\Sitemap\Model\Sitemap $sitemap */
/** @var ModelSitemap $sitemap */
$sitemap = $this->sitemapFactory->create();
$sitemap->load($id);
// delete file
Expand All @@ -78,7 +68,7 @@ public function execute()
// go to grid
$this->_redirect('adminhtml/*/');
return;
} catch (\Exception $e) {
} catch (Exception $e) {
// display error message
$this->messageManager->addErrorMessage($e->getMessage());
// go back to edit form
Expand Down
Loading