Merge pull request #31209 from ankush/purch_return_gle

fix: purchase invoice standalone return GLEs

erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py

@@ -1136,7 +1136,7 @@ class PurchaseInvoice(BuyingController):
 		# Stock ledger value is not matching with the warehouse amount
 		if (
 			self.update_stock
-			and voucher_wise_stock_value.get(item.name)
+			and voucher_wise_stock_value.get((item.name, item.warehouse))
 			and warehouse_debit_amount
 			!= flt(voucher_wise_stock_value.get((item.name, item.warehouse)), net_amt_precision)
 		):

erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py

@@ -27,12 +27,13 @@ from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import (
 	make_purchase_receipt,
 )
 from erpnext.stock.doctype.stock_entry.test_stock_entry import get_qty_after_transaction
+from erpnext.stock.tests.test_utils import StockTestMixin
 
 test_dependencies = ["Item", "Cost Center", "Payment Term", "Payment Terms Template"]
 test_ignore = ["Serial No"]
 
 
-class TestPurchaseInvoice(unittest.TestCase):
+class TestPurchaseInvoice(unittest.TestCase, StockTestMixin):
 	@classmethod
 	def setUpClass(self):
 		unlink_payment_on_cancel_of_invoice()
@@ -693,6 +694,80 @@ class TestPurchaseInvoice(unittest.TestCase):
 			self.assertEqual(expected_values[gle.account][0], gle.debit)
 			self.assertEqual(expected_values[gle.account][1], gle.credit)
 
+	def test_standalone_return_using_pi(self):
+		from erpnext.stock.doctype.stock_entry.test_stock_entry import make_stock_entry
+
+		item = self.make_item().name
+		company = "_Test Company with perpetual inventory"
+		warehouse = "Stores - TCP1"
+
+		make_stock_entry(item_code=item, target=warehouse, qty=50, rate=120)
+
+		return_pi = make_purchase_invoice(
+			is_return=1,
+			item=item,
+			qty=-10,
+			update_stock=1,
+			rate=100,
+			company=company,
+			warehouse=warehouse,
+			cost_center="Main - TCP1",
+		)
+
+		# assert that stock consumption is with actual rate
+		self.assertGLEs(
+			return_pi,
+			[{"credit": 1200, "debit": 0}],
+			gle_filters={"account": "Stock In Hand - TCP1"},
+		)
+
+		# assert loss booked in COGS
+		self.assertGLEs(
+			return_pi,
+			[{"credit": 0, "debit": 200}],
+			gle_filters={"account": "Cost of Goods Sold - TCP1"},
+		)
+
+	def test_return_with_lcv(self):
+		from erpnext.controllers.sales_and_purchase_return import make_return_doc
+		from erpnext.stock.doctype.landed_cost_voucher.test_landed_cost_voucher import (
+			create_landed_cost_voucher,
+		)
+
+		item = self.make_item().name
+		company = "_Test Company with perpetual inventory"
+		warehouse = "Stores - TCP1"
+		cost_center = "Main - TCP1"
+
+		pi = make_purchase_invoice(
+			item=item,
+			company=company,
+			warehouse=warehouse,
+			cost_center=cost_center,
+			update_stock=1,
+			qty=10,
+			rate=100,
+		)
+
+		# Create landed cost voucher - will increase valuation of received item by 10
+		create_landed_cost_voucher("Purchase Invoice", pi.name, pi.company, charges=100)
+		return_pi = make_return_doc(pi.doctype, pi.name)
+		return_pi.save().submit()
+
+		# assert that stock consumption is with actual in rate
+		self.assertGLEs(
+			return_pi,
+			[{"credit": 1100, "debit": 0}],
+			gle_filters={"account": "Stock In Hand - TCP1"},
+		)
+
+		# assert loss booked in COGS
+		self.assertGLEs(
+			return_pi,
+			[{"credit": 0, "debit": 100}],
+			gle_filters={"account": "Cost of Goods Sold - TCP1"},
+		)
+
 	def test_multi_currency_gle(self):
 		pi = make_purchase_invoice(
 			supplier="_Test Supplier USD",

erpnext/controllers/sales_and_purchase_return.py

@@ -316,7 +316,7 @@ def get_returned_qty_map_for_row(return_against, party, row_name, doctype):
 	return data[0]
 
 
-def make_return_doc(doctype, source_name, target_doc=None):
+def make_return_doc(doctype: str, source_name: str, target_doc=None):
 	from frappe.model.mapper import get_mapped_doc
 
 	from erpnext.stock.doctype.serial_no.serial_no import get_serial_nos

erpnext/stock/doctype/stock_entry/stock_entry_utils.py

@@ -2,11 +2,37 @@
 # See license.txt
 
 
+from typing import TYPE_CHECKING, Optional, overload
+
 import frappe
 from frappe.utils import cint, flt
 
 import erpnext
 
+if TYPE_CHECKING:
+	from erpnext.stock.doctype.stock_entry.stock_entry import StockEntry
+
+
+@overload
+def make_stock_entry(
+	*,
+	item_code: str,
+	qty: float,
+	company: Optional[str] = None,
+	from_warehouse: Optional[str] = None,
+	to_warehouse: Optional[str] = None,
+	rate: Optional[float] = None,
+	serial_no: Optional[str] = None,
+	batch_no: Optional[str] = None,
+	posting_date: Optional[str] = None,
+	posting_time: Optional[str] = None,
+	purpose: Optional[str] = None,
+	do_not_save: bool = False,
+	do_not_submit: bool = False,
+	inspection_required: bool = False,
+) -> "StockEntry":
+	...
+
 
 @frappe.whitelist()
 def make_stock_entry(**args):

erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py

@@ -24,9 +24,10 @@ from erpnext.stock.doctype.stock_reconciliation.test_stock_reconciliation import
 	create_stock_reconciliation,
 )
 from erpnext.stock.stock_ledger import get_previous_sle
