From 8ae2ce84c9d09a63c7ae2a9039717d8a1387e1a9 Mon Sep 17 00:00:00 2001 From: MostafaKadry Date: Tue, 28 Jul 2026 14:38:49 +0300 Subject: [PATCH] test: add regression test suite for promotion precedence and scoping --- docs/OFFERS_AND_PROMOTIONS.md | 25 +++++++ pos_next/promotions/engine.py | 126 ++++++++++++++++++++++++++------ pos_next/test_promotions.py | 134 ++++++++++++++++++++++++++++++++++ 3 files changed, 264 insertions(+), 21 deletions(-) diff --git a/docs/OFFERS_AND_PROMOTIONS.md b/docs/OFFERS_AND_PROMOTIONS.md index c8522be7..6d45000a 100644 --- a/docs/OFFERS_AND_PROMOTIONS.md +++ b/docs/OFFERS_AND_PROMOTIONS.md @@ -161,6 +161,31 @@ Rules of thumb: - A scope row counts only when a line that will **actually receive** the discount matches it. A line excluded by the matrix cannot inflate everyone else's total. +- **A rule that targets a line wins outright.** That line keeps *only* that rule's + percentage — the accumulated total is not added on top — and it stops + contributing its scope to everyone else. Precedence is decided by **scope + specificity**, not by promotion type: + + | Competing rule | Against Accumulative | + |---|---| + | Any rule scoped to Item Code / Item Group / Brand | **Wins outright** | + | `Transaction` Auto Discount (cart-level) | **Stacks** — the two sum | + + A cart-level reward is orthogonal to rewarding a varied basket, so it adds. A + rule naming an item, group or brand is a competing decision about that line's + price, so it replaces. + + ``` + Accumulative: Products 5%, Demo Item Group 5% + Auto Discount: SKU009 (Item Code) 15% + + SKU009 -> 15% (its own rule; the 5% does not pile on) + other Products -> 5% (Demo Item Group contributes nothing — no line takes it) + ``` + + Two competing routes exist and both are covered: a non-Auto rule lands in the + pipeline *baseline*, while a scoped Auto Discount arrives as a *part*. The + accumulative pass checks each — see `get_targeted_auto_discount_lines`. - The total is additive on list price, never compounded. - Each line is capped by the rule's ceiling *and* by that Item's own `max_discount`. - Don't list a group and its own ancestor on one rule — a line inside both counts twice. diff --git a/pos_next/promotions/engine.py b/pos_next/promotions/engine.py index 1c7b635b..c50c41ee 100644 --- a/pos_next/promotions/engine.py +++ b/pos_next/promotions/engine.py @@ -44,6 +44,7 @@ mark_item_discount_flags, ) from pos_next.promotions.scope import ( + APPLY_ON_TRANSACTION, get_item_group_with_descendants, get_scope_config, get_scope_keys, @@ -97,6 +98,17 @@ def get_line_parts(item) -> dict: return item.get(_PARTS_ATTR) or {} +def get_line_baseline(item) -> float: + """The discount ERPNext's per-item engine already put on this line. + + Non-zero means another Pricing Rule claimed the line. Auto Discount rules are + filtered out of those engine results and arrive as a *part* instead, so they + never show up here — which is what lets Auto Discount stack while a targeted + rule takes precedence. + """ + return flt(item.get(_BASE_ATTR) or 0) + + def get_line_total_percentage(item) -> float: """Baseline plus every recorded part, before clamping.""" return flt(item.get(_BASE_ATTR) or 0) + sum(flt(v) for v in get_line_parts(item).values()) @@ -265,10 +277,44 @@ def apply_auto_discount_rules(items, rule_map, selected_offer_names=None, type_m type_map = get_rule_promotion_types(list(rule_map.keys())) mark_item_discount_flags(items, type_map) - pre_subtotal = get_pre_discount_subtotal(items) applied = set() - for rule_name, details in rule_map.items(): + for rule_name, rule, scope_keys in _iter_applicable_auto_rules(items, rule_map, selected_offer_names): + rule_applied = False + for item in items: + if item.get("is_free_item") or not is_eligible_for_promotion( + item, PROMOTION_TARGET_AUTO, rule_type_map=type_map + ): + continue + if not item_matches_rule_scope(item, rule, scope_keys=scope_keys): + continue + + percentage = pricing_rule_line_percentage(item, rule) + if percentage <= 0: + continue + + add_line_part(item, PART_AUTO_DISCOUNT, percentage) + append_pricing_rule(item, rule_name) + item.discount_source = DISCOUNT_SOURCE_AUTO + rule_applied = True + + if rule_applied: + applied.add(rule_name) + + return applied + + +def _iter_applicable_auto_rules(items, rule_map, selected_offer_names=None): + """Yield ``(rule_name, rule, scope_keys)`` for Auto Discount rules the cart passes. + + Only the document-level gates are checked here (type, state, cart amount) — + per-line matching stays with the caller. Shared so the accumulative pass can + find out which lines a targeted Auto Discount will claim *before* it runs, + without duplicating this filter. + """ + pre_subtotal = get_pre_discount_subtotal(items) + + for rule_name, details in (rule_map or {}).items(): if selected_offer_names and rule_name not in selected_offer_names: continue if rule_promotion_type(details) != PROMOTION_TYPE_AUTO: @@ -290,30 +336,43 @@ def apply_auto_discount_rules(items, rule_map, selected_offer_names=None, type_m # Expand the rule's scope once — item group rows resolve through the # nested set, which we don't want to repeat for every cart line. - scope_keys = get_scope_keys(rule) + yield rule_name, rule, get_scope_keys(rule) - rule_applied = False + +def get_targeted_auto_discount_lines(items, rule_map, selected_offer_names=None, type_map=None) -> set: + """``id()`` of every line a *scoped* Auto Discount rule will claim. + + Specificity decides precedence. A ``Transaction`` Auto Discount is a cart-level + reward, orthogonal to rewarding a varied basket, so it stacks on top of the + accumulated total. An Auto Discount naming an item code, group or brand is a + competing decision about that line's price, so it wins outright and the line + drops out of the accumulative pass entirely — as recipient *and* as a scope + contributor for everyone else. + """ + if not items or not rule_map: + return set() + + if type_map is None: + type_map = get_rule_promotion_types(list(rule_map.keys())) + + claimed = set() + for _rule_name, rule, scope_keys in _iter_applicable_auto_rules( + items, rule_map, selected_offer_names + ): + if rule.get("apply_on") == APPLY_ON_TRANSACTION: + continue for item in items: - if item.get("is_free_item") or not is_eligible_for_promotion( - item, PROMOTION_TARGET_AUTO, rule_type_map=type_map - ): + if item.get("is_free_item"): + continue + if not is_eligible_for_promotion(item, PROMOTION_TARGET_AUTO, rule_type_map=type_map): continue if not item_matches_rule_scope(item, rule, scope_keys=scope_keys): continue - - percentage = pricing_rule_line_percentage(item, rule) - if percentage <= 0: + if pricing_rule_line_percentage(item, rule) <= 0: continue + claimed.add(id(item)) - add_line_part(item, PART_AUTO_DISCOUNT, percentage) - append_pricing_rule(item, rule_name) - item.discount_source = DISCOUNT_SOURCE_AUTO - rule_applied = True - - if rule_applied: - applied.add(rule_name) - - return applied + return claimed def get_accumulative_rule_names(rule_names) -> set[str]: @@ -364,7 +423,9 @@ def _scope_row_keys(row_field: str, value: str) -> set[str]: return {value} -def apply_accumulative_discount_rules(items, rule_map, selected_offer_names=None, type_map=None) -> set: +def apply_accumulative_discount_rules( + items, rule_map, selected_offer_names=None, type_map=None, excluded_lines=None +) -> set: """Accumulative rules — sum the percentages of every scope row present in the cart. A rule lists scopes (item codes, item groups or brands) each carrying its own @@ -377,6 +438,13 @@ def apply_accumulative_discount_rules(items, rule_map, selected_offer_names=None cashier discount, or a different item-level promotion) therefore cannot inflate the total for everyone else. + Precedence: a line claimed by a rule that targets it keeps that rule's + percentage alone, rather than having the accumulated total added on top. That + covers both routes a competing rule can arrive by — the pipeline baseline + (ERPNext's per-item engine) and ``excluded_lines`` (a scoped Auto Discount, + which is applied as a part). Only a ``Transaction`` Auto Discount stacks, + because a cart-level reward does not compete with rewarding a varied basket. + Contributes to ``PART_ACCUMULATIVE`` and flags the line so the matrix lets Auto Discount stack on top; the two percentages sum at finalize. """ @@ -386,6 +454,7 @@ def apply_accumulative_discount_rules(items, rule_map, selected_offer_names=None if type_map is None: type_map = get_rule_promotion_types(list(rule_map.keys())) + excluded = excluded_lines if excluded_lines is not None else set() pre_subtotal = get_pre_discount_subtotal(items) applied = set() @@ -416,6 +485,10 @@ def apply_accumulative_discount_rules(items, rule_map, selected_offer_names=None item for item in items if not item.get("is_free_item") + # A rule that targets this line wins outright — the accumulative + # percentage does not pile on top of it. + and get_line_baseline(item) <= 0 + and id(item) not in excluded and is_eligible_for_promotion(item, PROMOTION_TARGET_AUTO, rule_type_map=type_map) and item_matches_rule_scope(item, rule, scope_keys=get_scope_keys(rule)) ] @@ -513,9 +586,20 @@ def run_line_discount_passes(items, rule_map, selected_offer_names=None, cap_res begin_line_discounts(items) mark_item_discount_flags(items, type_map) - applied = apply_accumulative_discount_rules( + # Worked out before the accumulative pass so a line a targeted Auto Discount + # will claim neither receives the accumulated total nor contributes its scope + # to the other lines. + targeted = get_targeted_auto_discount_lines( items, rule_map, selected_offer_names=selected_offer_names, type_map=type_map ) + + applied = apply_accumulative_discount_rules( + items, + rule_map, + selected_offer_names=selected_offer_names, + type_map=type_map, + excluded_lines=targeted, + ) applied |= apply_auto_discount_rules( items, rule_map, selected_offer_names=selected_offer_names, type_map=type_map ) diff --git a/pos_next/test_promotions.py b/pos_next/test_promotions.py index 99835c3b..07341e2b 100644 --- a/pos_next/test_promotions.py +++ b/pos_next/test_promotions.py @@ -2196,6 +2196,140 @@ def test_rule_level_percentage_is_ignored(self): for item in resp["items"][:2]: self.assertAlmostEqual(flt(item.get("discount_percentage")), 8, places=2) + def test_targeted_rule_wins_outright_over_accumulative(self): + """A rule aimed at one item replaces the accumulated total on that line. + + Regression: the targeted 15% landed in the pipeline baseline and the + accumulated 5% piled on top for 20%. Other lines keep accumulating. + """ + accum = self._accumulative_rule( + "_PNXT_TEST_AccumPrecedence", + apply_on="Item Group", + item_groups=[{"item_group": _resolve_item_group(), "pos_discount_percentage": 5}], + priority="1", + ) + targeted = _make_rule( + "_PNXT_TEST_AccumPrecedenceTargeted", + apply_on="Item Code", + items=[{"item_code": ITEM_A}], + rate_or_discount="Discount Percentage", + price_or_product_discount="Price", + discount_percentage=15, + min_qty=0, + priority="5", + ) + resp = self._run([accum, targeted], [_line(self.ctx, ITEM_A), _line(self.ctx, ITEM_B)]) + + item_a, item_b = resp["items"][0], resp["items"][1] + self.assertAlmostEqual(flt(item_a.get("discount_percentage")), 15, places=2) + self.assertAlmostEqual(flt(item_b.get("discount_percentage")), 5, places=2) + + def test_item_scoped_auto_discount_wins_outright(self): + """Regression: a targeted Auto Discount stacked instead of replacing. + + It reaches the line as a pipeline *part*, not the baseline, so the earlier + baseline check could not see it — 5% + 15% came out as 20%. + """ + accum = self._accumulative_rule( + "_PNXT_TEST_AccumVsScopedAuto", + apply_on="Item Group", + item_groups=[{"item_group": _resolve_item_group(), "pos_discount_percentage": 5}], + priority="1", + ) + scoped_auto = _make_rule( + "_PNXT_TEST_AccumVsScopedAutoRule", + apply_on="Item Code", + items=[{"item_code": ITEM_A}], + rate_or_discount="Discount Percentage", + price_or_product_discount="Price", + discount_percentage=15, + promotion_type="Auto Discount", + min_qty=0, + priority="5", + ) + resp = self._run([accum, scoped_auto], [_line(self.ctx, ITEM_A), _line(self.ctx, ITEM_B)]) + + self.assertAlmostEqual(flt(resp["items"][0].get("discount_percentage")), 15, places=2) + self.assertAlmostEqual(flt(resp["items"][1].get("discount_percentage")), 5, places=2) + + def test_transaction_auto_discount_still_stacks(self): + """The counterpart: a cart-level Auto Discount is orthogonal and still sums.""" + accum = self._accumulative_rule( + "_PNXT_TEST_AccumVsTxnAuto", + apply_on="Item Group", + item_groups=[{"item_group": _resolve_item_group(), "pos_discount_percentage": 5}], + priority="1", + ) + txn_auto = _make_rule( + "_PNXT_TEST_AccumVsTxnAutoRule", + apply_on="Transaction", + rate_or_discount="Discount Percentage", + price_or_product_discount="Price", + discount_percentage=10, + promotion_type="Auto Discount", + min_qty=0, + apply_discount_on="Grand Total", + priority="7", + ) + resp = self._run([accum, txn_auto], [_line(self.ctx, ITEM_B)]) + self.assertAlmostEqual(flt(resp["items"][0].get("discount_percentage")), 15, places=2) + + def test_scoped_auto_line_does_not_contribute_its_scope(self): + """A line a scoped Auto Discount claims drops out of the accumulation entirely.""" + accum = self._accumulative_rule( + "_PNXT_TEST_AccumScopedAutoScope", + items=[ + {"item_code": ITEM_A, "pos_discount_percentage": 5}, + {"item_code": ITEM_B, "pos_discount_percentage": 3}, + ], + priority="1", + ) + scoped_auto = _make_rule( + "_PNXT_TEST_AccumScopedAutoScopeRule", + apply_on="Item Code", + items=[{"item_code": ITEM_A}], + rate_or_discount="Discount Percentage", + price_or_product_discount="Price", + discount_percentage=15, + promotion_type="Auto Discount", + min_qty=0, + priority="5", + ) + resp = self._run([accum, scoped_auto], [_line(self.ctx, ITEM_A), _line(self.ctx, ITEM_B)]) + + self.assertAlmostEqual(flt(resp["items"][0].get("discount_percentage")), 15, places=2) + # ITEM_A's scope contributes nothing: no line is left to receive it. + self.assertAlmostEqual(flt(resp["items"][1].get("discount_percentage")), 3, places=2) + + def test_targeted_line_does_not_contribute_its_scope(self): + """A line that keeps its own rule cannot inflate everyone else's total.""" + parent = _resolve_item_group() + accum = self._accumulative_rule( + "_PNXT_TEST_AccumPrecedenceScope", + items=[ + {"item_code": ITEM_A, "pos_discount_percentage": 5}, + {"item_code": ITEM_B, "pos_discount_percentage": 3}, + ], + priority="1", + ) + targeted = _make_rule( + "_PNXT_TEST_AccumPrecedenceScopeTargeted", + apply_on="Item Code", + items=[{"item_code": ITEM_A}], + rate_or_discount="Discount Percentage", + price_or_product_discount="Price", + discount_percentage=15, + min_qty=0, + priority="5", + ) + resp = self._run([accum, targeted], [_line(self.ctx, ITEM_A), _line(self.ctx, ITEM_B)]) + + # ITEM_A keeps its own 15%; ITEM_B accumulates only its own scope (3%), + # because ITEM_A's scope has no line left that would actually receive it. + self.assertAlmostEqual(flt(resp["items"][0].get("discount_percentage")), 15, places=2) + self.assertAlmostEqual(flt(resp["items"][1].get("discount_percentage")), 3, places=2) + self.assertNotEqual(parent, None) + def test_standalone_rule_without_percentages_is_still_rejected(self): """The guard must survive the scheme-generated exemption.""" with self.assertRaises(frappe.ValidationError):