Apply out-of-stock-last ordering on search result pages#1260
Conversation
On search pages the products are ordered by relevance, built as a FIELD() list of the product ids returned by the core search. computeOrderByField() returned that relevance ordering early, before computeShowLast() ran, so the "show out of stock products last" behaviour - applied to category, manufacturer and other listings - was skipped for searches. Out-of-stock products could therefore show up before in-stock ones in search results. Route the search relevance ordering through computeShowLast() as well, so out of stock products are pushed to the end while the relevance order is preserved inside each group.
|
Hello @boo-code! This is your first pull request on ps_facetedsearch repository of the PrestaShop project. Thank you, and welcome to this Open Source community! |
|
Hello @boo-code CI is red. |
The new test used assertStringContainsString(), which does not exist in the PHPUnit 5.7 the module runs on, and mocked Product with Mockery::mock() plus a MockProxy Product stub. That stub declared a global Product class at bootstrap, so the existing computeShowLast tests could no longer create their Product mock with Mockery::namedMock() (class already exists). Drop the MockProxy Product stub, mock Product through namedMock() like the neighbouring tests, and assert with assertContains() so the suite is green again on PHPUnit 5.7.
|
Thanks. Fixed in 1d00b91. The new test used |
Hlavtox
left a comment
There was a problem hiding this comment.
Hello @boo-code, I have to disagree on this one, because I think it's the expected behavior.
Let me explain. When you search for anything, you would still expect the most relevant thing to be in the front, even if it's not in stock.
|
Thanks @Hlavtox, that is a fair point and I am happy to defer to your call on the module's direction. Two clarifications on what the PR actually does, in case they shift the picture:
The intent was consistency: category, manufacturer and other listings already route through |
Adapter\MySQL::computeOrderByField()builds as aFIELD(p.id_product, ...)list from the product pool returned by the core search. That branch returned early, beforecomputeShowLast()(the method that prepends the out-of-stock-last ordering) was reached — so out-of-stock products could appear before in-stock ones in search results only. The fix routes the relevance ordering throughcomputeShowLast()too, so out-of-stock products are pushed to the end while the relevance order is preserved within each group.MySQLTest::testGetQueryAppliesOutOfStockLastOnSearchRelevanceOrdering): withPS_LAYERED_FILTER_SHOW_OUT_OF_STOCK_LASTenabled and a search-style query (anid_productpool as the initial population, ordered byposition), the generatedORDER BYnow contains both the out-of-stock-last clause (IFNULL(p.quantity, 0) <= 0) and the relevance ordering (FIELD(p.id_product,1,2,3)) — before this change only the relevance ordering was present. Manually: enable "show out-of-stock products last", run a storefront search returning both in- and out-of-stock products, and compare with a category listing — they now order stock the same way.Before / after, the generated
ORDER BYfor a search query with the feature enabled:(The test needed
Productto be mockable, so a one-lineProductstub was added to the testMockProxyalongside the existing ones.)