Adds configurable columns to product grid

[empiricompany]

This add new configuration to choose columns for catalog product list, i think this is an important feature to add to OpenMage.

IMPORTANT: Original solution with trait has been replaced with a new flexible architecture that follow magento way, inspiration from massaction grid component. Also create an interface to implement custom columns in other grids easily keeping the code well organized. All the relevant classes that extend Mage_Adminhtml_Block_Widget_Grid have been restored to their original state.

Technical details

To modify the grid columns, two things need to be done:

• Add new columns data to the collection
• Add new columns to be printed in the grid

For this reason, the class Mage_Adminhtml_Block_Widget_Grid initialize an helper that must implements two methods for this purpose. The helper is bound by a contract with the interface Mage_Adminhtml_Helper_Widget_Grid_Config_Interface, which requires the implementation of the two methods:

• public function applyAdvancedGridCollection($collection);

/**
* Collection object
*
* @param Mage_Core_Model_Resource_Db_Collection_Abstract $collection
* return $this
*/
public function applyAdvancedGridCollection($collection);

This method is called by 'Mage_Adminhtml_Block_Widget_Grid' after _prepareCollection() and allows to modify the collection by accessing the $collection->addAttributeToSelect and other methods.

• public function applyAdvancedGridColumn($gridBlock);

/**
 * Adminhtml grid widget block
 *
 * @param Mage_Adminhtml_Block_Widget_Grid $gridBlock
 * return $this
 */
public function applyAdvancedGridColumn($gridBlock);

This method is called after _prepareColumns and allows adding new columns to the grid by accessing the $gridBlock->addColumnAfter() and all the public methods of Mage_Adminhtml_Block_Widget_Grid .

Here, for example, I quickly implemented a new field in the orders grid:

https://github.com/empiricompany/openmage/commit/b40ae3420f425f7ece5794218f3d734036f1720c (without system configuration)

---

The helper for advanced grid is initialized like this. Here, I have made a note that a factory class would be needed.

/**
* Get Helper Advanced Grid
*
* @return Mage_Adminhtml_Helper_Widget_Grid_Config_Abstract
*/
public function getHelperAdvancedGrid(): Mage_Adminhtml_Helper_Widget_Grid_Config_Abstract
{
  if (!$this->_advancedGridHelper) {
      // TODO create factory class map block id to helper - create a new grid.xml configuration file
      switch ($this->getId()) {
          case 'productGrid':
          case 'catalog_category_products':
          case 'related_product_grid':
          case 'up_sell_product_grid':
          case 'cross_sell_product_grid':
          case 'super_product_links':
              $this->setAdvancedGridHelperName('adminhtml/widget_grid_config_catalog_product');
              break;
          case 'sales_order_grid':
              $this->setAdvancedGridHelperName('adminhtml/widget_grid_config_sales_order');
              break;
      }

      $this->_advancedGridHelper = Mage::helper($this->getAdvancedGridHelperName());

      if (!($this->_advancedGridHelper instanceof Mage_Adminhtml_Helper_Widget_Grid_Config_Abstract)) {
          Mage::throwException(
              $this->__("Helper for advanced grid config doesn't implement required interface. %s must extends Mage_Adminhtml_Helper_Widget_Grid_Config_Abstract", get_class($this->_advancedGridHelper))
          );
      }
  }
  $this->_advancedGridHelper->setGridId($this->getId());
  return $this->_advancedGridHelper;
}

The identification of the correct helper by checking block id avoids setting the helper manually in each relevant class. I'm thinking about a new configuration file called grid.xml that maps grid IDs to helpers and a factory class. Otherwise, and it remains an option, we should specify an helper for advanced grid like this:

