Match the price filter to the range the slider displays - #1290
Open
boo-code wants to merge 1 commit into
Open
Conversation
2 tasks
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.
floor()of the lowest indexed price andceil()of the highest, while the filter compares the raw indexed column against the value the shopper picked. A product indexed at 5995.000001 is therefore offered as spanning 5995 to 5996 and then satisfies neither end, so dragging either handle empties the listing. The bounds now cover the range that was displayed for the product.How to test
Give a product a tax excluded price whose tax included value is not representable exactly, for instance 4995.833334 at 20 percent, and reindex. The shop then holds a single indexed value of 5995.000001.
Measured on a 9.2 shop before the change:
Both ends of the range the slider itself offered return nothing. That is the two symptoms of the report, the unexpected upper bound and the empty result on an exact price.
The change
price_min < floor(max) + 1is the same condition asfloor(price_min) <= max, and the mirror on the other side, so a product matches exactly the range it was shown as occupying. Writing it this way keeps the column alone on its side of the comparison, so the index onlayered_price_indexis still usable, whichFLOOR(price_min) <= :maxwould have prevented.The six decimals themselves are not the problem, they come from
ps_round($price * (100 + $taxRate) / 100, 6)at index time and are correct. Only the comparison was using a different rounding from the display.Tests
testInitSearchWidensPriceBoundsToTheDisplayedRangepins the produced bounds for a 5995 to 5995 selection, and the existing expectation intestInitSearchWithAllFiltersis updated to the new operators. Reverting the change turns both red.Related
#1281 and #1283 also touch price filtering, both in the post filter of
Filters/Products.php. This one changes the bounds sent to SQL inProduct/Search.php, so they do not overlap, but they are worth sequencing deliberately since all three affect which products a price range returns.