diff --git a/pos_next/api/sales_invoice_hooks.py b/pos_next/api/sales_invoice_hooks.py index 10304f89..75a3d16e 100644 --- a/pos_next/api/sales_invoice_hooks.py +++ b/pos_next/api/sales_invoice_hooks.py @@ -141,3 +141,35 @@ 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.""" + 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 + + 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..15f2ce90 --- /dev/null +++ b/pos_next/api/test_sales_invoice_hooks.py @@ -0,0 +1,74 @@ +# 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.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 + + 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.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() + + 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'},] 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()