Skip to content

Commit 5066da8

Browse files
nathanjosiahnathanjosiah
committed
AC-2461: Implement email and newsletter template compatibility CLI check
1 parent fa6a944 commit 5066da8

File tree

6 files changed

+146
-102
lines changed

6 files changed

+146
-102
lines changed

‎app/code/Magento/Email/Console/Command/DatabaseTemplateCompatibilityCommand.php

Lines changed: 45 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,9 @@
77

88
namespace Magento\Email\Console\Command;
99

10-
use Magento\Email\Model\ResourceModel\Template\Collection;
10+
use Magento\Email\Model\AbstractTemplate;
1111
use Magento\Email\Model\ResourceModel\Template\CollectionFactory;
12-
use Magento\Email\Model\Template;
13-
use Magento\Email\Model\Template\CompatibilityChecker;
12+
use Magento\Email\Model\Template\VariableCompatibilityChecker;
1413
use Symfony\Component\Console\Command\Command;
1514
use Symfony\Component\Console\Input\InputInterface;
1615
use Symfony\Component\Console\Output\OutputInterface;
@@ -19,21 +18,26 @@
1918
/**
2019
* Scan DB templates for directive incompatibilities
2120
*/
22-
class DbTemplateCheckCommand extends Command
21+
class DatabaseTemplateCompatibilityCommand extends Command
2322
{
2423
/**
2524
* @var CollectionFactory
2625
*/
2726
private CollectionFactory $templateCollection;
2827

2928
/**
30-
* @var CompatibilityChecker
29+
* @var VariableCompatibilityChecker
3130
*/
32-
private CompatibilityChecker $compatibilityChecker;
31+
private VariableCompatibilityChecker $compatibilityChecker;
32+
33+
/**
34+
* @var bool
35+
*/
36+
protected bool $hasErrors = false;
3337

3438
public function __construct(
39+
VariableCompatibilityChecker $compatibilityChecker,
3540
CollectionFactory $templateCollection,
36-
CompatibilityChecker $compatibilityChecker,
3741
string $name = null
3842
) {
3943
parent::__construct($name);
@@ -46,52 +50,54 @@ public function __construct(
4650
*/
4751
protected function configure()
4852
{
49-
$this->setName('setup:email-override-compatibility-check')
50-
->setDescription('Scans email template overrides for potential compatibility issues');
51-
52-
parent::configure();
53+
$this->setName('dev:email:override-compatibility-check')
54+
->setDescription('Scans email template overrides for potential variable usage compatibility issues');
5355
}
5456

5557
/**
56-
* Executes compatibility checker command
57-
*
58-
* @param InputInterface $input
59-
* @param OutputInterface $output
60-
* @return int|null
58+
* @inheritDoc
6159
*/
6260
protected function execute(InputInterface $input, OutputInterface $output)
6361
{
64-
/** @var Collection $collection */
6562
$collection = $this->templateCollection->create();
6663
$collection->load();
6764

68-
$hasErrors = false;
65+
$this->hasErrors = false;
6966
foreach ($collection as $template) {
70-
/** @var Template $template */
71-
$errors = $this->compatibilityChecker->getCompatibilityIssues($template->getTemplateText());
72-
if (!empty($errors)) {
73-
$hasErrors = true;
74-
$templateName = $template->getTemplateCode();
75-
$output->writeln(
76-
'<error>Template "' . $templateName . '" has the following compatibility issues:</error>'
77-
);
78-
$this->renderErrors($output, $errors);
79-
}
80-
$errors = $this->compatibilityChecker->getCompatibilityIssues($template->getTemplateSubject());
81-
if (!empty($errors)) {
82-
$hasErrors = true;
83-
$templateName = $template->getTemplateCode();
84-
$output->writeln(
85-
'<error>Template "' . $templateName . '" subject has the following compatibility issues:</error>'
86-
);
87-
$this->renderErrors($output, $errors);
88-
}
67+
$this->checkTemplate($template, $output);
8968
}
9069

91-
if (!$hasErrors) {
70+
if (!$this->hasErrors) {
9271
$output->writeln('<info>No errors detected</info>');
9372
}
94-
return $hasErrors ? Cli::RETURN_FAILURE : Cli::RETURN_SUCCESS;
73+
return $this->hasErrors ? Cli::RETURN_FAILURE : Cli::RETURN_SUCCESS;
74+
}
75+
76+
/**
77+
* Check the given template for compatibility issues
78+
*
79+
* @param AbstractTemplate $template
80+
* @param OutputInterface $output
81+
*/
82+
protected function checkTemplate(AbstractTemplate $template, OutputInterface $output): void
83+
{
84+
$errors = $this->compatibilityChecker->getCompatibilityIssues($template->getTemplateText());
85+
$templateName = $template->getTemplateCode();
86+
if (!empty($errors)) {
87+
$this->hasErrors = true;
88+
$output->writeln(
89+
'<error>Template "' . $templateName . '" has the following compatibility issues:</error>'
90+
);
91+
$this->renderErrors($output, $errors);
92+
}
93+
$errors = $this->compatibilityChecker->getCompatibilityIssues($template->getTemplateSubject());
94+
if (!empty($errors)) {
95+
$this->hasErrors = true;
96+
$output->writeln(
97+
'<error>Template "' . $templateName . '" subject has the following compatibility issues:</error>'
98+
);
99+
$this->renderErrors($output, $errors);
100+
}
95101
}
96102

97103
/**

‎app/code/Magento/Email/Model/Template/VariableCompatibilityChecker.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
/**
1414
* Scan an email template for compatibility with the strict resolver
1515
*/
16-
class CompatibilityChecker
16+
class VariableCompatibilityChecker
1717
{
1818
private const CONSTRUCTION_DEPEND_PATTERN = '/{{depend\s*(.*?)}}(.*?){{\\/depend\s*}}/si';
1919
private const CONSTRUCTION_IF_PATTERN = '/{{if\s*(.*?)}}(.*?)({{else}}(.*?))?{{\\/if\s*}}/si';

‎app/code/Magento/Email/etc/di.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@
9696
<type name="Magento\Framework\Console\CommandListInterface">
9797
<arguments>
9898
<argument name="commands" xsi:type="array">
99-
<item name="emailTemplateCheck" xsi:type="object">Magento\Email\Console\Command\DbTemplateCheckCommand</item>
99+
<item name="emailTemplateCheck" xsi:type="object">Magento\Email\Console\Command\DatabaseTemplateCompatibilityCommand</item>
100100
</argument>
101101
</arguments>
102102
</type>

‎app/code/Magento/Newsletter/Console/Command/TemplateCheckCommand.php

Lines changed: 16 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -7,49 +7,42 @@
77

88
namespace Magento\Newsletter\Console\Command;
99

10-
use Magento\Newsletter\Model\ResourceModel\Template\Collection;
10+
use Magento\Email\Console\Command\DatabaseTemplateCompatibilityCommand;
11+
use Magento\Email\Model\ResourceModel\Template\CollectionFactory as EmailCollectionFactory;
1112
use Magento\Newsletter\Model\ResourceModel\Template\CollectionFactory;
12-
use Magento\Newsletter\Model\Template;
13-
use Magento\Email\Model\Template\CompatibilityChecker;
14-
use Symfony\Component\Console\Command\Command;
13+
use Magento\Email\Model\Template\VariableCompatibilityChecker;
1514
use Symfony\Component\Console\Input\InputInterface;
1615
use Symfony\Component\Console\Output\OutputInterface;
1716
use Magento\Framework\Console\Cli;
1817

1918
/**
2019
* Scan DB templates for directive incompatibilities
2120
*/
22-
class TemplateCheckCommand extends Command
21+
class TemplateCheckCommand extends DatabaseTemplateCompatibilityCommand
2322
{
2423
/**
2524
* @var CollectionFactory
2625
*/
2726
private CollectionFactory $templateCollection;
2827

29-
/**
30-
* @var CompatibilityChecker
31-
*/
32-
private CompatibilityChecker $compatibilityChecker;
33-
3428
public function __construct(
35-
CollectionFactory $templateCollection,
36-
CompatibilityChecker $compatibilityChecker,
29+
VariableCompatibilityChecker $compatibilityChecker,
30+
EmailCollectionFactory $templateCollection,
31+
CollectionFactory $newsletterCollectionFactory,
3732
string $name = null
3833
) {
39-
parent::__construct($name);
40-
$this->templateCollection = $templateCollection;
41-
$this->compatibilityChecker = $compatibilityChecker;
34+
parent::__construct($compatibilityChecker, $templateCollection, $name);
35+
36+
$this->templateCollection = $newsletterCollectionFactory;
4237
}
4338

4439
/**
4540
* @inheritdoc
4641
*/
4742
protected function configure()
4843
{
49-
$this->setName('setup:newsletter-compatibility-check')
50-
->setDescription('Scans newsletter templates for potential compatibility issues');
51-
52-
parent::configure();
44+
$this->setName('dev:email:newsletter-compatibility-check')
45+
->setDescription('Scans newsletter templates for potential variable usage compatibility issues');
5346
}
5447

5548
/**
@@ -61,53 +54,17 @@ protected function configure()
6154
*/
6255
protected function execute(InputInterface $input, OutputInterface $output)
6356
{
64-
/** @var Collection $collection */
6557
$collection = $this->templateCollection->create();
6658
$collection->load();
6759

68-
$hasErrors = false;
60+
$this->hasErrors = false;
6961
foreach ($collection as $template) {
70-
/** @var Template $template */
71-
$errors = $this->compatibilityChecker->getCompatibilityIssues($template->getTemplateText());
72-
if (!empty($errors)) {
73-
$hasErrors = true;
74-
$templateName = $template->getTemplateCode();
75-
$output->writeln(
76-
'<error>Newsletter "' . $templateName . '" has the following compatibility issues:</error>'
77-
);
78-
$this->renderErrors($output, $errors);
79-
}
80-
$errors = $this->compatibilityChecker->getCompatibilityIssues($template->getTemplateSubject());
81-
if (!empty($errors)) {
82-
$hasErrors = true;
83-
$templateName = $template->getTemplateCode();
84-
$output->writeln(
85-
'<error>Newsletter "' . $templateName . '" subject has the following compatibility issues:</error>'
86-
);
87-
$this->renderErrors($output, $errors);
88-
}
62+
$this->checkTemplate($template, $output);
8963
}
9064

91-
if (!$hasErrors) {
65+
if (!$this->hasErrors) {
9266
$output->writeln('<info>No errors detected</info>');
9367
}
94-
return $hasErrors ? Cli::RETURN_FAILURE : Cli::RETURN_SUCCESS;
95-
}
96-
97-
/**
98-
* Render given errors
99-
*
100-
* @param OutputInterface $output
101-
* @param array $errors
102-
*/
103-
private function renderErrors(OutputInterface $output, array $errors): void
104-
{
105-
foreach ($errors as $error) {
106-
$error = str_replace(PHP_EOL, PHP_EOL . ' ', $error);
107-
$output->writeln(
108-
'<error> - ' . $error . '</error>'
109-
);
110-
}
111-
$output->writeln('');
68+
return $this->hasErrors ? Cli::RETURN_FAILURE : Cli::RETURN_SUCCESS;
11269
}
11370
}

‎dev/tests/integration/testsuite/Magento/Email/Model/Template/VariableCompatibilityCheckerTest.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
declare(strict_types=1);
88

9-
namespace Magento\Email\Test\Unit\Model\Template;
9+
namespace Magento\Email\Model\Template;
1010

1111
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
1212
use PHPUnit\Framework\TestCase;
@@ -15,6 +15,11 @@ class VariableCompatibilityCheckerTest extends TestCase
1515
{
1616
public function testCompatibilityCheck()
1717
{
18-
$checker = new
18+
$objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
19+
/** @var VariableCompatibilityChecker $checker */
20+
$checker = $objectManager->get(VariableCompatibilityChecker::class);
21+
$errors = $checker->getCompatibilityIssues(file_get_contents(__DIR__ . '/../_files/variables_template.html'));
22+
23+
self::assertCount(20, $errors);
1924
}
2025
}

‎dev/tests/integration/testsuite/Magento/Email/Model/_files/variables_template.html

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
<!--
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
-->
7+
<div>{{var foo.good}}</div>
8+
<div>{{var foo.good|stillfine}}</div>
9+
<div>{{var foo.bad()}}</div>
10+
<div>{{var foo.bad()|alsobad}}</div>
11+
<div>{{var foo.getGood()}}</div>
12+
<div>{{var foo.foo.getGood()}}</div>
13+
<div>{{var foo.getGood().fine}}</div>
14+
<div>{{var foo.getGood().fine|alsofine}}</div>
15+
<div>{{var foo.bad($bad.param())}}</div>
16+
<div>{{var foo.bad.baz()}}</div>
17+
<div>{{var foo.undeclared.baz}}</div>
18+
<div>{{trans "foo %bar" bar=$foo.good.trans}}</div>
19+
<div>{{trans "foo %bar" bar=$foo.bad.trans()}}</div>
20+
<div>{{trans "foo %bar" bar="something"}}</div>
21+
<div>{{trans "foo %bar" bar="something" bad="$bad.bad()" something=$undeclared.var.error}}</div>
22+
<div>{{something "
23+
<blah>foo %bar</blah>blah
24+
" bar="something"
25+
}}</div>
26+
<div>{{something "
27+
<blah>foo %bar</blah>blah
28+
" bar="something" bad=$bad.multiline()
29+
}}</div>
30+
31+
{{if foo.goodif}}
32+
<div>{{var foo.goodif2}}</div>
33+
{{/if}}
34+
35+
{{if foo.goodif}}
36+
<div>{{var foo.goodif2}}</div>
37+
{{else}}
38+
<div>{{var foo.goodif3}}</div>
39+
{{/if}}
40+
41+
{{if foo.badif().bad}}
42+
<div>{{var foo.badif2()}}</div>
43+
{{/if}}
44+
45+
{{if foo.badif3()}}
46+
<div>{{var foo.badif4()}}</div>
47+
{{else}}
48+
<div>{{var foo.badif5()}}</div>
49+
{{/if}}
50+
51+
{{depend foo.gooddepend}}
52+
<div>{{var foo.gooddepend2}}</div>
53+
{{/depend}}
54+
55+
{{depend foo.badDepend().bad}}
56+
<div>{{var foo.baddepend2()}}</div>
57+
{{/depend}}
58+
59+
{{for item in foo.goodFor}}
60+
<div>{{var foo.goodFor}}</div>
61+
<div>{{var foo.goodFor|stillfine}}</div>
62+
<div>{{var foo.badFor()}}</div>
63+
<div>{{var foo.badFor()|alsobad}}</div>
64+
{{/for}}
65+
66+
{{for item in foo.getGoodFor()}}
67+
<div>loopy</div>
68+
{{/for}}
69+
70+
{{for item in foo.badForLoop()}}
71+
<div>this loop has a bad variable</div>
72+
{{/for}}
73+
74+
{{depend iusefilterslater}}
75+
{{var iusefilterslater|raw}}
76+
{{/depend}}

0 commit comments

Comments
 (0)