Skip to content

Commit

Permalink
Merged PR 53281: Add PHPCS ignore rules for Magento2.Legacy.EscapeMet…
Browse files Browse the repository at this point in the history
…hodsOnBlockClass

## What's being changed

Various template phtml files.

## Why it's being changed

As of Magento 2.4.7, the Magento2 coding standard flags the use of escaper methods on `$block`, for example:
```
17 | WARNING | [x] Using escapeHtml on $block is deprecated. Please use equivalent method on $escaper
```
Unfortunately we must yet support 2.3.7 therefore we need to ignore these warnings for the time being.

In addition I have resolved a handful of actual `ERROR`s which had the potential to block our marketplace submissions after the new version is released. I've also removed usage `@codingStandardsIgnoreFile` which is a) deprecated and b) risky because errors can surface undetected.

## How to review / test this change

- In Magento 2.4.7-beta3, run:
```
vendor/bin/phpcs --standard=Magento2 --extensions=php,phtml --error-severity=10 app/code/Dotdigitalgroup/Email/ --ignore-annotations --report=json --report-file=report.json
```
- Scan the report for any errors (warnings are OK)
- See a green CI build once it has been updated to use latest 2.4-develop
- Smoke test HTML output where escaping has been added, in particular around WBT (see [this commit](5c48a78))

Related work items: #242925
  • Loading branch information
sta1r committed Apr 9, 2024
1 parent 6772634 commit fd40657
Show file tree
Hide file tree
Showing 33 changed files with 216 additions and 171 deletions.
8 changes: 5 additions & 3 deletions view/adminhtml/templates/dashboard/connector.phtml
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
<?php /** @var \Dotdigitalgroup\Email\Block\Adminhtml\Dashboard\Information $block */?>
<?php
/** @var \Dotdigitalgroup\Email\Block\Adminhtml\Dashboard\Information $block */
// phpcs:disable Magento2.Legacy.EscapeMethodsOnBlockClass
?>
<table>
<tr>
<th>
Expand Down Expand Up @@ -49,5 +52,4 @@
<td><?= $block->escapeHtml($block->getLastFailedAuth()) ?></td>
</tr>
</table>


