elasticsuite icon indicating copy to clipboard operation
elasticsuite copied to clipboard

Search results page shows that results are sorted in ascending relevance but are actually correctly sorted in descending relevance

Open timothyfisherdev opened this issue 3 years ago • 5 comments

When looking at a search results page, the toolbar shows that the results are sorted in ascending relevance, but they are actually correctly sorted in descending relevance. See here: http://demo.magento-elastic-suite.io/index.php/catalogsearch/result/index/?q=trek. The arrow in the toolbar is facing upwards showing that the results are in ascending order, but, given the search results, they are correctly being sorted in descending relevance.

The src/module-elasticsuite-catalog/Block/Plugin/ResultPlugin.php block plugin method aroundSetListOrders in https://github.com/Smile-SA/elasticsuite/commit/e6de284267bc59756d25f5cb48e28ab6722fdd17 is responsible for the behavior I think. However, I’ve noticed that if I remove this plugin, the arrow direction is fixed, but the search results flip opposite direction too. I’m still trying to debug and find where this is all happening at in the code, but all I know is that when I disable the elasticsuite extension, the issue goes away.

I'm also unsure what the purpose of setting the block to ascending is.

Preconditions

Magento Version : 2.3.3 CE

ElasticSuite Version : 2.6.0

Environment : Happens in both modes

Third party modules : N/A

Steps to reproduce

  1. Install smile/elasticsuite
  2. Search something in search bar
  3. See toolbar on search page

Expected result

  1. Arrow should be facing downwards to show results in descending relevance

Actual result

  1. Arrow is facing upwards showing results being sorted as ascending relevance, however it looks like they're still correctly sorted in descending relevance.

timothyfisherdev avatar Jun 07 '21 07:06 timothyfisherdev

We'll try to reproduce this on a vanilla Magento instance (without Elasticsuite).

In any cases, there is no point in having a direction for the "relevance" sort order, it's only relevant when sorting DESC.

romainruaud avatar Jun 08 '21 14:06 romainruaud

In a vanilla Magento instance search results are provided in descending relevance by default: https://demo-m2.bird.eu/catalogsearch/result/?q=tee (arrow facing downwards). When switching this to ascending relevance the results are of course flipped around. I think this extension is just making the toolbar arrow not truly reflect which direction the results are sorted in.

timothyfisherdev avatar Jun 08 '21 20:06 timothyfisherdev

Okay, so that's a frontend issue, probably we are enforcing DEC order for relevance by default, but not reflecting it into the layout.

We'll try to fix this one but that's a low priority for us at the moment.

romainruaud avatar Jun 10 '21 13:06 romainruaud

In doing some more digging on this, I think that there is some faulty logic in determining the correct sort order for the query that not only flips the arrow but also makes it difficult for this extension to be compatible with other extensions.

While the issue was that the toolbar was not visually reflecting which sort order the results were in, simply switching the direction of the arrow did not work, and in fact, switched the results around as well. So the arrow was always the opposite of what the results were showing.

If you take a look at your demo site and add &product_list_dir=desc to the end of the URL, you can see that the arrow direction is fixed, however, the results are now in ascending relevance rather than descending: http://demo.magento-elastic-suite.io/index.php/catalogsearch/result/index/?q=trek&product_list_dir=desc.

I traced this to the addDefaultSortOrders function in the SortOrderBuilder: https://github.com/Smile-SA/elasticsuite/blob/2.10.x/src/module-elasticsuite-core/Search/Request/SortOrder/SortOrderBuilder.php#L135.

if (count($orders) > 0) {
    $firstOrder = current($orders);
    if ($firstOrder['direction'] == SortOrderInterface::SORT_DESC) {
        $defaultOrders[SortOrderInterface::DEFAULT_SORT_FIELD] = SortOrderInterface::SORT_ASC;
        $defaultOrders[$mapping->getIdField()->getName()]      = SortOrderInterface::SORT_ASC;
    }
}

This just takes the first ordering item in the array and checks to see if it's descending, if it is then it sets the main sort order query to the opposite, which would be ascending.

I think the logic of this was explained by @rbayet in issue #1512:

where

  • the search_query.position component will apply the fixed position (1, 2, 3...) in ascending order for products which have been positionned in the search merchandizer for your query (here the query of id "8269")
  • and then sort the remaining products (those without a fixed position in the search merchandizer) by descending score

So if the requested sort order is DESC, then search_query.position should be set to ASC, and the _score and entity_id fields should be set to DESC. That's what I think this method is trying to do.

