-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Fix alias in grid massaction for eav collections #23452
Fix alias in grid massaction for eav collections #23452
Conversation
Hi @thomas-kl1. Thank you for your contribution
For more details, please, review the Magento Contributor Assistant documentation |
Hi @sidolov, thank you for the review. |
@thomas-kl1, thank you for collaboration. Can you please provide strong motivation why this PR should be merged? It works fine with Product collection, but it's only because of \Magento\Catalog\Model\ResourceModel\Product\Collection::MAIN_TABLE_ALIAS value. I'm not sure if your solution will work in other cases. |
Hi @engcom-Foxtrot |
@sidolov Have you any updates? I've seen a similar PR, I don't know what the best and maintainable solution is. Can you advice? |
$idsSelect = clone $collection->getSelect(); | ||
$idsSelect->reset(\Magento\Framework\DB\Select::ORDER); | ||
$idsSelect->reset(\Magento\Framework\DB\Select::LIMIT_COUNT); | ||
$idsSelect->reset(\Magento\Framework\DB\Select::LIMIT_OFFSET); | ||
$idsSelect->reset(\Magento\Framework\DB\Select::COLUMNS); | ||
$idsSelect->columns($this->getMassactionIdField(), 'main_table'); | ||
$idsSelect->columns($this->getMassactionIdField(), $alias); |
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.
Will it work for both cases if we remove alias at all?
@magento give me test instance |
Hi @thomas-kl1. Thank you for your request. I'm working on Magento instance for you |
Hi @thomas-kl1, here is your new Magento instance. |
Hi @sidolov, thank you for the review. |
Hi @thomas-kl1, thank you for your contribution! |
Related to M2.2.x: MAGETWO-96957
Related to M2.3.x: MAGETWO-98617
Description (*)
Impossible to use the massaction on Vanilla grid with an EAV collection.
Fixed Issues (if relevant)
Manual testing scenarios (*)
See related issue #23451
Questions or comments
As all grids should be built with the ui component, this exception should never happen. However, many third-part extension still use the deprecated grid system.
Contribution checklist (*)