From 92e28856315b1bd9d4ea200b88898bfdb9746cc9 Mon Sep 17 00:00:00 2001 From: MostafaKadry Date: Tue, 28 Jul 2026 14:03:53 +0300 Subject: [PATCH] feat: implement accumulative discount mode with automated slab management and validation --- docs/OFFERS_AND_PROMOTIONS.md | 245 ++++++++++- pos_next/hooks.py | 1 + pos_next/overrides/pricing_rule.py | 97 ++++ pos_next/patches.txt | 3 +- pos_next/patches/v2_1_0/__init__.py | 0 .../lift_accumulative_config_to_scheme.py | 73 +++ .../pos_next/custom/promotional_scheme.json | 416 +++++++++++++++++- pos_next/public/js/promotional_scheme.js | 44 +- pos_next/test_promotions.py | 135 ++++++ 9 files changed, 989 insertions(+), 25 deletions(-) create mode 100644 pos_next/patches/v2_1_0/__init__.py create mode 100644 pos_next/patches/v2_1_0/lift_accumulative_config_to_scheme.py diff --git a/docs/OFFERS_AND_PROMOTIONS.md b/docs/OFFERS_AND_PROMOTIONS.md index e4f74322d..c8522be7c 100644 --- a/docs/OFFERS_AND_PROMOTIONS.md +++ b/docs/OFFERS_AND_PROMOTIONS.md @@ -28,6 +28,43 @@ POS Next supports automatic offer application based on cart contents. When items - **Brand**: All items from a brand - **Transaction**: Entire cart +> A Pricing Rule stores its targets in the `items` / `item_groups` / `brands` +> **child tables**. It has no scalar `item_code` / `item_group` / `brand` field — +> reading one raises `AttributeError`. Always resolve scope through +> `pos_next/promotions/scope.py`, which also handles item-group lineage (a rule on +> `Electronics` covers an item in `Electronics > Phones`). + +### 4. Cross-Cart Price Modes + +`apply_discount_on_price` on a Price rule switches it into a mode the per-item +engine cannot evaluate on its own, because the decision depends on the whole cart: + +| Mode | Behaviour | +|---|---| +| *(blank)* | Normal — discount every matching line | +| `Min` / `Max` | Discount only the single cheapest / most expensive matching line | +| `Accumulative` | Sum the percentage of every scope row present, apply the total to each eligible line | + +All three force `mixed_conditions = 1` so ERPNext evaluates the document together. +They are mutually exclusive — one field, one mode per rule. + +## Promotion Types and the Interaction Matrix + +`promotion_type` classifies what a promotion *is*, and the **Promotion Interaction +Matrix** (`pos_next/api/promotion_exclusions.py`) decides which lines each kind may +touch. A line is classified into a state, then looked up against the target: + +| Item state | GWP | Auto Discount | Coupon | +|---|:---:|:---:|:---:| +| `eligible` | ✅ | ✅ | ✅ | +| `already_discounted` | ✅ | ❌ | ❌ | +| `excluded_brand` | ✅ | ✅ | ❌ | +| `accumulative` | ✅ | ✅ | ❌ | + +`already_discounted` covers a line carrying an Item Level Discount or a manual +cashier discount. `accumulative` is the one state that lets a second discount stack: +the accumulative and Auto Discount percentages **sum** on the same line. + ## Configuration in ERPNext ### Creating a Promotional Scheme @@ -66,6 +103,78 @@ This allows ERPNext to accumulate quantities across different items in the same When you save a Promotional Scheme, ERPNext automatically creates underlying Pricing Rules (e.g., `PRLE-0003`). POS Next works with these generated rules. +### Accumulative Discounts + +Rewards a customer for buying **across** scopes: the more distinct scopes the cart +touches, the bigger the discount on *everything* in it. + +**On a Promotional Scheme** (the usual route), tick **Accumulative Discount** +under `is_cumulative`. That hides the price/product discount tables and reveals +the accumulative settings — qty and amount limits, **Max Accumulated Discount %** +and **Min Scopes Required** — on the scheme itself. Then: + +1. **Apply On** — `Item Code`, `Item Group` or `Brand` (not `Transaction`) +2. Give each scope row its own **Discount %** (`pos_discount_percentage`) + +The checkbox is an *authoring* control. `apply_discount_on_price` on the generated +Pricing Rule stays the single source of truth for the engine, the offer payload and +the interaction matrix; `normalize_accumulative_scheme` (hooked on +`before_validate`) projects the checkbox onto it every save, so the two cannot +drift and a scheme built through the API is as correct as one built in the form. + +> **The slab is hidden, not gone.** ERPNext generates Pricing Rules *from* the +> discount slabs and skips an empty child table, so a scheme with no slab saves +> clean and silently discounts nothing. The server therefore maintains exactly one +> slab behind the hidden table, injecting `rate_or_discount`, zeroed discount +> values and the mandatory `rule_description`. Those injected values are all *true* +> for this mode — none is a placeholder to dodge validation. +> +> `before_validate`, not `validate`: ERPNext's own `PromotionalScheme.validate()` +> rejects a slab-less scheme and runs ahead of every hooked `validate`. + +**On a standalone Pricing Rule**, set **Apply Discount On** to `Accumulative` +directly; **Promotion Type** must be `Item Level Discount`. + +Picking `Accumulative` hides the rule-level discount fields (`price_or_product_discount`, +`rate_or_discount`, `rate`, `discount_amount`, `discount_percentage`) and, on a +Scheme, the product/free-item slabs. The percentage comes from the scope rows and +nowhere else — the engine ignores the rule-level value outright, so a stale one +cannot leak into the total. + +> The price discount **slab** stays visible on a Scheme even though its discount +> fields are hidden: ERPNext generates no Pricing Rule without a slab, and the slab +> is where the mode, the cap and Min Scopes Required live. + +``` +Rule: Group A = 5%, Group B = 3%, Group C = 4% +Separate scheme: Auto Discount 10% + +Cart: Phone (Group A) + Shirt (Group B) + scopes present = {A, B} → C contributes nothing + accumulative = 5 + 3 = 8% → uniform, every eligible line + auto discount = 10% → matrix permits stacking + line total = 18% off price_list_rate (additive, not compounded) + per line = min(18%, rule cap, Item.max_discount) +``` + +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. +- 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. +- Validation forces `pos_only` and `mixed_conditions`, and rejects `Transaction` + scope, empty scope tables, all-zero percentages and a non-Item-Level promotion type. + +**Scheme → Rule propagation.** ERPNext rebuilds each generated rule's scope table +with `pr.append(field, {apply_on: value, "uom": uom})`, dropping custom columns +every save. `sync_scope_percentages_to_pricing_rules` (hooked on Scheme `on_update`) +re-applies the percentages afterwards. + +> The POS **Promotion Management** UI cannot create these — it has no grid for +> per-row percentages. Configure them in the Frappe desk. + ## Frontend Architecture ### Stores @@ -136,6 +245,67 @@ def apply_offers(invoice_data, selected_offers=None): } ``` +### The discount pipeline + +`apply_offers` is the **only** engine that runs for a POS cart: `update_invoice` +sets `ignore_pricing_rule = 1` and clears every line's `pricing_rules` before +saving, so ERPNext never recomputes and whatever the pipeline produces is what gets +persisted. (A consequence: the Min/Max `validate` hook is a no-op for POS invoices — +it only does work for Sales Orders, Quotations, Delivery Notes and desk-created +invoices.) + +Line-level passes live in `pos_next/promotions/engine.py` and run in order: + +``` +begin_line_discounts() snapshot the ERPNext result as the baseline + ↓ +accumulative pass contributes PART_ACCUMULATIVE, flags its lines + ↓ +auto discount pass contributes PART_AUTO_DISCOUNT (matrix-gated) + ↓ +finalize_line_discounts() discount_percentage = baseline + Σ parts, clamped +``` + +Passes contribute **named parts** rather than assigning `discount_percentage`: +the same source replaces its earlier value, different sources sum. Parts are +rebuilt from scratch each run, so re-running is idempotent — critical because +`validate` can fire repeatedly and additive-on-current would compound +18% → 36% → 54%. + +Order matters. Accumulative runs first so its lines are flagged before the auto +pass consults the matrix; that's what lets the two stack. + +> Accumulative rules carry `promotion_type = "Item Level Discount"`, and ERPNext +> tags them onto the very lines they target. Left alone the matrix would read those +> lines as `already_discounted` and the rule would lock itself out of its own scope. +> `build_matrix_type_map()` neutralises accumulative rules in the type map for +> exactly this reason. + +### Offline behaviour + +Offline carts recompute discounts in `posCart.js`, so accumulative maths has to +exist on both sides. To avoid duplicating the engine, `get_offers` ships the rule +declaratively and the client only sums: + +```json +"accumulative_scope_field": "item_group", +"accumulative_scopes": [ + {"values": ["Electronics", "Phones", "Laptops"], "discount_percentage": 5}, + {"values": ["Apparel"], "discount_percentage": 3} +], +"max_accumulated_discount_percentage": 20, +"min_scopes_required": 2 +``` + +Scope values are **pre-expanded server-side** (item groups → descendants, template +items → variants) so the client matches by plain lookup. Rows keep their identity +because the client counts distinct *rows* present, not distinct keys. + +Known gap: the server clamps against `Item.max_discount` unconditionally, but the +client only does so when the cached item record carries that field — which it does +not today. An offline sale exceeding an item's ceiling can fail +`validate_max_discount` at sync. + ### Mixed Conditions Support The API passes the document to ERPNext's pricing engine for mixed conditions: @@ -198,6 +368,30 @@ If 1 Book + 1 Camera doesn't trigger an "Item Group" offer: 2. Enable `Mixed Conditions` on the Pricing Rule in ERPNext 3. Ensure the backend API is passing `doc` parameter (see code above) +### Accumulative Discount Not Applying + +1. **Is the mode actually set?** `apply_discount_on_price` must read `Accumulative` + on the *generated Pricing Rule*, not just the scheme. +2. **Do the scope rows carry percentages?** A row with `pos_discount_percentage = 0` + contributes nothing; a rule where every row is zero is rejected at validate. + After editing a Scheme, confirm `sync_scope_percentages_to_pricing_rules` copied + them down — ERPNext wipes those columns on every scheme save. +3. **Is `Min Scopes Required` too high?** Set to 2, a cart touching one scope earns + nothing. +4. **Is the line excluded by the matrix?** A manual cashier discount or a *different* + item-level promotion takes the line out — and with it, that scope's contribution. +5. **Is a ceiling clamping it?** Check the rule's **Max Accumulated Discount %** and + the Item's own `max_discount`. + +### Auto Discount Not Stacking on an Accumulative Line + +Expected: accumulative + auto **sum**. If the auto half is missing, check that the +accumulative pass ran first (it flags `is_accumulative_discount`) — offline, that +ordering comes from the sort in `applyOffersOffline`. + +Note that an *ordinary* Item Level Discount is supposed to block Auto Discount. +Only `Accumulative` stacks. + ### Offer Removed Unexpectedly Check the console for: @@ -226,6 +420,37 @@ This happens when: - Add items until subtotal reaches min_amt → Offer applies - Remove items below min_amt → Offer removed +4. **Accumulative:** + - Add an item from scope A only → A's percentage alone + - Add an item from scope B → both lines jump to A% + B% + - Remove the B item → both fall back to A% + - With an Auto Discount scheme also active → both lines show A% + B% + auto% + +### Automated Tests + +Everything lives in `pos_next/test_promotions.py`: + +| Class | Covers | +|---|---| +| `TestPromotions` | Every offer shape through the full submit pipeline | +| `TestPricingRuleScope` | Child-table scope resolution, item-group lineage | +| `TestAutoDiscountScopes` | Auto Discount across Item Code / Item Group / Brand | +| `TestDiscountPipeline` | The accumulator — parts sum, clamping, idempotency | +| `TestAccumulativeDiscount` | Accumulative maths, stacking, caps, matrix flags | +| `TestAccumulativeValidation` | Config guards | +| `TestAccumulativeOfflinePayload` | What `get_offers` ships to the offline engine | + +```bash +bench --site pos set-config allow_tests true +bench --site pos run-tests --module pos_next.test_promotions + +# On a dev site with real data, prefer this — run-tests would wipe it: +bench --site pos execute pos_next.test_promotions.run_all +``` + +The submit-path tests need a Fiscal Year covering today's date, or they fail with +`FiscalYearError` regardless of promotion logic. + ### Backend Testing ```python @@ -244,13 +469,31 @@ result = apply_offers({ print(result) ``` +## Known Limitations + +- **Min/Max vs Accumulative in one cart.** The two are mutually exclusive per rule, + but a cart holding one of each will conflict: the Min/Max pass runs after the + pipeline and *replaces* the discount on the line it targets rather than adding to it. +- **`Item.max_discount` offline.** Enforced server-side, best-effort on the client + (see *Offline behaviour*). +- **No frontend tests.** `vitest` is configured but the repo has no JS test files, + so `computeAccumulativeDiscount` is verified only against the Python it mirrors. +- **Nested scope rows double-count.** Listing a group and its ancestor on one rule + means a line inside both contributes twice. + ## Related Files - Frontend: - `POS/src/stores/posOffers.js` - Offer eligibility store - - `POS/src/stores/posCart.js` - Cart and offer application logic + - `POS/src/stores/posCart.js` - Cart and offer application logic, offline engine - `POS/src/components/sale/OffersDialog.vue` - Offers UI - Backend: - `pos_next/api/invoices.py` - `apply_offers()` API + - `pos_next/promotions/engine.py` - Ordered discount pipeline and its passes + - `pos_next/promotions/scope.py` - Pricing Rule scope resolution, item-group lineage + - `pos_next/api/promotion_exclusions.py` - Promotion Interaction Matrix + - `pos_next/api/offers.py` - Offer payload shipped to the client + - `pos_next/overrides/pricing_rule.py` - Validation, Min/Max pass, Scheme→Rule sync + - `pos_next/pos_next/custom/pricing_rule_*.json` - Per-scope `pos_discount_percentage` - ERPNext: `erpnext/accounts/doctype/pricing_rule/` - Pricing engine diff --git a/pos_next/hooks.py b/pos_next/hooks.py index 7e5b75aba..8676b321f 100644 --- a/pos_next/hooks.py +++ b/pos_next/hooks.py @@ -174,6 +174,7 @@ }, "POS Profile": {"on_update": "pos_next.realtime_events.emit_pos_profile_updated_event"}, "Promotional Scheme": { + "before_validate": "pos_next.overrides.pricing_rule.normalize_accumulative_scheme", "validate": [ "pos_next.overrides.pricing_rule.enforce_cross_cart_pricing_config", "pos_next.overrides.pricing_rule.validate_unique_promotion_type_per_item", diff --git a/pos_next/overrides/pricing_rule.py b/pos_next/overrides/pricing_rule.py index 10b25277b..05f811e6c 100644 --- a/pos_next/overrides/pricing_rule.py +++ b/pos_next/overrides/pricing_rule.py @@ -263,6 +263,103 @@ def validate_unique_promotion_type_per_item(doc, method=None): ) +# Slab fields an Accumulative scheme authors on the parent instead. Parent +# fieldname -> slab fieldname (they differ for the amount pair). +ACCUMULATIVE_PARENT_TO_SLAB = { + "min_qty": "min_qty", + "max_qty": "max_qty", + "min_amount": "min_amount", + "max_amount": "max_amount", + "max_accumulated_discount_percentage": "max_accumulated_discount_percentage", + "min_scopes_required": "min_scopes_required", +} + + +def normalize_accumulative_scheme(doc, method=None): + """Derive the canonical price discount slab from the parent's accumulative config. + + ``pos_is_accumulative`` is an *authoring* control only. ``apply_discount_on_price`` + on the generated Pricing Rule stays the single source of truth for the engine, + the offer payload and the interaction matrix — this function projects the + checkbox onto it. Running on every ``validate`` means the two can never drift, + and a scheme created through the API, a fixture or ``create_promotion`` ends up + exactly as correct as one built in the form. + + ERPNext generates Pricing Rules *from the slabs* (``get_pricing_rules`` skips an + empty child table), so a hidden slab table must still contain a row or the + scheme would save clean and silently discount nothing. + + Wired to ``before_validate``: ERPNext's own ``PromotionalScheme.validate()`` + rejects a scheme with no discount slabs, and that class method runs ahead of + every hooked ``validate``, so the row must already exist by then. + """ + if doc.doctype != "Promotional Scheme": + return + if not frappe.db.has_column("Promotional Scheme", "pos_is_accumulative"): + return + if not doc.get("pos_is_accumulative"): + return + + slab = _resolve_accumulative_slab(doc) + + # Injected because each value is *true* for this mode, not to dodge validation: + # it is a price discount, expressed as a percentage, whose percentage lives on + # the scope rows. rule_description is the slab's only mandatory field. + slab.apply_discount_on_price = ACCUMULATIVE_MODE + slab.rate_or_discount = "Discount Percentage" + slab.rate = 0 + slab.discount_amount = 0 + slab.discount_percentage = 0 + if not slab.get("rule_description"): + slab.rule_description = _("Accumulative discount") + + for parent_field, slab_field in ACCUMULATIVE_PARENT_TO_SLAB.items(): + slab.set(slab_field, doc.get(parent_field) or 0) + + if flt(slab.min_scopes_required) < 1: + slab.min_scopes_required = 1 + + doc.mixed_conditions = 1 + doc.pos_only = 1 + if not doc.get("promotion_type"): + doc.promotion_type = PROMOTION_TYPE_ITEM_LEVEL + + +def _resolve_accumulative_slab(doc): + """The one price discount row an Accumulative scheme owns, created if absent. + + Kept to exactly one row: several would generate several identical Pricing + Rules, which ERPNext then reports as a MultiplePricingRuleConflict at the till. + """ + slabs = doc.get("price_discount_slabs") or [] + + conflicting = [s for s in slabs if s.get("apply_discount_on_price") in MIN_MAX_OPTIONS] + if conflicting: + frappe.throw( + _( + "A scheme cannot be Accumulative and use a Min/Max discount at " + "the same time. Remove the Min/Max price discount row, or clear " + "Accumulative Discount." + ), + title=_("Conflicting Discount Modes"), + ) + + if len(slabs) > 1: + frappe.throw( + _( + "An Accumulative scheme uses a single price discount row, but this " + "scheme has {0}. Remove the extra rows — the qty and amount limits are set " + "on the scheme itself." + ).format(len(slabs)), + title=_("Too Many Price Discount Rows"), + ) + + if slabs: + return slabs[0] + + return doc.append("price_discount_slabs", {}) + + def enforce_cross_cart_pricing_config(doc, method=None): """Validate-time guard for every cross-cart price mode. diff --git a/pos_next/patches.txt b/pos_next/patches.txt index c0f7ec258..018d4e9c3 100644 --- a/pos_next/patches.txt +++ b/pos_next/patches.txt @@ -5,4 +5,5 @@ [post_model_sync] # Patches added in this section will be executed after doctypes are migrated pos_next.patches.v1_7_0.reinstall_workspace -pos_next.patches.v2_0_0.remove_custom_company_fields \ No newline at end of file +pos_next.patches.v2_0_0.remove_custom_company_fields +pos_next.patches.v2_1_0.lift_accumulative_config_to_scheme \ No newline at end of file diff --git a/pos_next/patches/v2_1_0/__init__.py b/pos_next/patches/v2_1_0/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/pos_next/patches/v2_1_0/lift_accumulative_config_to_scheme.py b/pos_next/patches/v2_1_0/lift_accumulative_config_to_scheme.py new file mode 100644 index 000000000..69b894761 --- /dev/null +++ b/pos_next/patches/v2_1_0/lift_accumulative_config_to_scheme.py @@ -0,0 +1,73 @@ +# Copyright (c) 2026, BrainWise and contributors +# For license information, please see license.txt + +"""Lift Accumulative config from the price discount slab up to the Scheme. + +The slab table is hidden once ``pos_is_accumulative`` is on, which would strand +the qty/amount limits and the accumulative settings that used to be authored +there. They now live on the Promotional Scheme itself and are projected back +down on every validate (see ``normalize_accumulative_scheme``). + +Existing schemes are migrated by copying the slab's values up and ticking the +checkbox, so a promotion that was live before the upgrade keeps its behaviour +instead of silently resetting to 0 / 1. +""" + +import frappe +from frappe.utils import cint, flt + +ACCUMULATIVE_MODE = "Accumulative" + +# slab fieldname -> scheme fieldname +LIFTED_FIELDS = { + "min_qty": "min_qty", + "max_qty": "max_qty", + "min_amount": "min_amount", + "max_amount": "max_amount", + "max_accumulated_discount_percentage": "max_accumulated_discount_percentage", + "min_scopes_required": "min_scopes_required", +} + + +def execute(): + if not frappe.db.has_column("Promotional Scheme", "pos_is_accumulative"): + return + if not frappe.db.has_column("Promotional Scheme Price Discount", "apply_discount_on_price"): + return + + slab_columns = [ + slab_field + for slab_field in LIFTED_FIELDS + if frappe.db.has_column("Promotional Scheme Price Discount", slab_field) + ] + if not slab_columns: + return + + slabs = frappe.get_all( + "Promotional Scheme Price Discount", + filters={"apply_discount_on_price": ACCUMULATIVE_MODE, "parenttype": "Promotional Scheme"}, + fields=["parent", *slab_columns], + ) + if not slabs: + return + + # One row per scheme; if a scheme somehow has several, the first wins — the + # normalizer collapses it to a single row on the next save anyway. + seen = set() + for slab in slabs: + scheme = slab.get("parent") + if not scheme or scheme in seen: + continue + seen.add(scheme) + + values = {"pos_is_accumulative": 1} + for slab_field in slab_columns: + scheme_field = LIFTED_FIELDS[slab_field] + raw = slab.get(slab_field) + values[scheme_field] = ( + max(cint(raw) or 1, 1) if scheme_field == "min_scopes_required" else flt(raw) + ) + + frappe.db.set_value("Promotional Scheme", scheme, values, update_modified=False) + + frappe.db.commit() diff --git a/pos_next/pos_next/custom/promotional_scheme.json b/pos_next/pos_next/custom/promotional_scheme.json index de60814f3..27151e139 100644 --- a/pos_next/pos_next/custom/promotional_scheme.json +++ b/pos_next/pos_next/custom/promotional_scheme.json @@ -157,6 +157,420 @@ "translatable": 0, "unique": 0, "width": null + }, + { + "allow_in_quick_entry": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "default": "0", + "depends_on": null, + "description": "Take the discount from a per-row Discount % on the scope table instead of a single rule-level percentage. Every scope row present in the cart contributes, and the total applies to each eligible line. Hides the price discount rows \u2014 POS Next maintains the single row they need automatically.", + "docstatus": 0, + "dt": "Promotional Scheme", + "fieldname": "pos_is_accumulative", + "fieldtype": "Check", + "hidden": 0, + "idx": 6, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_preview": 0, + "in_standard_filter": 0, + "insert_after": "is_cumulative", + "is_system_generated": 0, + "is_virtual": 0, + "label": "Accumulative Discount", + "length": 0, + "mandatory_depends_on": null, + "module": null, + "name": "Promotional Scheme-pos_is_accumulative", + "no_copy": 0, + "non_negative": 0, + "options": null, + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "read_only": 0, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "show_dashboard": 0, + "sort_options": 0, + "translatable": 0, + "unique": 0, + "width": null + }, + { + "allow_in_quick_entry": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "default": null, + "depends_on": "eval:doc.pos_is_accumulative", + "description": null, + "docstatus": 0, + "dt": "Promotional Scheme", + "fieldname": "pos_accumulative_section", + "fieldtype": "Section Break", + "hidden": 0, + "idx": 7, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_preview": 0, + "in_standard_filter": 0, + "insert_after": "pos_is_accumulative", + "is_system_generated": 0, + "is_virtual": 0, + "label": "Accumulative Discount", + "length": 0, + "mandatory_depends_on": null, + "module": null, + "name": "Promotional Scheme-pos_accumulative_section", + "no_copy": 0, + "non_negative": 0, + "options": null, + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "read_only": 0, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "show_dashboard": 0, + "sort_options": 0, + "translatable": 0, + "unique": 0, + "width": null + }, + { + "allow_in_quick_entry": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "default": null, + "depends_on": "eval:doc.pos_is_accumulative", + "description": "Minimum total quantity before the discount applies. Measures volume \u2014 use Min Scopes Required to demand variety instead.", + "docstatus": 0, + "dt": "Promotional Scheme", + "fieldname": "min_qty", + "fieldtype": "Float", + "hidden": 0, + "idx": 8, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_preview": 0, + "in_standard_filter": 0, + "insert_after": "pos_accumulative_section", + "is_system_generated": 0, + "is_virtual": 0, + "label": "Min Qty", + "length": 0, + "mandatory_depends_on": null, + "module": null, + "name": "Promotional Scheme-min_qty", + "no_copy": 0, + "non_negative": 1, + "options": null, + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "read_only": 0, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "show_dashboard": 0, + "sort_options": 0, + "translatable": 0, + "unique": 0, + "width": null + }, + { + "allow_in_quick_entry": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "default": null, + "depends_on": "eval:doc.pos_is_accumulative", + "description": null, + "docstatus": 0, + "dt": "Promotional Scheme", + "fieldname": "max_qty", + "fieldtype": "Float", + "hidden": 0, + "idx": 9, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_preview": 0, + "in_standard_filter": 0, + "insert_after": "min_qty", + "is_system_generated": 0, + "is_virtual": 0, + "label": "Max Qty", + "length": 0, + "mandatory_depends_on": null, + "module": null, + "name": "Promotional Scheme-max_qty", + "no_copy": 0, + "non_negative": 1, + "options": null, + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "read_only": 0, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "show_dashboard": 0, + "sort_options": 0, + "translatable": 0, + "unique": 0, + "width": null + }, + { + "allow_in_quick_entry": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "default": null, + "depends_on": "eval:doc.pos_is_accumulative", + "description": null, + "docstatus": 0, + "dt": "Promotional Scheme", + "fieldname": "min_amount", + "fieldtype": "Currency", + "hidden": 0, + "idx": 10, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_preview": 0, + "in_standard_filter": 0, + "insert_after": "max_qty", + "is_system_generated": 0, + "is_virtual": 0, + "label": "Min Amount", + "length": 0, + "mandatory_depends_on": null, + "module": null, + "name": "Promotional Scheme-min_amount", + "no_copy": 0, + "non_negative": 1, + "options": null, + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "read_only": 0, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "show_dashboard": 0, + "sort_options": 0, + "translatable": 0, + "unique": 0, + "width": null + }, + { + "allow_in_quick_entry": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "default": null, + "depends_on": "eval:doc.pos_is_accumulative", + "description": null, + "docstatus": 0, + "dt": "Promotional Scheme", + "fieldname": "max_amount", + "fieldtype": "Currency", + "hidden": 0, + "idx": 11, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_preview": 0, + "in_standard_filter": 0, + "insert_after": "min_amount", + "is_system_generated": 0, + "is_virtual": 0, + "label": "Max Amount", + "length": 0, + "mandatory_depends_on": null, + "module": null, + "name": "Promotional Scheme-max_amount", + "no_copy": 0, + "non_negative": 1, + "options": null, + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "read_only": 0, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "show_dashboard": 0, + "sort_options": 0, + "translatable": 0, + "unique": 0, + "width": null + }, + { + "allow_in_quick_entry": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "default": null, + "depends_on": "eval:doc.pos_is_accumulative", + "description": null, + "docstatus": 0, + "dt": "Promotional Scheme", + "fieldname": "pos_accumulative_column", + "fieldtype": "Column Break", + "hidden": 0, + "idx": 12, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_preview": 0, + "in_standard_filter": 0, + "insert_after": "max_amount", + "is_system_generated": 0, + "is_virtual": 0, + "label": null, + "length": 0, + "mandatory_depends_on": null, + "module": null, + "name": "Promotional Scheme-pos_accumulative_column", + "no_copy": 0, + "non_negative": 0, + "options": null, + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "read_only": 0, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "show_dashboard": 0, + "sort_options": 0, + "translatable": 0, + "unique": 0, + "width": null + }, + { + "allow_in_quick_entry": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "default": "0", + "depends_on": "eval:doc.pos_is_accumulative", + "description": "Ceiling for the summed discount. Each line is additionally capped by that Item's own Max Discount. 0 means no rule-level cap.", + "docstatus": 0, + "dt": "Promotional Scheme", + "fieldname": "max_accumulated_discount_percentage", + "fieldtype": "Float", + "hidden": 0, + "idx": 13, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_preview": 0, + "in_standard_filter": 0, + "insert_after": "pos_accumulative_column", + "is_system_generated": 0, + "is_virtual": 0, + "label": "Max Accumulated Discount %", + "length": 0, + "mandatory_depends_on": null, + "module": null, + "name": "Promotional Scheme-max_accumulated_discount_percentage", + "no_copy": 0, + "non_negative": 1, + "options": null, + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "read_only": 0, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "show_dashboard": 0, + "sort_options": 0, + "translatable": 0, + "unique": 0, + "width": null + }, + { + "allow_in_quick_entry": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "default": "1", + "depends_on": "eval:doc.pos_is_accumulative", + "description": "How many distinct scope rows must be represented in the cart. 1 means a cart touching a single row still earns that row's percentage.", + "docstatus": 0, + "dt": "Promotional Scheme", + "fieldname": "min_scopes_required", + "fieldtype": "Int", + "hidden": 0, + "idx": 14, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_preview": 0, + "in_standard_filter": 0, + "insert_after": "max_accumulated_discount_percentage", + "is_system_generated": 0, + "is_virtual": 0, + "label": "Min Scopes Required", + "length": 0, + "mandatory_depends_on": null, + "module": null, + "name": "Promotional Scheme-min_scopes_required", + "no_copy": 0, + "non_negative": 1, + "options": null, + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "read_only": 0, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "show_dashboard": 0, + "sort_options": 0, + "translatable": 0, + "unique": 0, + "width": null } ], "custom_perms": [], @@ -164,4 +578,4 @@ "links": [], "property_setters": [], "sync_on_migrate": 1 -} \ No newline at end of file +} diff --git a/pos_next/public/js/promotional_scheme.js b/pos_next/public/js/promotional_scheme.js index 75fda8b3e..db423d7f6 100644 --- a/pos_next/public/js/promotional_scheme.js +++ b/pos_next/public/js/promotional_scheme.js @@ -33,6 +33,9 @@ frappe.ui.form.on("Promotional Scheme", { promotion_type(frm) { pn_toggle_gwp_fields(frm); }, + pos_is_accumulative(frm) { + pn_sync_accumulative(frm); + }, apply_on(frm) { pn_sync_accumulative(frm); pn_toggle_gwp_fields(frm); @@ -143,35 +146,35 @@ function pn_sync_min_max(frm) { } /** - * Accumulative schemes take their percentage from the per-scope rows. + * Presentation only — the server owns the data. + * + * `pos_is_accumulative` is an authoring control. `normalize_accumulative_scheme` + * projects it onto the canonical slab on every validate, so this function sets no + * value the server does not also set; it just gets the irrelevant controls out of + * the way. Anything enforced here is enforced there too, which is what keeps a + * scheme built via the API as correct as one built in this form. * - * The price discount slab itself stays visible — ERPNext generates no Pricing - * Rule without one, and the slab is where the mode, the cap and Min Scopes - * Required live. Only its discount-value fields are hidden, and the product - * (free item) slabs go away since an Item Level Discount cannot grant them. + * The price discount slab is hidden rather than removed: ERPNext generates no + * Pricing Rule from an empty child table, so the server keeps exactly one row + * alive behind the scenes. */ function pn_sync_accumulative(frm) { - const slabs = frm.doc.price_discount_slabs || []; - const is_accumulative = slabs.some((row) => row.apply_discount_on_price === "Accumulative"); + const is_accumulative = !!frm.doc.pos_is_accumulative; + + frm.toggle_display("price_discount_slabs", !is_accumulative); + // Free items and Accumulative are mutually exclusive. + frm.toggle_display("product_discount_slabs", !is_accumulative); if (is_accumulative) { - for (const row of slabs) { - if (row.apply_discount_on_price !== "Accumulative") continue; - // ERPNext needs a rate_or_discount to generate a Price rule at all. - if (row.rate_or_discount !== "Discount Percentage") { - frappe.model.set_value(row.doctype, row.name, "rate_or_discount", "Discount Percentage"); - } - for (const fieldname of ["rate", "discount_amount", "discount_percentage"]) { - if (row[fieldname]) { - frappe.model.set_value(row.doctype, row.name, fieldname, 0); - } - } - } if (!frm.doc.mixed_conditions) { frm.set_value("mixed_conditions", 1); } + if (!frm.doc.min_scopes_required) { + frm.set_value("min_scopes_required", 1); + } } + // Mirrors the server, which pins these on the slab it maintains. const slab_grid = frm.fields_dict.price_discount_slabs?.grid; if (slab_grid) { for (const fieldname of PN_SLAB_DISCOUNT_FIELDS) { @@ -180,9 +183,6 @@ function pn_sync_accumulative(frm) { } } - // Free items and Accumulative are mutually exclusive. - frm.set_df_property("product_discount_slabs", "hidden", is_accumulative ? 1 : 0); - pn_toggle_scope_percentage(frm, is_accumulative); } diff --git a/pos_next/test_promotions.py b/pos_next/test_promotions.py index ef73da96c..99835c3b6 100644 --- a/pos_next/test_promotions.py +++ b/pos_next/test_promotions.py @@ -2278,6 +2278,140 @@ def test_valid_rule_gets_mixed_conditions_and_pos_only_forced(self): self.assertTrue(doc.pos_only) +class TestAccumulativeSchemeCheckbox(FrappeTestCase): + """`pos_is_accumulative` is an authoring control; the slab stays canonical. + + The checkbox hides the price discount table, so the server must materialise + the single slab ERPNext needs — an empty child table generates no Pricing Rule + at all and the scheme would save clean while discounting nothing. + """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.ctx = _ctx() + + def _scheme_doc(self, name, **overrides): + if frappe.db.exists("Promotional Scheme", name): + frappe.delete_doc("Promotional Scheme", name, force=True, ignore_permissions=True) + self.addCleanup( + lambda: frappe.db.exists("Promotional Scheme", name) + and frappe.delete_doc("Promotional Scheme", name, force=True, ignore_permissions=True) + ) + payload = { + "doctype": "Promotional Scheme", + "name": name, + "company": _resolve_company(), + "apply_on": "Item Code", + "selling": 1, + "buying": 0, + "valid_from": nowdate(), + "promotion_type": "Item Level Discount", + "pos_is_accumulative": 1, + "min_scopes_required": 1, + "items": [ + {"item_code": ITEM_A, "pos_discount_percentage": 5}, + {"item_code": ITEM_B, "pos_discount_percentage": 3}, + ], + } + payload.update(overrides) + return frappe.get_doc(payload) + + def test_checkbox_alone_materialises_the_slab_and_generates_a_rule(self): + """No slab supplied at all — the API path a form would never exercise.""" + scheme = self._scheme_doc("_PNXT_TEST_AccumCheckbox") + self.assertFalse(scheme.get("price_discount_slabs")) + scheme.insert(ignore_permissions=True) + + slabs = scheme.get("price_discount_slabs") or [] + self.assertEqual(len(slabs), 1, "server must maintain exactly one slab") + self.assertEqual(slabs[0].apply_discount_on_price, "Accumulative") + self.assertEqual(slabs[0].rate_or_discount, "Discount Percentage") + self.assertTrue(slabs[0].rule_description, "slab's only reqd field must be injected") + self.assertTrue(scheme.mixed_conditions) + self.assertTrue(scheme.pos_only) + + rules = frappe.get_all("Pricing Rule", filters={"promotional_scheme": scheme.name}, pluck="name") + self.assertEqual(len(rules), 1, "an empty slab table would have generated nothing") + self.assertEqual( + frappe.db.get_value("Pricing Rule", rules[0], "apply_discount_on_price"), "Accumulative" + ) + + def test_parent_config_is_projected_onto_the_generated_rule(self): + scheme = self._scheme_doc( + "_PNXT_TEST_AccumCheckboxCfg", + min_qty=2, + min_amount=50, + max_accumulated_discount_percentage=25, + min_scopes_required=2, + ) + scheme.insert(ignore_permissions=True) + + rule = frappe.get_doc( + "Pricing Rule", + frappe.get_all("Pricing Rule", filters={"promotional_scheme": scheme.name}, pluck="name")[0], + ) + self.assertAlmostEqual(flt(rule.min_qty), 2, places=2) + self.assertAlmostEqual(flt(rule.min_amt), 50, places=2) + self.assertAlmostEqual(flt(rule.max_accumulated_discount_percentage), 25, places=2) + self.assertEqual(int(rule.min_scopes_required), 2) + + def test_toggling_off_and_on_keeps_exactly_one_slab(self): + """Idempotency: repeated saves must not append a slab (or a Pricing Rule) each time.""" + scheme = self._scheme_doc("_PNXT_TEST_AccumCheckboxToggle") + scheme.insert(ignore_permissions=True) + + for _ in range(3): + scheme.reload() + scheme.save(ignore_permissions=True) + + scheme.reload() + self.assertEqual(len(scheme.get("price_discount_slabs") or []), 1) + self.assertEqual( + len(frappe.get_all("Pricing Rule", filters={"promotional_scheme": scheme.name})), 1 + ) + + def test_accumulative_and_min_max_together_are_rejected(self): + """The checkbox re-admits a state the single Select made impossible.""" + scheme = self._scheme_doc( + "_PNXT_TEST_AccumCheckboxConflict", + price_discount_slabs=[ + { + "rule_description": "min/max row", + "rate_or_discount": "Discount Percentage", + "discount_percentage": 10, + "apply_discount_on_price": "Min", + } + ], + ) + with self.assertRaises(frappe.ValidationError): + scheme.insert(ignore_permissions=True) + + def test_multiple_slabs_are_rejected(self): + """Several rows would generate several identical rules -> conflict at the till.""" + scheme = self._scheme_doc( + "_PNXT_TEST_AccumCheckboxMultiSlab", + price_discount_slabs=[ + {"rule_description": "a", "rate_or_discount": "Discount Percentage", "min_qty": 1}, + {"rule_description": "b", "rate_or_discount": "Discount Percentage", "min_qty": 5}, + ], + ) + with self.assertRaises(frappe.ValidationError): + scheme.insert(ignore_permissions=True) + + def test_discount_applies_end_to_end_from_the_checkbox(self): + import json + + scheme = self._scheme_doc("_PNXT_TEST_AccumCheckboxApply") + scheme.insert(ignore_permissions=True) + rules = frappe.get_all("Pricing Rule", filters={"promotional_scheme": scheme.name}, pluck="name") + + payload = _cart_payload(self.ctx, [_line(self.ctx, ITEM_A), _line(self.ctx, ITEM_B)]) + resp = apply_offers(invoice_data=json.dumps(payload), selected_offers=json.dumps(rules)) + for item in resp["items"][:2]: + self.assertAlmostEqual(flt(item.get("discount_percentage")), 8, places=2) + + class TestAccumulativeOfflinePayload(FrappeTestCase): """`get_offers` must ship enough for the offline engine to reproduce the math. @@ -2422,6 +2556,7 @@ def run_all(): TestDiscountPipelineIdempotency, TestAccumulativeDiscount, TestAccumulativeValidation, + TestAccumulativeSchemeCheckbox, TestAccumulativeOfflinePayload, ) )