So, back over to the arrow, there is a aroundSetListOrders method in ResultPlugin (https://github.com/Smile-SA/elasticsuite/blob/2.10.x/src/module-elasticsuite-catalog/Block/Plugin/ResultPlugin.php#L143) that explicitly sets ASC on the toolbar which forces the arrow in the up position.

public function aroundSetListOrders(Result $resultBlock, \Closure $proceed)
{
    $proceed();
    $resultBlock->getListBlock()->setDefaultDirection('asc');

    return $resultBlock;
}

I think the purpose of this was for the category product listing because those are normally sorted by the position which you would want ascending. This messes up the arrow on search results pages though. Perhaps a check could be done here to only force ascending if this is a result block for a category page and not a search page.

Removing this wasn't enough to fix the issue though. The arrow fixes itself on the search page, however, it no longer works correctly when you click it and set it to the ascending direction. Here's a visualization of this with the debug mode on:

Default object (arrow facing upwards showing ascending direction, but really sorted in descending direction):

{
    "search_query.position": {
        "direction": "asc",
        "sortField": "search_query.position"
    },
    "_score": {
        "direction": "desc"
    },
    "entity_id": {
        "direction": "desc"
    }
}

When you turn the result plugin off so it doesn't enforce ASC order (position should be asc and _score should be desc):

{
    "search_query.position": {
        "direction": "desc",
        "sortField": "search_query.position"
    },
    "_score": {
        "direction": "asc"
    },
    "entity_id": {
        "direction": "asc"
    }
}

So that function seems like it's blindly switching between ascending/descending without doing appropriate checks. The way I see it, the default sort order for _score should be DESC, and then the search_query.position direction should be opposite of that, as stated by @rbayet.

I wrote a custom module with two classes to get around the issue and fix it, but ultimately I think the logic for the addDefaultSortOrders needs to change.

This plugin goes around the ResultPlugin:

public function aroundAroundSetListOrders(
    ElasticsuiteResultPlugin $subject,
    Closure $callPlugin,
    Result $resultBlock,
    Closure $originalFunction
) {
    // Instead of explicitly setting it back to descending, I've put an around interceptor
    // around their around interceptor in an effort to be non-invasive and not change 
    // whatever default sorting order is already set. Potentially, we could check 
    // here to see if the request is a search or a category page though too.
    $originalFunction();

    return $resultBlock;
}

This one flips the orders correctly after doing some checks:

public function beforeBuildSordOrders(SortOrderBuilder $sortOrderBuilder, ContainerConfigurationInterface $containerConfig, array $orders)
{
    if (isset($orders['search_query.position']['direction'])) {
        // Set the position sorting order to the opposite of what the toolbar is. The toolbar
        // getDirection method can return NULL if the product_list_dir parameter is not in 
        // the request, so set it to default back to descending like it should be at.
        $toolbarDir = $this->toolbar->getDirection() ?? SortOrderInterface::SORT_DESC;
        $orders['search_query.position']['direction'] = $this->flipDir($toolbarDir);

        // The intercepted method internally calls a private method called addDefaultSortOrders.
        // There is faulty logic there for setting up the sort order. The following lines fix 
        // this and make it compatible with other plugins that may add their own ordering.
        reset($orders);

        while (key($orders) !== 'search_query.position') {
            next($orders);
        }
    }

    return [$containerConfig, $orders];
}

private function flipDir(string $currentDir)
{
    return $currentDir === SortOrderInterface::SORT_ASC ? SortOrderInterface::SORT_DESC : SortOrderInterface::SORT_ASC;
}

Some extensions add their own orders to the array, like this for example:

Array
(
    [non_images_last] => Array
        (
            [direction] => DESC
        )

    [out_of_stock_last] => Array
        (
            [direction] => DESC
        )

    [search_query.position] => Array
        (
            [direction] => desc
            [sortField] => search_query.position
            [nestedPath] => search_query
            [nestedFilter] => Array
                (
                    [search_query.query_id] => 2147
                )

        )

)

So doing something like this won't work and be reliable:

$firstOrder = current($orders);
if ($firstOrder['direction'] == SortOrderInterface::SORT_DESC) {

Especially since the direction in non_images_last for example, is uppercase, and SortOrderInterface::SORT_DESC is lowercase, and 'desc' == 'DESC' would return false. My while loop was kind of a hacky way to fix it and get the array pointer back to search_query.position so that addDefaultSortOrders could do its thing.

Hope this all makes sense but if not I would be happy to provide the module I wrote to fix the sorting issue. I wrote it primarily for my client in order to make the two extensions compatible, but I think the correct solution is to change the logic of the functions.

timothyfisherdev avatar Jun 11 '21 03:06 timothyfisherdev

@timothyfisherdev could you share the module to fix the sort order issue?

cesarcardoso-mcfadyen avatar Feb 28 '23 15:02 cesarcardoso-mcfadyen