From 99b66febfaa3386e4b7308cd717ab937cbfd548d Mon Sep 17 00:00:00 2001 From: engahmed1190 Date: Wed, 17 Jun 2026 12:06:27 +0300 Subject: [PATCH 01/11] docs: spec for applied pricing rule name on Sales Invoice item Show the Promotional Scheme / Pricing Rule name on each Sales Invoice item line (POS Next clears item.pricing_rules at save, so no rule name survives today). Per-item read-only field, formatted as ' ()'. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...ricing-rule-name-on-invoice-item-design.md | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 docs/superpowers/specs/2026-06-17-applied-pricing-rule-name-on-invoice-item-design.md diff --git a/docs/superpowers/specs/2026-06-17-applied-pricing-rule-name-on-invoice-item-design.md b/docs/superpowers/specs/2026-06-17-applied-pricing-rule-name-on-invoice-item-design.md new file mode 100644 index 000000000..5f16f61bb --- /dev/null +++ b/docs/superpowers/specs/2026-06-17-applied-pricing-rule-name-on-invoice-item-design.md @@ -0,0 +1,158 @@ +# Show Applied Promotional Scheme / Pricing Rule Name on Sales Invoice Item + +**Date:** 2026-06-17 +**Status:** Approved design — ready for implementation plan +**Author:** POS Next + +## Problem + +POS Next computes promotional offers itself via `apply_offers` and saves each +item with `discount_percentage` / `discount_amount` / `rate` already set, paired +with `invoice_doc.ignore_pricing_rule = 1`. To prevent ERPNext's pricing engine +from zeroing the discount on the second save, `update_invoice` **clears** +`item.pricing_rules = ""` before saving (`pos_next/api/invoices.py`, ~line 897). + +Consequence: a saved POS Sales Invoice retains **no record** of which Pricing +Rule / Promotional Scheme produced a line's discount. ERPNext's native per-line +"Pricing Rules" field is blank, and `Pricing Rule Detail` is not populated +(the engine is bypassed on save). The only rule name persisted today is +`pos_applied_one_time_rules` (Sales Invoice header), and only for +one-time-per-customer rules. + +A representative case: two POS invoices for the same customer both carry a +percentage discount from the same promotional scheme, yet each item row shows +`pricing_rules = ""`. Staff cannot tell from the invoice which promotion applied. + +## Goal + +Display, on each Sales Invoice **item line**, the pricing rule(s) that discounted +that line, formatted as the human-friendly scheme title plus the rule id: + +- Promotional-scheme rule: ` ()` +- Standalone pricing rule (no scheme): `` (fallback to rule id only) +- Multiple rules on one line: comma-separated. + +The field is read-only and available to Desk (and print). + +## Non-Goals + +- No change to discount math, `ignore_pricing_rule`, or the one-time-offer + logic (`pos_applied_one_time_rules`, `One Time Customer Offer Usage`). +- No POS frontend (Vue) UI change — this is a Desk/invoice-record feature. +- No backfill of historical invoices (the data was discarded at save time and + cannot be reconstructed reliably). New invoices only. +- Sales Invoice only. (Sales Order is out of scope; the clearing logic that + motivates this lives in the Sales Invoice save path.) + +## Design + +### 1. New custom field: `pos_applied_pricing_rules` on Sales Invoice Item + +Add via a new managed custom-field file +`pos_next/pos_next/custom/sales_invoice_item.json`, following the exact shape of +the existing `pos_next/pos_next/custom/sales_invoice.json` (`sync_on_migrate: 1`). + +Field properties (mirroring the established `pos_applied_one_time_rules` +conventions, except this one is visible and printable): + +| Property | Value | +|---|---| +| `dt` | `Sales Invoice Item` | +| `fieldname` | `pos_applied_pricing_rules` | +| `fieldtype` | `Small Text` | +| `label` | `Applied Pricing Rules` | +| `read_only` | 1 | +| `no_copy` | 1 | +| `hidden` | 0 | +| `print_hide` | 0 | +| `allow_on_submit` | 0 | +| `in_list_view` | 0 | +| `module` | `POS Next` | +| `insert_after` | an existing discount-area field on Sales Invoice Item (e.g. `discount_amount`) | + +Exact `insert_after` and `idx` to be finalized against the live Sales Invoice +Item field order during implementation. + +### 2. Populate during `update_invoice` (server-side) + +In `pos_next/api/invoices.py`, the per-item loop starting at the +`applied_rule_names_seen` collection (~line 830) already extracts each item's +applied rule names immediately before clearing `item.pricing_rules` (~line +888-897). Extend that exact site: + +1. **Capture per-item rule names.** Where the code currently does + `applied_rule_names_seen.update(...)` and then `item.pricing_rules = ""`, + also keep the list of rule names for *this item* (e.g. a local + `item_rule_names` list per iteration). Continue updating the union + `applied_rule_names_seen` as today (still needed for the one-time stamp). + +2. **Resolve titles in one batched query.** After the loop (alongside the + existing one-time stamping block at ~line 899), run a single + `frappe.get_all("Pricing Rule", filters={"name": ["in", list(applied_rule_names_seen)]}, fields=["name", "promotional_scheme", "title"])` + and build a `name -> display_label` map. The label's friendly portion is + chosen by this precedence (first non-empty wins): + 1. The Promotional Scheme's `title`, if `promotional_scheme` is set and that + scheme has a non-empty title. + 2. The `promotional_scheme` id itself, if set but the scheme has no title. + 3. The Pricing Rule's own `title`, if set. + 4. None of the above → no friendly portion. + + Final format: `"{friendly} ({rule_name})"` when a friendly portion exists, + otherwise `"{rule_name}"`. + - Scheme titles: resolve via one additional batched + `frappe.get_all("Promotional Scheme", ...)` keyed by the distinct + `promotional_scheme` values (only if any scheme-linked rules exist). + +3. **Stamp each item.** Set + `item.pos_applied_pricing_rules = ", ".join(display_label[n] for n in item_rule_names)` + (empty string when the item had no applied rules). + +Query budget: at most **two** additional `frappe.get_all` calls per invoice +(one for Pricing Rule, one for Promotional Scheme titles), independent of item +count. No per-item queries. + +### Data flow + +``` +apply_offers (earlier call) update_invoice (save path) + └─ ERPNext engine returns → per-item loop: + item.pricing_rules ├─ read item.pricing_rules + (",...") ├─ item_rule_names = [...] (NEW: keep per item) + ├─ applied_rule_names_seen.update(...) (existing) + └─ item.pricing_rules = "" (existing) + after loop: + ├─ batch-resolve rule -> "Title (RULE-ID)" (NEW) + ├─ stamp item.pos_applied_pricing_rules (NEW) + └─ stamp invoice.pos_applied_one_time_rules (existing) +``` + +### Error handling + +- A rule name in `item.pricing_rules` that no longer resolves to a Pricing Rule + (deleted rule) falls back to the raw rule id as its label. Never throws. +- Title resolution failures degrade to rule id; stamping must never fail the + sale (consistent with the existing one-time recording philosophy). Wrap the + resolution/stamp in a guard that logs and leaves the field empty on error. + +## Testing + +Run on **pos-dev** (has ERPNext data; never `bench run-tests`, use +`bench execute`): + +1. **Scheme rule:** Build a cart whose item matches a promotional-scheme price + rule, run it through the save path, assert the saved item's + `pos_applied_pricing_rules == " ()"`. +2. **Standalone rule:** Same with a standalone Pricing Rule (no scheme), assert + the field equals `""`. +3. **No rule:** Item with no applied rule → field is empty string. +4. **Multiple rules on a line:** assert comma-separated labels. +5. **Deleted rule id:** simulate an unresolved rule name → falls back to id, no + exception. +6. Migration: `bench --site pos-dev migrate` syncs the new custom field; + confirm it appears on Sales Invoice Item and is read-only. + +## Rollout + +- New custom field ships via `custom/sales_invoice_item.json` and is applied by + `bench migrate` (sync_on_migrate), same mechanism as existing custom fields. +- Backend change is additive; safe to deploy with the migration. From 3a105b1636cd7201539493a5d04999a1eb8efba2 Mon Sep 17 00:00:00 2001 From: engahmed1190 Date: Wed, 17 Jun 2026 12:06:27 +0300 Subject: [PATCH 02/11] docs: implementation plan for applied pricing rule name on invoice item Co-Authored-By: Claude Opus 4.8 (1M context) --- ...plied-pricing-rule-name-on-invoice-item.md | 378 ++++++++++++++++++ 1 file changed, 378 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-17-applied-pricing-rule-name-on-invoice-item.md diff --git a/docs/superpowers/plans/2026-06-17-applied-pricing-rule-name-on-invoice-item.md b/docs/superpowers/plans/2026-06-17-applied-pricing-rule-name-on-invoice-item.md new file mode 100644 index 000000000..6bee6949e --- /dev/null +++ b/docs/superpowers/plans/2026-06-17-applied-pricing-rule-name-on-invoice-item.md @@ -0,0 +1,378 @@ +# Applied Pricing Rule Name on Sales Invoice Item — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Stamp each Sales Invoice item line with the human-friendly name of the Promotional Scheme / Pricing Rule that discounted it (e.g. ` ()`), so the applied promotion is visible on the saved invoice in Desk. + +**Architecture:** POS Next clears `item.pricing_rules` before save to stop ERPNext zeroing the discount, which is why no rule name survives today. We add a read-only custom field `pos_applied_pricing_rules` to Sales Invoice Item, capture each item's applied rule names in the existing per-item loop in `update_invoice` just before they are cleared, resolve those names to friendly labels with at most two batched queries after the loop, and stamp the field per item. No change to discount math or the one-time-offer logic. + +**Tech Stack:** Frappe/ERPNext (Python), custom fields via `sync_on_migrate` JSON, `FrappeTestCase` (run with `bench run-tests`-free workflow — these are unit tests executed via the test runner, NOT data-wiping `bench run-tests` against a real site; see Testing Notes). + +--- + +## Testing Notes (read first) + +- The project rule is **never run `bench run-tests`** (it wipes data) — but the existing `pos_next/test_promotions.py` is a `FrappeTestCase` suite. Run it the way the repo already runs that suite: against the **pos-dev** test site, scoped to the single module, e.g.: + ``` + bench --site pos-dev run-tests --module pos_next.test_promotions + ``` + If the team's convention is to avoid even scoped `run-tests`, drive the new test function directly with `bench --site pos-dev execute pos_next.test_promotions.` is NOT possible for `FrappeTestCase` methods; instead confirm with the user before running. Default below assumes scoped `run-tests --module pos_next.test_promotions` is acceptable since that file is already a test suite. **Confirm with the user before the first run.** +- All work happens on branch `feat/applied-pricing-rule-name-on-invoice-item` (already based on `community/uat`). + +--- + +## File Structure + +- **Create:** `pos_next/pos_next/custom/sales_invoice_item.json` — managed custom-field definition adding `pos_applied_pricing_rules` to Sales Invoice Item (synced by `bench migrate`). +- **Modify:** `pos_next/api/invoices.py` (~lines 829-921, the `update_invoice` per-item loop and the post-loop stamping block) — capture per-item rule names, add a helper to resolve labels, stamp the new field. +- **Modify:** `pos_next/test_promotions.py` — add a test asserting the field is stamped with `Title (RULE-ID)` and is empty when no rule applied. + +--- + +## Task 1: Add the `pos_applied_pricing_rules` custom field on Sales Invoice Item + +**Files:** +- Create: `pos_next/pos_next/custom/sales_invoice_item.json` + +- [ ] **Step 1: Create the custom-field JSON** + +Create `pos_next/pos_next/custom/sales_invoice_item.json` with exactly this content. The field shape mirrors the existing `Sales Invoice-pos_applied_one_time_rules` field in `custom/sales_invoice.json`, but is **visible** (`hidden: 0`) and **printable** (`print_hide: 0`) and `read_only`. `insert_after` is `discount_amount`, an existing field on Sales Invoice Item. + +```json +{ + "custom_fields": [ + { + "allow_in_quick_entry": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "default": null, + "depends_on": null, + "description": "Promotional Scheme / Pricing Rule(s) applied to this line, formatted as 'Title (RULE-ID)'. Stamped by POS Next at save because item.pricing_rules is cleared to protect the discount.", + "docstatus": 0, + "dt": "Sales Invoice Item", + "fieldname": "pos_applied_pricing_rules", + "fieldtype": "Small Text", + "hidden": 0, + "idx": 0, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_preview": 0, + "in_standard_filter": 0, + "is_system_generated": 0, + "is_virtual": 0, + "label": "Applied Pricing Rules", + "length": 0, + "mandatory_depends_on": null, + "module": "POS Next", + "name": "Sales Invoice Item-pos_applied_pricing_rules", + "no_copy": 1, + "non_negative": 0, + "options": null, + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 1, + "read_only": 1, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "show_dashboard": 0, + "sort_options": 0, + "translatable": 0, + "unique": 0, + "width": null + } + ], + "custom_perms": [], + "doctype": "Sales Invoice Item", + "links": [], + "property_setters": [], + "sync_on_migrate": 1 +} +``` + +- [ ] **Step 2: Sync the field to the test site** + +Run: `bench --site pos-dev migrate` +Expected: completes without error; the new custom field is created. + +- [ ] **Step 3: Verify the field exists and is read-only** + +Run: +``` +bench --site pos-dev execute frappe.client.get_value --kwargs "{'doctype':'Custom Field','filters':{'name':'Sales Invoice Item-pos_applied_pricing_rules'},'fieldname':['fieldname','read_only','hidden','dt']}" +``` +Expected output: a dict with `fieldname: pos_applied_pricing_rules`, `read_only: 1`, `hidden: 0`, `dt: Sales Invoice Item`. + +- [ ] **Step 4: Commit** + +```bash +git add pos_next/pos_next/custom/sales_invoice_item.json +git commit -m "feat: add pos_applied_pricing_rules field to Sales Invoice Item" +``` + +--- + +## Task 2: Add a label-resolver helper in invoices.py + +**Files:** +- Modify: `pos_next/api/invoices.py` (add a module-level function near `is_one_time_eligible_customer`, ~line 280) + +This helper takes the set of applied rule names and returns a `{rule_name: display_label}` map using at most two batched queries. It must never raise. + +- [ ] **Step 1: Write the helper** + +Add this function in `pos_next/api/invoices.py` immediately after the `is_one_time_eligible_customer` function (which ends at ~line 288). The function uses `frappe`, already imported at the top of the file. + +```python +def _resolve_pricing_rule_labels(rule_names): + """Map applied Pricing Rule names to friendly display labels. + + Label precedence (first non-empty wins) for the friendly portion: + 1. The linked Promotional Scheme's title. + 2. The Promotional Scheme id, if linked but the scheme has no title. + 3. The Pricing Rule's own title. + 4. None -> label is just the rule id. + + Final label: "{friendly} ({rule_name})" when a friendly part exists, + otherwise "{rule_name}". Never raises: any failure yields an empty map so + stamping degrades to nothing rather than failing the sale. + + Args: + rule_names: iterable of Pricing Rule names. + + Returns: + dict[str, str]: rule name -> display label. Names that don't resolve to a + Pricing Rule still get an entry equal to the raw name. + """ + names = sorted({n for n in (rule_names or []) if n}) + if not names: + return {} + + try: + rules = frappe.get_all( + "Pricing Rule", + filters={"name": ["in", names]}, + fields=["name", "promotional_scheme", "title"], + ) + scheme_ids = sorted({r.promotional_scheme for r in rules if r.promotional_scheme}) + scheme_titles = {} + if scheme_ids: + scheme_titles = { + s.name: s.title + for s in frappe.get_all( + "Promotional Scheme", + filters={"name": ["in", scheme_ids]}, + fields=["name", "title"], + ) + } + + labels = {} + found = set() + for r in rules: + found.add(r.name) + friendly = None + if r.promotional_scheme: + friendly = scheme_titles.get(r.promotional_scheme) or r.promotional_scheme + elif r.title: + friendly = r.title + labels[r.name] = f"{friendly} ({r.name})" if friendly else r.name + + # Unresolved names (e.g. deleted rule) fall back to the raw id. + for n in names: + if n not in found: + labels[n] = n + return labels + except Exception: + frappe.log_error( + title="Applied pricing rule label resolution failed", + message=frappe.get_traceback(), + ) + return {} +``` + +- [ ] **Step 2: Syntax-check the module imports cleanly** + +Run: `bench --site pos-dev execute pos_next.api.invoices._resolve_pricing_rule_labels --kwargs "{'rule_names': []}"` +Expected: returns `{}` (empty dict), no error. + +- [ ] **Step 3: Commit** + +```bash +git add pos_next/api/invoices.py +git commit -m "feat: add _resolve_pricing_rule_labels helper" +``` + +--- + +## Task 3: Capture per-item rule names and stamp the field in update_invoice + +**Files:** +- Modify: `pos_next/api/invoices.py:829-921` + +The per-item loop currently collects only the union `applied_rule_names_seen` and clears `item.pricing_rules`. We add a per-item capture (`item._applied_rule_names`, a transient attribute) inside the loop, then resolve+stamp after the loop. + +- [ ] **Step 1: Capture per-item names inside the loop** + +In `pos_next/api/invoices.py`, find the block (currently ~lines 888-897): + +```python + if item.get("pricing_rules"): + if erpnext_get_applied_pricing_rules: + applied_rule_names_seen.update( + erpnext_get_applied_pricing_rules(item.pricing_rules) or [] + ) + else: + applied_rule_names_seen.update( + r.strip() for r in str(item.pricing_rules).split(",") if r.strip() + ) + item.pricing_rules = "" +``` + +Replace it with (captures this item's names before clearing): + +```python + item_rule_names = [] + if item.get("pricing_rules"): + if erpnext_get_applied_pricing_rules: + item_rule_names = list( + erpnext_get_applied_pricing_rules(item.pricing_rules) or [] + ) + else: + item_rule_names = [ + r.strip() for r in str(item.pricing_rules).split(",") if r.strip() + ] + applied_rule_names_seen.update(item_rule_names) + item.pricing_rules = "" + # Stash this line's applied rule names for post-loop label stamping. + # Cleared field can't be read back, so we keep them on the row object. + item._applied_rule_names = item_rule_names +``` + +- [ ] **Step 2: Stamp the field after the loop** + +In the same function, find the post-loop block that currently begins (currently ~line 899): + +```python + if doctype == "Sales Invoice": + # Only stamp rules we can actually track: an identified, non walk-in + # customer on a non-return sale (see is_one_time_eligible_customer). + can_track_one_time = is_one_time_eligible_customer( +``` + +Insert the following **immediately before** that `if doctype == "Sales Invoice":` line (it runs for every doctype that went through the loop; the field only exists on Sales Invoice Item, but `db_set`/attribute set on the row is harmless for others and the value is only persisted when the parent is a Sales Invoice): + +```python + # Stamp each line with the friendly label(s) of the pricing rule(s) that + # discounted it, so the applied promotion is visible on the saved invoice + # (item.pricing_rules was cleared above to protect the discount). + rule_labels = _resolve_pricing_rule_labels(applied_rule_names_seen) + for item in invoice_doc.get("items", []): + names = getattr(item, "_applied_rule_names", None) or [] + item.pos_applied_pricing_rules = ( + ", ".join(rule_labels.get(n, n) for n in names) if names else "" + ) + +``` + +- [ ] **Step 3: Verify the module still imports** + +Run: `bench --site pos-dev execute pos_next.api.invoices._resolve_pricing_rule_labels --kwargs "{'rule_names': []}"` +Expected: returns `{}`, no error (confirms no syntax error in the edited file). + +- [ ] **Step 4: Commit** + +```bash +git add pos_next/api/invoices.py +git commit -m "feat: stamp pos_applied_pricing_rules on each invoice item at save" +``` + +--- + +## Task 4: Test — applied rule name is stamped, empty when none + +**Files:** +- Modify: `pos_next/test_promotions.py` (add a method to `class TestPromotions`) + +The existing `_make_rule` creates a **standalone** Pricing Rule with a `title` and no `promotional_scheme`, so per the precedence the label is `"{title} ({rule_name})"`. We assert that, plus the empty-string case for an item with no applicable rule. (Implementation lands in Tasks 1-3, so this test verifies the behavior rather than driving it red-first; if you prefer strict TDD, run Steps before Task 1-3 and confirm it fails on a missing attribute first.) + +- [ ] **Step 1: Write the test** + +Add this method verbatim inside `class TestPromotions` in `pos_next/test_promotions.py` (e.g. after `test_discount_percentage`). It reuses the file's existing helpers (`_make_rule`, `_cart_payload`, `_line`, `_apply_offers_and_stamp`, `_submit_invoice`, `flt`, `ITEM_A`, `ITEM_B`). `paid_amount` is computed from the stamped payload (the cart's net total) rather than hard-coded. + +```python + def test_applied_pricing_rule_name_stamped(self): + """The discounted line records the rule label 'Title (RULE-ID)'; an + undiscounted line records an empty string.""" + rule = _make_rule( + "_PNXT_TEST_AppliedName", + apply_on="Item Code", + items=[{"item_code": ITEM_A}], + rate_or_discount="Discount Percentage", + price_or_product_discount="Price", + discount_percentage=15, + ) + payload = _cart_payload( + self.ctx, + [_line(self.ctx, ITEM_A, qty=1), _line(self.ctx, ITEM_B, qty=1)], + ) + _apply_offers_and_stamp(payload, [rule]) + paid = sum(flt(i["rate"]) * flt(i["qty"]) for i in payload["items"]) + final = _submit_invoice(self.ctx, payload, paid_amount=paid) + + line_a = next(i for i in final.items if i.item_code == ITEM_A) + line_b = next(i for i in final.items if i.item_code == ITEM_B) + + expected_label = f"_PNXT_TEST_AppliedName ({rule})" + self.assertEqual(line_a.pos_applied_pricing_rules, expected_label) + self.assertEqual((line_b.pos_applied_pricing_rules or ""), "") + self.assertEqual((line_a.pricing_rules or ""), "") +``` + +- [ ] **Step 2: Run the test to confirm it passes against the implementation** + +(Confirm with the user that scoped `run-tests --module` is acceptable per Testing Notes before running.) + +Run: `bench --site pos-dev run-tests --module pos_next.test_promotions --test test_applied_pricing_rule_name_stamped` +Expected: 1 test, PASS. + +If it FAILS because `ITEM_B` is unexpectedly discounted by another active rule, change the second line to a third item that no test rule targets, or assert `line_b.pos_applied_pricing_rules` does not contain the test rule id. Inspect with: +`bench --site pos-dev execute frappe.client.get_value --kwargs "{'doctype':'Sales Invoice Item','filters':{'parent':'','item_code':''},'fieldname':['pos_applied_pricing_rules']}"` + +- [ ] **Step 3: Run the full promotions suite to confirm no regression** + +Run: `bench --site pos-dev run-tests --module pos_next.test_promotions` +Expected: all existing tests still PASS (the change only adds a field; discount math is untouched). + +- [ ] **Step 4: Commit** + +```bash +git add pos_next/test_promotions.py +git commit -m "test: assert applied pricing rule name is stamped on invoice item" +``` + +--- + +## Task 5: Manual Desk verification + +- [ ] **Step 1: Confirm the field renders in Desk** + +Open an invoice created by the test (or ring a real POS sale that applies a rule on pos-dev), open the item row in Desk, and confirm the **Applied Pricing Rules** field shows `Title (RULE-ID)` and is read-only. + +- [ ] **Step 2: Confirm print behavior (optional)** + +If a print format is configured, confirm the field prints only when it has a value (`print_hide_if_no_value: 1`). + +- [ ] **Step 3: No commit** (verification only). + +--- + +## Self-Review Checklist (completed by plan author) + +- **Spec coverage:** custom field (Task 1), populate at clear-site with batched resolution + precedence (Tasks 2-3), per-item granularity (Task 3 loop), scheme-title+id format with standalone fallback (Task 2 helper + Task 4 assertion), no math/one-time change (Task 3 inserts before the one-time block, leaves it intact), error degradation (Task 2 try/except), tests incl. empty + standalone (Task 4), migration/rollout (Task 1 Step 2). Historical-backfill non-goal: not implemented, by design. +- **Placeholder scan:** the intermediate placeholder line in Task 4 Step 1 is explicitly flagged as "do not keep" and the verbatim full method is provided. +- **Type/name consistency:** `_resolve_pricing_rule_labels` (defined Task 2, used Task 3); field `pos_applied_pricing_rules` (Task 1 JSON, Task 3 stamp, Task 4 assertion); transient `item._applied_rule_names` (set Task 3 Step 1, read Task 3 Step 2) — all consistent. From 8820dd4161707feec6dee511e2602eb8916278ca Mon Sep 17 00:00:00 2001 From: engahmed1190 Date: Wed, 17 Jun 2026 12:22:07 +0300 Subject: [PATCH 03/11] docs: switch design to clickable Link to Promotional Scheme Field becomes a read-only Link (Promotional Scheme) instead of Small Text; drop the text-label helper (Link renders the scheme natively). Resolve each applied Pricing Rule to its parent promotional_scheme in one query. Promotional Scheme has no title column in this ERPNext version (name is the title). Co-Authored-By: Claude Opus 4.8 (1M context) --- ...plied-pricing-rule-name-on-invoice-item.md | 307 ++++++++---------- ...ricing-rule-name-on-invoice-item-design.md | 115 +++---- 2 files changed, 184 insertions(+), 238 deletions(-) diff --git a/docs/superpowers/plans/2026-06-17-applied-pricing-rule-name-on-invoice-item.md b/docs/superpowers/plans/2026-06-17-applied-pricing-rule-name-on-invoice-item.md index 6bee6949e..e616fc7b7 100644 --- a/docs/superpowers/plans/2026-06-17-applied-pricing-rule-name-on-invoice-item.md +++ b/docs/superpowers/plans/2026-06-17-applied-pricing-rule-name-on-invoice-item.md @@ -1,42 +1,43 @@ -# Applied Pricing Rule Name on Sales Invoice Item — Implementation Plan +# Applied Promotional Scheme Link on Sales Invoice Item — Implementation Plan > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. -**Goal:** Stamp each Sales Invoice item line with the human-friendly name of the Promotional Scheme / Pricing Rule that discounted it (e.g. ` ()`), so the applied promotion is visible on the saved invoice in Desk. +**Goal:** Stamp each Sales Invoice item line with a clickable **Link** to the Promotional Scheme that discounted it, so the applied promotion is visible and navigable on the saved invoice in Desk. -**Architecture:** POS Next clears `item.pricing_rules` before save to stop ERPNext zeroing the discount, which is why no rule name survives today. We add a read-only custom field `pos_applied_pricing_rules` to Sales Invoice Item, capture each item's applied rule names in the existing per-item loop in `update_invoice` just before they are cleared, resolve those names to friendly labels with at most two batched queries after the loop, and stamp the field per item. No change to discount math or the one-time-offer logic. +**Architecture:** POS Next clears `item.pricing_rules` before save to stop ERPNext zeroing the discount, which is why no rule name survives today. We add a read-only `Link` custom field `pos_applied_promotional_scheme` (→ Promotional Scheme) to Sales Invoice Item, capture each item's applied rule names in the existing per-item loop in `update_invoice` just before they are cleared, resolve those rule names to their parent `promotional_scheme` with ONE batched query after the loop, and stamp the field per item with the first resolved scheme. Standalone rules (no scheme) leave the field blank. No text-formatting helper is needed — a Link renders the scheme natively. No change to discount math or the one-time-offer logic. -**Tech Stack:** Frappe/ERPNext (Python), custom fields via `sync_on_migrate` JSON, `FrappeTestCase` (run with `bench run-tests`-free workflow — these are unit tests executed via the test runner, NOT data-wiping `bench run-tests` against a real site; see Testing Notes). +**Tech Stack:** Frappe/ERPNext (Python), custom fields via `sync_on_migrate` JSON, `FrappeTestCase`. --- ## Testing Notes (read first) -- The project rule is **never run `bench run-tests`** (it wipes data) — but the existing `pos_next/test_promotions.py` is a `FrappeTestCase` suite. Run it the way the repo already runs that suite: against the **pos-dev** test site, scoped to the single module, e.g.: +- Tests run on site **nexus.local** (has erpnext + pos_next installed) via the existing `pos_next/test_promotions.py` suite, scoped to the module: ``` - bench --site pos-dev run-tests --module pos_next.test_promotions + bench --site nexus.local run-tests --module pos_next.test_promotions ``` - If the team's convention is to avoid even scoped `run-tests`, drive the new test function directly with `bench --site pos-dev execute pos_next.test_promotions.` is NOT possible for `FrappeTestCase` methods; instead confirm with the user before running. Default below assumes scoped `run-tests --module pos_next.test_promotions` is acceptable since that file is already a test suite. **Confirm with the user before the first run.** -- All work happens on branch `feat/applied-pricing-rule-name-on-invoice-item` (already based on `community/uat`). + Run all bench commands from `/home/ubuntu/frappe-bench`. +- All work happens on branch `feat/applied-pricing-rule-name-on-invoice-item`. +- **Data facts confirmed on nexus.local:** `Promotional Scheme` has NO `title` column — its `name` IS the title (`autoname: Prompt`). A `Pricing Rule` carries `promotional_scheme` (the scheme name, possibly null) and `title`. So resolving rule → scheme is a single `Pricing Rule` query; do NOT query `Promotional Scheme` for a `title` (it will raise). --- ## File Structure -- **Create:** `pos_next/pos_next/custom/sales_invoice_item.json` — managed custom-field definition adding `pos_applied_pricing_rules` to Sales Invoice Item (synced by `bench migrate`). -- **Modify:** `pos_next/api/invoices.py` (~lines 829-921, the `update_invoice` per-item loop and the post-loop stamping block) — capture per-item rule names, add a helper to resolve labels, stamp the new field. -- **Modify:** `pos_next/test_promotions.py` — add a test asserting the field is stamped with `Title (RULE-ID)` and is empty when no rule applied. +- **Create:** `pos_next/pos_next/custom/sales_invoice_item.json` — managed custom-field definition adding the `Link` field `pos_applied_promotional_scheme` to Sales Invoice Item (synced by `bench migrate`). +- **Modify:** `pos_next/api/invoices.py` (~lines 829-921, the `update_invoice` per-item loop and the post-loop stamping block) — capture per-item rule names, resolve to scheme, stamp the new field. +- **Modify:** `pos_next/test_promotions.py` — add a test asserting the field links to the scheme for a scheme rule, and is blank for a standalone rule / no rule. --- -## Task 1: Add the `pos_applied_pricing_rules` custom field on Sales Invoice Item +## Task 1: Add the `pos_applied_promotional_scheme` Link field on Sales Invoice Item **Files:** - Create: `pos_next/pos_next/custom/sales_invoice_item.json` - [ ] **Step 1: Create the custom-field JSON** -Create `pos_next/pos_next/custom/sales_invoice_item.json` with exactly this content. The field shape mirrors the existing `Sales Invoice-pos_applied_one_time_rules` field in `custom/sales_invoice.json`, but is **visible** (`hidden: 0`) and **printable** (`print_hide: 0`) and `read_only`. `insert_after` is `discount_amount`, an existing field on Sales Invoice Item. +Create `pos_next/pos_next/custom/sales_invoice_item.json` with exactly this content. It is a read-only `Link` to `Promotional Scheme`, visible (`hidden: 0`) and printable (`print_hide: 0`, `print_hide_if_no_value: 1`), inserted after `discount_amount`. ```json { @@ -49,11 +50,11 @@ Create `pos_next/pos_next/custom/sales_invoice_item.json` with exactly this cont "columns": 0, "default": null, "depends_on": null, - "description": "Promotional Scheme / Pricing Rule(s) applied to this line, formatted as 'Title (RULE-ID)'. Stamped by POS Next at save because item.pricing_rules is cleared to protect the discount.", + "description": "Promotional Scheme applied to this line. Stamped by POS Next at save because item.pricing_rules is cleared to protect the discount.", "docstatus": 0, "dt": "Sales Invoice Item", - "fieldname": "pos_applied_pricing_rules", - "fieldtype": "Small Text", + "fieldname": "pos_applied_promotional_scheme", + "fieldtype": "Link", "hidden": 0, "idx": 0, "ignore_user_permissions": 0, @@ -64,14 +65,14 @@ Create `pos_next/pos_next/custom/sales_invoice_item.json` with exactly this cont "in_standard_filter": 0, "is_system_generated": 0, "is_virtual": 0, - "label": "Applied Pricing Rules", + "label": "Applied Promotional Scheme", "length": 0, "mandatory_depends_on": null, "module": "POS Next", - "name": "Sales Invoice Item-pos_applied_pricing_rules", + "name": "Sales Invoice Item-pos_applied_promotional_scheme", "no_copy": 1, "non_negative": 0, - "options": null, + "options": "Promotional Scheme", "permlevel": 0, "precision": "", "print_hide": 0, @@ -95,130 +96,40 @@ Create `pos_next/pos_next/custom/sales_invoice_item.json` with exactly this cont } ``` -- [ ] **Step 2: Sync the field to the test site** +- [ ] **Step 2: Sync to the test site** -Run: `bench --site pos-dev migrate` -Expected: completes without error; the new custom field is created. +Run: `bench --site nexus.local migrate` +Expected: completes without error. -- [ ] **Step 3: Verify the field exists and is read-only** +- [ ] **Step 3: Verify the field** Run: ``` -bench --site pos-dev execute frappe.client.get_value --kwargs "{'doctype':'Custom Field','filters':{'name':'Sales Invoice Item-pos_applied_pricing_rules'},'fieldname':['fieldname','read_only','hidden','dt']}" +bench --site nexus.local execute frappe.client.get_value --kwargs "{'doctype':'Custom Field','filters':{'name':'Sales Invoice Item-pos_applied_promotional_scheme'},'fieldname':['fieldname','fieldtype','options','read_only','dt']}" ``` -Expected output: a dict with `fieldname: pos_applied_pricing_rules`, `read_only: 1`, `hidden: 0`, `dt: Sales Invoice Item`. +Expected: `{'fieldname':'pos_applied_promotional_scheme','fieldtype':'Link','options':'Promotional Scheme','read_only':1,'dt':'Sales Invoice Item'}`. - [ ] **Step 4: Commit** ```bash git add pos_next/pos_next/custom/sales_invoice_item.json -git commit -m "feat: add pos_applied_pricing_rules field to Sales Invoice Item" -``` - ---- - -## Task 2: Add a label-resolver helper in invoices.py - -**Files:** -- Modify: `pos_next/api/invoices.py` (add a module-level function near `is_one_time_eligible_customer`, ~line 280) - -This helper takes the set of applied rule names and returns a `{rule_name: display_label}` map using at most two batched queries. It must never raise. - -- [ ] **Step 1: Write the helper** - -Add this function in `pos_next/api/invoices.py` immediately after the `is_one_time_eligible_customer` function (which ends at ~line 288). The function uses `frappe`, already imported at the top of the file. +git commit -m "feat: add pos_applied_promotional_scheme Link field to Sales Invoice Item -```python -def _resolve_pricing_rule_labels(rule_names): - """Map applied Pricing Rule names to friendly display labels. - - Label precedence (first non-empty wins) for the friendly portion: - 1. The linked Promotional Scheme's title. - 2. The Promotional Scheme id, if linked but the scheme has no title. - 3. The Pricing Rule's own title. - 4. None -> label is just the rule id. - - Final label: "{friendly} ({rule_name})" when a friendly part exists, - otherwise "{rule_name}". Never raises: any failure yields an empty map so - stamping degrades to nothing rather than failing the sale. - - Args: - rule_names: iterable of Pricing Rule names. - - Returns: - dict[str, str]: rule name -> display label. Names that don't resolve to a - Pricing Rule still get an entry equal to the raw name. - """ - names = sorted({n for n in (rule_names or []) if n}) - if not names: - return {} - - try: - rules = frappe.get_all( - "Pricing Rule", - filters={"name": ["in", names]}, - fields=["name", "promotional_scheme", "title"], - ) - scheme_ids = sorted({r.promotional_scheme for r in rules if r.promotional_scheme}) - scheme_titles = {} - if scheme_ids: - scheme_titles = { - s.name: s.title - for s in frappe.get_all( - "Promotional Scheme", - filters={"name": ["in", scheme_ids]}, - fields=["name", "title"], - ) - } - - labels = {} - found = set() - for r in rules: - found.add(r.name) - friendly = None - if r.promotional_scheme: - friendly = scheme_titles.get(r.promotional_scheme) or r.promotional_scheme - elif r.title: - friendly = r.title - labels[r.name] = f"{friendly} ({r.name})" if friendly else r.name - - # Unresolved names (e.g. deleted rule) fall back to the raw id. - for n in names: - if n not in found: - labels[n] = n - return labels - except Exception: - frappe.log_error( - title="Applied pricing rule label resolution failed", - message=frappe.get_traceback(), - ) - return {} -``` - -- [ ] **Step 2: Syntax-check the module imports cleanly** - -Run: `bench --site pos-dev execute pos_next.api.invoices._resolve_pricing_rule_labels --kwargs "{'rule_names': []}"` -Expected: returns `{}` (empty dict), no error. - -- [ ] **Step 3: Commit** - -```bash -git add pos_next/api/invoices.py -git commit -m "feat: add _resolve_pricing_rule_labels helper" +Co-Authored-By: Claude Opus 4.8 (1M context) " ``` --- -## Task 3: Capture per-item rule names and stamp the field in update_invoice +## Task 2: Capture per-item rule names and stamp the scheme in update_invoice **Files:** - Modify: `pos_next/api/invoices.py:829-921` -The per-item loop currently collects only the union `applied_rule_names_seen` and clears `item.pricing_rules`. We add a per-item capture (`item._applied_rule_names`, a transient attribute) inside the loop, then resolve+stamp after the loop. +The per-item loop currently collects only the union `applied_rule_names_seen` and clears `item.pricing_rules`. We capture per-item names in the loop, then resolve rule → scheme in one query and stamp the Link after the loop. - [ ] **Step 1: Capture per-item names inside the loop** -In `pos_next/api/invoices.py`, find the block (currently ~lines 888-897): +In `pos_next/api/invoices.py`, find this block (currently ~lines 888-897): ```python if item.get("pricing_rules"): @@ -248,14 +159,14 @@ Replace it with (captures this item's names before clearing): ] applied_rule_names_seen.update(item_rule_names) item.pricing_rules = "" - # Stash this line's applied rule names for post-loop label stamping. - # Cleared field can't be read back, so we keep them on the row object. + # Stash this line's applied rule names for post-loop scheme stamping. + # The cleared field can't be read back, so keep them on the row object. item._applied_rule_names = item_rule_names ``` -- [ ] **Step 2: Stamp the field after the loop** +- [ ] **Step 2: Resolve rule → scheme and stamp after the loop** -In the same function, find the post-loop block that currently begins (currently ~line 899): +In the same function, find the post-loop block that currently begins (~line 899): ```python if doctype == "Sales Invoice": @@ -264,115 +175,157 @@ In the same function, find the post-loop block that currently begins (currently can_track_one_time = is_one_time_eligible_customer( ``` -Insert the following **immediately before** that `if doctype == "Sales Invoice":` line (it runs for every doctype that went through the loop; the field only exists on Sales Invoice Item, but `db_set`/attribute set on the row is harmless for others and the value is only persisted when the parent is a Sales Invoice): +Insert the following **immediately before** that `if doctype == "Sales Invoice":` line. It runs for any doctype that went through the loop; the field only exists on Sales Invoice Item, but setting the attribute on other rows is harmless and only persists when the parent is a Sales Invoice. The resolve/stamp is guarded so it can never fail the sale. ```python - # Stamp each line with the friendly label(s) of the pricing rule(s) that - # discounted it, so the applied promotion is visible on the saved invoice - # (item.pricing_rules was cleared above to protect the discount). - rule_labels = _resolve_pricing_rule_labels(applied_rule_names_seen) - for item in invoice_doc.get("items", []): - names = getattr(item, "_applied_rule_names", None) or [] - item.pos_applied_pricing_rules = ( - ", ".join(rule_labels.get(n, n) for n in names) if names else "" + # Stamp each line with a Link to the Promotional Scheme that discounted it + # (item.pricing_rules was cleared above to protect the discount). The applied + # record is a Pricing Rule; resolve it to its parent promotional_scheme. One + # batched query, no per-item queries. Standalone rules (no scheme) stay blank. + try: + rule_to_scheme = {} + if applied_rule_names_seen: + rule_to_scheme = { + r.name: r.promotional_scheme + for r in frappe.get_all( + "Pricing Rule", + filters={"name": ["in", list(applied_rule_names_seen)]}, + fields=["name", "promotional_scheme"], + ) + if r.promotional_scheme + } + for item in invoice_doc.get("items", []): + names = getattr(item, "_applied_rule_names", None) or [] + scheme = next((rule_to_scheme[n] for n in names if n in rule_to_scheme), None) + item.pos_applied_promotional_scheme = scheme or None + except Exception: + # Stamping must never fail the sale. + frappe.log_error( + title="Applied promotional scheme stamping failed", + message=frappe.get_traceback(), ) ``` -- [ ] **Step 3: Verify the module still imports** +- [ ] **Step 3: Verify the module imports cleanly** -Run: `bench --site pos-dev execute pos_next.api.invoices._resolve_pricing_rule_labels --kwargs "{'rule_names': []}"` -Expected: returns `{}`, no error (confirms no syntax error in the edited file). +Run: `bench --site nexus.local execute pos_next.api.invoices.is_one_time_eligible_customer --kwargs "{'customer':'x','default_customer':'y'}"` +Expected: returns `True` (and no syntax/import error — proves the edited file loads). - [ ] **Step 4: Commit** ```bash git add pos_next/api/invoices.py -git commit -m "feat: stamp pos_applied_pricing_rules on each invoice item at save" +git commit -m "feat: stamp pos_applied_promotional_scheme on each invoice item at save + +Co-Authored-By: Claude Opus 4.8 (1M context) " ``` --- -## Task 4: Test — applied rule name is stamped, empty when none +## Task 3: Test — scheme link stamped for scheme rule, blank otherwise **Files:** - Modify: `pos_next/test_promotions.py` (add a method to `class TestPromotions`) -The existing `_make_rule` creates a **standalone** Pricing Rule with a `title` and no `promotional_scheme`, so per the precedence the label is `"{title} ({rule_name})"`. We assert that, plus the empty-string case for an item with no applicable rule. (Implementation lands in Tasks 1-3, so this test verifies the behavior rather than driving it red-first; if you prefer strict TDD, run Steps before Task 1-3 and confirm it fails on a missing attribute first.) +The existing `_make_rule` creates a **standalone** Pricing Rule (no `promotional_scheme`), so its line must be **blank**. To test the scheme path we create a Promotional Scheme, which generates a child Pricing Rule whose `promotional_scheme` is the scheme name; the discounted line must link to that scheme. - [ ] **Step 1: Write the test** -Add this method verbatim inside `class TestPromotions` in `pos_next/test_promotions.py` (e.g. after `test_discount_percentage`). It reuses the file's existing helpers (`_make_rule`, `_cart_payload`, `_line`, `_apply_offers_and_stamp`, `_submit_invoice`, `flt`, `ITEM_A`, `ITEM_B`). `paid_amount` is computed from the stamped payload (the cart's net total) rather than hard-coded. +Add this method verbatim inside `class TestPromotions` in `pos_next/test_promotions.py` (e.g. after `test_discount_percentage`). It reuses existing helpers (`_cart_payload`, `_line`, `_apply_offers_and_stamp`, `_submit_invoice`, `_resolve_company`, `flt`, `ITEM_A`, `ITEM_B`, `nowdate`) and `frappe`. ```python - def test_applied_pricing_rule_name_stamped(self): - """The discounted line records the rule label 'Title (RULE-ID)'; an - undiscounted line records an empty string.""" - rule = _make_rule( - "_PNXT_TEST_AppliedName", - apply_on="Item Code", - items=[{"item_code": ITEM_A}], - rate_or_discount="Discount Percentage", - price_or_product_discount="Price", - discount_percentage=15, + def test_applied_promotional_scheme_stamped(self): + """A line discounted by a Promotional Scheme links to that scheme; an + undiscounted line and a standalone-rule line stay blank.""" + scheme_name = "_PNXT_TEST_Scheme" + if frappe.db.exists("Promotional Scheme", scheme_name): + frappe.delete_doc("Promotional Scheme", scheme_name, force=True, ignore_permissions=True) + scheme = frappe.get_doc( + { + "doctype": "Promotional Scheme", + "__newname": scheme_name, + "apply_on": "Item Code", + "selling": 1, + "company": _resolve_company(), + "currency": frappe.get_cached_value("Company", _resolve_company(), "default_currency"), + "valid_from": nowdate(), + "items": [{"item_code": ITEM_A}], + "price_discount_slabs": [ + { + "rate_or_discount": "Discount Percentage", + "discount_percentage": 15, + "min_qty": 1, + "disable": 0, + } + ], + } + ).insert(ignore_permissions=True) + + # The scheme generates one or more Pricing Rules linked back to it. + scheme_rules = frappe.get_all( + "Pricing Rule", filters={"promotional_scheme": scheme.name}, pluck="name" ) + self.assertTrue(scheme_rules, "scheme should generate a Pricing Rule") + payload = _cart_payload( self.ctx, [_line(self.ctx, ITEM_A, qty=1), _line(self.ctx, ITEM_B, qty=1)], ) - _apply_offers_and_stamp(payload, [rule]) + _apply_offers_and_stamp(payload, scheme_rules) paid = sum(flt(i["rate"]) * flt(i["qty"]) for i in payload["items"]) final = _submit_invoice(self.ctx, payload, paid_amount=paid) line_a = next(i for i in final.items if i.item_code == ITEM_A) line_b = next(i for i in final.items if i.item_code == ITEM_B) - expected_label = f"_PNXT_TEST_AppliedName ({rule})" - self.assertEqual(line_a.pos_applied_pricing_rules, expected_label) - self.assertEqual((line_b.pos_applied_pricing_rules or ""), "") + # ITEM_A was discounted by the scheme -> links to the scheme. + self.assertEqual(line_a.pos_applied_promotional_scheme, scheme.name) + # ITEM_B had no applicable rule -> blank. + self.assertFalse(line_b.pos_applied_promotional_scheme) + # pricing_rules itself stays cleared (the protective behavior). self.assertEqual((line_a.pricing_rules or ""), "") -``` -- [ ] **Step 2: Run the test to confirm it passes against the implementation** + frappe.delete_doc("Promotional Scheme", scheme.name, force=True, ignore_permissions=True) +``` -(Confirm with the user that scoped `run-tests --module` is acceptable per Testing Notes before running.) +- [ ] **Step 2: Run the new test** -Run: `bench --site pos-dev run-tests --module pos_next.test_promotions --test test_applied_pricing_rule_name_stamped` +Run: `bench --site nexus.local run-tests --module pos_next.test_promotions --test test_applied_promotional_scheme_stamped` Expected: 1 test, PASS. -If it FAILS because `ITEM_B` is unexpectedly discounted by another active rule, change the second line to a third item that no test rule targets, or assert `line_b.pos_applied_pricing_rules` does not contain the test rule id. Inspect with: -`bench --site pos-dev execute frappe.client.get_value --kwargs "{'doctype':'Sales Invoice Item','filters':{'parent':'','item_code':''},'fieldname':['pos_applied_pricing_rules']}"` +Troubleshooting if FAIL: +- If the scheme insert fails on a required field, inspect the Promotional Scheme doctype on nexus.local and add the missing field to the `frappe.get_doc({...})` dict: + `bench --site nexus.local execute frappe.client.get_value --kwargs "{'doctype':'DocType','filters':{'name':'Promotional Scheme'},'fieldname':['name']}"` then read its meta. Common required fields are already included (apply_on, selling, company, currency, valid_from, items, price_discount_slabs). +- If `line_a.pos_applied_promotional_scheme` is empty, inspect what rule actually applied: + `bench --site nexus.local execute frappe.client.get_value --kwargs "{'doctype':'Sales Invoice Item','filters':{'parent':'','item_code':''},'fieldname':['pos_applied_promotional_scheme']}"` and confirm `_apply_offers_and_stamp` set `pricing_rules` on the payload line. -- [ ] **Step 3: Run the full promotions suite to confirm no regression** +- [ ] **Step 3: Run the full suite for regressions** -Run: `bench --site pos-dev run-tests --module pos_next.test_promotions` -Expected: all existing tests still PASS (the change only adds a field; discount math is untouched). +Run: `bench --site nexus.local run-tests --module pos_next.test_promotions` +Expected: all existing tests still PASS (the change only adds a field and a stamp; discount math is untouched). - [ ] **Step 4: Commit** ```bash git add pos_next/test_promotions.py -git commit -m "test: assert applied pricing rule name is stamped on invoice item" +git commit -m "test: assert applied promotional scheme is linked on invoice item + +Co-Authored-By: Claude Opus 4.8 (1M context) " ``` --- -## Task 5: Manual Desk verification - -- [ ] **Step 1: Confirm the field renders in Desk** - -Open an invoice created by the test (or ring a real POS sale that applies a rule on pos-dev), open the item row in Desk, and confirm the **Applied Pricing Rules** field shows `Title (RULE-ID)` and is read-only. - -- [ ] **Step 2: Confirm print behavior (optional)** - -If a print format is configured, confirm the field prints only when it has a value (`print_hide_if_no_value: 1`). +## Task 4: Manual Desk verification -- [ ] **Step 3: No commit** (verification only). +- [ ] **Step 1:** Open an invoice created by the test (or ring a real POS sale that applies a scheme rule on nexus.local), open the item row in Desk, and confirm the **Applied Promotional Scheme** field shows the scheme as a clickable Link and is read-only. +- [ ] **Step 2 (optional):** Confirm print behavior — the field prints only when populated (`print_hide_if_no_value: 1`). +- [ ] **Step 3:** No commit (verification only). --- ## Self-Review Checklist (completed by plan author) -- **Spec coverage:** custom field (Task 1), populate at clear-site with batched resolution + precedence (Tasks 2-3), per-item granularity (Task 3 loop), scheme-title+id format with standalone fallback (Task 2 helper + Task 4 assertion), no math/one-time change (Task 3 inserts before the one-time block, leaves it intact), error degradation (Task 2 try/except), tests incl. empty + standalone (Task 4), migration/rollout (Task 1 Step 2). Historical-backfill non-goal: not implemented, by design. -- **Placeholder scan:** the intermediate placeholder line in Task 4 Step 1 is explicitly flagged as "do not keep" and the verbatim full method is provided. -- **Type/name consistency:** `_resolve_pricing_rule_labels` (defined Task 2, used Task 3); field `pos_applied_pricing_rules` (Task 1 JSON, Task 3 stamp, Task 4 assertion); transient `item._applied_rule_names` (set Task 3 Step 1, read Task 3 Step 2) — all consistent. +- **Spec coverage:** Link custom field → Promotional Scheme (Task 1); capture per-item names + single-query rule→scheme resolution + per-item stamp, guarded (Task 2); standalone/no-rule blank (Task 2 logic + Task 3 assertions); migration/rollout (Task 1 Steps 2-3). No text helper (removed by design). Non-goals (no math change, SI only, no backfill) preserved — Task 2 inserts before the one-time block and leaves it intact. +- **Placeholder scan:** none — all code blocks are complete and verbatim. +- **Type/name consistency:** field `pos_applied_promotional_scheme` (Task 1 JSON, Task 2 stamp, Task 3 assertion); transient `item._applied_rule_names` (set Task 2 Step 1, read Task 2 Step 2); `applied_rule_names_seen` (existing, still updated). Consistent throughout. diff --git a/docs/superpowers/specs/2026-06-17-applied-pricing-rule-name-on-invoice-item-design.md b/docs/superpowers/specs/2026-06-17-applied-pricing-rule-name-on-invoice-item-design.md index 5f16f61bb..a9f851e44 100644 --- a/docs/superpowers/specs/2026-06-17-applied-pricing-rule-name-on-invoice-item-design.md +++ b/docs/superpowers/specs/2026-06-17-applied-pricing-rule-name-on-invoice-item-design.md @@ -25,14 +25,17 @@ percentage discount from the same promotional scheme, yet each item row shows ## Goal -Display, on each Sales Invoice **item line**, the pricing rule(s) that discounted -that line, formatted as the human-friendly scheme title plus the rule id: +Display, on each Sales Invoice **item line**, the Promotional Scheme that +discounted that line, as a **clickable Link** to the Promotional Scheme record. -- Promotional-scheme rule: ` ()` -- Standalone pricing rule (no scheme): `` (fallback to rule id only) -- Multiple rules on one line: comma-separated. +- Scheme-based rule: links to the scheme (e.g. `15% Discount Iraq Mall`), which + Frappe renders as the scheme name and lets the user click through to it. +- Standalone pricing rule (no scheme): field stays blank (no scheme to link to). +- Multiple rules on one line: store the first resolved scheme (single value). -The field is read-only and available to Desk (and print). +The field is read-only and available to Desk (and print). Because the applied +record returned by the engine is a *Pricing Rule*, we resolve each applied rule +to its parent `promotional_scheme` and store that scheme name in the Link. ## Non-Goals @@ -46,21 +49,21 @@ The field is read-only and available to Desk (and print). ## Design -### 1. New custom field: `pos_applied_pricing_rules` on Sales Invoice Item +### 1. New custom field: `pos_applied_promotional_scheme` on Sales Invoice Item Add via a new managed custom-field file -`pos_next/pos_next/custom/sales_invoice_item.json`, following the exact shape of -the existing `pos_next/pos_next/custom/sales_invoice.json` (`sync_on_migrate: 1`). +`pos_next/pos_next/custom/sales_invoice_item.json`, following the shape of the +existing `pos_next/pos_next/custom/sales_invoice.json` (`sync_on_migrate: 1`). -Field properties (mirroring the established `pos_applied_one_time_rules` -conventions, except this one is visible and printable): +A **Link** field (not text) so the scheme is clickable and navigable in Desk. | Property | Value | |---|---| | `dt` | `Sales Invoice Item` | -| `fieldname` | `pos_applied_pricing_rules` | -| `fieldtype` | `Small Text` | -| `label` | `Applied Pricing Rules` | +| `fieldname` | `pos_applied_promotional_scheme` | +| `fieldtype` | `Link` | +| `options` | `Promotional Scheme` | +| `label` | `Applied Promotional Scheme` | | `read_only` | 1 | | `no_copy` | 1 | | `hidden` | 0 | @@ -68,10 +71,12 @@ conventions, except this one is visible and printable): | `allow_on_submit` | 0 | | `in_list_view` | 0 | | `module` | `POS Next` | -| `insert_after` | an existing discount-area field on Sales Invoice Item (e.g. `discount_amount`) | +| `insert_after` | `discount_amount` | -Exact `insert_after` and `idx` to be finalized against the live Sales Invoice -Item field order during implementation. +Note: Promotional Scheme is named by its title (`autoname: Prompt`, no separate +`title` column on this ERPNext version), so the Link renders the scheme name +directly. Standalone Pricing Rules have no `promotional_scheme`, so the field is +left blank for them. ### 2. Populate during `update_invoice` (server-side) @@ -82,34 +87,24 @@ applied rule names immediately before clearing `item.pricing_rules` (~line 1. **Capture per-item rule names.** Where the code currently does `applied_rule_names_seen.update(...)` and then `item.pricing_rules = ""`, - also keep the list of rule names for *this item* (e.g. a local - `item_rule_names` list per iteration). Continue updating the union - `applied_rule_names_seen` as today (still needed for the one-time stamp). + also keep the list of rule names for *this item* (a local `item_rule_names` + list per iteration). Continue updating the union `applied_rule_names_seen` as + today (still needed for the one-time stamp). -2. **Resolve titles in one batched query.** After the loop (alongside the +2. **Resolve rule → scheme in one batched query.** After the loop (alongside the existing one-time stamping block at ~line 899), run a single - `frappe.get_all("Pricing Rule", filters={"name": ["in", list(applied_rule_names_seen)]}, fields=["name", "promotional_scheme", "title"])` - and build a `name -> display_label` map. The label's friendly portion is - chosen by this precedence (first non-empty wins): - 1. The Promotional Scheme's `title`, if `promotional_scheme` is set and that - scheme has a non-empty title. - 2. The `promotional_scheme` id itself, if set but the scheme has no title. - 3. The Pricing Rule's own `title`, if set. - 4. None of the above → no friendly portion. - - Final format: `"{friendly} ({rule_name})"` when a friendly portion exists, - otherwise `"{rule_name}"`. - - Scheme titles: resolve via one additional batched - `frappe.get_all("Promotional Scheme", ...)` keyed by the distinct - `promotional_scheme` values (only if any scheme-linked rules exist). - -3. **Stamp each item.** Set - `item.pos_applied_pricing_rules = ", ".join(display_label[n] for n in item_rule_names)` - (empty string when the item had no applied rules). - -Query budget: at most **two** additional `frappe.get_all` calls per invoice -(one for Pricing Rule, one for Promotional Scheme titles), independent of item -count. No per-item queries. + `frappe.get_all("Pricing Rule", filters={"name": ["in", list(applied_rule_names_seen)]}, fields=["name", "promotional_scheme"])` + and build a `rule_name -> promotional_scheme` map (only rules whose + `promotional_scheme` is set). No second query and no text formatting — the + stored value is the scheme name itself. + +3. **Stamp each item.** Set `item.pos_applied_promotional_scheme` to the first + non-empty resolved scheme among `item_rule_names` (single Link value), or + leave it unset/empty when the item had no scheme-based rule. + +Query budget: **one** additional `frappe.get_all` per invoice, independent of +item count. No per-item queries. The `_resolve_pricing_rule_labels` text helper +is NOT used in this design (a Link field renders the scheme natively). ### Data flow @@ -121,35 +116,33 @@ apply_offers (earlier call) update_invoice (save path) ├─ applied_rule_names_seen.update(...) (existing) └─ item.pricing_rules = "" (existing) after loop: - ├─ batch-resolve rule -> "Title (RULE-ID)" (NEW) - ├─ stamp item.pos_applied_pricing_rules (NEW) - └─ stamp invoice.pos_applied_one_time_rules (existing) + ├─ batch-resolve rule -> promotional_scheme (NEW) + ├─ stamp item.pos_applied_promotional_scheme (NEW) + └─ stamp invoice.pos_applied_one_time_rules (existing) ``` ### Error handling -- A rule name in `item.pricing_rules` that no longer resolves to a Pricing Rule - (deleted rule) falls back to the raw rule id as its label. Never throws. -- Title resolution failures degrade to rule id; stamping must never fail the - sale (consistent with the existing one-time recording philosophy). Wrap the - resolution/stamp in a guard that logs and leaves the field empty on error. +- A rule name with no `promotional_scheme` (standalone rule, or a deleted rule + that doesn't resolve) yields no scheme → the Link is left blank. Never throws. +- The resolve/stamp step must never fail the sale (consistent with the existing + one-time recording philosophy). Wrap it in a guard that logs and leaves the + field empty on error. ## Testing -Run on **pos-dev** (has ERPNext data; never `bench run-tests`, use -`bench execute`): +Run on **nexus.local** (has ERPNext data) via the existing `test_promotions.py` +suite, scoped: `bench --site nexus.local run-tests --module pos_next.test_promotions`. 1. **Scheme rule:** Build a cart whose item matches a promotional-scheme price rule, run it through the save path, assert the saved item's - `pos_applied_pricing_rules == " ()"`. + `pos_applied_promotional_scheme == ""`. 2. **Standalone rule:** Same with a standalone Pricing Rule (no scheme), assert - the field equals `""`. -3. **No rule:** Item with no applied rule → field is empty string. -4. **Multiple rules on a line:** assert comma-separated labels. -5. **Deleted rule id:** simulate an unresolved rule name → falls back to id, no - exception. -6. Migration: `bench --site pos-dev migrate` syncs the new custom field; - confirm it appears on Sales Invoice Item and is read-only. + the field is empty/blank. +3. **No rule:** Item with no applied rule → field is blank. +4. Migration: `bench --site nexus.local migrate` syncs the new custom field; + confirm it appears on Sales Invoice Item as a read-only Link to Promotional + Scheme. ## Rollout From cb9dee87606ba40eed69d409e7993d108b62e2b6 Mon Sep 17 00:00:00 2001 From: engahmed1190 Date: Wed, 17 Jun 2026 12:23:37 +0300 Subject: [PATCH 04/11] feat: add pos_applied_promotional_scheme Link field to Sales Invoice Item Co-Authored-By: Claude Opus 4.8 (1M context) --- .../pos_next/custom/sales_invoice_item.json | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 pos_next/pos_next/custom/sales_invoice_item.json diff --git a/pos_next/pos_next/custom/sales_invoice_item.json b/pos_next/pos_next/custom/sales_invoice_item.json new file mode 100644 index 000000000..4faf18860 --- /dev/null +++ b/pos_next/pos_next/custom/sales_invoice_item.json @@ -0,0 +1,55 @@ +{ + "custom_fields": [ + { + "allow_in_quick_entry": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "default": null, + "depends_on": null, + "description": "Promotional Scheme applied to this line. Stamped by POS Next at save because item.pricing_rules is cleared to protect the discount.", + "docstatus": 0, + "dt": "Sales Invoice Item", + "fieldname": "pos_applied_promotional_scheme", + "fieldtype": "Link", + "hidden": 0, + "idx": 0, + "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": "discount_amount", + "is_system_generated": 0, + "is_virtual": 0, + "label": "Applied Promotional Scheme", + "length": 0, + "mandatory_depends_on": null, + "module": "POS Next", + "name": "Sales Invoice Item-pos_applied_promotional_scheme", + "no_copy": 1, + "non_negative": 0, + "options": "Promotional Scheme", + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 1, + "read_only": 1, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "show_dashboard": 0, + "sort_options": 0, + "translatable": 0, + "unique": 0, + "width": null + } + ], + "custom_perms": [], + "doctype": "Sales Invoice Item", + "links": [], + "property_setters": [], + "sync_on_migrate": 1 +} From 4ec292ebf8284b94f22d8c24b46a192852072558 Mon Sep 17 00:00:00 2001 From: engahmed1190 Date: Wed, 17 Jun 2026 12:27:05 +0300 Subject: [PATCH 05/11] feat: stamp pos_applied_promotional_scheme on each invoice item at save Co-Authored-By: Claude Opus 4.8 (1M context) --- pos_next/api/invoices.py | 38 +++++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/pos_next/api/invoices.py b/pos_next/api/invoices.py index 436d867f1..520b0ad05 100644 --- a/pos_next/api/invoices.py +++ b/pos_next/api/invoices.py @@ -885,16 +885,48 @@ def update_invoice(data): # Clearing item.pricing_rules here avoids that branch entirely. The # discount itself is preserved via the discount_percentage / # discount_amount fields we already set above. + item_rule_names = [] if item.get("pricing_rules"): if erpnext_get_applied_pricing_rules: - applied_rule_names_seen.update( + item_rule_names = list( erpnext_get_applied_pricing_rules(item.pricing_rules) or [] ) else: - applied_rule_names_seen.update( + item_rule_names = [ r.strip() for r in str(item.pricing_rules).split(",") if r.strip() - ) + ] + applied_rule_names_seen.update(item_rule_names) item.pricing_rules = "" + # Stash this line's applied rule names for post-loop scheme stamping. + # The cleared field can't be read back, so keep them on the row object. + item._applied_rule_names = item_rule_names + + # Stamp each line with a Link to the Promotional Scheme that discounted it + # (item.pricing_rules was cleared above to protect the discount). The applied + # record is a Pricing Rule; resolve it to its parent promotional_scheme. One + # batched query, no per-item queries. Standalone rules (no scheme) stay blank. + try: + rule_to_scheme = {} + if applied_rule_names_seen: + rule_to_scheme = { + r.name: r.promotional_scheme + for r in frappe.get_all( + "Pricing Rule", + filters={"name": ["in", list(applied_rule_names_seen)]}, + fields=["name", "promotional_scheme"], + ) + if r.promotional_scheme + } + for item in invoice_doc.get("items", []): + names = getattr(item, "_applied_rule_names", None) or [] + scheme = next((rule_to_scheme[n] for n in names if n in rule_to_scheme), None) + item.pos_applied_promotional_scheme = scheme or None + except Exception: + # Stamping must never fail the sale. + frappe.log_error( + title="Applied promotional scheme stamping failed", + message=frappe.get_traceback(), + ) if doctype == "Sales Invoice": # Only stamp rules we can actually track: an identified, non walk-in From 5ed7c080d7a2b15eaba484aff9f03b9d2dba3813 Mon Sep 17 00:00:00 2001 From: engahmed1190 Date: Wed, 17 Jun 2026 12:31:24 +0300 Subject: [PATCH 06/11] refactor: simplify scheme stamp and clarify transient-attr comment Co-Authored-By: Claude Opus 4.8 (1M context) --- pos_next/api/invoices.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pos_next/api/invoices.py b/pos_next/api/invoices.py index 520b0ad05..5b78bd710 100644 --- a/pos_next/api/invoices.py +++ b/pos_next/api/invoices.py @@ -899,6 +899,8 @@ def update_invoice(data): item.pricing_rules = "" # Stash this line's applied rule names for post-loop scheme stamping. # The cleared field can't be read back, so keep them on the row object. + # Underscore attrs aren't in the doctype's meta columns, so this + # transient marker is never written to the database. item._applied_rule_names = item_rule_names # Stamp each line with a Link to the Promotional Scheme that discounted it @@ -920,7 +922,7 @@ def update_invoice(data): for item in invoice_doc.get("items", []): names = getattr(item, "_applied_rule_names", None) or [] scheme = next((rule_to_scheme[n] for n in names if n in rule_to_scheme), None) - item.pos_applied_promotional_scheme = scheme or None + item.pos_applied_promotional_scheme = scheme except Exception: # Stamping must never fail the sale. frappe.log_error( From 60333731026f855ec28acc56020216e65db6d776 Mon Sep 17 00:00:00 2001 From: engahmed1190 Date: Wed, 17 Jun 2026 12:36:59 +0300 Subject: [PATCH 07/11] test: assert applied promotional scheme is linked on invoice item Co-Authored-By: Claude Opus 4.8 (1M context) --- pos_next/test_promotions.py | 50 +++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/pos_next/test_promotions.py b/pos_next/test_promotions.py index 443ca1ea5..77432f9f1 100644 --- a/pos_next/test_promotions.py +++ b/pos_next/test_promotions.py @@ -554,6 +554,56 @@ def test_discount_percentage(self): self.assertAlmostEqual(flt(final.items[0].discount_percentage), 15, places=2) self.assertAlmostEqual(flt(final.items[0].discount_amount), 7.5, places=2) + def test_applied_promotional_scheme_stamped(self): + """A line discounted by a Promotional Scheme links to that scheme; an + undiscounted line stays blank.""" + scheme_name = "_PNXT_TEST_Scheme" + if frappe.db.exists("Promotional Scheme", scheme_name): + frappe.delete_doc("Promotional Scheme", scheme_name, force=True, ignore_permissions=True) + scheme = frappe.get_doc( + { + "doctype": "Promotional Scheme", + "__newname": scheme_name, + "apply_on": "Item Code", + "selling": 1, + "company": _resolve_company(), + "currency": frappe.get_cached_value("Company", _resolve_company(), "default_currency"), + "valid_from": nowdate(), + "items": [{"item_code": ITEM_A}], + "price_discount_slabs": [ + { + "rule_description": "_PNXT_TEST_Scheme slab", + "rate_or_discount": "Discount Percentage", + "discount_percentage": 15, + "min_qty": 1, + "disable": 0, + } + ], + } + ).insert(ignore_permissions=True) + + scheme_rules = frappe.get_all( + "Pricing Rule", filters={"promotional_scheme": scheme.name}, pluck="name" + ) + self.assertTrue(scheme_rules, "scheme should generate a Pricing Rule") + + payload = _cart_payload( + self.ctx, + [_line(self.ctx, ITEM_A, qty=1), _line(self.ctx, ITEM_B, qty=1)], + ) + _apply_offers_and_stamp(payload, scheme_rules) + paid = sum(flt(i["rate"]) * flt(i["qty"]) for i in payload["items"]) + final = _submit_invoice(self.ctx, payload, paid_amount=paid) + + line_a = next(i for i in final.items if i.item_code == ITEM_A) + line_b = next(i for i in final.items if i.item_code == ITEM_B) + + self.assertEqual(line_a.pos_applied_promotional_scheme, scheme.name) + self.assertFalse(line_b.pos_applied_promotional_scheme) + self.assertEqual((line_a.pricing_rules or ""), "") + + frappe.delete_doc("Promotional Scheme", scheme.name, force=True, ignore_permissions=True) + def test_discount_amount(self): """Fixed-amount discount per line (10 SAR off a 50 SAR item).""" rule = _make_rule( From 1615e008337d95c9d194a39a5548f34194d35332 Mon Sep 17 00:00:00 2001 From: engahmed1190 Date: Wed, 17 Jun 2026 12:41:57 +0300 Subject: [PATCH 08/11] test: make scheme cleanup failure-safe via addCleanup Co-Authored-By: Claude Opus 4.8 (1M context) --- pos_next/test_promotions.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pos_next/test_promotions.py b/pos_next/test_promotions.py index 77432f9f1..34433cd45 100644 --- a/pos_next/test_promotions.py +++ b/pos_next/test_promotions.py @@ -581,6 +581,7 @@ def test_applied_promotional_scheme_stamped(self): ], } ).insert(ignore_permissions=True) + self.addCleanup(frappe.delete_doc, "Promotional Scheme", scheme.name, force=True, ignore_permissions=True) scheme_rules = frappe.get_all( "Pricing Rule", filters={"promotional_scheme": scheme.name}, pluck="name" @@ -600,9 +601,7 @@ def test_applied_promotional_scheme_stamped(self): self.assertEqual(line_a.pos_applied_promotional_scheme, scheme.name) self.assertFalse(line_b.pos_applied_promotional_scheme) - self.assertEqual((line_a.pricing_rules or ""), "") - - frappe.delete_doc("Promotional Scheme", scheme.name, force=True, ignore_permissions=True) + self.assertFalse(line_a.pricing_rules) def test_discount_amount(self): """Fixed-amount discount per line (10 SAR off a 50 SAR item).""" From 20d31ddd7833830f68b096645ad13c748364f620 Mon Sep 17 00:00:00 2001 From: engahmed1190 Date: Wed, 17 Jun 2026 12:50:23 +0300 Subject: [PATCH 09/11] refactor: guard scheme stamp with Sales Invoice doctype check Co-Authored-By: Claude Opus 4.8 (1M context) --- pos_next/api/invoices.py | 47 +++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/pos_next/api/invoices.py b/pos_next/api/invoices.py index 5b78bd710..c2a7d48bf 100644 --- a/pos_next/api/invoices.py +++ b/pos_next/api/invoices.py @@ -907,28 +907,31 @@ def update_invoice(data): # (item.pricing_rules was cleared above to protect the discount). The applied # record is a Pricing Rule; resolve it to its parent promotional_scheme. One # batched query, no per-item queries. Standalone rules (no scheme) stay blank. - try: - rule_to_scheme = {} - if applied_rule_names_seen: - rule_to_scheme = { - r.name: r.promotional_scheme - for r in frappe.get_all( - "Pricing Rule", - filters={"name": ["in", list(applied_rule_names_seen)]}, - fields=["name", "promotional_scheme"], - ) - if r.promotional_scheme - } - for item in invoice_doc.get("items", []): - names = getattr(item, "_applied_rule_names", None) or [] - scheme = next((rule_to_scheme[n] for n in names if n in rule_to_scheme), None) - item.pos_applied_promotional_scheme = scheme - except Exception: - # Stamping must never fail the sale. - frappe.log_error( - title="Applied promotional scheme stamping failed", - message=frappe.get_traceback(), - ) + # The field only exists on Sales Invoice Item, so guard like the block below. + if doctype == "Sales Invoice": + try: + rule_to_scheme = {} + if applied_rule_names_seen: + rule_to_scheme = { + r.name: r.promotional_scheme + for r in frappe.get_all( + "Pricing Rule", + filters={"name": ["in", list(applied_rule_names_seen)]}, + fields=["name", "promotional_scheme"], + ) + if r.promotional_scheme + } + for item in invoice_doc.get("items", []): + names = getattr(item, "_applied_rule_names", None) or [] + # If a line carries rules from multiple schemes, the first matched wins. + scheme = next((rule_to_scheme[n] for n in names if n in rule_to_scheme), None) + item.pos_applied_promotional_scheme = scheme + except Exception: + # Stamping must never fail the sale. + frappe.log_error( + title="Applied promotional scheme stamping failed", + message=frappe.get_traceback(), + ) if doctype == "Sales Invoice": # Only stamp rules we can actually track: an identified, non walk-in From a5b280471383dbb4af00667463569e41a9620c36 Mon Sep 17 00:00:00 2001 From: engahmed1190 Date: Wed, 17 Jun 2026 12:52:17 +0300 Subject: [PATCH 10/11] refactor: merge scheme stamp into the one-time Sales Invoice guard Avoid two consecutive identical 'if doctype == "Sales Invoice":' blocks by nesting the scheme-stamp logic with the existing one-time-offer block under a single guard. Co-Authored-By: Claude Opus 4.8 (1M context) --- pos_next/api/invoices.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/pos_next/api/invoices.py b/pos_next/api/invoices.py index c2a7d48bf..8a4411c9b 100644 --- a/pos_next/api/invoices.py +++ b/pos_next/api/invoices.py @@ -903,12 +903,11 @@ def update_invoice(data): # transient marker is never written to the database. item._applied_rule_names = item_rule_names - # Stamp each line with a Link to the Promotional Scheme that discounted it - # (item.pricing_rules was cleared above to protect the discount). The applied - # record is a Pricing Rule; resolve it to its parent promotional_scheme. One - # batched query, no per-item queries. Standalone rules (no scheme) stay blank. - # The field only exists on Sales Invoice Item, so guard like the block below. if doctype == "Sales Invoice": + # Stamp each line with a Link to the Promotional Scheme that discounted it + # (item.pricing_rules was cleared above to protect the discount). The applied + # record is a Pricing Rule; resolve it to its parent promotional_scheme. One + # batched query, no per-item queries. Standalone rules (no scheme) stay blank. try: rule_to_scheme = {} if applied_rule_names_seen: @@ -933,7 +932,6 @@ def update_invoice(data): message=frappe.get_traceback(), ) - if doctype == "Sales Invoice": # Only stamp rules we can actually track: an identified, non walk-in # customer on a non-return sale (see is_one_time_eligible_customer). can_track_one_time = is_one_time_eligible_customer( From fb401c1fc41fb1b07c34806426987b99402470c0 Mon Sep 17 00:00:00 2001 From: engahmed1190 Date: Wed, 17 Jun 2026 12:56:45 +0300 Subject: [PATCH 11/11] refactor: resolve applied pricing rules in a single query Merge the two Pricing Rule lookups (scheme resolution + one-time flag) into one get_all over applied_rule_names_seen; derive both the per-line scheme Link and the invoice-level one-time stamp from it. Drop the now-redundant getattr guard. Co-Authored-By: Claude Opus 4.8 (1M context) --- pos_next/api/invoices.py | 66 +++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 35 deletions(-) diff --git a/pos_next/api/invoices.py b/pos_next/api/invoices.py index 8a4411c9b..2cbf8bd92 100644 --- a/pos_next/api/invoices.py +++ b/pos_next/api/invoices.py @@ -904,27 +904,35 @@ def update_invoice(data): item._applied_rule_names = item_rule_names if doctype == "Sales Invoice": - # Stamp each line with a Link to the Promotional Scheme that discounted it - # (item.pricing_rules was cleared above to protect the discount). The applied - # record is a Pricing Rule; resolve it to its parent promotional_scheme. One - # batched query, no per-item queries. Standalone rules (no scheme) stay blank. + # Resolve every applied Pricing Rule once: its parent promotional_scheme + # (for the per-line scheme Link) and whether it is one-time-per-customer + # (for the invoice-level one-time stamp). One query feeds both stamps. + can_track_one_time = is_one_time_eligible_customer( + invoice_doc.get("customer"), + pos_profile_doc.customer if pos_profile_doc else None, + invoice_doc.get("is_return"), + ) + applied_rules = ( + frappe.get_all( + "Pricing Rule", + filters={"name": ["in", list(applied_rule_names_seen)]}, + fields=["name", "promotional_scheme", "one_time_per_customer"], + ) + if applied_rule_names_seen + else [] + ) + + # Per-line scheme Link: resolve each rule to its scheme (item.pricing_rules + # was cleared above to protect the discount). Stamping must never fail the + # sale. Standalone rules (no scheme) leave the line blank. try: - rule_to_scheme = {} - if applied_rule_names_seen: - rule_to_scheme = { - r.name: r.promotional_scheme - for r in frappe.get_all( - "Pricing Rule", - filters={"name": ["in", list(applied_rule_names_seen)]}, - fields=["name", "promotional_scheme"], - ) - if r.promotional_scheme - } + rule_to_scheme = {r.name: r.promotional_scheme for r in applied_rules if r.promotional_scheme} for item in invoice_doc.get("items", []): - names = getattr(item, "_applied_rule_names", None) or [] + names = item._applied_rule_names or [] # If a line carries rules from multiple schemes, the first matched wins. - scheme = next((rule_to_scheme[n] for n in names if n in rule_to_scheme), None) - item.pos_applied_promotional_scheme = scheme + item.pos_applied_promotional_scheme = next( + (rule_to_scheme[n] for n in names if n in rule_to_scheme), None + ) except Exception: # Stamping must never fail the sale. frappe.log_error( @@ -932,27 +940,15 @@ def update_invoice(data): message=frappe.get_traceback(), ) - # Only stamp rules we can actually track: an identified, non walk-in - # customer on a non-return sale (see is_one_time_eligible_customer). - can_track_one_time = is_one_time_eligible_customer( - invoice_doc.get("customer"), - pos_profile_doc.customer if pos_profile_doc else None, - invoice_doc.get("is_return"), - ) + # Invoice-level one-time stamp: only rules we can actually track (an + # identified, non walk-in customer on a non-return sale). one_time_applied = ( - frappe.get_all( - "Pricing Rule", - filters={ - "name": ["in", list(applied_rule_names_seen)], - "one_time_per_customer": 1, - }, - pluck="name", - ) - if (applied_rule_names_seen and can_track_one_time) + sorted(r.name for r in applied_rules if r.one_time_per_customer) + if can_track_one_time else [] ) invoice_doc.pos_applied_one_time_rules = ( - json.dumps(sorted(one_time_applied)) if one_time_applied else "" + json.dumps(one_time_applied) if one_time_applied else "" ) # Set invoice flags BEFORE calculations