From 86ab46f161e4f504cf858929426ce4bbe25720ad Mon Sep 17 00:00:00 2001 From: Mathias Elle Date: Fri, 6 Nov 2026 14:50:57 +0100 Subject: [PATCH 1/4] =?UTF-8?q?=E2=9C=A8=20feat(template):=20copy=20whole?= =?UTF-8?q?=20template=20directory=20with=20template:override?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #245 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/commands_reference.md | 8 +- .../Command/Template/OverrideCommand.php | 131 +++++++++++++++++- src/Model/TemplateReference.php | 12 ++ .../TemplateFallbackResolver.php | 41 ++++++ .../TemplateOverride/TemplatePathParser.php | 91 ++++++++---- .../Command/Template/OverrideCommandTest.php | 79 +++++++++++ .../TemplateFallbackResolverTest.php | 31 +++++ .../TemplatePathParserTest.php | 63 ++++++++- 8 files changed, 426 insertions(+), 30 deletions(-) diff --git a/docs/commands_reference.md b/docs/commands_reference.md index e4705a76..69e43535 100644 --- a/docs/commands_reference.md +++ b/docs/commands_reference.md @@ -141,17 +141,20 @@ bin/magento mageforge:template:override 'Magento_Catalog::product/view/details.p bin/magento mageforge:template:override vendor/magento/module-catalog/view/frontend/templates/product/view/details.phtml -t Vendor/theme bin/magento mageforge:template:override 'Magento_Sales::order/new.html' --theme Vendor/theme bin/magento mageforge:template:override 'Magento_Theme::css/source/_module.less' --theme Vendor/theme +bin/magento mageforge:template:override vendor/magento/module-catalog/view/frontend/templates --theme Vendor/theme ``` **Arguments:** -- `template` — The view file to override. Accepts the `Module_Name::path/to/file.phtml` +- `template` — The view file or directory to override. Accepts the `Module_Name::path/to/file.phtml` notation or a file path (absolute, or relative to the Magento root / current directory). File paths may point into a module's `view//templates`, `view//email` or `view//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 `//email/` or `//web/` respectively. + A directory path (e.g. `vendor/magento/module-catalog/view/frontend/templates`) copies + all files inside it, including subdirectories. **Options:** @@ -174,7 +177,8 @@ bin/magento mageforge:template:override 'Magento_Theme::css/source/_module.less' - After copying, the command re-resolves the file to verify the new override wins and cleans the relevant caches (`full_page`, `block_html`, `layout`, `translate`). - If the file is already overridden in the target theme, nothing is copied unless `--force` - is given. + is given. When copying a directory, already overridden files are skipped and the rest is + copied; with `--force` they are replaced as well. --- diff --git a/src/Console/Command/Template/OverrideCommand.php b/src/Console/Command/Template/OverrideCommand.php index bd603813..0a748f92 100644 --- a/src/Console/Command/Template/OverrideCommand.php +++ b/src/Console/Command/Template/OverrideCommand.php @@ -69,7 +69,7 @@ protected function configure(): void ->addArgument( 'template', InputArgument::OPTIONAL, - 'Template to override (Module_Name::path/to/template.phtml or a file path)', + 'Template to override (Module_Name::path/to/template.phtml, a file path or a directory path)', ) ->addOption('theme', 't', InputOption::VALUE_REQUIRED, 'Target theme code (format: Vendor/theme)') ->addOption( @@ -138,6 +138,19 @@ protected function executeCommand(InputInterface $input, OutputInterface $output )); return Cli::RETURN_FAILURE; } + + if ($reference->isDirectory()) { + return $this->overrideDirectory( + $reference, + $theme, + $fallbackDirs, + $targetDir, + $isDryRun, + $isForce, + $output, + ); + } + $targetFile = $targetDir . '/' . $templatePath; $sourceFile = $this->fallbackResolver->findFirstExistingFile($fallbackDirs, $templatePath); @@ -215,6 +228,120 @@ protected function executeCommand(InputInterface $input, OutputInterface $output return Cli::RETURN_SUCCESS; } + /** + * Copy all files of a template directory into the theme + * + * Every file is resolved through the fallback chain like a single template, so the copied + * files are the ones Magento currently uses. Files that are already overridden are skipped + * unless --force is given. + * + * @param TemplateReference $reference + * @param ThemeInterface $theme + * @param string[] $fallbackDirs + * @param string $targetDir + * @param bool $isDryRun + * @param bool $isForce + * @param OutputInterface $output + * @return int + */ + private function overrideDirectory( + TemplateReference $reference, + ThemeInterface $theme, + array $fallbackDirs, + string $targetDir, + bool $isDryRun, + bool $isForce, + OutputInterface $output, + ): int { + $subPath = $reference->getTemplatePath(); + $sources = $this->fallbackResolver->findDirectoryFiles($fallbackDirs, $subPath, $targetDir); + if ($sources === []) { + $this->io->error(sprintf('No files found in any fallback location for %s.', $reference->getTemplateId())); + $this->displayFallbackDirs($fallbackDirs, null, $targetDir, $subPath); + return Cli::RETURN_FAILURE; + } + + $targetBase = $subPath === '' ? $targetDir : $targetDir . '/' . $subPath; + $copies = []; + $skipped = 0; + foreach ($sources as $relativePath => $sourceFile) { + $templatePath = $subPath === '' ? $relativePath : $subPath . '/' . $relativePath; + $alreadyOverridden = $this->fallbackResolver->findFirstExistingFile([$targetDir], $templatePath) !== null; + if ($alreadyOverridden && !$isForce) { + $skipped++; + continue; + } + $copies[$templatePath] = ['source' => $sourceFile, 'target' => $targetDir . '/' . $templatePath]; + } + + $skippedNote = $skipped > 0 && !$isForce ? ' (skipped, use --force to replace)' : ''; + $this->io->newLine(); + $this->io->definitionList( + ['Directory' => $reference->getTemplateId()], + ['Theme' => $theme->getCode()], + ['Target' => $this->toRelativePath($targetBase)], + ['Files to copy' => (string) count($copies)], + ['Already overridden' => $skipped . $skippedNote], + ); + + if ($copies === []) { + $this->io->info('All files of this directory are already overridden in this theme.'); + return Cli::RETURN_SUCCESS; + } + + if ($isDryRun || $this->isVerbose($output)) { + $this->io->table( + ['Source', 'Target'], + array_map( + fn(array $copy) => [$this->toRelativePath($copy['source']), $this->toRelativePath($copy['target'])], + array_values($copies), + ), + ); + } + + if ($isDryRun) { + $this->io->success('Dry run: no files were copied.'); + return Cli::RETURN_SUCCESS; + } + + $unresolved = []; + foreach ($copies as $templatePath => $copy) { + $sourceModuleName = $this->resolveSourceModuleName($copy['source'], $fallbackDirs, $reference); + $this->templateCopier->copy($copy['source'], $copy['target'], $sourceModuleName); + + // Verify that Magento's fallback now resolves the file to the new override + $resolved = $this->fallbackResolver->findFirstExistingFile($fallbackDirs, $templatePath); + if ($resolved !== $copy['target']) { + $unresolved[] = $templatePath; + } + } + + if ($unresolved !== []) { + $this->io->warning(sprintf( + "The files were copied, but Magento does not resolve these to the new override:\n - %s", + implode("\n - ", $unresolved), + )); + return Cli::RETURN_FAILURE; + } + + if (!$this->cacheCleaner->clean($this->io, $this->isVerbose($output))) { + $this->io->warning(sprintf( + '%d file(s) copied to %s, but cleaning the caches failed. ' + . "Run 'bin/magento cache:clean full_page block_html layout translate' manually.", + count($copies), + $this->toRelativePath($targetBase), + )); + return Cli::RETURN_FAILURE; + } + + $this->io->success(sprintf( + '%d file(s) copied to %s', + count($copies), + $this->toRelativePath($targetBase), + )); + return Cli::RETURN_SUCCESS; + } + /** * Get the template input from the argument or an interactive prompt * @@ -454,6 +581,8 @@ private function displayUsage(): void ); $this->io->writeln('The template can also be given as a file path, e.g.'); $this->io->writeln(' vendor/magento/module-catalog/view/frontend/templates/product/view/details.phtml'); + $this->io->writeln('Pass a directory path to copy all files inside it, e.g.'); + $this->io->writeln(' vendor/magento/module-catalog/view/frontend/templates'); } /** diff --git a/src/Model/TemplateReference.php b/src/Model/TemplateReference.php index 8ef81c13..b1e064dc 100644 --- a/src/Model/TemplateReference.php +++ b/src/Model/TemplateReference.php @@ -13,11 +13,13 @@ class TemplateReference * @param string $moduleName Module name in Vendor_Module notation * @param string $templatePath Path relative to the module's templates directory * @param TemplateType $type Whether this is a block template or an email template + * @param bool $isDirectory Whether the reference points to a directory instead of a single file */ public function __construct( private readonly string $moduleName, private readonly string $templatePath, private readonly TemplateType $type = TemplateType::TEMPLATE, + private readonly bool $isDirectory = false, ) { } @@ -51,6 +53,16 @@ public function getType(): TemplateType return $this->type; } + /** + * Check whether the reference points to a directory (all files inside are meant) + * + * @return bool + */ + public function isDirectory(): bool + { + return $this->isDirectory; + } + /** * Get the full template identifier in Module_Name::path/to/template.phtml notation * diff --git a/src/Service/TemplateOverride/TemplateFallbackResolver.php b/src/Service/TemplateOverride/TemplateFallbackResolver.php index 0074cef5..aa8ed0d4 100644 --- a/src/Service/TemplateOverride/TemplateFallbackResolver.php +++ b/src/Service/TemplateOverride/TemplateFallbackResolver.php @@ -160,6 +160,47 @@ public function findFirstExistingFile( return null; } + /** + * Collect all files below a directory across the fallback directories + * + * For every relative file path, the first fallback directory containing it wins, exactly like + * for a single template. The optional excluded directory (the override target) is not searched. + * + * @param string[] $fallbackDirs + * @param string $subPath Directory path relative to the fallback directories, empty for their root + * @param string|null $excludedDir + * @return array Source file paths keyed by their path relative to $subPath + * @throws \Magento\Framework\Exception\FileSystemException + */ + public function findDirectoryFiles(array $fallbackDirs, string $subPath, ?string $excludedDir = null): array + { + $files = []; + foreach ($fallbackDirs as $dir) { + if ($dir === $excludedDir) { + continue; + } + + $base = $subPath === '' ? $dir : $dir . '/' . $subPath; + if (!$this->fileDriver->isDirectory($base)) { + continue; + } + + $prefix = rtrim(str_replace('\\', '/', $base), '/') . '/'; + foreach ($this->fileDriver->readDirectoryRecursively($base) as $entry) { + $entry = str_replace('\\', '/', $entry); + if (!str_starts_with($entry, $prefix) || !$this->fileDriver->isFile($entry)) { + continue; + } + + $files[substr($entry, strlen($prefix))] ??= $entry; + } + } + + ksort($files); + + return $files; + } + /** * Get the fallback directory located within the given theme itself (the override target) * diff --git a/src/Service/TemplateOverride/TemplatePathParser.php b/src/Service/TemplateOverride/TemplatePathParser.php index 335e2dfd..638aa5d0 100644 --- a/src/Service/TemplateOverride/TemplatePathParser.php +++ b/src/Service/TemplateOverride/TemplatePathParser.php @@ -92,39 +92,42 @@ private function parseTemplateId(string $input): TemplateReference */ private function parseFilePath(string $input): TemplateReference { - $absolutePath = $this->locateFile(str_replace('\\', '/', $input)); - if ($absolutePath === null) { + $located = $this->locatePath(str_replace('\\', '/', $input)); + if ($located === null) { throw new \InvalidArgumentException( - "Template file '$input' not found. Pass an existing file path or use the " + "Template file or directory '$input' not found. Pass an existing file or directory path or use the " . 'Module_Name::path/to/template.phtml notation.', ); } - $moduleReference = $this->matchModuleFile($absolutePath); + [$absolutePath, $isDirectory] = $located; + + $moduleReference = $this->matchModuleFile($absolutePath, $isDirectory); if ($moduleReference !== null) { return $moduleReference; } - $themeReference = $this->matchThemeFile($absolutePath); + $themeReference = $this->matchThemeFile($absolutePath, $isDirectory); if ($themeReference !== null) { return $themeReference; } throw new \InvalidArgumentException( - "The file '$input' does not belong to a registered module or theme view directory.", + "The path '$input' does not belong to a registered module or theme view directory.", ); } /** - * Locate a file by its path, trying the working directory and the Magento root + * Locate a file or directory by its path, trying the working directory and the Magento root * * Relative paths are first resolved against the current working directory and then * against the Magento root, so paths like vendor/module/... work from any directory. * * @param string $path - * @return string|null The normalized absolute path, or null when the file does not exist + * @return array{0: string, 1: bool}|null The normalized absolute path and whether it is a directory, + * or null when nothing exists at the given path */ - private function locateFile(string $path): ?string + private function locatePath(string $path): ?array { $candidates = [$path]; if (!str_starts_with($path, '/')) { @@ -135,12 +138,16 @@ private function locateFile(string $path): ?string } foreach ($candidates as $candidate) { + $isDirectory = false; if (!$this->fileDriver->isFile($candidate)) { - continue; + if (!$this->fileDriver->isDirectory($candidate)) { + continue; + } + $isDirectory = true; } $realPath = $this->fileDriver->getRealPath($candidate); if (is_string($realPath)) { - return str_replace('\\', '/', $realPath); + return [rtrim(str_replace('\\', '/', $realPath), '/'), $isDirectory]; } } @@ -148,13 +155,14 @@ private function locateFile(string $path): ?string } /** - * Match a file inside a module's view//templates, email, web or layout directory + * Match a path inside a module's view//templates, email, web or layout directory * * @param string $absolutePath + * @param bool $isDirectory * @return TemplateReference|null * @throws \InvalidArgumentException */ - private function matchModuleFile(string $absolutePath): ?TemplateReference + private function matchModuleFile(string $absolutePath, bool $isDirectory): ?TemplateReference { $match = $this->findOwningComponent($absolutePath, ComponentRegistrar::MODULE); if ($match === null) { @@ -163,25 +171,31 @@ private function matchModuleFile(string $absolutePath): ?TemplateReference [$moduleName, $relativePath] = $match; $matches = []; - if (!preg_match('#^view/[a-z_]+/(templates|email|web|layout)/(.+)$#', $relativePath, $matches)) { + if (!preg_match($this->moduleViewPattern($isDirectory), $relativePath, $matches)) { throw new \InvalidArgumentException(sprintf( - "The file belongs to module '%s' but is not inside a view//templates, " + "The path belongs to module '%s' but is not inside a view//templates, " . 'view//email, view//web or view//layout directory.', $moduleName, )); } - return $this->createReference($moduleName, $matches[2], $this->typeFromViewDir($matches[1])); + return $this->createReference( + $moduleName, + $matches[2] ?? '', + $this->typeFromViewDir($matches[1]), + $isDirectory, + ); } /** - * Match a file inside a theme's /templates, email, web or layout directory + * Match a path inside a theme's /templates, email, web or layout directory * * @param string $absolutePath + * @param bool $isDirectory * @return TemplateReference|null * @throws \InvalidArgumentException */ - private function matchThemeFile(string $absolutePath): ?TemplateReference + private function matchThemeFile(string $absolutePath, bool $isDirectory): ?TemplateReference { $match = $this->findOwningComponent($absolutePath, ComponentRegistrar::THEME); if ($match === null) { @@ -190,24 +204,45 @@ private function matchThemeFile(string $absolutePath): ?TemplateReference [$themeFullPath, $relativePath] = $match; $matches = []; - if (!preg_match('#^([A-Za-z0-9]+_[A-Za-z0-9]+)/(templates|email|web|layout)/(.+)$#', $relativePath, $matches)) { + $pattern = $isDirectory + ? '#^([A-Za-z0-9]+_[A-Za-z0-9]+)/(templates|email|web|layout)(?:/(.*))?$#' + : '#^([A-Za-z0-9]+_[A-Za-z0-9]+)/(templates|email|web|layout)/(.+)$#'; + if (!preg_match($pattern, $relativePath, $matches)) { throw new \InvalidArgumentException(sprintf( - "The file belongs to theme '%s' but is not a module template override " + "The path belongs to theme '%s' but is not a module template override " . '(expected /templates/..., /email/... ' . '/web/... or /layout/...).', $themeFullPath, )); } - return $this->createReference($matches[1], $matches[3], $this->typeFromViewDir($matches[2])); + return $this->createReference( + $matches[1], + $matches[3] ?? '', + $this->typeFromViewDir($matches[2]), + $isDirectory, + ); } /** - * Find the registered component (longest path match) containing the given file + * Get the pattern matching a module's view directory path; directories may be the view directory itself + * + * @param bool $isDirectory + * @return string + */ + private function moduleViewPattern(bool $isDirectory): string + { + return $isDirectory + ? '#^view/[a-z_]+/(templates|email|web|layout)(?:/(.*))?$#' + : '#^view/[a-z_]+/(templates|email|web|layout)/(.+)$#'; + } + + /** + * Find the registered component (longest path match) containing the given path * * @param string $absolutePath * @param string $componentType - * @return array{0: string, 1: string}|null Component name and file path relative to the component root + * @return array{0: string, 1: string}|null Component name and path relative to the component root */ private function findOwningComponent(string $absolutePath, string $componentType): ?array { @@ -244,6 +279,7 @@ private function findOwningComponent(string $absolutePath, string $componentType * @param string $moduleName * @param string $templatePath * @param TemplateType $type + * @param bool $isDirectory * @return TemplateReference * @throws \InvalidArgumentException */ @@ -251,10 +287,11 @@ private function createReference( string $moduleName, string $templatePath, TemplateType $type = TemplateType::TEMPLATE, + bool $isDirectory = false, ): TemplateReference { $originalModules = $this->compatModuleResolver->getOriginalModules($moduleName); if ($originalModules === []) { - return new TemplateReference($moduleName, $templatePath, $type); + return new TemplateReference($moduleName, $templatePath, $type, $isDirectory); } // Compat modules may keep templates in a subdirectory named after the original module @@ -264,12 +301,16 @@ private function createReference( $originalModule, substr($templatePath, strlen($originalModule) + 1), $type, + $isDirectory, ); } + if ($isDirectory && $templatePath === $originalModule) { + return new TemplateReference($originalModule, '', $type, true); + } } if (count($originalModules) === 1) { - return new TemplateReference($originalModules[0], $templatePath, $type); + return new TemplateReference($originalModules[0], $templatePath, $type, $isDirectory); } throw new \InvalidArgumentException(sprintf( diff --git a/tests/Unit/Console/Command/Template/OverrideCommandTest.php b/tests/Unit/Console/Command/Template/OverrideCommandTest.php index 100f027c..30365a3c 100644 --- a/tests/Unit/Console/Command/Template/OverrideCommandTest.php +++ b/tests/Unit/Console/Command/Template/OverrideCommandTest.php @@ -8,6 +8,7 @@ use Magento\Framework\Console\Cli; use OpenForgeProject\MageForge\Console\Command\Template\OverrideCommand; use OpenForgeProject\MageForge\Model\TemplateReference; +use OpenForgeProject\MageForge\Model\TemplateType; use OpenForgeProject\MageForge\Model\ThemeList; use OpenForgeProject\MageForge\Service\CacheCleaner; use OpenForgeProject\MageForge\Service\TemplateOverride\AreaEmulator; @@ -344,6 +345,84 @@ public function testWarnsWhenVerificationDoesNotResolveToNewOverride(): void $this->assertStringContainsString('Magento resolves the template to', $tester->getDisplay()); } + public function testCopiesAllFilesOfDirectoryAndSkipsExistingOverrides(): void + { + $this->themeList->method('getAllThemes')->willReturn([new FakeTheme('Vendor/theme')]); + $this->templatePathParser + ->method('parse') + ->willReturn(new TemplateReference('Magento_Catalog', '', TemplateType::TEMPLATE, true)); + $this->fallbackResolver->method('getFallbackDirs')->willReturn([self::THEME_DIR, self::MODULE_DIR]); + $this->fallbackResolver->method('getThemeTargetDir')->willReturn(self::THEME_DIR); + $this->fallbackResolver + ->method('findDirectoryFiles') + ->with([self::THEME_DIR, self::MODULE_DIR], '', self::THEME_DIR) + ->willReturn([ + 'a.phtml' => self::MODULE_DIR . '/a.phtml', + 'sub/b.phtml' => self::MODULE_DIR . '/sub/b.phtml', + 'c.phtml' => self::MODULE_DIR . '/c.phtml', + ]); + $this->fallbackResolver + ->method('findFirstExistingFile') + ->willReturnCallback( + static fn(array $dirs, string $path): ?string => match (true) { + $dirs === [self::THEME_DIR] => $path === 'c.phtml' ? self::THEME_DIR . '/c.phtml' : null, + default => self::THEME_DIR . '/' . $path, + }, + ); + $copied = []; + $this->templateCopier + ->expects($this->exactly(2)) + ->method('copy') + ->willReturnCallback(static function (string $source, string $target) use (&$copied): void { + $copied[$source] = $target; + }); + $this->cacheCleaner->expects($this->once())->method('clean')->willReturn(true); + + $tester = new CommandTester($this->command); + $exitCode = $tester->execute([ + 'template' => self::MODULE_DIR, + '--theme' => 'Vendor/theme', + ]); + + $this->assertSame(Cli::RETURN_SUCCESS, $exitCode); + $this->assertSame( + [ + self::MODULE_DIR . '/a.phtml' => self::THEME_DIR . '/a.phtml', + self::MODULE_DIR . '/sub/b.phtml' => self::THEME_DIR . '/sub/b.phtml', + ], + $copied, + ); + $this->assertStringContainsString('2 file(s) copied', $tester->getDisplay()); + } + + public function testDirectoryDryRunDoesNotCopy(): void + { + $this->themeList->method('getAllThemes')->willReturn([new FakeTheme('Vendor/theme')]); + $this->templatePathParser + ->method('parse') + ->willReturn(new TemplateReference('Magento_Catalog', 'product', TemplateType::TEMPLATE, true)); + $this->fallbackResolver->method('getFallbackDirs')->willReturn([self::THEME_DIR, self::MODULE_DIR]); + $this->fallbackResolver->method('getThemeTargetDir')->willReturn(self::THEME_DIR); + $this->fallbackResolver + ->method('findDirectoryFiles') + ->willReturn(['a.phtml' => self::MODULE_DIR . '/product/a.phtml']); + $this->fallbackResolver->method('findFirstExistingFile')->willReturn(null); + $this->templateCopier->expects($this->never())->method('copy'); + $this->cacheCleaner->expects($this->never())->method('clean'); + + $tester = new CommandTester($this->command); + $exitCode = $tester->execute([ + 'template' => self::MODULE_DIR . '/product', + '--theme' => 'Vendor/theme', + '--dry-run' => true, + ]); + + $this->assertSame(Cli::RETURN_SUCCESS, $exitCode); + $display = $tester->getDisplay(); + $this->assertStringContainsString('Dry run: no files were copied.', $display); + $this->assertStringContainsString('product/a.phtml', $display); + } + public function testReportsParserErrors(): void { $this->themeList->method('getAllThemes')->willReturn([new FakeTheme('Vendor/theme')]); diff --git a/tests/Unit/Service/TemplateOverride/TemplateFallbackResolverTest.php b/tests/Unit/Service/TemplateOverride/TemplateFallbackResolverTest.php index bd9c2325..74dd4381 100644 --- a/tests/Unit/Service/TemplateOverride/TemplateFallbackResolverTest.php +++ b/tests/Unit/Service/TemplateOverride/TemplateFallbackResolverTest.php @@ -295,4 +295,35 @@ public function testGetThemeTargetDirNormalizesBackslashesAndTrailingSlash(): vo $this->assertSame('C:\\app\\design\\frontend\\Vendor\\theme\\Magento_Catalog\\templates', $result); } + + public function testFindDirectoryFilesMergesFallbackDirsFirstDirWinsAndSkipsExcluded(): void + { + $this->fileDriver->method('isDirectory')->willReturn(true); + $this->fileDriver + ->method('readDirectoryRecursively') + ->willReturnMap([ + ['/compat/templates/product', ['/compat/templates/product/a.phtml', '/compat/templates/product/sub']], + [ + '/module/templates/product', + ['/module/templates/product/a.phtml', '/module/templates/product/sub/b.phtml'], + ], + ]); + $this->fileDriver + ->method('isFile') + ->willReturnCallback(static fn(string $path): bool => str_ends_with($path, '.phtml')); + + $files = $this->resolver->findDirectoryFiles( + ['/theme/templates', '/compat/templates', '/module/templates'], + 'product', + '/theme/templates', + ); + + $this->assertSame( + [ + 'a.phtml' => '/compat/templates/product/a.phtml', + 'sub/b.phtml' => '/module/templates/product/sub/b.phtml', + ], + $files, + ); + } } diff --git a/tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php b/tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php index 2e291bd9..6e173bd3 100644 --- a/tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php +++ b/tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php @@ -358,12 +358,71 @@ public function testParsesEmailThemeFilePath(): void $this->assertSame(TemplateType::EMAIL, $reference->getType()); } + public function testParsesModuleDirectoryPath(): void + { + $directory = '/magento/vendor/magento/module-catalog/view/frontend/templates'; + $this->fileDriver->method('isFile')->willReturn(false); + $this->fileDriver->method('isDirectory')->with($directory)->willReturn(true); + $this->fileDriver->method('getRealPath')->with($directory)->willReturn($directory); + $this->componentRegistrar + ->method('getPaths') + ->with(ComponentRegistrar::MODULE) + ->willReturn(['Magento_Catalog' => '/magento/vendor/magento/module-catalog']); + $this->compatModuleResolver->method('getOriginalModules')->willReturn([]); + + $reference = $this->parser->parse($directory); + + $this->assertTrue($reference->isDirectory()); + $this->assertSame('Magento_Catalog', $reference->getModuleName()); + $this->assertSame('', $reference->getTemplatePath()); + $this->assertSame(TemplateType::TEMPLATE, $reference->getType()); + } + + public function testParsesThemeSubdirectoryPath(): void + { + $directory = '/magento/app/design/frontend/Vendor/theme/Magento_Sales/email/order'; + $this->fileDriver->method('isFile')->willReturn(false); + $this->fileDriver->method('isDirectory')->willReturn(true); + $this->fileDriver->method('getRealPath')->willReturn($directory); + $this->componentRegistrar + ->method('getPaths') + ->willReturnMap([ + [ComponentRegistrar::MODULE, []], + [ComponentRegistrar::THEME, ['frontend/Vendor/theme' => '/magento/app/design/frontend/Vendor/theme']], + ]); + $this->compatModuleResolver->method('getOriginalModules')->willReturn([]); + + $reference = $this->parser->parse($directory); + + $this->assertTrue($reference->isDirectory()); + $this->assertSame('Magento_Sales', $reference->getModuleName()); + $this->assertSame('order', $reference->getTemplatePath()); + $this->assertSame(TemplateType::EMAIL, $reference->getType()); + } + + public function testRejectsModuleDirectoryOutsideViewDirectories(): void + { + $directory = '/magento/vendor/magento/module-catalog/Model'; + $this->fileDriver->method('isFile')->willReturn(false); + $this->fileDriver->method('isDirectory')->willReturn(true); + $this->fileDriver->method('getRealPath')->willReturn($directory); + $this->componentRegistrar + ->method('getPaths') + ->with(ComponentRegistrar::MODULE) + ->willReturn(['Magento_Catalog' => '/magento/vendor/magento/module-catalog']); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessageMatches('/is not inside a view/'); + + $this->parser->parse($directory); + } + public function testRejectsMissingFile(): void { $this->fileDriver->method('isFile')->willReturn(false); $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessageMatches("/Template file '\/nowhere\/file\.phtml' not found/"); + $this->expectExceptionMessageMatches("/Template file or directory '\/nowhere\/file\.phtml' not found/"); $this->parser->parse('/nowhere/file.phtml'); } @@ -495,7 +554,7 @@ public function testFileNotFoundMessageContainsBothHints(): void $this->fileDriver->method('isFile')->willReturn(false); $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessageMatches('/Pass an existing file path.*Module_Name::path\/to\/template\.phtml/s'); + $this->expectExceptionMessageMatches('/Pass an existing file or directory path.*Module_Name::path\/to\/template\.phtml/s'); $this->parser->parse('/nowhere/file.phtml'); } From a0aa09a78f288152573f424ad347c3d9302912ed Mon Sep 17 00:00:00 2001 From: Mathias Elle Date: Fri, 6 Nov 2026 15:09:55 +0100 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=8E=A8=20style(template):=20fix=20mag?= =?UTF-8?q?o=20formatting=20and=20phpcs=20line=20length?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Command/Template/OverrideCommand.php | 25 +++++++------------ .../TemplatePathParserTest.php | 4 ++- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/Console/Command/Template/OverrideCommand.php b/src/Console/Command/Template/OverrideCommand.php index 0a748f92..a996b343 100644 --- a/src/Console/Command/Template/OverrideCommand.php +++ b/src/Console/Command/Template/OverrideCommand.php @@ -290,13 +290,10 @@ private function overrideDirectory( } if ($isDryRun || $this->isVerbose($output)) { - $this->io->table( - ['Source', 'Target'], - array_map( - fn(array $copy) => [$this->toRelativePath($copy['source']), $this->toRelativePath($copy['target'])], - array_values($copies), - ), - ); + $this->io->table(['Source', 'Target'], array_map(fn(array $copy) => [ + $this->toRelativePath($copy['source']), + $this->toRelativePath($copy['target']), + ], array_values($copies))); } if ($isDryRun) { @@ -317,10 +314,10 @@ private function overrideDirectory( } if ($unresolved !== []) { - $this->io->warning(sprintf( - "The files were copied, but Magento does not resolve these to the new override:\n - %s", - implode("\n - ", $unresolved), - )); + $list = implode("\n - ", $unresolved); + $this->io->warning( + "The files were copied, but Magento does not resolve these to the new override:\n - {$list}", + ); return Cli::RETURN_FAILURE; } @@ -334,11 +331,7 @@ private function overrideDirectory( return Cli::RETURN_FAILURE; } - $this->io->success(sprintf( - '%d file(s) copied to %s', - count($copies), - $this->toRelativePath($targetBase), - )); + $this->io->success(sprintf('%d file(s) copied to %s', count($copies), $this->toRelativePath($targetBase))); return Cli::RETURN_SUCCESS; } diff --git a/tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php b/tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php index 6e173bd3..e114f21c 100644 --- a/tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php +++ b/tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php @@ -554,7 +554,9 @@ public function testFileNotFoundMessageContainsBothHints(): void $this->fileDriver->method('isFile')->willReturn(false); $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessageMatches('/Pass an existing file or directory path.*Module_Name::path\/to\/template\.phtml/s'); + $this->expectExceptionMessageMatches( + '/Pass an existing file or directory path.*Module_Name::path\/to\/template\.phtml/s', + ); $this->parser->parse('/nowhere/file.phtml'); } From 976a382b3832ac2e92f1f145987eba9eae4cdc78 Mon Sep 17 00:00:00 2001 From: Mathias Elle Date: Fri, 6 Nov 2026 15:12:27 +0100 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=90=9B=20fix(template):=20count=20the?= =?UTF-8?q?me-only=20files=20as=20overridden=20and=20handle=20numeric=20fi?= =?UTF-8?q?le=20names?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Command/Template/OverrideCommand.php | 21 ++++-- .../TemplateFallbackResolver.php | 2 +- .../Command/Template/OverrideCommandTest.php | 68 ++++++++++++++----- 3 files changed, 67 insertions(+), 24 deletions(-) diff --git a/src/Console/Command/Template/OverrideCommand.php b/src/Console/Command/Template/OverrideCommand.php index a996b343..e2a15268 100644 --- a/src/Console/Command/Template/OverrideCommand.php +++ b/src/Console/Command/Template/OverrideCommand.php @@ -255,7 +255,8 @@ private function overrideDirectory( ): int { $subPath = $reference->getTemplatePath(); $sources = $this->fallbackResolver->findDirectoryFiles($fallbackDirs, $subPath, $targetDir); - if ($sources === []) { + $overridden = $this->fallbackResolver->findDirectoryFiles([$targetDir], $subPath); + if ($sources === [] && $overridden === []) { $this->io->error(sprintf('No files found in any fallback location for %s.', $reference->getTemplateId())); $this->displayFallbackDirs($fallbackDirs, null, $targetDir, $subPath); return Cli::RETURN_FAILURE; @@ -263,18 +264,23 @@ private function overrideDirectory( $targetBase = $subPath === '' ? $targetDir : $targetDir . '/' . $subPath; $copies = []; - $skipped = 0; foreach ($sources as $relativePath => $sourceFile) { - $templatePath = $subPath === '' ? $relativePath : $subPath . '/' . $relativePath; - $alreadyOverridden = $this->fallbackResolver->findFirstExistingFile([$targetDir], $templatePath) !== null; - if ($alreadyOverridden && !$isForce) { - $skipped++; + // Numeric file names become integer array keys + $relativePath = (string) $relativePath; + if (isset($overridden[$relativePath]) && !$isForce) { continue; } + $templatePath = $subPath === '' ? $relativePath : $subPath . '/' . $relativePath; $copies[$templatePath] = ['source' => $sourceFile, 'target' => $targetDir . '/' . $templatePath]; } + // Files that only exist in the theme have no other source to copy from + $skipped = count($overridden) - ($isForce ? count(array_intersect_key($overridden, $sources)) : 0); - $skippedNote = $skipped > 0 && !$isForce ? ' (skipped, use --force to replace)' : ''; + $skippedNote = match (true) { + $skipped === 0 => '', + $isForce => ' (not present elsewhere in the fallback chain)', + default => ' (skipped, use --force to replace)', + }; $this->io->newLine(); $this->io->definitionList( ['Directory' => $reference->getTemplateId()], @@ -303,6 +309,7 @@ private function overrideDirectory( $unresolved = []; foreach ($copies as $templatePath => $copy) { + $templatePath = (string) $templatePath; $sourceModuleName = $this->resolveSourceModuleName($copy['source'], $fallbackDirs, $reference); $this->templateCopier->copy($copy['source'], $copy['target'], $sourceModuleName); diff --git a/src/Service/TemplateOverride/TemplateFallbackResolver.php b/src/Service/TemplateOverride/TemplateFallbackResolver.php index aa8ed0d4..8de4fe1d 100644 --- a/src/Service/TemplateOverride/TemplateFallbackResolver.php +++ b/src/Service/TemplateOverride/TemplateFallbackResolver.php @@ -196,7 +196,7 @@ public function findDirectoryFiles(array $fallbackDirs, string $subPath, ?string } } - ksort($files); + ksort($files, SORT_STRING); return $files; } diff --git a/tests/Unit/Console/Command/Template/OverrideCommandTest.php b/tests/Unit/Console/Command/Template/OverrideCommandTest.php index 30365a3c..98875f08 100644 --- a/tests/Unit/Console/Command/Template/OverrideCommandTest.php +++ b/tests/Unit/Console/Command/Template/OverrideCommandTest.php @@ -355,23 +355,26 @@ public function testCopiesAllFilesOfDirectoryAndSkipsExistingOverrides(): void $this->fallbackResolver->method('getThemeTargetDir')->willReturn(self::THEME_DIR); $this->fallbackResolver ->method('findDirectoryFiles') - ->with([self::THEME_DIR, self::MODULE_DIR], '', self::THEME_DIR) - ->willReturn([ - 'a.phtml' => self::MODULE_DIR . '/a.phtml', - 'sub/b.phtml' => self::MODULE_DIR . '/sub/b.phtml', - 'c.phtml' => self::MODULE_DIR . '/c.phtml', - ]); - $this->fallbackResolver - ->method('findFirstExistingFile') ->willReturnCallback( - static fn(array $dirs, string $path): ?string => match (true) { - $dirs === [self::THEME_DIR] => $path === 'c.phtml' ? self::THEME_DIR . '/c.phtml' : null, - default => self::THEME_DIR . '/' . $path, - }, + static fn(array $dirs): array => $dirs === [self::THEME_DIR] + ? [ + 'c.phtml' => self::THEME_DIR . '/c.phtml', + 'theme-only.phtml' => self::THEME_DIR . '/theme-only.phtml', + ] + : [ + 'a.phtml' => self::MODULE_DIR . '/a.phtml', + 'sub/b.phtml' => self::MODULE_DIR . '/sub/b.phtml', + 'c.phtml' => self::MODULE_DIR . '/c.phtml', + // Numeric file names become integer array keys + 123 => self::MODULE_DIR . '/123', + ], ); + $this->fallbackResolver + ->method('findFirstExistingFile') + ->willReturnCallback(static fn(array $dirs, string $path): string => self::THEME_DIR . '/' . $path); $copied = []; $this->templateCopier - ->expects($this->exactly(2)) + ->expects($this->exactly(3)) ->method('copy') ->willReturnCallback(static function (string $source, string $target) use (&$copied): void { $copied[$source] = $target; @@ -389,10 +392,40 @@ public function testCopiesAllFilesOfDirectoryAndSkipsExistingOverrides(): void [ self::MODULE_DIR . '/a.phtml' => self::THEME_DIR . '/a.phtml', self::MODULE_DIR . '/sub/b.phtml' => self::THEME_DIR . '/sub/b.phtml', + self::MODULE_DIR . '/123' => self::THEME_DIR . '/123', ], $copied, ); - $this->assertStringContainsString('2 file(s) copied', $tester->getDisplay()); + $display = $tester->getDisplay(); + $this->assertStringContainsString('3 file(s) copied', $display); + $this->assertStringContainsString('Already overridden', $display); + } + + public function testDirectoryWithOnlyThemeFilesSucceedsWithoutCopying(): void + { + $this->themeList->method('getAllThemes')->willReturn([new FakeTheme('Vendor/theme')]); + $this->templatePathParser + ->method('parse') + ->willReturn(new TemplateReference('Magento_Catalog', '', TemplateType::TEMPLATE, true)); + $this->fallbackResolver->method('getFallbackDirs')->willReturn([self::THEME_DIR, self::MODULE_DIR]); + $this->fallbackResolver->method('getThemeTargetDir')->willReturn(self::THEME_DIR); + $this->fallbackResolver + ->method('findDirectoryFiles') + ->willReturnCallback( + static fn(array $dirs): array => $dirs === [self::THEME_DIR] + ? ['a.phtml' => self::THEME_DIR . '/a.phtml'] + : [], + ); + $this->templateCopier->expects($this->never())->method('copy'); + + $tester = new CommandTester($this->command); + $exitCode = $tester->execute([ + 'template' => self::THEME_DIR, + '--theme' => 'Vendor/theme', + ]); + + $this->assertSame(Cli::RETURN_SUCCESS, $exitCode); + $this->assertStringContainsString('already overridden', $tester->getDisplay()); } public function testDirectoryDryRunDoesNotCopy(): void @@ -405,8 +438,11 @@ public function testDirectoryDryRunDoesNotCopy(): void $this->fallbackResolver->method('getThemeTargetDir')->willReturn(self::THEME_DIR); $this->fallbackResolver ->method('findDirectoryFiles') - ->willReturn(['a.phtml' => self::MODULE_DIR . '/product/a.phtml']); - $this->fallbackResolver->method('findFirstExistingFile')->willReturn(null); + ->willReturnCallback( + static fn(array $dirs): array => $dirs === [self::THEME_DIR] + ? [] + : ['a.phtml' => self::MODULE_DIR . '/product/a.phtml'], + ); $this->templateCopier->expects($this->never())->method('copy'); $this->cacheCleaner->expects($this->never())->method('clean'); From c85c928b78b6022eb0308a976fe9aeadde4a75d3 Mon Sep 17 00:00:00 2001 From: Mathias Elle Date: Fri, 6 Nov 2026 15:30:57 +0100 Subject: [PATCH 4/4] =?UTF-8?q?=E2=9C=85=20test(template):=20cover=20direc?= =?UTF-8?q?tory=20override=20paths=20and=20drop=20redundant=20casts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Command/Template/OverrideCommand.php | 12 +- .../TemplateFallbackResolver.php | 2 +- .../Command/Template/OverrideCommandTest.php | 142 +++++++++++++++ tests/Unit/Model/TemplateReferenceTest.php | 6 + .../TemplateFallbackResolverTest.php | 39 +++++ .../TemplatePathParserTest.php | 164 ++++++++++++++++++ 6 files changed, 360 insertions(+), 5 deletions(-) diff --git a/src/Console/Command/Template/OverrideCommand.php b/src/Console/Command/Template/OverrideCommand.php index e2a15268..3d4457d1 100644 --- a/src/Console/Command/Template/OverrideCommand.php +++ b/src/Console/Command/Template/OverrideCommand.php @@ -271,7 +271,11 @@ private function overrideDirectory( continue; } $templatePath = $subPath === '' ? $relativePath : $subPath . '/' . $relativePath; - $copies[$templatePath] = ['source' => $sourceFile, 'target' => $targetDir . '/' . $templatePath]; + $copies[] = [ + 'path' => $templatePath, + 'source' => $sourceFile, + 'target' => $targetDir . '/' . $templatePath, + ]; } // Files that only exist in the theme have no other source to copy from $skipped = count($overridden) - ($isForce ? count(array_intersect_key($overridden, $sources)) : 0); @@ -299,7 +303,7 @@ private function overrideDirectory( $this->io->table(['Source', 'Target'], array_map(fn(array $copy) => [ $this->toRelativePath($copy['source']), $this->toRelativePath($copy['target']), - ], array_values($copies))); + ], $copies)); } if ($isDryRun) { @@ -308,8 +312,8 @@ private function overrideDirectory( } $unresolved = []; - foreach ($copies as $templatePath => $copy) { - $templatePath = (string) $templatePath; + foreach ($copies as $copy) { + $templatePath = $copy['path']; $sourceModuleName = $this->resolveSourceModuleName($copy['source'], $fallbackDirs, $reference); $this->templateCopier->copy($copy['source'], $copy['target'], $sourceModuleName); diff --git a/src/Service/TemplateOverride/TemplateFallbackResolver.php b/src/Service/TemplateOverride/TemplateFallbackResolver.php index 8de4fe1d..fcfe2790 100644 --- a/src/Service/TemplateOverride/TemplateFallbackResolver.php +++ b/src/Service/TemplateOverride/TemplateFallbackResolver.php @@ -169,7 +169,7 @@ public function findFirstExistingFile( * @param string[] $fallbackDirs * @param string $subPath Directory path relative to the fallback directories, empty for their root * @param string|null $excludedDir - * @return array Source file paths keyed by their path relative to $subPath + * @return array Source file paths keyed by their path relative to $subPath * @throws \Magento\Framework\Exception\FileSystemException */ public function findDirectoryFiles(array $fallbackDirs, string $subPath, ?string $excludedDir = null): array diff --git a/tests/Unit/Console/Command/Template/OverrideCommandTest.php b/tests/Unit/Console/Command/Template/OverrideCommandTest.php index 98875f08..35d7ad57 100644 --- a/tests/Unit/Console/Command/Template/OverrideCommandTest.php +++ b/tests/Unit/Console/Command/Template/OverrideCommandTest.php @@ -428,6 +428,148 @@ public function testDirectoryWithOnlyThemeFilesSucceedsWithoutCopying(): void $this->assertStringContainsString('already overridden', $tester->getDisplay()); } + public function testDirectoryForceReplacesOverriddenFilesAndReportsThemeOnlyFiles(): void + { + $this->setUpDirectoryTemplate( + sources: ['a.phtml' => self::MODULE_DIR . '/a.phtml'], + overridden: ['a.phtml' => self::THEME_DIR . '/a.phtml', 'theme-only.phtml' => self::THEME_DIR . '/x.phtml'], + ); + $this->fallbackResolver + ->method('findFirstExistingFile') + ->willReturn(self::THEME_DIR . '/a.phtml'); + $this->templateCopier + ->expects($this->once()) + ->method('copy') + ->with(self::MODULE_DIR . '/a.phtml', self::THEME_DIR . '/a.phtml', 'Magento_Catalog'); + $this->cacheCleaner->expects($this->once())->method('clean')->willReturn(true); + + $tester = new CommandTester($this->command); + $exitCode = $tester->execute([ + 'template' => self::MODULE_DIR, + '--theme' => 'Vendor/theme', + '--force' => true, + ]); + + $this->assertSame(Cli::RETURN_SUCCESS, $exitCode); + $display = $tester->getDisplay(); + $this->assertStringContainsString('Files to copy', $display); + $this->assertStringContainsString('not present elsewhere in the fallback chain', $display); + $this->assertStringContainsString('1 file(s) copied to', $display); + } + + public function testDirectorySkipsOverriddenFilesWithoutForce(): void + { + $this->setUpDirectoryTemplate( + sources: ['a.phtml' => self::MODULE_DIR . '/a.phtml'], + overridden: ['a.phtml' => self::THEME_DIR . '/a.phtml'], + ); + $this->templateCopier->expects($this->never())->method('copy'); + + $tester = new CommandTester($this->command); + $exitCode = $tester->execute(['template' => self::MODULE_DIR, '--theme' => 'Vendor/theme']); + + $this->assertSame(Cli::RETURN_SUCCESS, $exitCode); + $display = $tester->getDisplay(); + $this->assertStringContainsString('skipped, use --force to replace', $display); + $this->assertStringContainsString('All files of this directory are already overridden', $display); + } + + public function testDirectoryFailsWhenNoFilesAreFound(): void + { + $this->setUpDirectoryTemplate(sources: [], overridden: []); + $this->templateCopier->expects($this->never())->method('copy'); + + $tester = new CommandTester($this->command); + $exitCode = $tester->execute(['template' => self::MODULE_DIR, '--theme' => 'Vendor/theme']); + + $this->assertSame(Cli::RETURN_FAILURE, $exitCode); + $display = $tester->getDisplay(); + $this->assertStringContainsString('No files found in any fallback location for Magento_Catalog::', $display); + $this->assertStringContainsString('Fallback search order', $display); + } + + public function testDirectoryWarnsWhenFilesDoNotResolveToNewOverride(): void + { + $this->setUpDirectoryTemplate( + sources: ['a.phtml' => self::MODULE_DIR . '/a.phtml', 'b.phtml' => self::MODULE_DIR . '/b.phtml'], + overridden: [], + ); + $this->fallbackResolver + ->method('findFirstExistingFile') + ->willReturnCallback( + static fn(array $dirs, string $path): string => $path === 'a.phtml' + ? self::THEME_DIR . '/a.phtml' + : self::MODULE_DIR . '/' . $path, + ); + $this->templateCopier->expects($this->exactly(2))->method('copy'); + $this->cacheCleaner->expects($this->never())->method('clean'); + + $tester = new CommandTester($this->command); + $exitCode = $tester->execute(['template' => self::MODULE_DIR, '--theme' => 'Vendor/theme']); + + $this->assertSame(Cli::RETURN_FAILURE, $exitCode); + $display = $tester->getDisplay(); + $this->assertStringContainsString('Magento does not resolve these to the new override', $display); + $this->assertStringContainsString('- b.phtml', $display); + $this->assertStringNotContainsString('- a.phtml', $display); + } + + public function testDirectoryReportsFailedCacheCleaning(): void + { + $this->setUpDirectoryTemplate(sources: ['a.phtml' => self::MODULE_DIR . '/a.phtml'], overridden: []); + $this->fallbackResolver->method('findFirstExistingFile')->willReturn(self::THEME_DIR . '/a.phtml'); + $this->cacheCleaner->method('clean')->willReturn(false); + + $tester = new CommandTester($this->command); + $exitCode = $tester->execute(['template' => self::MODULE_DIR, '--theme' => 'Vendor/theme']); + + $this->assertSame(Cli::RETURN_FAILURE, $exitCode); + $this->assertStringContainsString('1 file(s) copied to', $tester->getDisplay()); + $this->assertStringContainsString('but cleaning the caches', $tester->getDisplay()); + } + + public function testDirectoryCopiesIntoSubPathOfTheme(): void + { + $this->setUpDirectoryTemplate( + sources: ['a.phtml' => self::MODULE_DIR . '/product/a.phtml'], + overridden: [], + subPath: 'product', + ); + $this->fallbackResolver + ->method('findFirstExistingFile') + ->with([self::THEME_DIR, self::MODULE_DIR], 'product/a.phtml') + ->willReturn(self::THEME_DIR . '/product/a.phtml'); + $this->templateCopier + ->expects($this->once()) + ->method('copy') + ->with(self::MODULE_DIR . '/product/a.phtml', self::THEME_DIR . '/product/a.phtml'); + $this->cacheCleaner->method('clean')->willReturn(true); + + $tester = new CommandTester($this->command); + $exitCode = $tester->execute(['template' => self::MODULE_DIR . '/product', '--theme' => 'Vendor/theme']); + + $this->assertSame(Cli::RETURN_SUCCESS, $exitCode); + } + + /** + * @param array $sources + * @param array $overridden + */ + private function setUpDirectoryTemplate(array $sources, array $overridden, string $subPath = ''): void + { + $this->themeList->method('getAllThemes')->willReturn([new FakeTheme('Vendor/theme')]); + $this->templatePathParser + ->method('parse') + ->willReturn(new TemplateReference('Magento_Catalog', $subPath, TemplateType::TEMPLATE, true)); + $this->fallbackResolver->method('getFallbackDirs')->willReturn([self::THEME_DIR, self::MODULE_DIR]); + $this->fallbackResolver->method('getThemeTargetDir')->willReturn(self::THEME_DIR); + $this->fallbackResolver + ->method('findDirectoryFiles') + ->willReturnCallback( + static fn(array $dirs): array => $dirs === [self::THEME_DIR] ? $overridden : $sources, + ); + } + public function testDirectoryDryRunDoesNotCopy(): void { $this->themeList->method('getAllThemes')->willReturn([new FakeTheme('Vendor/theme')]); diff --git a/tests/Unit/Model/TemplateReferenceTest.php b/tests/Unit/Model/TemplateReferenceTest.php index 97b96679..ee8b0157 100644 --- a/tests/Unit/Model/TemplateReferenceTest.php +++ b/tests/Unit/Model/TemplateReferenceTest.php @@ -26,6 +26,12 @@ public function testBuildsTemplateIdInModuleNotation(): void $this->assertSame('Magento_Checkout::cart/item/default.phtml', $reference->getTemplateId()); } + public function testIsNotADirectoryByDefaultAndExposesDirectoryFlag(): void + { + $this->assertFalse((new TemplateReference('Magento_Catalog', 'product'))->isDirectory()); + $this->assertTrue((new TemplateReference('Magento_Catalog', 'product', isDirectory: true))->isDirectory()); + } + public function testExposesTemplateType(): void { $reference = new TemplateReference( diff --git a/tests/Unit/Service/TemplateOverride/TemplateFallbackResolverTest.php b/tests/Unit/Service/TemplateOverride/TemplateFallbackResolverTest.php index 74dd4381..b984285d 100644 --- a/tests/Unit/Service/TemplateOverride/TemplateFallbackResolverTest.php +++ b/tests/Unit/Service/TemplateOverride/TemplateFallbackResolverTest.php @@ -296,6 +296,45 @@ public function testGetThemeTargetDirNormalizesBackslashesAndTrailingSlash(): vo $this->assertSame('C:\\app\\design\\frontend\\Vendor\\theme\\Magento_Catalog\\templates', $result); } + public function testFindDirectoryFilesUsesDirectoryRootWithoutSubPathAndSkipsMissingDirs(): void + { + $this->fileDriver + ->method('isDirectory') + ->willReturnCallback(static fn(string $path): bool => $path !== '/missing'); + $this->fileDriver + ->expects($this->once()) + ->method('readDirectoryRecursively') + ->with('/module/templates') + ->willReturn([ + '/module/templates/z.phtml', + '/module/templates/b.phtml', + '/other/place/ignored.phtml', + '/module/templates/sub', + ]); + $this->fileDriver + ->method('isFile') + ->willReturnCallback(static fn(string $path): bool => str_ends_with($path, '.phtml')); + + $files = $this->resolver->findDirectoryFiles(['/missing', '/module/templates'], ''); + + $this->assertSame( + ['b.phtml' => '/module/templates/b.phtml', 'z.phtml' => '/module/templates/z.phtml'], + $files, + 'Files must be sorted by path; entries outside the directory and non-files are ignored', + ); + } + + public function testFindDirectoryFilesNormalizesBackslashes(): void + { + $this->fileDriver->method('isDirectory')->willReturn(true); + $this->fileDriver->method('readDirectoryRecursively')->willReturn(['C:\\module\\templates\\sub\\a.phtml']); + $this->fileDriver->method('isFile')->willReturn(true); + + $files = $this->resolver->findDirectoryFiles(['C:\\module\\templates'], ''); + + $this->assertSame(['sub/a.phtml' => 'C:/module/templates/sub/a.phtml'], $files); + } + public function testFindDirectoryFilesMergesFallbackDirsFirstDirWinsAndSkipsExcluded(): void { $this->fileDriver->method('isDirectory')->willReturn(true); diff --git a/tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php b/tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php index e114f21c..43bbab91 100644 --- a/tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php +++ b/tests/Unit/Service/TemplateOverride/TemplatePathParserTest.php @@ -11,6 +11,7 @@ use OpenForgeProject\MageForge\Model\TemplateType; use OpenForgeProject\MageForge\Service\TemplateOverride\CompatModuleResolver; use OpenForgeProject\MageForge\Service\TemplateOverride\TemplatePathParser; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -400,6 +401,169 @@ public function testParsesThemeSubdirectoryPath(): void $this->assertSame(TemplateType::EMAIL, $reference->getType()); } + public function testParsesModuleSubdirectoryPathWithTrailingSlashFromRealPath(): void + { + $directory = '/magento/vendor/magento/module-catalog/view/frontend/templates/product/view'; + $this->fileDriver->method('isFile')->willReturn(false); + $this->fileDriver->method('isDirectory')->willReturn(true); + $this->fileDriver->method('getRealPath')->willReturn($directory . '/'); + $this->componentRegistrar + ->method('getPaths') + ->with(ComponentRegistrar::MODULE) + ->willReturn(['Magento_Catalog' => '/magento/vendor/magento/module-catalog']); + $this->compatModuleResolver->method('getOriginalModules')->willReturn([]); + + $reference = $this->parser->parse($directory); + + $this->assertTrue($reference->isDirectory()); + $this->assertSame('product/view', $reference->getTemplatePath()); + } + + public function testParsesThemeDirectoryRootWithoutSubPath(): void + { + $directory = '/magento/app/design/frontend/Vendor/theme/Magento_Sales/email'; + $this->fileDriver->method('isFile')->willReturn(false); + $this->fileDriver->method('isDirectory')->willReturn(true); + $this->fileDriver->method('getRealPath')->willReturn($directory); + $this->componentRegistrar + ->method('getPaths') + ->willReturnMap([ + [ComponentRegistrar::MODULE, []], + [ComponentRegistrar::THEME, ['frontend/Vendor/theme' => '/magento/app/design/frontend/Vendor/theme']], + ]); + $this->compatModuleResolver->method('getOriginalModules')->willReturn([]); + + $reference = $this->parser->parse($directory); + + $this->assertTrue($reference->isDirectory()); + $this->assertSame('', $reference->getTemplatePath()); + $this->assertSame(TemplateType::EMAIL, $reference->getType()); + } + + public function testMapsCompatModuleDirectoryNamedAfterOriginalModuleToItsRoot(): void + { + $directory = '/magento/vendor/hyva-themes/compat/view/frontend/templates/Magento_Catalog'; + $this->fileDriver->method('isFile')->willReturn(false); + $this->fileDriver->method('isDirectory')->willReturn(true); + $this->fileDriver->method('getRealPath')->willReturn($directory); + $this->componentRegistrar + ->method('getPaths') + ->with(ComponentRegistrar::MODULE) + ->willReturn(['Hyva_CompatCatalog' => '/magento/vendor/hyva-themes/compat']); + $this->compatModuleResolver + ->method('getOriginalModules') + ->with('Hyva_CompatCatalog') + ->willReturn(['Magento_Catalog', 'Magento_Checkout']); + + $reference = $this->parser->parse($directory); + + $this->assertTrue($reference->isDirectory()); + $this->assertSame('Magento_Catalog', $reference->getModuleName()); + $this->assertSame('', $reference->getTemplatePath()); + } + + public function testStripsOriginalModuleDirectoryFromCompatSubdirectoryPath(): void + { + $directory = '/magento/vendor/hyva-themes/compat/view/frontend/templates/Magento_Catalog/product'; + $this->fileDriver->method('isFile')->willReturn(false); + $this->fileDriver->method('isDirectory')->willReturn(true); + $this->fileDriver->method('getRealPath')->willReturn($directory); + $this->componentRegistrar + ->method('getPaths') + ->with(ComponentRegistrar::MODULE) + ->willReturn(['Hyva_CompatCatalog' => '/magento/vendor/hyva-themes/compat']); + $this->compatModuleResolver + ->method('getOriginalModules') + ->willReturn(['Magento_Catalog', 'Magento_Checkout']); + + $reference = $this->parser->parse($directory); + + $this->assertTrue($reference->isDirectory()); + $this->assertSame('Magento_Catalog', $reference->getModuleName()); + $this->assertSame('product', $reference->getTemplatePath()); + } + + public function testMapsSingleCompatModuleDirectoryToOriginalModule(): void + { + $directory = '/magento/vendor/hyva-themes/compat/view/frontend/templates/product'; + $this->fileDriver->method('isFile')->willReturn(false); + $this->fileDriver->method('isDirectory')->willReturn(true); + $this->fileDriver->method('getRealPath')->willReturn($directory); + $this->componentRegistrar + ->method('getPaths') + ->with(ComponentRegistrar::MODULE) + ->willReturn(['Hyva_CompatCatalog' => '/magento/vendor/hyva-themes/compat']); + $this->compatModuleResolver->method('getOriginalModules')->willReturn(['Magento_Catalog']); + + $reference = $this->parser->parse($directory); + + $this->assertTrue($reference->isDirectory()); + $this->assertSame('Magento_Catalog', $reference->getModuleName()); + $this->assertSame('product', $reference->getTemplatePath()); + } + + #[DataProvider('invalidModuleDirectoryProvider')] + public function testRejectsModuleDirectoriesOutsideViewDirectories(string $relativePath): void + { + $directory = '/magento/vendor/magento/module-catalog/' . $relativePath; + $this->fileDriver->method('isFile')->willReturn(false); + $this->fileDriver->method('isDirectory')->willReturn(true); + $this->fileDriver->method('getRealPath')->willReturn($directory); + $this->componentRegistrar + ->method('getPaths') + ->with(ComponentRegistrar::MODULE) + ->willReturn(['Magento_Catalog' => '/magento/vendor/magento/module-catalog']); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessageMatches('/is not inside a view/'); + + $this->parser->parse($directory); + } + + /** + * @return array + */ + public static function invalidModuleDirectoryProvider(): array + { + return [ + 'prefix before view' => ['Block/view/frontend/templates'], + 'suffix after view dir' => ['view/frontend/templates-old'], + 'view area only' => ['view/frontend'], + ]; + } + + #[DataProvider('invalidThemeDirectoryProvider')] + public function testRejectsThemeDirectoriesOutsideModuleViewDirectories(string $relativePath): void + { + $directory = '/magento/app/design/frontend/Vendor/theme/' . $relativePath; + $this->fileDriver->method('isFile')->willReturn(false); + $this->fileDriver->method('isDirectory')->willReturn(true); + $this->fileDriver->method('getRealPath')->willReturn($directory); + $this->componentRegistrar + ->method('getPaths') + ->willReturnMap([ + [ComponentRegistrar::MODULE, []], + [ComponentRegistrar::THEME, ['frontend/Vendor/theme' => '/magento/app/design/frontend/Vendor/theme']], + ]); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessageMatches('/is not a module template override/'); + + $this->parser->parse($directory); + } + + /** + * @return array + */ + public static function invalidThemeDirectoryProvider(): array + { + return [ + 'prefix before module' => ['web/Magento_Catalog/templates'], + 'suffix after view dir' => ['Magento_Catalog/templates-old'], + 'module root only' => ['Magento_Catalog'], + ]; + } + public function testRejectsModuleDirectoryOutsideViewDirectories(): void { $directory = '/magento/vendor/magento/module-catalog/Model';