fix: restore coupon usage when invoices are cancelled

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()}",
+		)

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)

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/<path:app_path>', 'to_route': 'pos'},]
+website_route_rules = [{'from_route': '/pos/<path:app_path>', 'to_route': 'pos'},]

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

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()