+from erpnext.stock.tests.test_utils import StockTestMixin
 
 
-class TestStockLedgerEntry(FrappeTestCase):
+class TestStockLedgerEntry(FrappeTestCase, StockTestMixin):
 	def setUp(self):
 		items = create_items()
 		reset("Stock Entry")
@@ -541,30 +542,6 @@ class TestStockLedgerEntry(FrappeTestCase):
 				"Incorrect 'Incoming Rate' values fetched for DN items",
 			)
 
-	def assertSLEs(self, doc, expected_sles, sle_filters=None):
-		"""Compare sorted SLEs, useful for vouchers that create multiple SLEs for same line"""
-
-		filters = {"voucher_no": doc.name, "voucher_type": doc.doctype, "is_cancelled": 0}
-		if sle_filters:
-			filters.update(sle_filters)
-		sles = frappe.get_all(
-			"Stock Ledger Entry",
-			fields=["*"],
-			filters=filters,
-			order_by="timestamp(posting_date, posting_time), creation",
-		)
-
-		for exp_sle, act_sle in zip(expected_sles, sles):
-			for k, v in exp_sle.items():
-				act_value = act_sle[k]
-				if k == "stock_queue":
-					act_value = json.loads(act_value)
-					if act_value and act_value[0][0] == 0:
-						# ignore empty fifo bins
-						continue
-
-				self.assertEqual(v, act_value, msg=f"{k} doesn't match \n{exp_sle}\n{act_sle}")
-
 	def test_batchwise_item_valuation_stock_reco(self):
 		item, warehouses, batches = setup_item_valuation_test()
 		state = {"stock_value": 0.0, "qty": 0.0}

erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py

@@ -10,7 +10,7 @@ from frappe.tests.utils import FrappeTestCase, change_settings
 from frappe.utils import add_days, cstr, flt, nowdate, nowtime, random_string
 
 from erpnext.accounts.utils import get_stock_and_account_balance