<?php // phpcs:enable ?>
6 changes: 5 additions & 1 deletion view/adminhtml/templates/dashboard/information.phtml
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
<?php /** @var \Dotdigitalgroup\Email\Block\Adminhtml\Dashboard\Information $block */?>
<?php
/** @var \Dotdigitalgroup\Email\Block\Adminhtml\Dashboard\Information $block */
// phpcs:disable Magento2.Legacy.EscapeMethodsOnBlockClass
?>
<table>
<tr>
<th>
Expand Down Expand Up @@ -43,3 +46,4 @@
</tr>
<?php endforeach; ?>
</table>
<?php // phpcs:enable ?>
6 changes: 5 additions & 1 deletion view/adminhtml/templates/dashboard/main.phtml
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
<?php /** @var \Dotdigitalgroup\Email\Block\Adminhtml\Dashboard $block */?>
<?php
/** @var \Dotdigitalgroup\Email\Block\Adminhtml\Dashboard $block */
// phpcs:disable Magento2.Legacy.EscapeMethodsOnBlockClass
?>
<div class="content-header">
<?= $block->getChildHtml('adminhtml.system.config.switcher');?>
</div>
Expand Down Expand Up @@ -32,3 +35,4 @@
}
}
</script>
<?php // phpcs:enable ?>
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php
/** @var $block Dotdigitalgroup\Email\Block\Adminhtml\Email\Items */
// phpcs:disable Magento2.Legacy.EscapeMethodsOnBlockClass
?>
<?php if ($block->hasItems()) { ?>
<p><strong>Automation errors</strong></p>
Expand Down Expand Up @@ -47,4 +48,6 @@
)
); ?>
</p>
<?php } ?>
<?php }
// phpcs:enable
?>
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php
/** @var $block Dotdigitalgroup\Email\Block\Adminhtml\Email\Items */
// phpcs:disable Magento2.Legacy.EscapeMethodsOnBlockClass
?>
<?php if ($block->hasItems()) { ?>
<p><strong>Campaign errors</strong></p>
Expand Down Expand Up @@ -32,4 +33,6 @@
</table>

<p><?= $block->escapeHtml(__('Please go to the Campaign Sends report to review.')); ?></p>
<?php } ?>
<?php }
// phpcs:enable
?>
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php
/** @var $block Dotdigitalgroup\Email\Block\Adminhtml\Email\Items */
// phpcs:disable Magento2.Legacy.EscapeMethodsOnBlockClass
?>
<?php if ($block->hasItems()) { ?>
<p><strong>Cron errors</strong></p>
Expand Down Expand Up @@ -28,4 +29,6 @@
</table>

<p><?= $block->escapeHtml(__('Please go to the Cron Tasks report to review.')); ?></p>
<?php } ?>
<?php }
// phpcs:enable
?>
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php
/** @var $block Dotdigitalgroup\Email\Block\Adminhtml\Email\Items */
// phpcs:disable Magento2.Legacy.EscapeMethodsOnBlockClass
?>
<?php if ($block->hasItems()) { ?>
<p><strong>Importer errors</strong></p>
Expand Down Expand Up @@ -33,4 +34,6 @@
</table>

<p><?= $block->escapeHtml(__('Please go to the Import Report to review.')); ?></p>
<?php } ?>
<?php }
// phpcs:enable
?>
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php
/** @var $block Dotdigitalgroup\Email\Block\Adminhtml\Email\Items */
// phpcs:disable Magento2.Legacy.EscapeMethodsOnBlockClass
?>
<?php if ($block->hasItems()) { ?>
<p><strong>SMTP errors</strong></p>
Expand All @@ -26,4 +27,6 @@
</tbody>
</table>

<?php } ?>
<?php }
// phpcs:enable
?>
12 changes: 7 additions & 5 deletions view/adminhtml/templates/engagement_cloud/trial.phtml
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
<?php /** @var \Dotdigitalgroup\Email\Block\Adminhtml\EngagementCloudTrialInterface $block */
// @codingStandardsIgnoreFile
<?php
/** @var \Dotdigitalgroup\Email\Block\Adminhtml\EngagementCloudTrialInterface $block */
// phpcs:disable Magento2.Legacy.EscapeMethodsOnBlockClass
?>
<script type="text/x-magento-init">
{
"*": {
"Dotdigitalgroup_Email/js/engagementCloudEmbed": {
"iframeSrc": "<?= $block->getAction() ?>"
"iframeSrc": "<?= $block->escapeHtml($block->getAction()) ?>"
},
"Dotdigitalgroup_Email/js/engagementCloudTrial": {
"callback": "<?= $block->getLocalCallbackUrl() ?>",
"micrositeUrl": "<?= $block->getTrialSignupHostAndScheme() ?>"
"callback": "<?= $block->escapeHtml($block->getLocalCallbackUrl()) ?>",
"micrositeUrl": "<?= $block->escapeHtml($block->getTrialSignupHostAndScheme()) ?>"
}
}
}
</script>
<?php // phpcs:enable ?>
8 changes: 6 additions & 2 deletions view/adminhtml/templates/log.phtml
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
<?php /** @var \Dotdigitalgroup\Email\Block\Adminhtml\Logviewer $block */?>
<?php
/** @var \Dotdigitalgroup\Email\Block\Adminhtml\Logviewer $block */
// phpcs:disable Magento2.Legacy.EscapeMethodsOnBlockClass
?>
<div class="switcher">
<label style="font-weight: bold; padding-right: 10px;" for="connector-log-selector">
<?= $block->escapeHtml(__('Select Log File:')); ?>
Expand Down Expand Up @@ -36,4 +39,5 @@
}
}
}
</script>
</script>
<?php // phpcs:enable ?>
2 changes: 2 additions & 0 deletions view/adminhtml/templates/rulesJsInit.phtml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php
/** @var \Magento\Backend\Block\Template $block */
// phpcs:disable Magento2.Legacy.EscapeMethodsOnBlockClass
?>
<script type="text/x-magento-init">
{
Expand All @@ -18,3 +19,4 @@
}
}
</script>
<?php // phpcs:enable ?>
101 changes: 43 additions & 58 deletions view/adminhtml/templates/store/switcher.phtml
Original file line number Diff line number Diff line change
@@ -1,50 +1,45 @@
<?php
// @codingStandardsIgnoreFile

