From 1e9b0c3cd9aec92bea887b2853bd6382ed02c991 Mon Sep 17 00:00:00 2001 From: MT Date: Wed, 25 Mar 2026 23:10:19 +0200 Subject: [PATCH 1/3] fix: restore coupon usage when invoices are cancelled --- pos_next/api/sales_invoice_hooks.py | 29 +++++++++++++++++ pos_next/api/test_sales_invoice_hooks.py | 40 ++++++++++++++++++++++++ pos_next/hooks.py | 7 +++-- 3 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 pos_next/api/test_sales_invoice_hooks.py diff --git a/pos_next/api/sales_invoice_hooks.py b/pos_next/api/sales_invoice_hooks.py index 10304f89..9c3a8ff9 100644 --- a/pos_next/api/sales_invoice_hooks.py +++ b/pos_next/api/sales_invoice_hooks.py @@ -141,3 +141,32 @@ def before_cancel(doc, method=None): alert=True, indicator="orange" ) + + +def on_cancel(doc, method=None): + """ + On Cancel hook for Sales Invoice. + Roll back coupon usage only after the invoice has cancelled successfully. + + Args: + doc: Sales Invoice document + method: Hook method name (unused) + """ + rollback_coupon_usage(doc) + + +def rollback_coupon_usage(doc): + """Restore coupon usage for cancelled invoices that consumed a POS coupon.""" + coupon_code = doc.get("coupon_code") + if not coupon_code or not frappe.db.table_exists("POS Coupon"): + return + + try: + from pos_next.pos_next.doctype.pos_coupon.pos_coupon import decrement_coupon_usage + + decrement_coupon_usage(coupon_code) + except Exception as e: + frappe.log_error( + title="Coupon Usage Rollback Failed", + message=f"Invoice: {doc.name}, Coupon: {coupon_code}, Error: {str(e)}\n{frappe.get_traceback()}", + ) diff --git a/pos_next/api/test_sales_invoice_hooks.py b/pos_next/api/test_sales_invoice_hooks.py new file mode 100644 index 00000000..43b24d23 --- /dev/null +++ b/pos_next/api/test_sales_invoice_hooks.py @@ -0,0 +1,40 @@ +# Copyright (c) 2025, BrainWise and contributors +# For license information, please see license.txt + +import unittest +from unittest.mock import Mock, patch + +from pos_next.api.sales_invoice_hooks import on_cancel, rollback_coupon_usage + + +class TestSalesInvoiceHooks(unittest.TestCase): + @patch("pos_next.api.sales_invoice_hooks.frappe.db") + @patch("pos_next.pos_next.doctype.pos_coupon.pos_coupon.decrement_coupon_usage") + def test_rollback_coupon_usage_for_cancelled_invoice(self, mock_decrement, mock_db): + doc = Mock() + doc.get.return_value = "SAVE10" + doc.name = "ACC-SINV-0001" + mock_db.table_exists.return_value = True + + rollback_coupon_usage(doc) + + mock_decrement.assert_called_once_with("SAVE10") + + @patch("pos_next.api.sales_invoice_hooks.frappe.db") + @patch("pos_next.pos_next.doctype.pos_coupon.pos_coupon.decrement_coupon_usage") + def test_rollback_coupon_usage_skips_invoices_without_coupon(self, mock_decrement, mock_db): + doc = Mock() + doc.get.return_value = None + + rollback_coupon_usage(doc) + + mock_db.table_exists.assert_not_called() + mock_decrement.assert_not_called() + + @patch("pos_next.api.sales_invoice_hooks.rollback_coupon_usage") + def test_on_cancel_delegates_to_coupon_rollback(self, mock_rollback): + doc = Mock() + + on_cancel(doc) + + mock_rollback.assert_called_once_with(doc) diff --git a/pos_next/hooks.py b/pos_next/hooks.py index 7b422cf4..8f40e73e 100644 --- a/pos_next/hooks.py +++ b/pos_next/hooks.py @@ -190,7 +190,10 @@ "pos_next.realtime_events.emit_stock_update_event", "pos_next.api.wallet.process_loyalty_to_wallet" ], - "on_cancel": "pos_next.realtime_events.emit_stock_update_event", + "on_cancel": [ + "pos_next.api.sales_invoice_hooks.on_cancel", + "pos_next.realtime_events.emit_stock_update_event", + ], "after_insert": "pos_next.realtime_events.emit_invoice_created_event" }, "POS Profile": { @@ -294,4 +297,4 @@ # } -website_route_rules = [{'from_route': '/pos/', 'to_route': 'pos'},] \ No newline at end of file +website_route_rules = [{'from_route': '/pos/', 'to_route': 'pos'},] From 747b3993e6cf96583c757f05c0cc030737e0014f Mon Sep 17 00:00:00 2001 From: MT Date: Sat, 28 Mar 2026 14:41:39 +0200 Subject: [PATCH 2/3] fix: skip coupon rollback for return invoices --- pos_next/api/sales_invoice_hooks.py | 3 ++ pos_next/api/test_sales_invoice_hooks.py | 38 ++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/pos_next/api/sales_invoice_hooks.py b/pos_next/api/sales_invoice_hooks.py index 9c3a8ff9..75a3d16e 100644 --- a/pos_next/api/sales_invoice_hooks.py +++ b/pos_next/api/sales_invoice_hooks.py @@ -157,6 +157,9 @@ def on_cancel(doc, method=None): def rollback_coupon_usage(doc): """Restore coupon usage for cancelled invoices that consumed a POS coupon.""" + if doc.get("is_return"): + return + coupon_code = doc.get("coupon_code") if not coupon_code or not frappe.db.table_exists("POS Coupon"): return diff --git a/pos_next/api/test_sales_invoice_hooks.py b/pos_next/api/test_sales_invoice_hooks.py index 43b24d23..15f2ce90 100644 --- a/pos_next/api/test_sales_invoice_hooks.py +++ b/pos_next/api/test_sales_invoice_hooks.py @@ -12,7 +12,10 @@ class TestSalesInvoiceHooks(unittest.TestCase): @patch("pos_next.pos_next.doctype.pos_coupon.pos_coupon.decrement_coupon_usage") def test_rollback_coupon_usage_for_cancelled_invoice(self, mock_decrement, mock_db): doc = Mock() - doc.get.return_value = "SAVE10" + doc.get.side_effect = lambda key, default=None: { + "is_return": 0, + "coupon_code": "SAVE10", + }.get(key, default) doc.name = "ACC-SINV-0001" mock_db.table_exists.return_value = True @@ -24,13 +27,44 @@ def test_rollback_coupon_usage_for_cancelled_invoice(self, mock_decrement, mock_ @patch("pos_next.pos_next.doctype.pos_coupon.pos_coupon.decrement_coupon_usage") def test_rollback_coupon_usage_skips_invoices_without_coupon(self, mock_decrement, mock_db): doc = Mock() - doc.get.return_value = None + doc.get.side_effect = lambda key, default=None: { + "is_return": 0, + "coupon_code": None, + }.get(key, default) rollback_coupon_usage(doc) mock_db.table_exists.assert_not_called() mock_decrement.assert_not_called() + @patch("pos_next.api.sales_invoice_hooks.frappe.db") + @patch("pos_next.pos_next.doctype.pos_coupon.pos_coupon.decrement_coupon_usage") + def test_rollback_coupon_usage_skips_return_invoices(self, mock_decrement, mock_db): + doc = Mock() + doc.get.side_effect = lambda key, default=None: { + "is_return": 1, + "coupon_code": "SAVE10", + }.get(key, default) + + rollback_coupon_usage(doc) + + mock_db.table_exists.assert_not_called() + mock_decrement.assert_not_called() + + @patch("pos_next.api.sales_invoice_hooks.frappe.db") + @patch("pos_next.pos_next.doctype.pos_coupon.pos_coupon.decrement_coupon_usage") + def test_rollback_coupon_usage_skips_when_coupon_table_missing(self, mock_decrement, mock_db): + doc = Mock() + doc.get.side_effect = lambda key, default=None: { + "is_return": 0, + "coupon_code": "SAVE10", + }.get(key, default) + mock_db.table_exists.return_value = False + + rollback_coupon_usage(doc) + + mock_decrement.assert_not_called() + @patch("pos_next.api.sales_invoice_hooks.rollback_coupon_usage") def test_on_cancel_delegates_to_coupon_rollback(self, mock_rollback): doc = Mock() From e9db0ebce684be68ff779674bbc713cd6ce6acda Mon Sep 17 00:00:00 2001 From: MT Date: Fri, 3 Apr 2026 23:55:41 +0200 Subject: [PATCH 3/3] fix: make coupon rollback transactional --- .../pos_next/doctype/pos_coupon/pos_coupon.py | 43 ++++++++++++++++--- .../doctype/pos_coupon/test_pos_coupon.py | 34 ++++++++++++++- 2 files changed, 69 insertions(+), 8 deletions(-) diff --git a/pos_next/pos_next/doctype/pos_coupon/pos_coupon.py b/pos_next/pos_next/doctype/pos_coupon/pos_coupon.py index e57b832b..c4636ba9 100644 --- a/pos_next/pos_next/doctype/pos_coupon/pos_coupon.py +++ b/pos_next/pos_next/doctype/pos_coupon/pos_coupon.py @@ -171,16 +171,47 @@ def increment_coupon_usage(coupon_code): ) +def rollback_coupon_usage(coupon_code): + """Atomically decrement coupon usage inside the caller's transaction.""" + coupon_code = coupon_code.upper() + locked_coupon = frappe.db.sql( + """ + SELECT name, used + FROM `tabPOS Coupon` + WHERE coupon_code = %s + FOR UPDATE + """, + (coupon_code,), + as_dict=True, + ) + + if not locked_coupon: + return None + + coupon_row = locked_coupon[0] + current_used = flt(coupon_row.get("used") or 0) + if current_used <= 0: + return coupon_row + + updated_used = current_used - 1 + frappe.db.set_value( + "POS Coupon", + coupon_row["name"], + "used", + updated_used, + update_modified=False, + ) + coupon_row["used"] = updated_used + return coupon_row + + def decrement_coupon_usage(coupon_code): - """Decrement the usage counter for a coupon (for cancelled invoices)""" + """Backward-compatible wrapper around transactional coupon rollback.""" try: - coupon = frappe.get_doc("POS Coupon", {"coupon_code": coupon_code.upper()}) - if coupon.used and coupon.used > 0: - coupon.used = coupon.used - 1 - coupon.db_set('used', coupon.used) - frappe.db.commit() + return rollback_coupon_usage(coupon_code) except Exception as e: frappe.log_error( title="Coupon Usage Decrement Failed", message=f"Failed to decrement usage for coupon {coupon_code}: {str(e)}" ) + raise diff --git a/pos_next/pos_next/doctype/pos_coupon/test_pos_coupon.py b/pos_next/pos_next/doctype/pos_coupon/test_pos_coupon.py index f9c38823..244113d9 100644 --- a/pos_next/pos_next/doctype/pos_coupon/test_pos_coupon.py +++ b/pos_next/pos_next/doctype/pos_coupon/test_pos_coupon.py @@ -1,9 +1,39 @@ # Copyright (c) 2021, Youssef Restom and Contributors # See license.txt -# import frappe import unittest +from unittest.mock import patch + +from pos_next.pos_next.doctype.pos_coupon.pos_coupon import ( + decrement_coupon_usage, + rollback_coupon_usage, +) class TestPOSCoupon(unittest.TestCase): - pass + @patch("pos_next.pos_next.doctype.pos_coupon.pos_coupon.frappe.db") + def test_rollback_coupon_usage_locks_and_updates_without_commit(self, mock_db): + mock_db.sql.return_value = [{"name": "PROMO-0001", "used": 2}] + + result = rollback_coupon_usage("promo10") + + self.assertEqual(result["used"], 1) + mock_db.sql.assert_called_once() + mock_db.set_value.assert_called_once_with( + "POS Coupon", + "PROMO-0001", + "used", + 1, + update_modified=False, + ) + self.assertFalse(mock_db.commit.called) + + @patch("pos_next.pos_next.doctype.pos_coupon.pos_coupon.rollback_coupon_usage") + @patch("pos_next.pos_next.doctype.pos_coupon.pos_coupon.frappe.log_error") + def test_decrement_coupon_usage_re_raises_failures(self, mock_log_error, mock_rollback): + mock_rollback.side_effect = RuntimeError("boom") + + with self.assertRaises(RuntimeError): + decrement_coupon_usage("promo10") + + mock_log_error.assert_called_once()