refactor: replace template override with plugin-based logo injection#4
Open
rhoerr wants to merge 4 commits intoelement119:masterfrom
Open
refactor: replace template override with plugin-based logo injection#4rhoerr wants to merge 4 commits intoelement119:masterfrom
rhoerr wants to merge 4 commits intoelement119:masterfrom
Conversation
Eliminate the fragile full-template override of Magento_Backend::page/header.phtml by using a plugin on Header::toHtml() and Header::getViewFileUrl() instead. The core already differentiates login vs. menu logos via layout handles (default.xml vs admin_login.xml setting logo_image_src). This refactor leverages that mechanism: layout XML passes config path and upload directory as block arguments, and the plugin reads config, builds the media URL, and sets logo_image_src before rendering. A companion afterGetViewFileUrl method passes full URLs through without asset repository resolution. Removes 4 files (template, ViewModel, Model, Scope/Config) and adds 3 (plugin class, di.xml, admin_login layout). The module now works with any theme's default logos and is immune to core template changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cover all branches of beforeToHtml (no config path, no upload dir, no filename in config, menu logo URL, login logo URL) and afterGetViewFileUrl (https passthrough, http passthrough, view file fallback). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Wrap storeManager->getStore() in try-catch for NoSuchEntityException to prevent admin lockout from a cosmetic module - Add is_string() type guard on config value (getValue returns mixed) - Add basename() sanitization to strip path traversal from filename - Inject LoggerInterface and log warning on store resolution failure - Add @param/@return PHPDoc tags for Magento2 PHPCS compliance - Add class-level docblock explaining the plugin design rationale - Fix license header: LICENCE.txt → LICENSE across all module files - Add 4 new test cases: empty string config, non-string config, path traversal stripping, NoSuchEntityException handling Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… config - Add show_part !== 'logo' guard to both plugin methods so they skip non-logo Header block instances (e.g. the 'user' block) - Catch \Throwable in addition to NoSuchEntityException so unexpected errors degrade gracefully instead of breaking the admin header - Fix system.xml group scope to showInWebsite=0/showInStore=0 matching the field visibility (prevents empty group at non-default scopes) - Add missing composer.json dependencies (framework, backend, store) - Remove redundant _getUploadDir() overrides from backend models (parent File class already resolves path from system.xml config) - Fix LESS license header (LICENCE.txt -> LICENSE) - Update tests: add show_part guard and \Throwable coverage (15 tests) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
I think this will resolve #3, by doing away with the template override. I still need to test it, so leaving as draft for now. |
Contributor
Author
|
Tested. Works as intended (for me). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the full template override of
Magento_Backend::page/header.phtmlwith a lightweight plugin onMagento\Backend\Block\Page\Header, eliminating template drift and improving maintainability.beforeToHtmlreads config path and upload directory from layout XML arguments, builds a media URL, and setslogo_image_srcon the block.afterGetViewFileUrlpasses absolute URLs through unchanged so the core template renders them without static file resolution.default.xmlpasses menu logo config;admin_login.xmloverrides with login logo config. No login-page detection logic needed.show_partguard limits plugin to thelogoblock instance only;is_string()type guard andbasename()path-traversal sanitization on config values;NoSuchEntityException+\Throwablecatch with PSR-3 logging ensures the plugin never breaks the admin header._getUploadDir()overrides from backend models (parent resolves fromsystem.xml); fixedsystem.xmlgroup scope mismatch; added missingcomposer.jsondependencies; fixed LESS license header.Files removed
view/adminhtml/templates/page/header.phtml— template override (the whole point)ViewModel/AdminLogo.php— login-page detection replaced by layout handlesModel/AdminLogo.php— URL construction moved to plugin (also fixesFileDrivermisuse)Scope/Config.php— config reading moved to plugin viaScopeConfigInterfaceFiles added
Plugin/Backend/Block/Page/HeaderPlugin.phpetc/adminhtml/di.xmlview/adminhtml/layout/admin_login.xmlTest/Unit/Plugin/Backend/Block/Page/HeaderPluginTest.php— 15 unit tests, 20 assertionsTest plan
bin/magento setup:upgrade— module.xml sequence changedbin/magento setup:di:compile— new plugin class, deleted classesbin/magento cache:flush<img>src: media URL when custom, static view file URL when defaultvendor/bin/phpunit app/code/Element119/CustomAdminLogo/Test/— 15 tests, 20 assertions, all pass🤖 Generated with Claude Code