/* @var $block \Magento\Backend\Block\Store\Switcher */
// phpcs:disable Magento2.Legacy.EscapeMethodsOnBlockClass
?>
<?php if ($websites = $block->getWebsites()): ?>

<div class="store-switcher store-view">
<span class="store-switcher-label"><?php echo $block->escapeHtml(__('Websites View:')) ?></span>
<span class="store-switcher-label"><?= $block->escapeHtml(__('Websites View:')) ?></span>
<div class="actions dropdown closable">
<input type="hidden" name="store_switcher" id="store_switcher"
data-role="store-view-id" data-param="<?php echo $block->escapeHtml($block->getStoreVarName()) ?>"
value="<?php echo $block->escapeHtml($block->getStoreId()) ?>"
onchange="switchScope(this);"<?php echo $block->escapeHtml($block->getUiId()) ?> />
data-role="store-view-id" data-param="<?= $block->escapeHtml($block->getStoreVarName()) ?>"
value="<?= $block->escapeHtml($block->getStoreId()) ?>"
onchange="switchScope(this);"<?= $block->escapeHtml($block->getUiId()) ?> />
<input type="hidden" name="store_group_switcher" id="store_group_switcher"
data-role="store-group-id" data-param="<?php echo $block->escapeHtml($block->getStoreGroupVarName()) ?>"
value="<?php echo $block->escapeHtml($block->getStoreGroupId()) ?>"
onchange="switchScope(this);"<?php echo $block->escapeHtml($block->getUiId()) ?> />
data-role="store-group-id" data-param="<?= $block->escapeHtml($block->getStoreGroupVarName()) ?>"
value="<?= $block->escapeHtml($block->getStoreGroupId()) ?>"
onchange="switchScope(this);"<?= $block->escapeHtml($block->getUiId()) ?> />
<input type="hidden" name="website_switcher" id="website_switcher"
data-role="website-id" data-param="<?php echo $block->escapeHtml($block->getWebsiteVarName()) ?>"
value="<?php echo $block->escapeHtml($block->getWebsiteId()) ?>"
onchange="switchScope(this);"<?php echo $block->escapeHtml($block->getUiId()) ?> />
data-role="website-id" data-param="<?= $block->escapeHtml($block->getWebsiteVarName()) ?>"
value="<?= $block->escapeHtml($block->getWebsiteId()) ?>"
onchange="switchScope(this);"<?= $block->escapeHtml($block->getUiId()) ?> />
<button
type="button"
class="admin__action-dropdown"
data-mage-init='{"dropdown":{}}'
data-toggle="dropdown"
aria-haspopup="true"
id="store-change-button">
<?php echo $block->escapeHtml($block->getCurrentSelectionName()) ?>
<?= $block->escapeHtml($block->getCurrentSelectionName()) ?>
</button>
<ul class="dropdown-menu" data-role="stores-list">
<?php if ($block->hasDefaultOption()): ?>
<li class="store-switcher-all <?php if ( ! ($block->getDefaultSelectionName() != $block->getCurrentSelectionName())) {
echo "disabled";
} ?> <?php if ( ! $block->hasScopeSelected()) {
?> current<?php
} ?>">
<li class="store-switcher-all <?php if (! ($block->getDefaultSelectionName() != $block->getCurrentSelectionName())) { echo "disabled"; } ?> <?php if (! $block->hasScopeSelected()) { ?> current<?php } // phpcs:ignore Generic.Files.LineLength.TooLong ?>">
<?php if ($block->getDefaultSelectionName() != $block->getCurrentSelectionName()) {
?>
<a data-role="store-view-id" data-value="" href="#">
<?php echo $block->escapeHtml($block->getDefaultSelectionName()) ?>
<?= $block->escapeHtml($block->getDefaultSelectionName()) ?>
</a>
<?php
} else {
?>
<span><?php echo $block->escapeHtml($block->getDefaultSelectionName()) ?></span>
<span><?= $block->escapeHtml($block->getDefaultSelectionName()) ?></span>
<?php
} ?>
</li>
Expand All @@ -56,67 +51,55 @@
<?php foreach ($block->getStores($group) as $store): ?>
<?php if ($showWebsite == false): ?>
<?php $showWebsite = true; ?>
<li class="store-switcher-website <?php if ( ! ($block->isWebsiteSwitchEnabled() && ! $block->isWebsiteSelected($website))) {
echo "disabled";
} ?> <?php if ($block->isWebsiteSelected($website)) {
?> current<?php
} ?>">
<?php if ($block->isWebsiteSwitchEnabled() && ! $block->isWebsiteSelected($website)) {
?>
<a data-role="website-id" data-value="<?php echo $block->escapeHtml($website->getId()) ?>" href="#">
<?php echo $block->escapeHtml($website->getName()) ?>
<li class="store-switcher-website <?php if (! ($block->isWebsiteSwitchEnabled() && ! $block->isWebsiteSelected($website))) { echo "disabled"; } ?> <?php if ($block->isWebsiteSelected($website)) { ?> current<?php } // phpcs:ignore Generic.Files.LineLength.TooLong ?>">
<?php if ($block->isWebsiteSwitchEnabled() && ! $block->isWebsiteSelected($website)) { // phpcs:ignore Generic.Files.LineLength.TooLong ?>
<a data-role="website-id"
data-value="<?= $block->escapeHtml($website->getId()) ?>" href="#">
<?= $block->escapeHtml($website->getName()) ?>
</a>
<?php
} else {
?>
<span><?php echo $block->escapeHtml($website->getName()) ?></span>
<span><?= $block->escapeHtml($website->getName()) ?></span>
<?php
} ?>
</li>
<?php endif; ?>
<?php if ($showGroup == false): ?>
<?php $showGroup = true; ?>
<li class="store-switcher-store <?php if ( ! ($block->isStoreGroupSwitchEnabled() && ! $block->isStoreGroupSelected($group))) {
echo "disabled";
} ?> <?php if ($block->isStoreGroupSelected($group)) {
?> current<?php
} ?>">
<?php if ($block->isStoreGroupSwitchEnabled() && ! $block->isStoreGroupSelected($group)) {
?>
<a data-role="store-group-id" data-value="<?php echo $block->escapeHtml($group->getId()) ?>" href="#">
<?php echo $block->escapeHtml($group->getName()) ?>
<li class="store-switcher-store <?php if (! ($block->isStoreGroupSwitchEnabled() && ! $block->isStoreGroupSelected($group))) { echo "disabled"; } ?> <?php if ($block->isStoreGroupSelected($group)) { ?> current<?php } // phpcs:ignore Generic.Files.LineLength.TooLong ?>">
<?php if ($block->isStoreGroupSwitchEnabled() && ! $block->isStoreGroupSelected($group)) { // phpcs:ignore Generic.Files.LineLength.TooLong ?>
<a data-role="store-group-id"
data-value="<?= $block->escapeHtml($group->getId()) ?>" href="#">
<?= $block->escapeHtml($group->getName()) ?>
</a>
<?php
} else {
?>
<span><?php echo $block->escapeHtml($group->getName()) ?></span>
<span><?= $block->escapeHtml($group->getName()) ?></span>
<?php
} ?>
</li>
<?php endif; ?>
<li class="store-switcher-store-view <?php if ( ! ($block->isStoreSwitchEnabled() && ! $block->isStoreSelected($store))) {
echo "disabled";
} ?> <?php if ($block->isStoreSelected($store)) {
?> current<?php
} ?>">
<?php if ($block->isStoreSwitchEnabled() && ! $block->isStoreSelected($store)) {
?>
<a data-role="store-view-id" data-value="<?php echo $block->escapeHtml($store->getId()) ?>" href="#">
<?php echo $block->escapeHtml($store->getName()) ?>
<li class="store-switcher-store-view <?php if (! ($block->isStoreSwitchEnabled() && ! $block->isStoreSelected($store))) { echo "disabled"; } ?> <?php if ($block->isStoreSelected($store)) { ?> current<?php } // phpcs:ignore Generic.Files.LineLength.TooLong ?>">
<?php if ($block->isStoreSwitchEnabled() && ! $block->isStoreSelected($store)) { ?>
<a data-role="store-view-id"
data-value="<?= $block->escapeHtml($store->getId()) ?>" href="#">
<?= $block->escapeHtml($store->getName()) ?>
</a>
<?php
} else {
?>
<span><?php echo $block->escapeHtml($store->getName()) ?></span>
<span><?= $block->escapeHtml($store->getName()) ?></span>
<?php
} ?>
</li>
<?php endforeach; ?>
<?php endforeach; ?>
<?php endforeach; ?>
<?php if ($block->getShowManageStoresLink() && $block->getAuthorization()->isAllowed('Magento_Backend::store')): ?>
<?php if ($block->getShowManageStoresLink() && $block->getAuthorization()->isAllowed('Magento_Backend::store')): // phpcs:ignore Generic.Files.LineLength.TooLong ?>
<li class="dropdown-toolbar">
<a href="<?php echo $block->escapeUrl($block->getUrl('*/system_store')) ?>"><?php echo $block->escapeHtml(__('Stores Configuration')) ?></a>
<a href="<?= $block->escapeUrl($block->getUrl('*/system_store')) ?>"><?= $block->escapeHtml(__('Stores Configuration')) // phpcs:ignore Generic.Files.LineLength.TooLong ?></a>
</li>
<?php endif; ?>
</ul>
Expand All @@ -128,13 +111,15 @@
{
"*": {
"Dotdigitalgroup_Email/js/switcher":{
"getUseConfirm":"<?php echo (int)$block->getUseConfirm(); ?>",
"getContent":"<?php echo $block->escapeHtml(__('Please confirm scope switching. All data that hasn\'t been saved will be lost.')) ?>",
"objVal":"<?php echo $block->escapeHtml($block->getStoreId()) ?>",
"isUsingIframe":"<?php echo $block->escapeHtml($block->isUsingIframe()); ?>",
"getSwitchUrl":"<?php echo $block->escapeUrl($block->getSwitchUrl()) ?>"
"getUseConfirm":"<?= (int)$block->getUseConfirm(); ?>",
"getContent":"<?= $block->escapeHtml(__('Please confirm scope switching. All data that hasn\'t been saved will be lost.')) // phpcs:ignore Generic.Files.LineLength.TooLong ?>",
"objVal":"<?= $block->escapeHtml($block->getStoreId()) ?>",
"isUsingIframe":"<?= $block->escapeHtml($block->isUsingIframe()); ?>",
"getSwitchUrl":"<?= $block->escapeUrl($block->getSwitchUrl()) ?>"
}
}
}
</script>
<?php endif; ?>
<?php endif;
// phpcs:enable
?>
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
<?php /** @var \Dotdigitalgroup\Email\Block\Adminhtml\Config\Createaddressbook $block */?>
<?php
/** @var \Dotdigitalgroup\Email\Block\Adminhtml\Config\Createaddressbook $block */
// phpcs:disable Magento2.Legacy.EscapeMethodsOnBlockClass
?>
<div class="actions actions-create-addressbook">
<div id="validation_result" class="message-validation hidden"></div>
<button class="action-create-addressbook" type="button" id="<?= $block->getHtmlId() ?>">
Expand All @@ -14,4 +17,5 @@
}
}
}
</script>
</script>
<?php // phpcs:enable ?>
8 changes: 6 additions & 2 deletions view/adminhtml/templates/system/config/createdatafield.phtml
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
<?php /** @var \Dotdigitalgroup\Email\Block\Adminhtml\Config\Createdatafield $block */?>
<?php
/** @var \Dotdigitalgroup\Email\Block\Adminhtml\Config\Createdatafield $block */
// phpcs:disable Magento2.Legacy.EscapeMethodsOnBlockClass
?>
<div class="actions actions-create-datafield">
<div id="validation_result" class="message-validation hidden"></div>
<button class="action-create-datafield" type="button" id="<?= $block->getHtmlId() ?>">
Expand All @@ -14,4 +17,5 @@
}
}
}
</script>
</script>
<?php // phpcs:enable ?>
6 changes: 5 additions & 1 deletion view/adminhtml/templates/system/config/reportlink.phtml
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
<?php /** @var \Dotdigitalgroup\Email\Block\Adminhtml\Config\Report\Order $block */?>
<?php
/** @var \Dotdigitalgroup\Email\Block\Adminhtml\Config\Report\Order $block */
// phpcs:disable Magento2.Legacy.EscapeMethodsOnBlockClass
?>
<div>
<a href="<?= $block->escapeHtml($block->getLink()); ?>">
<?= $block->escapeHtml(__('Click here for status')); ?>
</a>
</div>
<?php // phpcs:enable ?>
Loading

0 comments on commit fd40657

Please sign in to comment.