Skip to content
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

\Smile\ElasticsuiteVirtualCategory\Plugin\Widget\ProductsListPlugin::afterCreateCollection causing sort direction to be an array #2789

Closed
nmarquez2 opened this issue Nov 22, 2022 · 7 comments

Comments

@nmarquez2
Copy link

Preconditions

Magento Version : 2.4.5-p1 EE

ElasticSuite Version : 2.10.12.1

Environment : Production

Third party modules :

Steps to reproduce

  1. apply ACSD-46466_2.4.5.patch see attachment, this fixes large catalog performance issues with search results page
  2. sort_order is set to 'poistion_by_sku'
  3. condition_option is set to 'sku'
  4. condition_option_value is a non empty list of sku

Expected result

  1. setOrder Method is called with a string attributeand string direction param

Actual result

  1. setOrder method is called with string attribute ("_script") and array direction resulting from \Smile\ElasticsuiteVirtualCategory\Model\Widget\SortOrder\SkuPosition\Builder::buildSortOrder
  2. Since the patch adds a plugin with strict type string on attribute and direction params there is a string type error
  3. TypeError: Magento\InventoryCatalog\Plugin\Catalog\Model\ResourceModel\Product\CollectionPlugin::beforeSetOrder(): Argument Append french translations. #3 ($dir) must be of type string, array given

ACSD-46466_2.4.5.patch.txt

@romainruaud
Copy link
Collaborator

Hello @nmarquez2

What is the source of this patch ? I'd like to see the official page from Adobe describing it.

If you are using Elasticsuite, for me you don't need this patch actually.

The fix we made in this PR is sufficient : #2774

I'll have a look at the patch you provided and how it could live together with Elasticsuite, but most of its content is dirty code by Adobe struggling to work properly with Elasticsearch... as always ...

@romainruaud
Copy link
Collaborator

Okay so to keep it short :

=> I cannot see this plugin on the Magento inventory repository : https://github.com/magento/inventory/tree/develop/InventoryCatalog

=> if this patch does his way until a live release on Magento, we'll immediately disable the plugin called "outOfStockSorting" on the Collection class as part of the Elasticsuite codebase, as we previously made in #2774 for the plugin on Toolbar.

So for now, I suggest you use the fix we published in #2774 to get rid of performances issues.

If you still want to apply this Magento patch, you can do it, but also disable this new plugin, it has no sense when using Elasticsuite.

Why do I say it has no sense ?

Because Magento is struggling to sort products based on stock status because they fail to index it properly.

That's the search engine that should be responsible of sorting, like we do with Elasticsuite : we index stock data, and then Elasticsearch can sort/filter on it.

With their implementation, Adobe is actually trying to mix between data indexed in ES (product data) and data existing only in SQL (stock data). This cannot end good...

Regards

@nmarquez2
Copy link
Author

Hello @romainruaud

The patch was provided by Adobe Commerce's support team in response to a ticket, it is not publicly available. We have more than 100000 product and the "display out of stock" feature of Magento Inventory is kill the serach result page. This is why we needed to apply the patch.

The patch adds the plugin itself this is why it is not in the repository. We'll test the fix you provided and see if the patch is still needed.

Thanks!

@romainruaud
Copy link
Collaborator

okay, so I'm pretty sure that using the patch provided in #2774 will solve your issue.

It's just about de-activating a buggy plugin in Magento_InventoryCatalog, you can even reproduce it in your own codebase :

    <!-- Disable erratic plugin on toolbar.
         This plugin is not needed because Elasticsuite already filters properly the products.
         So we have accurate product count in the toolbar from the ES response. -->
    <type name="\Magento\Catalog\Block\Product\ProductList\Toolbar">
        <!-- This plugin is defined in magento/module-inventory-catalog -->
        <plugin name="update_toolbar_count" disabled="true"/>
    </type>

@nmarquez2
Copy link
Author

@romainruaud I see that the fix is not part of 2.10.12.1, do you know when this is going to be released?

@romainruaud
Copy link
Collaborator

I'm actually not sure if we well release a "hotfix" version (2.10.12.2) or release a "minor" version (2.10.13).

In the meantime you can use the 2.10.x branch directly, or report the fix in your codebase as described in my comment above.

Regards

@nmarquez2
Copy link
Author

Ok perfect!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants