Couldn't select all items on Review grid

[paul-at-straker]

I couldn't select all items on review grid.

Preconditions

1. Magento 2.2.1
2. PHP 7.1.12

Steps to reproduce

1. login Magento admin
2. from main menu, select MARKETING -> User Content -> Reviews
3. select Select All under Actions checkbox

Expected result

1. it should show 346 records found (346 selected)

Actual result

1. it should show 346 records found (20 selected)

[magento-engcom-team]

@paul-at-straker, thank you for your report.
We've created internal ticket(s) MAGETWO-85227 to track progress on the issue.

[osrecio]

The problem is due to: MassAction/Extended.php. When try to get: getColumValues from Review\Product\Collection the collection is loaded and only get the first 20 Product Reviews.

In this moment I don't know what is the best way to solve this issue. I will continue investigating.

Thanks for the catch @paul-at-straker

[paul-at-straker]

As far as know getGridIdsJson() in the version of 2.1.x uses getAllIds instead of getColumValues(). I have tried to replace the entire getGridIdsJson() with the code copied from version 2.1.x, it works. Although I haven't looked at it deeply, it might be helpful.

[osrecio]

Yes, This changes was made to allow get Id's not only for primary key: #9610.

[osrecio]

Same happens in Grid Notification.

Ivestigating possibilities to fix #9610

[sheenu3091]

I am working in at #dmcdindia

[magento-engcom-team]

@sheenu3091 thank you for joining. Please accept team invitation here and self-assign the issue.

[jspoe]

I would suggest to change this into an ui-component with a proper data-provider instead of the old Grid-block method, to make it future proof and generic with the other grids.
This should fix this bug too.

[jspoe]

@magento-engcom-team @okorshenko Do you agree with this approach? If that is the case, I will continue to work on this solution.

[VladimirZaets]

Hi @jspoe. I agree that this issue should be fixed only in the ui-component grid, no sense to fix it in the old grids.
The correct fix for this issue is replacing the old grid with ui-component grid.
@jspoe As I see the ui-component grid works correctly (screenshots are attached. We can select items per page or all items in all pages), so I don't fully understand the solution in the ui-component grid you said, could you please provide more information?

[jspoe]

@VladimirZaets The scenario where this can be reproduced (Marketing -> User Content -> Reviews) is an old grid. This should be transformed from an old grid into an UiComponent instead of just fix the selection bug of the grid.

Before I spent a couple of hours in transforming the old grid into an UiComponent and removing all the references and files of the old grid. It would be a waste of time if this change was already made in another issue or by another team. Therefor I just wanted to have the confirmation to make this whole change instead of only fix the selection bug. :)

[raisaev]

Transforming the Reviews grid to the version based on UiComponent is definitely a good thing.

But the issue can be reproduced on ANY of grids that still use the old approach. Yes, perhaps this is really the last of the grids that do not use UiComponent in Magento Core but there can be a lot in other extensions written for M2.

A good solution is to do both:

• fix the selection bug for all old grids
• transform Reviews grid to the UiComponent approach

[jspoe]

Agreed, I'll work on the bugfix first then and make a pull request for the UiComponent afterwards.

[jspoe]

Debugged this right down to the problem, with the given information of #9610

The problem is the fact that the change to getColumnValues() did not take in account that a collection already could be loaded with a given limit. In the case of the toolbar of the review grid. The collection gets loaded and the default methods to reset it are not possible, because of an afterLoad in Model/ResourceModel/Review.php that joins the store table onto it. That will result in a correlation error because the afterLoad method will be called twice.

Solution in this case might be to have a cloned version of an (yet) unloaded collection in Magento/Backend/Block/Widget/Grid/Extended.php. But that is not a solid solution either, because you'll have a second collection in your object and thus maks the object heavier than it should be. I also thought of the getSelectCountSql() that is available in most collections and in the AbstractDb class, but that might also result in some unwanted constructions because you could miss the column that you want to use.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 14 days if no further activity occurs. Thank you for your contributions.