Smile-SA / elasticsuite

Smile ElasticSuite - Magento 2 merchandising and search engine built on ElasticSearch
https://elasticsuite.io
Open Software License 3.0
763 stars 342 forks source link

Virtual Categories and $category->getProductCollection() #2887

Open Nuranto opened 1 year ago

Nuranto commented 1 year ago

I went into the $category->getProductCollection() problem. I saw that there is many issues on this problem, and I saw the proposed solution.

However, it makes no sense to me. I think $category->getProductCollection() should return the correct results, without having to add custom code to make it work.

I'm trying to find a solution for this, and I was first thinking at adding a plugin to Category model to return a Fulltext\Collection for getProductCollection() if category is virtual. But that will probably cause issues if we don't always return the same model, right ?

So maybe another approach, would be to create an Indexer that populates catalog_category_product from the virtual category results. However, it would probably a be a very high resource-consuming indexer, so I'm not sure about that solution either.

Third solution I was thinking about : a Plugin for Magento\Catalog\Model\ResourceModel\Product\Collection, for addCategoryFilter method : If virtual : exec the request, and add a filter on product ID, else use parent method. It could result at filtering on thousands of IDs, which I don't like, but I think it is already working like this elsewhere, so why not ?

What do you think about it ? Do you see a better solution ?

romainruaud commented 1 year ago

I went into the $category->getProductCollection() problem. I saw that there is many issues on this problem, and I saw the proposed solution.

However, it makes no sense to me. I think $category->getProductCollection() should return the correct results, without having to add custom code to make it work.

I'm trying to find a solution for this, and I was first thinking at adding a plugin to Category model to return a Fulltext\Collection for getProductCollection() if category is virtual. But that will probably cause issues if we don't always return the same model, right ?

Yes, this is likely to break many things and probably any other extensions that are waiting for a clearly scoped Model.

So maybe another approach, would be to create an Indexer that populates catalog_category_product from the virtual category results. However, it would probably a be a very high resource-consuming indexer, so I'm not sure about that solution either.

Nope, we made virtual categories to avoid having doing such things.

Third solution I was thinking about : a Plugin for Magento\Catalog\Model\ResourceModel\Product\Collection, for addCategoryFilter method : If virtual : exec the request, and add a filter on product ID, else use parent method. It could result at filtering on thousands of IDs, which I don't like, but I think it is already working like this elsewhere, so why not ?

We already have a plugin for that : https://github.com/Smile-SA/elasticsuite/blob/2.11.x/src/module-elasticsuite-virtual-category/Plugin/Collection/AddVirtualCategoryQuery.php#L53

What do you think about it ? Do you see a better solution ?

Regards

Nuranto commented 1 year ago

Third solution I was thinking about : a Plugin for Magento\Catalog\Model\ResourceModel\Product\Collection, for addCategoryFilter method : If virtual : exec the request, and add a filter on product ID, else use parent method. It could result at filtering on thousands of IDs, which I don't like, but I think it is already working like this elsewhere, so why not ?

We already have a plugin for that : https://github.com/Smile-SA/elasticsuite/blob/2.11.x/src/module-elasticsuite-virtual-category/Plugin/Collection/AddVirtualCategoryQuery.php#L53

This plugins applies on Smile\ElasticsuiteCatalog\Model\ResourceModel\Product\Fulltext\Collection which overrides (through preferences) magento core's fulltext & widget collection models, but not Magento\Catalog\Model\ResourceModel\Product\Collection which is the model used/returned by Category::getProductCollection. Am I missing something ?

Nuranto commented 1 year ago

@romainruaud I'm willing to make a PR, but I need to be sure I'm not missing something first, since your last answer confused me.