fix: enforce one-time-per-customer offers (server gate + offline/online sync)#320
Merged
Merged
Conversation
PR #319's one-time-per-customer offer feature did not actually enforce the limit. This fixes the server gate, the transaction-rule bypass, and the offline/online frontend consistency, and hardens the recording hook. Backend (pos_next/api): - apply_offers: the existence check used a double-colon name (f"{customer}::{rule}") that never matched the doctype's single-colon autoname, so the gate never saw prior redemptions. Replaced with a field-filter lookup {customer, pricing_rule}, immune to the autoname. - apply_offers: transaction-scoped one-time rules bypassed the gate entirely (no one_time_per_customer field, no skip). Now gated by the same shared _skip_one_time_rule, and the customer's redeemed set is fetched once into a set (1 query instead of one exists() per rule). - update_invoice: only stamp pos_applied_one_time_rules for an identified, non-default, non-return customer (via is_one_time_eligible_customer), so a redemption is never recorded against the shared walk-in customer. - record_one_time_offer_usage: never fail the sale — guard against an over-long composite name and log-and-skip any insert error; rely on ignore_if_duplicate for idempotency. Frontend (POS/src): - Restore offline redemption recording (the call had been dropped in a refactor), wired into the real offline checkout path in POSSale.vue and keyed by the offline invoice id. - Release cached offline redemptions when an offline sale is voided, deleted, superseded, or synced, so a void doesn't permanently block the customer. - Make the online fetch authoritative: replace the cached server set (so a server-side release self-heals) while preserving not-yet-synced offline redemptions. - Single chokepoint posSync.supersedeInvoice bundles supersede + release; keyed release on the sync hot path (no full-table scan per invoice). - Add unit tests for the offline/online redemption cache. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
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
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.
Summary
PR #319 introduced the One-Time Offer per Customer feature, but a review found the limit was not actually being enforced — online or offline. This PR fixes the server-side gate, closes a transaction-rule bypass, makes the offline/online frontend consistent, and hardens the recording hook so it can never break a sale.
No schema change and no data migration — the original
One Time Customer Offer Usagedoctype and itsformat:{customer}:{pricing_rule}autoname are kept. All lookups now use field filters instead of reconstructing the composite name.The bugs this fixes
🔴 1. Server gate never matched recorded redemptions
apply_offerschecked existence with a double-colon key —f"{customer}::{record.name}"— but the doctype autoname generates a single-colon name (CUST:RULE). The lookup never matched, so a customer could redeem the same one-time offer unlimited times online. Proven on a live site.Fix: field-filter lookup
frappe.db.exists("One Time Customer Offer Usage", {"customer": customer, "pricing_rule": record.name})— immune to the autoname format.🔴 2. Transaction-scoped one-time offers bypassed the gate entirely
The transaction-rule top-up loop didn't select
one_time_per_customerand applied no gate, so cart-level one-time offers applied on every invoice, including walk-ins and repeat customers.Fix: added the field to the query and routed both loops through a shared
_skip_one_time_rule.🔴 3. Offline redemption recording was silently dropped
A refactor (commit
cf035dc2) removed the call that caches a redemption on offline checkout, leaving the wholerecordOfflineRedemptionschain orphaned. The same customer could re-apply a one-time offer on every offline sale; on sync the server strips the discount, causing a receipt/cash vs. server grand_total mismatch.Fix: recording restored in the real offline checkout path (
POSSale.vue), keyed by the offline invoice id.🟠 4. Walk-in / return sales recorded redemptions
update_invoicestampedpos_applied_one_time_rulesunconditionally, which would write a usage row against the shared default/walk-in customer (blocking everyone) or consume a redemption on a credit note.Fix: stamp only for an identified, non-default, non-return customer via the new
is_one_time_eligible_customerhelper.Changes
Backend —
pos_next/api/invoices.pyapply_offers: field-filter existence check (docs: update README to clarify POS access steps #1); shared_skip_one_time_ruleapplied in both the per-item and transaction loops (fix: cart header layout. #2); customer's redeemed set fetched once into aset(1 query instead of oneexists()per rule).update_invoice: walk-in/return guard on stamping (feat: add lazy loading to item images for improved performance #4).is_one_time_eligible_customer(customer, default_customer, is_return)— single source of truth shared by the grant side and record side.sales_invoice_hooks.pyrecord_one_time_offer_usage: never fails the sale — guards an over-long composite name and logs-and-skips any insert error; idempotency viaignore_if_duplicate. Removed the redundant per-rule pre-skip.Frontend —
POS/src/utils/offline/db.js— redemption cache row reshaped to{ serverRules, offlineRules: { [invoiceId]: [...] } }:serverRules— authoritative, replaced wholesale on each online fetch so a server-side release self-heals.offlineRules— per-invoice unsynced redemptions, so a voided offline sale releases only its own.rulesrows. Shared_putRedemptionRowread-mutate-write helper;releaseOfflineRedemptionsreturns the effective set.stores/posOffers.js— online fetch is authoritative (replace, preserve offline);recordOfflineRedemptionskeyed by invoice id; newreleaseOfflineRedemptionsForInvoicerefreshes the in-memory gate without a re-read.stores/posCart.js—recordOfflineRedemptionsForSalecomputes applied one-time rules and delegates to the offers store.stores/posSync.js—deletePendingreleases redemptions on void; new single-chokepointsupersedeInvoicebundles worker-supersede + release.utils/offline/sync.js—markInvoiceSyncedreleases the offline-bucket entry once synced, keyed by customer (no full-table scan per invoice on the sync hot path).pages/POSSale.vue— records redemptions on offline checkout; routes both supersede sites throughposSync.supersedeInvoice.Offline ⇄ online parity matrix
serverRuleTests
POS/src/utils/offline/__tests__/oneTimeRedemptions.test.js— 7 passing vitest cases covering record (P0), server∪offline union, server-authoritative self-heal (P3), per-invoice release (P2), no-customer scan, release-returns-effective-set, and legacy-row backward compat.pos-dev: gate now fires (incl. customer names containing:), duplicate insert rejected, field-based lookup correct, long-name checkout degrades gracefully.🤖 Generated with Claude Code