class Mage_Adminhtml_Block_Catalog_Product_Grid extends Mage_Adminhtml_Block_Widget_Grid
{
    public function __construct()
    {
        parent::__construct();
        $this->setId('productGrid');
        
        // Set helperAdvancedGrid
        $this->setAdvancedGridHelperName('adminhtml/widget_grid_config_catalog_product'); 
        
        $this->setDefaultSort('entity_id');
        $this->setDefaultDir('DESC');
        $this->setSaveParametersInSession(true);
        $this->setUseAjax(true);
        $this->setVarNameFilter('product_filter');
    }

---

Column sorting

To manage column sorting the class Mage_Adminhtml_Block_Widget_Grid adds the block adminhtml/widget_grid_advanced (as massaction) to the 'grid.phtml' template.

The block is then called in grid.phtml, after massactionblock:

<?php if($this->getAdvancedGridBlock()->isAvailable()): ?>
    <?php echo $this->getAdvancedGridBlock()->getJavaScript() ?>
 <?php endif ?>

This block inserts the JavaScript that will handle the sorting by initializing 'varienGridAdvanced' (included in grid.js) PS: This block can also be used to insert a reset button for column order.

For this feature a new controller Mage_Adminhtml_GridController has been created. The columns can be sorted using drag & drop. Upon the drop event, an AJAX call to adminhtml/grid/saveColumnOrder is made, and the sequence of columns is saved in system_config advanced_grid/_id_of_grid_/order. Then the grid reloads as usual through AJAX, similar to a search.

---

Features:

• [x] Enable/disable custom columns for single grids:
• Catalog Product list
• Catalog Category Products tab
• Catalog Product View Related, Up-sells, Cross-sells and Associated Products tabs
• [x] Choose columns to display for single grids
• [x] Show Columns Created At and Updated At
• [x] Choose Image Preview Columns: Base Image, Small Image, Thumbnail
• [x] Set custom width for product image (default 64px)
• [x] Render correctly product images and field by type like: select, multiselect, date, price and text/textarea
• [x] Add a second horizontal scroll on top if the columns don't fit in the window
• [x] Set order columns by drag&drop

I have added a new column renderer for product images. The images are resized to 64px by default to reduce file size, and they are generated in media/catalog/product/cache/0/64x64/ directory. Image previews are linked to the original url.

This is a proof of concept - Not ready for production

I have not tested yet with php 7.4, only php 8.2

It should be perfectly compatible with any existing columns added by overwriting the classes that extend Mage_Adminhtml_Block_Widget_Grid, like Mage_Adminhtml_Block_Catalog_Product_Grid.

Knowed Bugs / Issues:

• [ ] Missing translations
• [x] In multi grid product view there are issues with columns sorting
• [x] ~~Phpstan doesn't like constants in traits, even though in theory they seem to not cause any problems when called with self like: self::CONFIG_PATH_GRID_ENABLED ref: https://wiki.php.net/rfc/constants_in_traits Costants moved to a dedicated helper Mage_Adminhtml_Helper_Widget_Grid_Config~~

Improvements:

• [x] Reset columns order button
• [ ] Force column drag only to x axis
• [ ] Role permission to restrict the ability to sort columns
• [ ] Possibility to remove default columns
• [ ] Configuration field image width must show only if this columns are selected: image, small_image, thumbnail

Code Refactoring

• [x] Move css for column grid sorting to backend theme (now is injected in grid.phtml)
• [ ] Create a dedicate helper that configure Mage_Adminhtml_Block_Widget_Grid and separete from helper class that retrive data from config

[pquerner]

Imho this should be made optional. Our instance doesnt have images, instead they are coming from CDN anyways and we've overwritten the parts, but it would make it (I think) like we have "none" images for any product - and simply add a column for no reason.

Also I think some vendors doesnt want that even if they have their product images attached to magento.

[addison74]

We should have the implementation of Magento 2 grids in OpenMage, but more extensive changes are needed.

There are a lot of implementations all over the Internet and for a while I considered displaying an image in the grid as useful for quick visual identification of the product, but the feedback I received was negative. First of all, the grid was getting bigger, especially when 200 products were viewed on the page. The page loading time increases, there are requests for each individual image.

I could agree with this PR only if there is also the option to disable the display of the column in product grid, which should be implicitly set in Backend to No.

[addison74]

... there must be the option of control in the Backend for each individual grid where images are displayed: Product Grid, Category Grid, Related/Up-sells/Cross-sells Grids.

[empiricompany]

We should have the implementation of Magento 2 grids in OpenMage, but more extensive changes are needed.

There are a lot of implementations all over the Internet and for a while I considered displaying an image in the grid as useful for quick visual identification of the product, but the feedback I received was negative. First of all, the grid was getting bigger, especially when 200 products were viewed on the page. The page loading time increases, there are requests for each individual image.

I could agree with this PR only if there is also the option to disable the display of the column in product grid, which should be implicitly set in Backend to No.

I agree, I will convert this PR into a draft because I want to immediately configure the choice of columns to display, which by default does nothing in order to avoid any compatibility and performance issues. I will complete the PR in the next few days while waiting for further feedback.

[addison74]

In addition, we must set up in the Backend, in the same section that controls each individual grid, the size of the image.

Displaying an image of 64 px height compared to 200 displayed products on the page or maybe even more if someone uses other custom values, will increase the use of the scroll, will at least triple the display.

[empiricompany]

What do you think about last commit https://github.com/OpenMage/magento-lts/pull/3451/commits/be9d683327cdf446669b341dc0ebb21cd510c418?

I've used traits to add columns to the grid and i've added configuration to enable product image column and setting the width. Trait should be the safest way to avoid compatibility issues.

If this path can be ok, I can proceed with additional features and reorganize the configuration more effectively, for which I would dedicate a separate tab.

Same questions about:

• there is better location where to place the traits?
• i have to implement same sort of method to configure the order of columns, now i've just placed product image after entity_id

PS: phpstan report that traits cannot have constant, but using self it's ok i've to implement an interface like that? https://wiki.php.net/rfc/constants_in_traits

then

interface Mage_Adminhtml_Block_Widget_Grid_Config_Product_Columns_Interface
{
    public const CONFIG_PATH_GRID_COLUMNS = 'admin/grid_catalog/columns';
    public const CONFIG_PATH_GRID_COLUMN_IMAGE_WIDTH = 'admin/grid_catalog/imagewith';
}

class Mage_Adminhtml_Block_Catalog_Product_Grid extends Mage_Adminhtml_Block_Widget_Grid implements Mage_Adminhtml_Block_Widget_Grid_Config_Product_Columns_Interface
{
    use Mage_Adminhtml_Block_Widget_Grid_Config_Product_Columns;

waiting for feedback

[addison74]

I would like to be able to control the display of the column in each of the 5 grids where it appears: Product, Category, Up-Sells, Cross-Sells, Related.

From the attached image it appears that it is a unitary control and not an individual one.

The idea of using multiselect is not a bad one

• Catalog Grid Configuration - it is OK, it requires the translation label
• Columns - OK too
• Product Image - here I would control the 5 grids for the image, adding the 5 options like this

Product - Image Thumbnail Category - Image Thumbnail Cross-Sells - Image Thumbnail Related - Image Thumbnail Up-Sells - Image Thumbnail

But if he has other custom grids then he will no longer have control.

I think it would have been better to have a discussion in the dedicated section about this idea and then you could proceed to its implementation.

[empiricompany]

I would like to be able to control the display of the column in each of the 5 grids where it appears: Product, Category, Up-Sells, Cross-Sells, Related.

From the attached image it appears that it is a unitary control and not an individual one.

The idea of using multiselect is not a bad one

* **Catalog Grid Configuration** - it is OK, it requires the translation label

* **Columns** - OK too

* **Product Image** - here I would control the 5 grids for the image, adding the 5 options like this

Product - Image Thumbnail Category - Image Thumbnail Cross-Sells - Image Thumbnail Related - Image Thumbnail Up-Sells - Image Thumbnail

But if he has other custom grids then he will no longer have control.

I think it would have been better to have a discussion in the dedicated section about this idea and then you could proceed to its implementation.

Yes, through the trait and the way I have organized the code, I would be able to implement dedicated configurations for each type of grid (catalog_product, catalog_category/product, upsell, related, crossell, and other grids like sales), where you can set additional columns besides the product image a its order. i want feedback about current implementation with traits

[addison74]

Some ideas related to the UI

1 - The label name Enable Grids creates confusion, because we do not allow enable and disable those grids. Another expression must be used, e.g. Available Grids.

2 - I would name the options in the multiselect list based on a convention (main category + action), ordered alphabetically. We are removing the Catalog to reduce its length and we must quickly understand where the grids are

Categories / Products Products / List Products / Edit / Associated Products / Edit / Cross-Sells Products / Edit / Related Products / Edit / Up-Sells

3 - When the product is configurable, a grid appears in Associated Products tab. I added it to the list of grids above.

[empiricompany]

@ADDISON74, please provide a PR to explain your vision in detail and what you are trying to achieve

[addison74]

@empiricompany - first of all, I don't have time to implement this PR in the way I would like it. Secondly, I don't consider this feature important at the moment, I like the one in Magento 2, but I can live without it as before.

Because you proposed a PR, I tested your implementation and provided you a feedback. If it ends up in OpenMage, it must be functional and useful.

[sreichel]

Mhh maybe better to fork https://github.com/jayelkaake/enhancedgrid or https://github.com/blmage/mage-enhanced-admin-grids?

Btw constants in traits are only for php 8.2.

[sreichel]

PS: phpstan report that traits cannot have constant, but using self it's ok i've to implement an interface like that?

Think "yes", interface should work.

Untested: i guess it would also work to add that constants to a block. or helper class. Constants should be accessible from inside the trait?

[empiricompany]

Mhh maybe better to fork https://github.com/jayelkaake/enhancedgrid or https://github.com/blmage/mage-enhanced-admin-grids?

Btw constants in traits are only for php 8.2.

I used https://github.com/blmage/mage-enhanced-admin-grids, but it is no longer compatible with OpenMage and PHP 8.2. The code is very messy and heavily relies on create_function and hacks. I haven't tested it yet, but this one seems more interesting: https://github.com/jayelkaake/enhancedgrid.

Are you suggesting that it's better to create a separate extension rather than including it in the core of OpenMage?

[addison74]

Sven's feedback is welcome. I think the best option would be using an extension. Those who need it can install it, in addition, we don't have to maintain it when problems are reported. It is what we did with a lot of core modules like Backup, Poll, both having their repositories

I remain of the opinion that such proposals should be discussed first and then proceed to their implementation or not.

[fballiano]

I've a different opinion (sorry, I'm getting back on track, I was away for a few days), I think such funcionality would be a nice addition to the core and at least it would be maintained, many extensions become unmaintained with time sadly.

Backup/Poll were ancient module that IMHO have no reason to exist anymore in 2023, that's why they were removed, but grid customization would be awesome to have.

If there was a really good working module out there, than by any mean let's use it and don't reinvent the wheel, but with my customers we old old clanky modules that half-work nowadays so... I don't know... it doesn't seem there's any complete solution out there to reccommend.

EDIT: I didn't have time to test this implementation yet, but I'll try to do that asap.

[empiricompany]

Thanks to everyone, I would like a lot of feedback on this. I am working on it a lot. The latest features seem very useful to me, such as displaying the creation/update date (I use this column a lot to identify products updated through external integrations) and the addition of a second horizontal scroll at the top if the columns don't all fit in the grid PS: Tested with the OpenMage theme and the old theme.

[addison74]

@empiricompany - I will help you with testing and feedback if you want to continue with this PR.

We should analyze if it can be implemented as an extension in OpenMage to be enabled/disabled. In case someone does not want to load it to have full control.

[empiricompany]

@empiricompany - I will help you with testing and feedback if you want to continue with this PR.

We should analyze if it can be implemented as an extension in OpenMage to be enabled/disabled. In case someone does not want to load it to have full control.

@ADDISON74 - The beauty is that the new feature is disabled by default, it is non-invasive, and it doesn't load anything. I would like to have this functionality officially maintained with OpenMage and not have to rewrite it as an external extension. The idea is to extend the possibility of customization to other grids as well.

[fballiano]

So, I checked it out briefly, here's my personal feedback:

• I'm not sure about adding traits to the codebase, since it would be the first time (and I don't love them in general)
• @ADDISON74 it's right about the "Enable Grids" label, I don't understand what it really does
• why "created_at/updated_at" are not in the "columns" multiselect?
• I think we would need a separate configuration for every grid because having a single one seems kinda limiting

