-
-
Notifications
You must be signed in to change notification settings - Fork 436
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PHP 8: Fix cannot remove category on admin product edit categories tab #2205
PHP 8: Fix cannot remove category on admin product edit categories tab #2205
Conversation
This seems to be related to the database adapter. If you try getting the Id field of any model it will return a |
yes but |
Agreed |
@iamniels can you give a test to the version I pushed? hope it's fine :-) |
I made the same observation but the change e.g. with ID (integer) data from the database adapter, happened with PHP 8.1, in PHP 8.0 the DB delivers still string. We have code in our shops in place to take that into account since we frist tried out OM with PHP 8.1. |
@Sdfendor You must be right then as I only tested this with 7.4 and 8.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works well
Description (*)
On PHP 8.0, ids in the category tree JSON are rendered as integers instead of as strings on previous PHP versions. Whether this has something to do with
Mage_Adminhtml_Block_Catalog_Category_Tree::_getNodeJson
, withZend_Json::encode
or withjson_encode
is unknown to me and not relevant for this PR and worth another discussion.The issue fixed in this PR is that with PHP 8.0+ a product cannot be removed from categories in the categories tab of the product edit view. This is because the ids in the JSON provided by Magento to populate the categories tree (with
bildCategoryTree
) are integers instead of strings previously. The JS functioncategoryRemove()
in categories.phtml can only handle ids as strings.The best solution imho is to make
categoryRemove()
a bit more robust and support both integers and strings.Manual testing scenario of solved issue