Sort the wishlist by when products were added, as the page says it does - #541
Draft
boo-code wants to merge 1 commit into
Draft
Sort the wishlist by when products were added, as the page says it does#541boo-code wants to merge 1 commit into
boo-code wants to merge 1 commit into
Conversation
The wishlist page advertises "Last added" as its sort and offers no other
one, but the query was built with the catalogue ordering - PS_PRODUCTS_ORDER_BY,
which is position ascending on a stock install. So the list came back ordered
by where each product sits in its default category, which has nothing to do
with when it was added, and adding a product could place it anywhere.
That also explains why the report could not be reproduced by everyone: the
outcome depends on the chosen products' catalogue positions.
Verified the emitted clause: SortOrder('wishlist_product',
'id_wishlist_product', 'DESC') yields "id_wishlist_product desc", which both
Validate::isOrderBy() and Validate::isOrderWay() accept, and the column is
unambiguous because the provider inner joins wishlist_product as wp. It is
also the exact order the provider already offers as the only sort option.
Reported as PrestaShop/PrestaShop#36496.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
products-list.tplrendersdata-default-sort="{l s='Last added'}"andWishListProductSearchProvideroffers that as the only sort order - butWishlistViewController::getProductSearchQuery()built the query with the catalogue ordering instead,Tools::getProductsOrder('by'/'way'), which isPS_PRODUCTS_ORDER_BYand resolves tocp.positionascending on a stock install. The list therefore came back ordered by where each product sits in its default category, which is unrelated to when it was added, so adding a product could place it anywhere in the list.Verified rather than assumed
The replacement is the same
SortOrderthe provider already exposes, and its emitted clause was checked against core:The column is unambiguous because the provider inner joins
wishlist_productaswp, andcpis only there because the provider left joinscategory_productfor the category filter - so the old clause was valid SQL, just answering the wrong question.Ordering by a column outside the
GROUP BYis fine here: PrestaShop clears the sessionsql_modeon connect (SET SESSION sql_mode = ''in bothDbPDOandDbMySQLi), soONLY_FULL_GROUP_BYis not in force, and the existing "Last added" option already relies on this.Why the report was hard to reproduce
The outcome depended entirely on the catalogue positions of the products chosen for the test, which is why @Progi1984 saw it and @florine2623 did not.
public/assets are untouched: this is a PHP controller change with no front-end build involved.