fix: Serial No not updated correctly via Inter Company Stock Transfer (#25006)

* fix: Serial No not updated correctly via Inter Company Stock Transfer

* chore: Added More Test Cases for inter company Serial Transfer

* fix: Test for serial no duplication

- fixed serial no test
- made errors more meaningful on serial no validation

* fix: Stock Reco Test

Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com>

erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py

@@ -582,6 +582,7 @@ class TestPurchaseReceipt(unittest.TestCase):
 			serial_no=serial_no, basic_rate=100, do_not_submit=True)
 		se.submit()
 
+		se.cancel()
 		dn.cancel()
 		pr1.cancel()
 

erpnext/stock/doctype/serial_no/serial_no.py

@@ -14,6 +14,7 @@ from frappe import _, ValidationError
 from erpnext.controllers.stock_controller import StockController
 from six import string_types
 from six.moves import map
+
 class SerialNoCannotCreateDirectError(ValidationError): pass
 class SerialNoCannotCannotChangeError(ValidationError): pass
 class SerialNoNotRequiredError(ValidationError): pass
@@ -322,11 +323,35 @@ def validate_serial_no(sle, item_det):
 			frappe.throw(_("Serial Nos Required for Serialized Item {0}").format(sle.item_code),
 				SerialNoRequiredError)
 	elif serial_nos:
+		# SLE is being cancelled and has serial nos
 		for serial_no in serial_nos:
-			sr = frappe.db.get_value("Serial No", serial_no, ["name", "warehouse"], as_dict=1)
+			check_serial_no_validity_on_cancel(serial_no, sle)
-			if sr and cint(sle.actual_qty) < 0 and sr.warehouse != sle.warehouse:
+
-				frappe.throw(_("Cannot cancel {0} {1} because Serial No {2} does not belong to the warehouse {3}")
+def check_serial_no_validity_on_cancel(serial_no, sle):
-					.format(sle.voucher_type, sle.voucher_no, serial_no, sle.warehouse))
+	sr = frappe.db.get_value("Serial No", serial_no, ["name", "warehouse", "company", "status"], as_dict=1)
+	sr_link = frappe.utils.get_link_to_form("Serial No", serial_no)
+	doc_link = frappe.utils.get_link_to_form(sle.voucher_type, sle.voucher_no)
+	actual_qty = cint(sle.actual_qty)
+	is_stock_reco = sle.voucher_type == "Stock Reconciliation"
+	msg = None
+
+	if sr and (actual_qty < 0 or is_stock_reco) and sr.warehouse != sle.warehouse:
+		# receipt(inward) is being cancelled
+		msg = _("Cannot cancel {0} {1} as Serial No {2} does not belong to the warehouse {3}").format(
+			sle.voucher_type, doc_link, sr_link, frappe.bold(sle.warehouse))
+	elif sr and actual_qty > 0 and not is_stock_reco:
+		# delivery is being cancelled, check for warehouse.
+		if sr.warehouse:
+			# serial no is active in another warehouse/company.
+			msg = _("Cannot cancel {0} {1} as Serial No {2} is active in warehouse {3}").format(
+				sle.voucher_type, doc_link, sr_link, frappe.bold(sr.warehouse))
+		elif sr.company != sle.company and sr.status == "Delivered":
+			# serial no is inactive (allowed) or delivered from another company (block).
+			msg = _("Cannot cancel {0} {1} as Serial No {2} does not belong to the company {3}").format(
+				sle.voucher_type, doc_link, sr_link, frappe.bold(sle.company))
+
+	if msg:
+		frappe.throw(msg, title=_("Cannot cancel"))
 
 def validate_material_transfer_entry(sle_doc):
 	sle_doc.update({

erpnext/stock/doctype/serial_no/test_serial_no.py

@@ -40,16 +40,139 @@ class TestSerialNo(unittest.TestCase):
 		se = make_serialized_item(target_warehouse="_Test Warehouse - _TC")
 		serial_nos = get_serial_nos(se.get("items")[0].serial_no)
 
-		create_delivery_note(item_code="_Test Serialized Item With Series", qty=1, serial_no=serial_nos[0])
+		dn = create_delivery_note(item_code="_Test Serialized Item With Series", qty=1, serial_no=serial_nos[0])
+
+		serial_no = frappe.get_doc("Serial No", serial_nos[0])
+
+		# check Serial No details after delivery
+		self.assertEqual(serial_no.status, "Delivered")
+		self.assertEqual(serial_no.warehouse, None)
+		self.assertEqual(serial_no.company, "_Test Company")
+		self.assertEqual(serial_no.delivery_document_type, "Delivery Note")
+		self.assertEqual(serial_no.delivery_document_no, dn.name)
 
 		wh = create_warehouse("_Test Warehouse", company="_Test Company 1")
-		make_purchase_receipt(item_code="_Test Serialized Item With Series", qty=1, serial_no=serial_nos[0],
+		pr = make_purchase_receipt(item_code="_Test Serialized Item With Series", qty=1, serial_no=serial_nos[0],
 			company="_Test Company 1", warehouse=wh)
 
-		serial_no = frappe.db.get_value("Serial No", serial_nos[0], ["warehouse", "company"], as_dict=1)
+		serial_no.reload()
 
+		# check Serial No details after purchase in second company
+		self.assertEqual(serial_no.status, "Active")
 		self.assertEqual(serial_no.warehouse, wh)
 		self.assertEqual(serial_no.company, "_Test Company 1")
+		self.assertEqual(serial_no.purchase_document_type, "Purchase Receipt")
+		self.assertEqual(serial_no.purchase_document_no, pr.name)
+
+	def test_inter_company_transfer_intermediate_cancellation(self):
+		"""
+			Receive into and Deliver Serial No from one company.
+			Then Receive into and Deliver from second company.
+			Try to cancel intermediate receipts/deliveries to test if it is blocked.
+		"""
+		se = make_serialized_item(target_warehouse="_Test Warehouse - _TC")
+		serial_nos = get_serial_nos(se.get("items")[0].serial_no)
+
+		sn_doc = frappe.get_doc("Serial No", serial_nos[0])
+
+		# check Serial No details after purchase in first company
+		self.assertEqual(sn_doc.status, "Active")
+		self.assertEqual(sn_doc.company, "_Test Company")
+		self.assertEqual(sn_doc.warehouse, "_Test Warehouse - _TC")
+		self.assertEqual(sn_doc.purchase_document_no, se.name)
+
+		dn = create_delivery_note(item_code="_Test Serialized Item With Series",
+			qty=1, serial_no=serial_nos[0])
+		sn_doc.reload()
+		# check Serial No details after delivery from **first** company
+		self.assertEqual(sn_doc.status, "Delivered")
+		self.assertEqual(sn_doc.company, "_Test Company")
+		self.assertEqual(sn_doc.warehouse, None)
+		self.assertEqual(sn_doc.delivery_document_no, dn.name)
+
+		# try cancelling the first Serial No Receipt, even though it is delivered
+		# block cancellation is Serial No is out of the warehouse
+		self.assertRaises(frappe.ValidationError, se.cancel)
+
+		# receive serial no in second company
+		wh = create_warehouse("_Test Warehouse", company="_Test Company 1")
+		pr = make_purchase_receipt(item_code="_Test Serialized Item With Series",
+			qty=1, serial_no=serial_nos[0], company="_Test Company 1", warehouse=wh)
+		sn_doc.reload()
+
+		self.assertEqual(sn_doc.warehouse, wh)
+		# try cancelling the delivery from the first company
+		# block cancellation as Serial No belongs to different company
+		self.assertRaises(frappe.ValidationError, dn.cancel)
+
+		# deliver from second company
+		dn_2 = create_delivery_note(item_code="_Test Serialized Item With Series",
+			qty=1, serial_no=serial_nos[0], company="_Test Company 1", warehouse=wh)
+		sn_doc.reload()
+
+		# check Serial No details after delivery from **second** company
+		self.assertEqual(sn_doc.status, "Delivered")
+		self.assertEqual(sn_doc.company, "_Test Company 1")
+		self.assertEqual(sn_doc.warehouse, None)
+		self.assertEqual(sn_doc.delivery_document_no, dn_2.name)
+
+		# cannot cancel any intermediate document before last Delivery Note
+		self.assertRaises(frappe.ValidationError, se.cancel)
+		self.assertRaises(frappe.ValidationError, dn.cancel)
+		self.assertRaises(frappe.ValidationError, pr.cancel)
+
+	def test_inter_company_transfer_fallback_on_cancel(self):
+		"""
+			Test Serial No state changes on cancellation.
+			If Delivery cancelled, it should fall back on last Receipt in the same company.
+			If Receipt is cancelled, it should be Inactive in the same company.
+		"""
+		# Receipt in **first** company
+		se = make_serialized_item(target_warehouse="_Test Warehouse - _TC")
+		serial_nos = get_serial_nos(se.get("items")[0].serial_no)
+		sn_doc = frappe.get_doc("Serial No", serial_nos[0])
+
+		# Delivery from first company
+		dn = create_delivery_note(item_code="_Test Serialized Item With Series",
+			qty=1, serial_no=serial_nos[0])
+
+		# Receipt in **second** company
+		wh = create_warehouse("_Test Warehouse", company="_Test Company 1")
+		pr = make_purchase_receipt(item_code="_Test Serialized Item With Series",
+			qty=1, serial_no=serial_nos[0], company="_Test Company 1", warehouse=wh)
+
+		# Delivery from second company
+		dn_2 = create_delivery_note(item_code="_Test Serialized Item With Series",
+			qty=1, serial_no=serial_nos[0], company="_Test Company 1", warehouse=wh)
+		sn_doc.reload()
+
+		self.assertEqual(sn_doc.status, "Delivered")
+		self.assertEqual(sn_doc.company, "_Test Company 1")
+		self.assertEqual(sn_doc.delivery_document_no, dn_2.name)
+
+		dn_2.cancel()
+		sn_doc.reload()
+		# Fallback on Purchase Receipt if Delivery is cancelled
+		self.assertEqual(sn_doc.status, "Active")
+		self.assertEqual(sn_doc.company, "_Test Company 1")
+		self.assertEqual(sn_doc.warehouse, wh)
+		self.assertEqual(sn_doc.purchase_document_no, pr.name)
+
+		pr.cancel()
+		sn_doc.reload()
+		# Inactive in same company if Receipt cancelled
+		self.assertEqual(sn_doc.status, "Inactive")
+		self.assertEqual(sn_doc.company, "_Test Company 1")
+		self.assertEqual(sn_doc.warehouse, None)
+
+		dn.cancel()
+		sn_doc.reload()
+		# Fallback on Purchase Receipt in FIRST company if
+		# Delivery from FIRST company is cancelled
+		self.assertEqual(sn_doc.status, "Active")
+		self.assertEqual(sn_doc.company, "_Test Company")
+		self.assertEqual(sn_doc.warehouse, "_Test Warehouse - _TC")
+		self.assertEqual(sn_doc.purchase_document_no, se.name)
 
 	def tearDown(self):
 		frappe.db.rollback()

erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py

@@ -398,7 +398,7 @@ class StockReconciliation(StockController):
 		merge_similar_entries = {}
 
 		for d in sl_entries:
-			if not d.serial_no or d.actual_qty < 0:
+			if not d.serial_no or flt(d.get("actual_qty")) < 0:
 				new_sl_entries.append(d)
 				continue
 

erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py

@@ -32,7 +32,7 @@ class TestStockReconciliation(unittest.TestCase):
 		company = frappe.db.get_value('Warehouse', 'Stores - TCP1', 'company')
 		# [[qty, valuation_rate, posting_date,
 		#		posting_time, expected_stock_value, bin_qty, bin_valuation]]
-		
+
 		input_data = [
 			[50, 1000, "2012-12-26", "12:00"],
 			[25, 900, "2012-12-26", "12:00"],
@@ -86,7 +86,7 @@ class TestStockReconciliation(unittest.TestCase):
 		se1.cancel()
 
 	def test_get_items(self):
-		create_warehouse("_Test Warehouse Group 1", 
+		create_warehouse("_Test Warehouse Group 1",
 			{"is_group": 1, "company": "_Test Company", "parent_warehouse": "All Warehouses - _TC"})
 		create_warehouse("_Test Warehouse Ledger 1",
 			{"is_group": 0, "parent_warehouse": "_Test Warehouse Group 1 - _TC", "company": "_Test Company"})