fix: Disable global attribute field when editing on a store view scope - #5699
Open
wild82pl wants to merge 3 commits into
Open
fix: Disable global attribute field when editing on a store view scope#5699wild82pl wants to merge 3 commits into
wild82pl wants to merge 3 commits into
Conversation
Mage_Adminhtml_Block_Catalog_Form_Renderer_Fieldset_Element::checkFieldDisable() already disables the "use default" checkbox for global-scope attributes via canDisplayUseDefault(), but the field itself remained editable and savable when the fieldset was rendered for a specific store view (not the default/all-store-views scope). Saving from that state writes a store-scoped row for an attribute that should only ever have a global value, corrupting the attribute's data. Add isGlobalAttributeOnStoreScope() to detect that case (global-scope attribute + a data object with a non-default store id) and disable the field the same way canDisplayUseDefault()/usedDefault() already does, closing the gap that mechanism leaves open.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an admin product-edit form edge case where global-scope EAV attributes remained editable when editing a product in a specific store view scope, allowing an invalid store-scoped value row to be saved for a globally-scoped attribute.
Changes:
- Added
Mage_Adminhtml_Block_Catalog_Form_Renderer_Fieldset_Element::isGlobalAttributeOnStoreScope()to detect global attributes being edited in a non-default store scope. - Updated
checkFieldDisable()to disable the form element when this invalid edit context is detected. - Added a PHPUnit test + data provider covering global/default scope behavior and ensuring website/store-scoped attributes remain unaffected.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| app/code/core/Mage/Adminhtml/Block/Catalog/Form/Renderer/Fieldset/Element.php | Adds detection for global-attribute-on-store-scope and disables the field to prevent invalid store-scoped saves. |
| tests/unit/Mage/Adminhtml/Block/Catalog/Form/Renderer/Fieldset/ElementTest.php | Introduces a unit test validating the new detection logic via a mocked subject. |
| tests/unit/Traits/DataProvider/Mage/Adminhtml/Block/Catalog/Form/Renderer/Fieldset/ElementTrait.php | Adds a data provider with cases for global/website/store scope attributes across store-id contexts. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
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
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.



Description (*)
Mage_Adminhtml_Block_Catalog_Form_Renderer_Fieldset_Element::checkFieldDisable()already disables the "use default" checkbox for global-scope attributes viacanDisplayUseDefault(), but the field itself remains editable and savable when the fieldset is rendered for a specific store view (not the default/all-store-views scope). Saving from that state writes a store-scoped row for an attribute that should only ever have a global value, corrupting the attribute's data.This adds
isGlobalAttributeOnStoreScope()to detect that case (global-scope attribute + a data object with a non-default store id) and disable the field the same waycanDisplayUseDefault()/usedDefault()already do, closing the gap that mechanism leaves open.Manual testing scenarios (*)
Questions or comments
Added
isGlobalAttributeOnStoreScope()and a PHPUnit test (ElementTest/ElementTrait) covering: global attribute on a store view (disabled), global attribute on default scope (editable), and website/store-scope attributes on a store view (unaffected, editable). Verified locally: PHPUnit, PHPStan, ECS and PHPMD all pass clean on the changed files.Contribution checklist (*)