OpenMage / magento-lts

Official OpenMage LTS codebase | Migrate easily from Magento Community Edition in minutes! Download the source code for free or contribute to OpenMage LTS | Security vulnerability patches, bug fixes, performance improvements and more.
https://www.openmage.org
Open Software License 3.0
866 stars 436 forks source link

Fixed cannot sort by column in backend's grids #3985

Closed fballiano closed 4 months ago

fballiano commented 4 months ago

https://github.com/OpenMage/magento-lts/pull/3927 broke grid sorting functionality in the backend because the js was looking for the "name" attribute.

This PR rewrites the doSort methods fixing the bug

empiricompany commented 4 months ago

Sorry, but the patch solves the sorting issues in the default grid, but the module for advanced grids, widely used, mage-enhanced-admin-grids continues to not work.

The problem is here: https://github.com/OpenMage/magento-lts/commit/91b1944b2cad76349ce0e8899d6821eaaabd3ca7 where is being removed the attribute name

I have also try to change attribute name to data-name and update it here https://github.com/empiricompany/mage-enhanced-admin-grids/blob/7aa9a82bc86d1ea87c07dd00db7251ff1ea96162/js/bl/customgrid/config.js#L226

but not resolve the issues, the sorting and filters are not working.

fballiano commented 4 months ago

did you try element.parentNode.dataSet.columnId?

empiricompany commented 4 months ago

did you try element.parentNode.dataSet.columnId?

Tried now, but it doesn't work.

fballiano commented 4 months ago

I've re-added the "name" attribute to the "a", which btw is also reported by phpstorm

Screenshot 2024-05-09 alle 11 35 42
empiricompany commented 4 months ago

True, the tag ID should be used instead. For now, thank you for restoring "name".

We should find a solution for these old, widely used extensions. For example, I was thinking of creating a dedicated clone repository of OpenMage and keep it updated then store all patches in the PRs then keep synchronized with the main one. But i'm not sure i have so much time...

empiricompany commented 4 months ago

Sorry, for it to work, we also need to revert " js/mage/adminhtml/grid.js" to version 20.6.0

https://raw.githubusercontent.com/OpenMage/magento-lts/v20.6.0/js/mage/adminhtml/grid.js

---
 js/mage/adminhtml/grid.js | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/js/mage/adminhtml/grid.js b/js/mage/adminhtml/grid.js
index 55eb3b2ef05..9e2a168e5d4 100644
--- a/js/mage/adminhtml/grid.js
+++ b/js/mage/adminhtml/grid.js
@@ -141,15 +141,15 @@ varienGrid.prototype = {

     },
     doSort : function(event){
-        var element = Event.findElement(event, 'a');
+        event.preventDefault();
+        const element = event.target.closest('a');
+        const parentElement = element.parentNode;

-        if(element.name && element.title){
-            this.addVarToUrl(this.sortVar, element.name);
+        if (element && parentElement && parentElement.dataset.columnId && element.title) {
+            this.addVarToUrl(this.sortVar, parentElement.dataset.columnId);
             this.addVarToUrl(this.dirVar, element.title);
             this.reload(this.url);
         }
-        Event.stop(event);
-        return false;
     },
     loadByElement : function(element){
         if(element && element.name){
fballiano commented 4 months ago

done

empiricompany commented 4 months ago

For those who want to fix it immediately 20.0.7 without downgrade, you can apply the patch like this:

{
    [...]
    "require": {
        "aydin-hassan/magento-core-composer-installer": "~2.1.0",
        "openmage/magento-lts": "^20.7.0",
    [...]
    "extra": {
        "enable-patching": true,
        [...]
        "patches": {
            "openmage/magento-lts": {
                "Fixed cannot sort by column in backend's grids #3985": "https://patch-diff.githubusercontent.com/raw/OpenMage/magento-lts/pull/3985.patch"
            }
        }
    }
fballiano commented 4 months ago

I don't understand how the js thing can happen but whatever :-\