sorry, I'm not sure about this one as it is now, yes it allows to add images and to "turn off" some columns but at the moment it seems to come with a bit too many caveats, at least in my opinion.

[empiricompany]

@fballiano @ADDISON74, please check the latest version. We have now separated the configuration for each grid area

@sreichel, I have simplified the trait and moved all constants to a helper class. The future idea is to create a kind of "decorator" class for each grid type (catalog_product, sales_, etc.) that the trait will read to remain generic.

I would also like some feedback on the names of the new methods and where it is most appropriate to place the classes (I was thinking of a ?new folder for Traits?). @fballiano I chose to create a trait to add new methods to the class without having to repeat them in all relevant blocks and avoid duplicated code. In my opinion, it is a good strategy, and I use them a lot in Laravel.

[empiricompany]

i've updated PR description with new screenshots

[addison74]

A short feedback after looking over the updates.

1. If there are 1000 attributes in a store, that multiselect list will be very large. You could consider that in the management page of an attribute, add a new Backend Properties section to the Properties tab to have a Use in Product Grid with Yes/No options. In this way, everyone will be able to set their own short list of attributes for the grids and you won't have to list them all in the configuration.

3. Why is Product Image Preview a multiselect? I would only use the base image and not offer this option.

4. Please check that there is another grid to take into account. When the product type is configurable, this grid appears in the Associated Products tab.

