Skip to content

Include tier prices in catalog rule indexer to prevent rule from raising effective price - #272

Open
ddevallan wants to merge 3 commits into
mage-os:mainfrom
ddevallan:fix/catalog-rule-tier-prices
Open

Include tier prices in catalog rule indexer to prevent rule from raising effective price#272
ddevallan wants to merge 3 commits into
mage-os:mainfrom
ddevallan:fix/catalog-rule-tier-prices

Conversation

@ddevallan

@ddevallan ddevallan commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Description

The catalog rule indexer (RuleProductsSelectBuilder::buildSelect()) computed default_price from the regular product price only — tier prices were completely ignored.

When a catalog rule calculates a discounted price that is still higher than an existing tier price, the indexed rule price is used instead of the tier price, effectively raising the visible price for customers who qualify for both.

Example: Regular $100 · Tier $30 (all groups) · 50% rule → $50 without fix. With fix: LEAST($30,$100)=$30 → 50%=$15 ≤ $30 ✓

Fix

Added two LEFT JOINs on catalog_product_entity_tier_price for website-specific and global (website_id=0) tier prices, using OR all_groups = 1 to handle tier prices that apply to all customer groups. default_price is wrapped in LEAST():

LEAST(
    IFNULL(price_tier0.value, pp_default.value),  -- global tier or regular
    IFNULL(price_tier.value, pp_default.value),    -- website tier or regular
    IFNULL(pp_website.value, pp_default.value)     -- regular price
)

⚠️ Known limitations

qty = 1 only — intentional design decision

Only tier prices with qty = 1 are joined. This is deliberate: the catalog rule indexer's default_price represents the single-unit display price shown in product listings.

Consider a product with regular price $100, a qty ≥ 5 tier price of $20, and a 50% rule. Including the qty=5 tier in LEAST() would give $10 as the indexed rule price — but $10 only applies when buying 5+ units. A customer buying 1 unit would see $10 in listings and pay $50 at checkout. This would be misleading.

qty = 1 tier prices are semantically different: they apply from the very first unit (effectively group-specific prices) and are safe to include in the single-unit price calculation. The integration test testReindexFullTierPriceForQtyAboveOneIsNotConsideredByIndexer explicitly documents this behavior.

Other limitations

  • Performance — two additional LEFT JOINs on the indexer SELECT will slightly increase full reindex time on large catalogs
  • Duplicate rows edge case — if a product has both an all_groups = 1 tier AND a specific-group tier for the same website and qty=1, both match the JOIN and may produce duplicate result rows

Test results

Unit tests (3):

 ✔ Build select joins tier prices for website
 ✔ Build select includes tier prices in least expression
 ✔ Build select tier join includes all groups condition
OK (3 tests, 16 assertions)

Integration tests — full scenario matrix (6):

Scenario Description Result
All-groups global tier caps rule Tier $30 < rule $50 → rule price ≤ $30
Rule wins when lower than tier Tier $80 > rule $40 → rule price still ≤ $80
Specific customer group tier Only group 1 has tier — only group 1 is capped
Website-scoped tier (price_tier JOIN) Tier on website_id=1 respected
Global + website tier — LEAST picks lower $40 global vs $20 website → $20 wins
Volume discount (qty>1) — documents known limitation qty=5 tier ignored; rule applies to regular price
OK (6 tests, 57 assertions) in 12s

Existing indexer tests — no regression (3):

 ✔ Reindex full · ✔ Reindex by id · ✔ Reindex by ids
OK (3 tests, 7 assertions)

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • README.md files for modified modules are updated — N/A
  • All automated tests passed successfully (all builds are green)

…ing effective price

The catalog rule indexer built default_price from the regular product
price only. When a catalog rule calculated a discounted price higher than
an existing tier price, the indexed rule price was used instead of the
tier price, effectively raising the price for customers who qualified
for both.

Added LEFT JOINs for website-scoped and global tier prices at qty=1,
using OR all_groups=1 to correctly handle tier prices that apply to all
customer groups. The default_price column now uses LEAST() so the
catalog rule discount is applied against the lowest available price,
preventing the rule from ever producing a price higher than an existing
tier price.
@ddevallan
ddevallan requested a review from a team as a code owner June 1, 2026 19:39
@ddevallan
ddevallan marked this pull request as draft June 1, 2026 19:44
ddevallan added 2 commits June 2, 2026 08:37
Two integration tests against the full IndexerBuilder pipeline verify
the tier price fix across all customer groups:

1. testReindexFullRulePriceNeverExceedsTierPrice: with a  all-groups
   global tier and 50% rule (which would give ), the indexed rule
   price must be ≤  for all customer groups.

2. testReindexFullRulePriceWinsWhenLowerThanTierPrice: with a  tier
   and the same 50% rule (which gives ), the rule price wins and must
   still be ≤  for all groups.
… rule scenarios

Extends integration test coverage to the full scenario matrix:

1. All-groups global tier caps rule (already existed)
2. Rule wins when lower than tier (already existed)
3. Specific customer group tier applies only to that group
4. Website-scoped tier price (price_tier JOIN, not global)
5. Both global and website tiers exist — LEAST picks the lower one
6. Volume discount (qty>1): intentionally not considered by indexer
   — documents the known limitation with an explicit assertion
@ddevallan
ddevallan marked this pull request as ready for review June 2, 2026 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant