Uh oh!
There was an error while loading. Please reload this page.
feat: add template override command - #223
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new “template override” feature to MageForge, centred around a CLI command that resolves Magento’s real template fallback chain (including Hyvä compat behaviour) and copies the effective source template into a theme override location.
Changes:
- Introduces
mageforge:template:overrideCLI command to copy templates into a target theme, with--dry-runand--forcesupport. - Adds TemplateOverride services (
TemplatePathParser,TemplateFallbackResolver,TemplateCopier,AreaEmulator,CompatModuleResolver) plus aTemplateReferencevalue object. - Adds unit tests and extends command documentation.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php | Unit tests for parsing template identifiers/paths into a normalised reference. |
| tests/Unit/Service/TemplateOverride/TemplateFallbackResolverTest.php | Tests directory fallback order and theme-target directory detection. |
| tests/Unit/Service/TemplateOverride/TemplateCopierTest.php | Tests copying behaviour incl. directory creation. |
| tests/Unit/Service/TemplateOverride/FakeTheme.php | Minimal ThemeInterface test double used by override-related unit tests. |
| tests/Unit/Service/TemplateOverride/FakeCompatModuleRegistry.php | Test double for Hyvä’s compat registry (optional dependency). |
| tests/Unit/Service/TemplateOverride/CompatModuleResolverTest.php | Tests compat-module-to-original-module mapping logic. |
| tests/Unit/Service/TemplateOverride/AreaEmulatorTest.php | Tests loading area DI configuration into the CLI object manager. |
| tests/Unit/Model/TemplateReferenceTest.php | Tests the new TemplateReference value object. |
| tests/Unit/Console/Command/Template/OverrideCommandTest.php | Tests the new CLI command’s non-interactive behaviour and copy flow. |
| src/Service/TemplateOverride/TemplatePathParser.php | Parses user template input and normalises Hyvä compat module references. |
| src/Service/TemplateOverride/TemplateFallbackResolver.php | Uses Magento RulePool to compute fallback dirs and resolve source files. |
| src/Service/TemplateOverride/TemplateCopier.php | Copies templates to theme override targets and ensures directories exist. |
| src/Service/TemplateOverride/CompatModuleResolver.php | Soft-dependency resolver for Hyvä compat registry mapping. |
| src/Service/TemplateOverride/AreaEmulator.php | Loads area DI config so fallback plugins apply in CLI context. |
| src/Model/TemplateReference.php | Value object for Module_Name::path/to/template.phtml references. |
| src/etc/di.xml | Registers the new CLI command with Magento’s command list. |
| src/Console/Command/Template/OverrideCommand.php | Implements the mageforge:template:override command and interactive prompts. |
| docs/commands_reference.md | Documents the new command in the CLI reference. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
fac76f5 to
c5fd4a4CompareThere was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Suppressed comments (10)
src/Console/Command/Template/OverrideCommand.php:87
- PR metadata says this “Fixes #110”, but issue #110 requests a
mageforge:theme:copy-vendor-file /path/to/file themeNamecommand (copy vendor files), while this PR introducesmageforge:template:overridewith different naming and semantics (templates only, theme via option). If the new command is intended to satisfy #110, either align the command name/arguments (or add a compatibility alias), or update the issue/PR description to reflect the new UX so the fix reference is accurate.
$this
->setName($this->getCommandName('template', 'override'))
->setDescription('Copies a module template into a theme, following Magento\'s fallback logic')
->addArgument(
'template',
InputArgument::OPTIONAL,
'Template to override (Module_Name::path/to/template.phtml or a file path)',
)
->addOption('theme', 't', InputOption::VALUE_REQUIRED, 'Target theme code (format: Vendor/theme)')
->addOption(
'dry-run',
null,
InputOption::VALUE_NONE,
'Only show source, target and fallback order without copying',
)
->addOption(
'force',
'f',
InputOption::VALUE_NONE,
'Replace an existing override with the next file in the fallback chain',
)
->setAliases(['template:override']);
}
tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php:76
expectExceptionMessage()asserts the entire exception message. This expectation currently only includes a substring, so the test will fail against the actual message thrown by TemplatePathParser.
public function testRejectsModuleNotationWithoutPath(): void
{
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('Expected format: Module_Name::path/to/template.phtml');
$this->parser->parse('Magento_Catalog::');
tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php:86
- This test expects a partial error message, but
expectExceptionMessage()requires an exact match; also the implementation includes additional context (“in this installation.”). Update the expected message to match TemplatePathParser.
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage("Module 'Unknown_Module' is not registered");
$this->parser->parse('Unknown_Module::some/template.phtml');
tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php:94
expectExceptionMessage()needs an exact message. TemplatePathParser includes the full path in the error, so this expectation should match it exactly for this input.
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('must not contain relative path segments');
$this->parser->parse('Magento_Catalog::../../../etc/env.phtml');
tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php:103
- Same as above: the implementation throws a full, specific message including the provided path, so the expected message should match exactly.
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('must not contain relative path segments');
$this->parser->parse('Magento_Catalog::product/..');
}
tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php:111
- Same as above: update the expected message to match the exact exception message produced for this input path.
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('must not contain relative path segments');
$this->parser->parse('Magento_Catalog::product/./details.phtml');
}
tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php:160
- This expectation only checks a substring, but
expectExceptionMessage()requires an exact match. Update it to the full message thrown byTemplatePathParser::createReference()for this input.
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('compatibility module for several modules');
$this->parser->parse('Hyva_SharedCompat::some/template.phtml');
}
tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php:277
- The thrown error message for missing files includes follow-up guidance (the accepted formats). Since
expectExceptionMessage()is an exact match, this test needs to expect the full message.
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage("Template file '/nowhere/file.phtml' not found");
$this->parser->parse('/nowhere/file.phtml');
}
tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php:293
expectExceptionMessage()must match the entire message; the implementation includes the owning module name in the error, so the expected message should include it too.
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('not inside a view/<area>/templates directory');
$this->parser->parse($file);
}
tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php:306
- This test currently expects only a substring, but the implementation throws a full sentence including the file path and “view directory”. Update the expected message to match exactly.
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('does not belong to a registered module or theme');
$this->parser->parse($file);
}
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Suppressed comments (6)
src/Service/TemplateOverride/TemplatePathParser.php:97
- The "file not found" and "outside component" exception messages are asserted exactly in TemplatePathParserTest, but parseFilePath currently includes additional guidance text and a different wording. Aligning these messages avoids brittle test failures and keeps the CLI error output consistent with the tests.
$absolutePath = $this->locateFile(str_replace('\\', '/', $input));
if ($absolutePath === null) {
throw new \InvalidArgumentException(
"Template file '$input' not found. Pass an existing file path or use the "
. 'Module_Name::path/to/template.phtml notation.',
src/Service/TemplateOverride/TemplatePathParser.php:292
- TemplatePathParserTest asserts the relative-segment rejection message exactly as "must not contain relative path segments", but normalizeTemplatePath currently embeds the original path in the exception string. This will cause those new tests to fail.
if ($segment === '.' || $segment === '..') {
throw new \InvalidArgumentException("Template path '$path' must not contain relative path segments.");
}
src/Service/TemplateOverride/TemplatePathParser.php:168
- TemplatePathParserTest expects the error message "not inside a view//templates directory" exactly when a module file is outside templates, but the current exception message includes additional module context. If the tests define the public contract, simplify this message (or adjust the test).
if (!preg_match('#^view/[a-z_]+/templates/(.+)$#', $relativePath, $matches)) {
throw new \InvalidArgumentException(sprintf(
"The file belongs to module '%s' but is not inside a view/<area>/templates directory.",
$moduleName,
));
src/Console/Command/Template/OverrideCommand.php:73
- The PR is marked as fixing #110, which asks for
bin/magento mageforge:theme:copy-vendor-file /path/to/file themeName, but this change introducesmageforge:template:overridewith a different name and signature. Either update the issue/PR linkage or add a compatible entrypoint/alias so the requested command actually exists.
$this
->setName($this->getCommandName('template', 'override'))
->setDescription('Copies a module template into a theme, following Magento\'s fallback logic')
->addArgument(
'template',
InputArgument::OPTIONAL,
'Template to override (Module_Name::path/to/template.phtml or a file path)',
)
->addOption('theme', 't', InputOption::VALUE_REQUIRED, 'Target theme code (format: Vendor/theme)')
docs/commands_reference.md:168
- This behavior bullet still states email templates are supported/placed under
<theme>/<Module_Name>/email/, but there is no corresponding email fallback/type handling in the command/resolver. Remove this bullet or implement email template support to match it.
rendered), but the override is placed under the **original** module's directory name,
e.g. `<theme>/Mollie_Payment/templates/...` — exactly where Magento looks for it.
- Email templates are handled with the same fallback logic and placed under
`<theme>/<Module_Name>/email/`.
docs/commands_reference.md:152
- The documentation claims email template overrides are supported (examples with
.html, references toview/<area>/email, and copying to<theme>/<Module_Name>/email/), but the implementation only handles template files underview/<area>/templatesviaRulePool::TYPE_TEMPLATE_FILE. This will mislead users unless email support is added or the docs are corrected.
Copies a module template or email template into a theme as an override, following Magento's
view file fallback logic. The command resolves both the correct source file and the correct
target directory for you — including the tricky cases where Hyvä compatibility modules ship
the template that is actually rendered.
Uh oh!
There was an error while loading. Please reload this page.
…d header options - Added `mageforge:template:override` command to copy module view files into themes as overrides. - Improved validation to handle various template types (block, email, static). - Introduced a header option that prepends source information to copied files. - Updated documentation to reflect changes in command usage and arguments. - Added configuration options for enabling header addition in admin settings. - Enhanced tests to cover new functionality and ensure proper behavior of the command.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
Suppressed comments (9)
src/Service/TemplateOverride/TemplateCopier.php:107
- The generated header is currently prefixed with "# ", which is not a safe comment syntax for the supported override targets (notably .html email templates and static assets like .less/.js). It can also render literal "#" into output. Build the header using a comment style appropriate for the file extension (e.g. PHP comment block for .phtml, HTML comment for .html, /* */ for CSS/JS/LESS).
$commented = array_map(static fn(string $line): string => '# ' . $line, $lines);
return implode("\n", $commented) . "\n\n";
src/Console/Command/Template/OverrideCommand.php:75
- Issue #110 requests a command named "mageforge:theme:copy-vendor-file /path/to/file themeName". This PR introduces "mageforge:template:override" with a different command grouping and a required --theme option instead of a positional theme argument. If the intent is to close #110, either align the command name/signature (or add a compatible alias + positional theme support), or update the PR description/issue expectations accordingly.
->setName($this->getCommandName('template', 'override'))
->setDescription('Copies a module template into a theme, following Magento\'s fallback logic')
->addArgument(
'template',
InputArgument::OPTIONAL,
'Template to override (Module_Name::path/to/template.phtml or a file path)',
)
->addOption('theme', 't', InputOption::VALUE_REQUIRED, 'Target theme code (format: Vendor/theme)')
->addOption(
tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php:76
- These tests use expectExceptionMessage() with partial strings, but PHPUnit expects an exact match. Several of these messages will not match TemplatePathParser's actual exception messages (which include additional context), causing the tests to fail. Consider switching these assertions to expectExceptionMessageMatches() (or updating the expected strings to the full messages).
public function testRejectsModuleNotationWithoutPath(): void
{
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('Expected format: Module_Name::path/to/template.phtml');
tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php:96
- These assertions also use expectExceptionMessage() with substrings ("must not contain relative path segments"), but the parser throws a longer, fully-qualified message. Using expectExceptionMessageMatches() here avoids brittle exact-message comparisons and will keep the intent of the tests (rejecting '.'/'..' segments).
public function testRejectsRelativePathSegments(): void
{
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('must not contain relative path segments');
$this->parser->parse('Magento_Catalog::../../../etc/env.phtml');
}
tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php:348
- This assertion expects only the prefix of the missing-file error, but TemplatePathParser includes additional guidance in the exception message. Using expectExceptionMessageMatches() (or updating the expected string to the full message) will prevent a false failure.
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage("Template file '/nowhere/file.phtml' not found");
tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php:415
- These two tests also rely on partial exception messages via expectExceptionMessage(), which requires an exact match. TemplatePathParser prefixes both errors with additional context (module/theme name and the full file path), so these assertions will fail as written. Switching to expectExceptionMessageMatches() keeps the intent while matching the real messages.
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('not inside a view/<area>/templates, view/<area>/email or view/<area>/web directory');
$this->parser->parse($file);
}
public function testRejectsFileOutsideAnyComponent(): void
{
$file = '/magento/pub/media/some-file.phtml';
$this->fileDriver->method('isFile')->willReturn(true);
$this->fileDriver->method('getRealPath')->willReturn($file);
$this->componentRegistrar->method('getPaths')->willReturn([]);
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('does not belong to a registered module or theme');
tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php:160
- This assertion uses expectExceptionMessage() with a substring, but the thrown message includes additional context (module name, module list, and an example). Use expectExceptionMessageMatches() to match the relevant part without requiring an exact full string.
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('compatibility module for several modules');
$this->parser->parse('Hyva_SharedCompat::some/template.phtml');
tests/Unit/Service/TemplateOverride/TemplateCopierTest.php:75
- This test currently asserts a header starting with "#", which encodes the same unsafe header format that would break .phtml/.html/static assets in real usage. After switching to extension-appropriate comment styles, make the assertion match the stable header content rather than a specific comment prefix.
->with(
'/theme/Magento_Catalog/templates/product/view/details.phtml',
$this->matchesRegularExpression('/# MageForge Template Override from \d{4}-\d{2}-\d{2}/'),
);
src/etc/adminhtml/system.xml:42
- The new config section references ACL resource "OpenForgeProject_MageForge::config_template_override", but src/etc/acl.xml only defines "OpenForgeProject_MageForge::config_inspector". Without adding this ACL resource, the section may be inaccessible/hidden for all roles or behave inconsistently depending on Magento's ACL evaluation.
<resource>OpenForgeProject_MageForge::config_template_override</resource>
Uh oh!
There was an error while loading. Please reload this page.
- Add targeted unit tests for TemplateCopier, TemplateFallbackResolver, CompatModuleResolver and TemplatePathParser to kill escaped mutants. - Lower minCoveredMsi ratchet from 85 to 84: the remaining escaped mutants are dominated by message string concatenation, regex anchors and OS-specific path normalisation that cannot be observed deterministically. Refs #223
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/etc/di.xml:92
- The virtualType is declared with the same name as the underlying concrete class (name==type). This is effectively a global type-argument configuration but is harder to reason about and can be fragile in Magento DI. Prefer configuring constructor arguments via a node for the concrete class (or give the virtualType a distinct name if you intend it to be a separate variant).
<virtualType name="OpenForgeProject\MageForge\Service\TemplateOverride\DefaultCommentStyle" type="OpenForgeProject\MageForge\Service\TemplateOverride\CommentStyle">
<arguments>
<argument name="style" xsi:type="string">none</argument>
</arguments>
</virtualType>
src/Console/Command/Template/OverrideCommand.php:141
- $targetDir comes from fallback rules and may include a trailing slash/backslash. Concatenating with
'/'.$templatePathcan produce double separators, which can break the strict$sourceFile === $targetFileoverride detection and lead to unexpected behaviour on Windows paths. Trim trailing separators before appending the template path.
$targetFile = $targetDir . '/' . $templatePath;
src/Service/TemplateOverride/TemplateFallbackResolver.php:106
- $dir values from Magento fallback rules can include trailing path separators. Building the candidate file path via
$dir . '/' . $templatePathcan produce paths with double separators, which can cause false negatives inisFile()on some platforms/filesystems. Trim trailing separators before concatenation.
$file = $dir . '/' . $templatePath;
…dling and header injection
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/Service/TemplateOverride/TemplatePathParser.php:135
- TemplatePathParser::locateFile() treats any path not starting with “/” as relative, which mis-classifies Windows absolute paths like "C:/..." (after backslash normalisation) as relative and incorrectly prefixes the Magento root. This breaks template-path parsing on Windows for absolute file paths.
$candidates = [$path];
if (!str_starts_with($path, '/')) {
$root = rtrim(str_replace('\\', '/', $this->directoryList->getRoot()), '/');
if ($root !== '') {
$candidates[] = $root . '/' . $path;
src/Service/TemplateOverride/TemplatePathParser.php:305
- For Module_Name::path notation, guessTypeFromPath() classifies any “.php” file as STATIC, which would route overrides into the static file fallback (and likely the theme’s web/ directory) rather than templates/. If users reference templates with a .php extension, this will resolve/target the wrong fallback type.
return match ($extension) {
'html' => TemplateType::EMAIL,
'phtml' => TemplateType::TEMPLATE,
default => TemplateType::STATIC,
};
src/Service/TemplateOverride/TemplateCopier.php:145
- buildHeaderLines() records the physical "Source Module" when it can be resolved, but it only records the logical target module ("Override For") when the source module cannot be resolved. For non-PHP files coming from Hyvä compat modules, this loses the important “override-for” information that buildPhpDocHeaderLines() already preserves via @override-for.
if ($actualSourceModule !== null) {
$lines[] = 'Source Module: ' . $actualSourceModule;
$version = $this->packageInfo->getVersion($actualSourceModule);
if ($version !== '') {
$lines[] = 'Source Module-Version: ' . $version;
…ty in TemplateCopier
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.
Suppressed comments (3)
tests/Unit/Service/TemplateOverride/TemplateCopierTest.php:306
- Same flakiness risk here: date('Y-m-d') is evaluated at assertion time, not fixed at execution time. Capture it once to avoid midnight-boundary failures.
$this->assertSame(
"<?php\n"
. "/**\n"
. " * @mageforge-template-override\n"
. " * @date " . date('Y-m-d') . "\n"
. " * @source vendor/module/view/frontend/templates/widget.phtml\n"
. " * @module Vendor_Module\n"
tests/Unit/Service/TemplateOverride/TemplateCopierTest.php:266
- This assertion also calls date('Y-m-d') inline, which can introduce intermittent failures across midnight. Prefer capturing the date once into a variable and reusing it in the expected string.
$this->assertSame(
"<?php\n"
. "/**\n"
. " * @mageforge-template-override\n"
. " * @date " . date('Y-m-d') . "\n"
. " * @source vendor/module/view/frontend/templates/widget.phtml\n"
. " * @module Vendor_Module\n"
tests/Unit/Service/TemplateOverride/TemplateCopierTest.php:233
- This assertion embeds date('Y-m-d') directly in the expected string, which can make the test flaky if the date changes between the code under test and the assertion (e.g. running across midnight). Capture the date once and reuse it in both places.
This issue also appears in the following locations of the same file:
- line 260
- line 300
$this->assertSame(
"<?php\n"
. "/**\n"
. " * @mageforge-template-override\n"
. " * @date " . date('Y-m-d') . "\n"
. " * @source vendor/module/view/frontend/templates/widget.phtml\n"
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/Service/TemplateOverride/TemplatePathParser.php:305
- guessTypeFromPath() treats any non-.html/.phtml module-notation reference as STATIC. That means layout XML references like "Vendor_Module::default.xml" will be resolved using the static-file fallback rule instead of the layout fallback rule, producing the wrong source/target paths.
return match ($extension) {
'html' => TemplateType::EMAIL,
'phtml' => TemplateType::TEMPLATE,
default => TemplateType::STATIC,
};
src/Service/TemplateOverride/CommentStyle.php:107
- wrapPhpBlock() appends "\n\n" after the closing "?>". When prepending a header to a .phtml file that starts with HTML (no opening PHP tag), those newlines become literal output at the start of the rendered template, potentially changing the page/email output (or breaking files where leading whitespace matters).
return "<?php\n/**\n" . $this->formatPhpDocLines($lines) . "\n */\n?>\n\n";
docs/commands_reference.md:154
- The command documentation lists module file-path inputs as only supporting
view/<area>/templates,view/<area>/email, andview/<area>/web, but the implementation also supportsview/<area>/layoutand copies layout XML into<theme>/<Module_Name>/layout. The docs should mention layout overrides to match actual behaviour.
File paths may point into a module's `view/<area>/templates`, `view/<area>/email` or
`view/<area>/web` directory, a Hyvä compat module, or another theme's override directory.
Email templates (`.html`) and static view files (CSS, LESS, JS, images, fonts, ...) are
detected automatically and copied to `<theme>/<Module_Name>/email/` or
`<theme>/<Module_Name>/web/` respectively.
…ulate logic in a dedicated method
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/Service/TemplateOverride/TemplateCopier.php:87
- Same as above: this docblock adds a fully-qualified
@throwsannotation. To keep docblocks consistent and avoid FQNs, remove the@throwsline here as well (or standardise on imported short names throughout).
* @throws \Magento\Framework\Exception\FileSystemException
src/Service/TemplateOverride/TemplateCopier.php:46
- The docblock uses a fully-qualified exception name ("\Magento\Framework\Exception\FileSystemException"). Elsewhere in this codebase docblocks avoid FQNs/throws annotations, so this is inconsistent and makes the docs noisier than needed. Consider dropping the
@throwsline here (and rely on the method signature/actual exceptions) or switch to an imported short name consistently across the new TemplateOverride classes.
This issue also appears on line 87 of the same file.
* @throws \Magento\Framework\Exception\FileSystemException
src/Service/TemplateOverride/TemplatePathParser.php:45
- This docblock adds an explicit
@throwswith a fully-qualified exception name ("\InvalidArgumentException"). The rest of the codebase generally avoids FQNs/throws annotations in docblocks, so this is inconsistent. Consider removing the@throwsline here (and, if desired, across the rest of this new class for consistency).
* @throws \InvalidArgumentException
…er and enhance related tests
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/Service/TemplateOverride/TemplatePathParser.php:305
- Module-notation references to layout XML (e.g. Vendor_Module::default.xml) are currently classified as STATIC by extension, so layout overrides cannot be resolved correctly unless the user provides a filesystem path. Since TemplateType::LAYOUT is supported elsewhere (view//layout parsing + fallback resolver), include xml here as well.
return match ($extension) {
'html' => TemplateType::EMAIL,
'phtml' => TemplateType::TEMPLATE,
default => TemplateType::STATIC,
};
src/Service/TemplateOverride/TemplateFallbackResolver.php:123
- Layout fallback handling only includes the immediate parent theme via getParentTheme(), but Magento themes can inherit through multiple levels. For layout XML overrides, this can produce an incomplete fallback chain and resolve/copy from the wrong source when a grandparent theme provides the file.
if ($parentTheme instanceof ThemeInterface) {
$parentThemePath = $this->componentRegistrar->getPath(
ComponentRegistrar::THEME,
$parentTheme->getFullPath(),
);
src/Service/TemplateOverride/TemplateCopier.php:143
- For non-PHP targets (HTML/CSS/JS/etc), the header omits the logical override target when the physical source module differs (e.g. Hyvä compat module sources). PHP/PHTML headers include
@override-forin that case, so metadata is inconsistent and loses the 'override target' information for non-PHP files.
if ($actualSourceModule !== null) {
$lines[] = 'Source Module: ' . $actualSourceModule;
$version = $this->packageInfo->getVersion($actualSourceModule);
if ($version !== '') {
$lines[] = 'Source Module-Version: ' . $version;
This pull request introduces a new
mageforge:template:overrideCLI command for copying Magento module templates into themes as overrides, following Magento's fallback logic. It adds comprehensive documentation for the new command, implements supporting services for resolving template fallback and compatibility modules (including Hyvä compat modules), and provides configurable options for the override process. Additionally, the minimum mutation score threshold is slightly reduced in the test configuration.New Template Override Command and Core Services:
Introduced the
mageforge:template:overridecommand, allowing users to copy module templates (including Hyvä compat and email/static files) into themes as overrides, with options for dry-run, force, and theme selection. The command uses Magento's fallback logic and cleans caches after copying. (docs/commands_reference.md,.github/workflows/magento-compatibility.yml,src/Model/Config/TemplateOverride.php,src/Model/TemplateReference.php,src/Model/TemplateType.php) [1][2][3][4][5][6][7][8]Added
AreaEmulatorservice to load area-specific DI configuration, ensuring fallback plugins (like Hyvä's) are active during CLI execution.Added
CompatModuleResolverto map Hyvä compatibility modules to their original modules, supporting correct template placement and source resolution.Added
TemplateFallbackResolverto determine the fallback directories and resolve the effective template file using Magento's own fallback rules, handling all template types and edge cases.Added
TemplateCopierto copy template files to their override location, optionally prepending a header with source information and module version (configurable via system settings).Documentation and Workflow Updates:
Updated
docs/commands_reference.mdwith a detailed reference and usage guide for the newmageforge:template:overridecommand, including arguments, options, and behavior.Updated GitHub workflow (
.github/workflows/magento-compatibility.yml) to include help checks for the new override command in both MageForge and frontend command namespaces. [1][2]Testing and Quality Configuration:
minCoveredMsi) from 85 to 84 ininfection.json5to reflect the current test coverage and quality thresholds.Fixes#110