5. TBD which is the best place in the backend where to add the configuration section.

6. We must work on naming the labels and then add the translations. Let's not abuse comments where it really isn't necessary.

7. TBD if it's fine to use traits, considering that I haven't really come across it in the OpenMage code.

[empiricompany]

@ADDISON74 - appreciate your suggestions, thank you.

1. If there are 1000 attributes in a store, that multiselect list will be very large. You could consider that in the management page of an attribute, add a new Backend Properties section to the Properties tab to have a Use in Product Grid with Yes/No options. In this way, everyone will be able to set their own short list of attributes for the grids and you won't have to list them all in the configuration.

It could be a solution, but I didn't want to modify the database schema for now. It had to be as non-invasive as possible. The same applies to attribute management. Maybe a dedicated backend section could also be implemented with dedicated schema tables to store configurations where attributes are pagineted and more options, i think it would be easier to implement sorting, for example. Or we can create a new component form for multiselect with search...possibile solutions can be endless but it will definitely be something to consider.

3. Why is Product Image Preview a multiselect? I would only use the base image and not offer this option.

I wanted to give this option as well so that everyone can choose the image they prefer and any other media_image type attributes added.

4. Please check that there is another grid to take into account. When the product type is configurable, this grid appears in the Associated Products tab.

I know, I saw it, but unfortunately that grid is not standard and does not extend Mage_Adminhtml_Block_Widget_Grid. It's not that easy.

[addison74]

At point #3 it cannot be multiselect, Only one image from the list can be used.

[empiricompany]

At point #3 it cannot be multiselect, Only one image from the list can be used.

why?

[addison74]

At point #3 it cannot be multiselect, Only one image from the list can be used.

why?

If I select 10 images which one will be used in the grid?

[empiricompany]

At point #3 it cannot be multiselect, Only one image from the list can be used.

why?

If I select 10 images which one will be used in the grid?

All selected ones will be displayed. Please before quickly comment the PR test it.

[addison74]

I don't think it's a good idea to allow multiple images to be listed in the grid, but let's see other opinions. One alone is enough. Basically what is the role of the image? That of identifying the product faster when scrolling through the list. At the moment in OpenMage, finding one or more products can be done extremely easily by filtering the columns.

It was not a quick comment between two sips of coffee. The posted image helped me enough. I noticed that sometimes the authors of a PR tend to be subjective or to feel hurt by the comments. What can I say? It is an interesting experience to create PR's because you really have to convince as many people as possible.

[fballiano]

it could be interesting to have more than one images (although probably not the case for everybody) in case different "image attributes" are important to show important part of the products and combined in different way through the catalog so, at the end of the day, why not.