fix(woocommerce): subtract tax from price filter bounds - #4338
Conversation
The Filter by Price widget compared including-tax bounds against the excluding-tax price indexed in Elasticsearch, so products were dropped. Mirror WooCommerce core by subtracting inclusive tax from min/max bounds when prices are entered excluding tax but the shop shows including tax. Fixes 10up#4332
|
Thanks for the PR @faisalahammad and for the clear description. The approach is correct and it fixes the case in the issue. There is one problem to solve before we can merge this. The price filter compares prices using You can see this yourself. Set a product price to It would be great if you could add a test that uses a price with decimal numbers. The current test uses Let us know if you have any questions. |
The price filter range queries targeted meta._price.long, which stores prices as whole numbers via intval(). Decimal prices like 100.99 lost their decimal portion, and tax-adjusted bounds became fractional numbers that no longer matched the truncated stored value, excluding products from results. Switch the range query to meta._price.double, which is already indexed alongside long by prepare_meta_value_types() and used by InstantResults. No reindex required, no mapping change. The test now seeds a decimal product price (100.99) and an incl-tax bound of 121.188 to reproduce the rounding bug directly. Also update testPriceFilterWithoutTax's assertion, which was still expecting meta._price.long. Addresses PR 10up#4338 feedback. Fixes 10up#4332
|
Thanks for flagging the rounding issue. Fixed in 44f4ae5. The range query now targets meta._price.double (already indexed alongside long). meta._price.long truncates via intval() — 100.99 stored as 100, so tax-adjusted fractional bounds like 100.83 missed it. meta._price.double keeps the full decimal. Test updated: seeds regular_price 100.99 and min_price=121.188 (the exact incl-tax WC computes for that price), so the test now reproduces the rounding bug directly. Also updated testPriceFilterWithoutTax's stale meta._price.long assertion. No reindex needed. |
Forces WC_Tax::get_rates('') to look up against shop base country (GB),
where the seeded 20% tax rate row lives. Without this, a leftover session
or prior-test setting routes WC_Customer::get_taxable_address() to a
non-GB tuple, returns [] from get_rates, and skips the inclusive-tax
subtraction in Products::get_price_filter_tax_adjustment() -- causing
gte/lte = 121.188 instead of 100.99.
PHP 7.4+ compatible. References PR 10up#4338 (Fixes CI PHPUnit matrix).
Refs 10up#4338
CI Fix Summary — 1 PHPUnit failure resolved
Why the helper itself is unchanged
CleanupThe test option-snapshot loop already restores all captured options in Verification
Out of scope
|
felipeelia
left a comment
There was a problem hiding this comment.
Hey @faisalahammad! Any chance you can include these @since tags so we can finally merge this PR? Thanks!
|
@felipeelia Done — added the |
- merge origin/develop into fix/4305-hide-subscription-token (PR 10up#4338 price filter tax fix) - auto-fix: run phpcbf on includes/classes/ElementorUtils.php:111 equals-align warning - tests: wrap IS_EPIO_ENVIRONMENT in try/finally for isolation in 3 Settings tests Errors fixed: - PHPCS: equals sign not aligned correctly; expected 1 space but found 6 spaces - PHPUnit: testPriceFilterWithTax float drift (100.99000000000001 vs 100.99) Refs 10up#4324
Summary
On stores with WooCommerce tax enabled, "Prices entered with tax: NO" and "Display prices in the shop: Including tax", the Filter by Price widget returned wrong results. ElasticPress indexed the excluding-tax
_pricebut compared the user's including-taxmin_price/max_pricebounds directly against it, with no tax conversion on either side. Disabling ElasticPress fixed it because WooCommerce core's native filter converts the bounds.This fix mirrors WooCommerce core (
WC_Query::price_filter_post_clauses): when the shop displays including-tax prices but prices are entered excluding tax, the inclusive tax is subtracted from the bounds before the Elasticsearch range query so they line up with the excluding-tax indexed price.Fixes #4332
Changes
includes/classes/Feature/WooCommerce/Products.phpBefore:
After:
Why: A new
get_price_filter_tax_adjustment()helper subtracts the inclusive tax from each bound, applied once after the bounds are read so both the search and shop code paths reuse the converted values. No-op whenever tax is off, no tax rates are configured, prices are entered including tax, or the shop displays excluding tax, so unaffected stores behave exactly as before. No mapping or index change, so no reindex is needed.Testing
Test 1: Reproduce the bug fix (unit)
composer run setup-local-testsif not already set up (needs MySQL + Elasticsearch).vendor/bin/phpunit --filter testPriceFilterWithTax tests/php/features/WooCommerce/TestWooCommerceProduct.phpmin_price=120&max_price=120, and asserts the Elasticsearch range bound is reduced to100.0(the excluding-tax price).Result: passes.
Test 2: Existing price filters still green
vendor/bin/phpunit --filter testPriceFilter tests/php/features/WooCommerce/TestWooCommerceProduct.phpvendor/bin/phpunit --filter testPriceFilterWithSearchQuery tests/php/features/WooCommerce/TestWooCommerceProduct.phpResult: both pass (tax adjustment is a no-op when tax is off).
Test 3: Manual repro (issue #4332)
?min_price=120&max_price=120.Result: product appears (before fix it did not).