fix(promotions): preserve discounts during submit + support transaction-level pricing rules#264
Merged
Conversation
…on-enforcement fix(isolation): roll back custom_company enforcement that hid all rec…
…evel rules Two related bugs surfaced from a user report of a Sales Invoice ending up "Partly Paid" even though the cashier collected the discounted amount. Both fixes are needed to make every Pricing Rule shape POS Next claims to support behave correctly end-to-end (apply_offers → update_invoice → submit_invoice). 1. Partial-paid regression When POS Next set `ignore_pricing_rule=1` on the invoice doc (which it always does, since offers are computed manually via apply_offers) AND item.pricing_rules carried the offer name AND the doc already existed in DB, ERPNext's get_pricing_rule_for_item branched into remove_pricing_rule_for_item, which zeroed discount_percentage, discount_amount, and reset rate=price_list_rate on the next save. The first save (saveDraft) didn't trip it because the doc didn't exist yet; the second save (submit) did, silently restoring the pre-discount grand_total while paid_amount stayed at the collected (discounted) amount — producing "Partly Paid" status with a phantom outstanding equal to the discount. Fix: clear item.pricing_rules in _process_invoice before save. The discount itself is still carried via discount_percentage / discount_amount on the line, so the offer's effect is preserved. 2. Transaction-level Price discounts silently dropped _evaluate_transaction_offers only collected free-item rows after running ERPNext's apply_pricing_rule_on_transaction. When a Price-type Transaction rule fired (e.g. "10% off cart over 100"), ERPNext wrote additional_discount_percentage / discount_amount / apply_discount_on on the doc header — and POS Next never read them back. The cart received no signal that any rule applied, leaving applied_pricing_rules empty. Fix: snapshot the header fields before/after the engine call, surface them in the return dict, attribute the change to matching transaction- level Price rules in rule_map. apply_offers() forwards them to the client; the cart writes discountAmount into additionalDiscount.value, which the existing submit path already maps to invoice.discount_amount. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… in CI
Adds pos_next/test_promotions.py — 18 FrappeTestCase methods that drive the
full apply_offers → update_invoice → submit_invoice pipeline and assert
the saved Sales Invoice has the expected grand_total / paid_amount /
discount_percentage / discount_amount / outstanding_amount / status.
Coverage:
- 7 main offer shapes: Discount Percentage, Discount Amount, Rate override,
Free same-item, Free different-item, Transaction-level, Mixed multi-item.
- 6 edge cases: 100% off, discount equal to item price, multi-qty,
stacked line + transaction discounts, transaction rule below min_amt,
plain (no-offer) invoice.
- 2 explicit regression canaries:
* test_partial_paid_regression — pins down the ERPNext
remove_pricing_rule_for_item interaction so future ERPNext upgrades or
POS Next refactors that break it fail loudly.
* test_transaction_harvest_response — pins down the apply_offers
response shape so callers can rely on additional_discount_percentage /
discount_amount / apply_discount_on being surfaced.
- 4 negative tests: disabled rule, expired rule, wrong item scope,
transaction rule below threshold — all must yield empty
applied_pricing_rules and zero discount.
Test fixtures are deterministic: all items / customer / POS Profile /
Pricing Rules are prefixed _PNXT_TEST_ and created on demand, so the suite
runs cleanly on a fresh CI test_site or an existing dev site (data is
left in place at teardown — rules are disabled rather than deleted).
The previously-commented Run Tests step in .github/workflows/ci.yml is
enabled and scoped to this module so the integration suite gates every PR.
Local invocation on dev sites (where bench run-tests would wipe data):
bench --site <site> execute pos_next.test_promotions.run_all
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CI run on the new test_site revealed that the test setUp leaned on fixtures that exist only when a site has been seeded (nexus.local) but not on the bare ERPNext install CI provisions. Failures fixed: 1. 'All Customer Groups' is a Group node — Customer.validate rejects it. Resolve a leaf Customer Group via is_group=0, preferring ERPNext's _Test Customer Group fixture when present, falling back to creating one under the root. 2. Same gotcha applies to 'All Territories'. Added _resolve_territory() with the same is_group=0 + fallback-create pattern. 3. Item Group: 'All Item Groups' is also a Group node. Added _resolve_item_group() with the same shape. 4. Mode of Payment: a fresh test_site has the 'Cash' Mode of Payment but no Mode of Payment Account rows, so _set_payment_accounts would fail later. Wire up Cash → company.default_cash_account (or any cash-type leaf account, or any leaf account as last resort) if no Mode of Payment Account exists for the test company. All 18 tests still pass against nexus.local (3 consecutive runs). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-to-end fix(offers): preserve item discount on submit + harvest transaction-level rules
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
This PR fixes two critical issues in the promotional offers pipeline affecting invoice consistency and transaction-level pricing rules in POSNext.
Issues Fixed
1. Partial Paid Invoice Regression
Invoices with promotional discounts could incorrectly become
Partly Paidafter submission.Root Cause
ERPNext removed pricing rule effects during submit when:
ignore_pricing_rule=1item.pricing_rulesexistedThis silently restored original rates while keeping the collected payment amount unchanged.
Fix
Clear
item.pricing_rulesbefore save in_process_invoicePreserve discounts using:
discount_percentagediscount_amountResult:
grand_totalpaid_amount2. Transaction-Level Price Rules Not Reflected
Transaction-level discount rules were applied internally by ERPNext but never surfaced back to POSNext.
Fix
Capture invoice header discount changes before/after pricing engine execution
Forward:
additional_discount_percentagediscount_amountapply_discount_onProperly map transaction-level pricing rules into the frontend response
Result:
Tests Added
Added a comprehensive integration suite covering:
A total of 18 integration tests now validate the complete:
apply_offers → update_invoice → submit_invoicepipeline.
CI has also been updated to execute the promotions suite automatically.