-from erpnext.stock.doctype.item.test_item import create_item, make_item
+from erpnext.stock.doctype.item.test_item import create_item
 from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_purchase_receipt
 from erpnext.stock.doctype.serial_no.serial_no import get_serial_nos
 from erpnext.stock.doctype.stock_reconciliation.stock_reconciliation import (
@@ -19,10 +19,11 @@ from erpnext.stock.doctype.stock_reconciliation.stock_reconciliation import (
 )
 from erpnext.stock.doctype.warehouse.test_warehouse import create_warehouse
 from erpnext.stock.stock_ledger import get_previous_sle, update_entries_after
+from erpnext.stock.tests.test_utils import StockTestMixin
 from erpnext.stock.utils import get_incoming_rate, get_stock_value_on, get_valuation_method
 
 
-class TestStockReconciliation(FrappeTestCase):
+class TestStockReconciliation(FrappeTestCase, StockTestMixin):
 	@classmethod
 	def setUpClass(cls):
 		create_batch_or_serial_no_items()
@@ -40,7 +41,7 @@ class TestStockReconciliation(FrappeTestCase):
 		self._test_reco_sle_gle("Moving Average")
 
 	def _test_reco_sle_gle(self, valuation_method):
-		item_code = make_item(properties={"valuation_method": valuation_method}).name
+		item_code = self.make_item(properties={"valuation_method": valuation_method}).name
 
 		se1, se2, se3 = insert_existing_sle(warehouse="Stores - TCP1", item_code=item_code)
 		company = frappe.db.get_value("Warehouse", "Stores - TCP1", "company")
@@ -392,7 +393,7 @@ class TestStockReconciliation(FrappeTestCase):
 		SR4		| Reco	|	0	|	6	(posting date: today-1) [backdated]
 		PR3		| PR	|	1	|	7	(posting date: today) # can't post future PR
 		"""
-		item_code = make_item().name
+		item_code = self.make_item().name
 		warehouse = "_Test Warehouse - _TC"
 
 		frappe.flags.dont_execute_stock_reposts = True
@@ -458,7 +459,7 @@ class TestStockReconciliation(FrappeTestCase):
 		from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note
 		from erpnext.stock.stock_ledger import NegativeStockError
 
-		item_code = make_item().name
+		item_code = self.make_item().name
 		warehouse = "_Test Warehouse - _TC"
 
 		pr1 = make_purchase_receipt(
@@ -506,7 +507,7 @@ class TestStockReconciliation(FrappeTestCase):
 		from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note
 		from erpnext.stock.stock_ledger import NegativeStockError
 
-		item_code = make_item().name
+		item_code = self.make_item().name
 		warehouse = "_Test Warehouse - _TC"
 
 		sr = create_stock_reconciliation(
@@ -549,7 +550,7 @@ class TestStockReconciliation(FrappeTestCase):
 		# repost will make this test useless, qty should update in realtime without reposts
 		frappe.flags.dont_execute_stock_reposts = True
 
-		item_code = make_item().name
+		item_code = self.make_item().name
 		warehouse = "_Test Warehouse - _TC"
 
 		sr = create_stock_reconciliation(

erpnext/stock/tests/test_utils.py

@@ -1,16 +1,67 @@
+import json
+
 import frappe
 from frappe.tests.utils import FrappeTestCase
 
-from erpnext.stock.doctype.item.test_item import make_item
 from erpnext.stock.utils import scan_barcode
 
 
-class TestStockUtilities(FrappeTestCase):
+class StockTestMixin:
+	"""Mixin to simplfy stock ledger tests, useful for all stock transactions."""
+
+	def make_item(self, item_code=None, properties=None, *args, **kwargs):
+		from erpnext.stock.doctype.item.test_item import make_item
+
+		return make_item(item_code, properties, *args, **kwargs)
+
+	def assertSLEs(self, doc, expected_sles, sle_filters=None):
+		"""Compare sorted SLEs, useful for vouchers that create multiple SLEs for same line"""
+
+		filters = {"voucher_no": doc.name, "voucher_type": doc.doctype, "is_cancelled": 0}
+		if sle_filters:
+			filters.update(sle_filters)
+		sles = frappe.get_all(
+			"Stock Ledger Entry",
+			fields=["*"],
+			filters=filters,
+			order_by="timestamp(posting_date, posting_time), creation",
+		)
+
+		for exp_sle, act_sle in zip(expected_sles, sles):
+			for k, v in exp_sle.items():
+				act_value = act_sle[k]
+				if k == "stock_queue":
+					act_value = json.loads(act_value)
+					if act_value and act_value[0][0] == 0:
+						# ignore empty fifo bins
+						continue
+
+				self.assertEqual(v, act_value, msg=f"{k} doesn't match \n{exp_sle}\n{act_sle}")
+
+	def assertGLEs(self, doc, expected_gles, gle_filters=None, order_by=None):
+		filters = {"voucher_no": doc.name, "voucher_type": doc.doctype, "is_cancelled": 0}
+
+		if gle_filters:
+			filters.update(gle_filters)
+		actual_gles = frappe.get_all(
+			"GL Entry",
+			fields=["*"],
+			filters=filters,
+			order_by=order_by or "posting_date, creation",
+		)
+
+		for exp_gle, act_gle in zip(expected_gles, actual_gles):
+			for k, exp_value in exp_gle.items():
+				act_value = act_gle[k]
+				self.assertEqual(exp_value, act_value, msg=f"{k} doesn't match \n{exp_gle}\n{act_gle}")
+
+
+class TestStockUtilities(FrappeTestCase, StockTestMixin):
 	def test_barcode_scanning(self):
-		simple_item = make_item(properties={"barcodes": [{"barcode": "12399"}]})
+		simple_item = self.make_item(properties={"barcodes": [{"barcode": "12399"}]})
 		self.assertEqual(scan_barcode("12399")["item_code"], simple_item.name)
 
-		batch_item = make_item(properties={"has_batch_no": 1, "create_new_batch": 1})
+		batch_item = self.make_item(properties={"has_batch_no": 1, "create_new_batch": 1})
 		batch = frappe.get_doc(doctype="Batch", item=batch_item.name).insert()
 
 		batch_scan = scan_barcode(batch.name)
@@ -19,7 +70,7 @@ class TestStockUtilities(FrappeTestCase):
 		self.assertEqual(batch_scan["has_batch_no"], 1)
 		self.assertEqual(batch_scan["has_serial_no"], 0)
 
-		serial_item = make_item(properties={"has_serial_no": 1})
+		serial_item = self.make_item(properties={"has_serial_no": 1})
 		serial = frappe.get_doc(
 			doctype="Serial No", item_code=serial_item.name, serial_no=frappe.generate_hash()
 		).insert()