fix: Treat rows as Unit Price rows only until the qty is 0

- The unit price check should depend on the row qty being 0
- Once the row ceases to be 0, it is treated as an ordinary row
- test: PO, SO and Quotation

(cherry picked from commit 0447c7be0a)

# Conflicts:
#	erpnext/selling/doctype/quotation/test_quotation.py
#	erpnext/selling/doctype/sales_order/test_sales_order.py

erpnext/buying/doctype/purchase_order/purchase_order.py

@@ -725,8 +725,11 @@ def set_missing_values(source, target):
 def make_purchase_receipt(source_name, target_doc=None):
 	has_unit_price_items = frappe.db.get_value("Purchase Order", source_name, "has_unit_price_items")
 
+	def is_unit_price_row(source):
+		return has_unit_price_items and source.qty == 0
+
 	def update_item(obj, target, source_parent):
-		target.qty = flt(obj.qty) - flt(obj.received_qty) if not has_unit_price_items else 0
+		target.qty = flt(obj.qty) if is_unit_price_row(obj) else flt(obj.qty) - flt(obj.received_qty)
 		target.stock_qty = (flt(obj.qty) - flt(obj.received_qty)) * flt(obj.conversion_factor)
 		target.amount = (flt(obj.qty) - flt(obj.received_qty)) * flt(obj.rate)
 		target.base_amount = (
@@ -758,7 +761,7 @@ def make_purchase_receipt(source_name, target_doc=None):
 				},
 				"postprocess": update_item,
 				"condition": lambda doc: (
-					abs(doc.received_qty) < abs(doc.qty) if not has_unit_price_items else True
+					True if is_unit_price_row(doc) else abs(doc.received_qty) < abs(doc.qty)
 				)
 				and doc.delivered_by_supplier != 1,
 			},

erpnext/buying/doctype/purchase_order/test_purchase_order.py

@@ -1234,18 +1234,6 @@ class TestPurchaseOrder(FrappeTestCase):
 
 		po.reload()
 		self.assertEqual(po.items[0].received_qty, 5)
-		# PO still has qty 0, so received % should be unset
-		self.assertFalse(po.per_received)
-		self.assertEqual(po.status, "To Receive and Bill")
-
-		# Test: PR can be made against PO as long PO qty is 0 OR PO qty > received qty
-		pr2 = make_purchase_receipt(po.name)
-		self.assertEqual(pr2.items[0].qty, 0)
-		pr2.items[0].qty = 5
-		pr2.submit()
-
-		po.reload()
-		self.assertEqual(po.items[0].received_qty, 10)
 		self.assertFalse(po.per_received)
 		self.assertEqual(po.status, "To Receive and Bill")
 
@@ -1263,9 +1251,19 @@ class TestPurchaseOrder(FrappeTestCase):
 		)
 		update_child_qty_rate("Purchase Order", trans_item, po.name)
 
+		# Test: PR can be made against PO as long PO qty is 0 OR PO qty > received qty
+		pr2 = make_purchase_receipt(po.name)
+
+		po.reload()
+		self.assertEqual(po.items[0].qty, 10)
+		self.assertEqual(pr2.items[0].qty, 5)
+
+		pr2.submit()
+
 		# PO should be updated to 100% received
 		po.reload()
 		self.assertEqual(po.items[0].qty, 10)
+		self.assertEqual(po.items[0].received_qty, 10)
 		self.assertEqual(po.per_received, 100.0)
 		self.assertEqual(po.status, "To Bill")
 

erpnext/selling/doctype/quotation/quotation.py

@@ -380,10 +380,14 @@ def _make_sales_order(source_name, target_doc=None, ignore_permissions=False):
 			as_list=1,
 		)
 	)
+
+	selected_rows = [x.get("name") for x in frappe.flags.get("args", {}).get("selected_items", [])]
+
 	# 0 qty is accepted, as the qty uncertain for some items
 	has_unit_price_items = frappe.db.get_value("Quotation", source_name, "has_unit_price_items")
 
-	selected_rows = [x.get("name") for x in frappe.flags.get("args", {}).get("selected_items", [])]
+	def is_unit_price_row(source) -> bool:
+		return has_unit_price_items and source.qty == 0
 
 	def set_missing_values(source, target):
 		if customer:
@@ -413,7 +417,7 @@ def _make_sales_order(source_name, target_doc=None, ignore_permissions=False):
 		target.run_method("calculate_taxes_and_totals")
 
 	def update_item(obj, target, source_parent):
-		balance_qty = obj.qty - ordered_items.get(obj.item_code, 0.0)
+		balance_qty = obj.qty if is_unit_price_row(obj) else obj.qty - ordered_items.get(obj.item_code, 0.0)
 		target.qty = balance_qty if balance_qty > 0 else 0
 		target.stock_qty = flt(target.qty) * flt(obj.conversion_factor)
 
@@ -427,23 +431,22 @@ def _make_sales_order(source_name, target_doc=None, ignore_permissions=False):
 		Row mapping from Quotation to Sales order:
 		1. If no selections, map all non-alternative rows (that sum up to the grand total)
 		2. If selections: Is Alternative Item/Has Alternative Item: Map if selected and adequate qty
-		3. If selections: Simple row: Map if adequate qty
+		3. If no selections: Simple row: Map if adequate qty
 		"""
 		balance_qty = item.qty - ordered_items.get(item.item_code, 0.0)
-		if balance_qty <= 0 and not has_unit_price_items:
+		has_valid_qty: bool = (balance_qty > 0) or is_unit_price_row(item)
-			# False if qty <=0 in a 'normal' scenario
-			return False
 
-		has_qty: bool = (balance_qty > 0) or has_unit_price_items
+		if not has_valid_qty:
+			return False
 
 		if not selected_rows:
 			return not item.is_alternative
 
 		if selected_rows and (item.is_alternative or item.has_alternative_item):
-			return (item.name in selected_rows) and has_qty
+			return item.name in selected_rows
 
 		# Simple row
-		return has_qty
+		return True
 
 	doclist = get_mapped_doc(
 		"Quotation",

erpnext/selling/doctype/quotation/test_quotation.py

@@ -2,13 +2,49 @@
 # License: GNU General Public License v3. See license.txt
 
 import frappe
+<<<<<<< HEAD
 from frappe.tests.utils import FrappeTestCase
+=======
+from frappe.tests import IntegrationTestCase, UnitTestCase, change_settings
+>>>>>>> 0447c7be0a (fix: Treat rows as Unit Price rows only until the qty is 0)
 from frappe.utils import add_days, add_months, flt, getdate, nowdate
 
 test_dependencies = ["Product Bundle"]
 
 
+<<<<<<< HEAD
 class TestQuotation(FrappeTestCase):
+=======
+class UnitTestQuotation(UnitTestCase):
+	"""
+	Unit tests for Quotation.
+	Use this class for testing individual functions and methods.
+	"""
+
+	pass
+
+
+class TestQuotation(IntegrationTestCase):
+	def test_quotation_qty(self):
+		qo = make_quotation(qty=0, do_not_save=True)
+		with self.assertRaises(InvalidQtyError):
+			qo.save()
+
+		# No error with qty=1
+		qo.items[0].qty = 1
+		qo.save()
+		self.assertEqual(qo.items[0].qty, 1)
+
+	def test_quotation_zero_qty(self):
+		"""
+		Test if Quote with zero qty (Unit Price Item) is conditionally allowed.
+		"""
+		qo = make_quotation(qty=0, do_not_save=True)
+		with change_settings("Selling Settings", {"allow_zero_qty_in_quotation": 1}):
+			qo.save()
+			self.assertEqual(qo.items[0].qty, 0)
+
+>>>>>>> 0447c7be0a (fix: Treat rows as Unit Price rows only until the qty is 0)
 	def test_make_quotation_without_terms(self):
 		quotation = make_quotation(do_not_save=1)
 		self.assertFalse(quotation.get("payment_schedule"))
@@ -761,6 +797,39 @@ class TestQuotation(FrappeTestCase):
 		self.assertEqual(quotation.rounding_adjustment, 0)
 		self.assertEqual(quotation.rounded_total, 0)
 
+	@IntegrationTestCase.change_settings("Selling Settings", {"allow_zero_qty_in_quotation": 1})
+	def test_so_from_zero_qty_quotation(self):
+		from erpnext.selling.doctype.quotation.quotation import make_sales_order
+		from erpnext.stock.doctype.item.test_item import make_item
+
+		make_item("_Test Item 2", {"is_stock_item": 1})
+		quotation = make_quotation(qty=0, do_not_save=1)
+		quotation.append("items", {"item_code": "_Test Item 2", "qty": 10, "rate": 100})
+		quotation.submit()
+
+		sales_order = make_sales_order(quotation.name)
+		sales_order.delivery_date = nowdate()
+		self.assertEqual(sales_order.items[0].qty, 0)
+		self.assertEqual(sales_order.items[1].qty, 10)
+
+		sales_order.items[0].qty = 10
+		sales_order.items[1].qty = 5
+		sales_order.submit()
+
+		quotation.reload()
+		self.assertEqual(quotation.status, "Partially Ordered")
+
+		sales_order_2 = make_sales_order(quotation.name)
+		sales_order_2.delivery_date = nowdate()
+		self.assertEqual(sales_order_2.items[0].qty, 0)
+		self.assertEqual(sales_order_2.items[1].qty, 5)
+
+		del sales_order_2.items[0]
+		sales_order_2.submit()
+
+		quotation.reload()
+		self.assertEqual(quotation.status, "Ordered")
+
 
 test_records = frappe.get_test_records("Quotation")
 

erpnext/selling/doctype/sales_order/sales_order.py

@@ -962,9 +962,6 @@ def make_delivery_note(source_name, target_doc=None, kwargs=None):
 
 	kwargs = frappe._dict(kwargs)
 
-	# 0 qty is accepted, as the qty is uncertain for some items
-	has_unit_price_items = frappe.db.get_value("Sales Order", source_name, "has_unit_price_items")
-
 	sre_details = {}
 	if kwargs.for_reserved_stock:
 		sre_details = get_sre_reserved_qty_details_for_voucher("Sales Order", source_name)
@@ -975,6 +972,12 @@ def make_delivery_note(source_name, target_doc=None, kwargs=None):
 		"Sales Team": {"doctype": "Sales Team", "add_if_empty": True},
 	}
 
+	# 0 qty is accepted, as the qty is uncertain for some items
+	has_unit_price_items = frappe.db.get_value("Sales Order", source_name, "has_unit_price_items")
+
+	def is_unit_price_row(source):
+		return has_unit_price_items and source.qty == 0
+
 	def set_missing_values(source, target):
 		if kwargs.get("ignore_pricing_rule"):
 			# Skip pricing rule when the dn is creating from the pick list
@@ -1012,13 +1015,15 @@ def make_delivery_note(source_name, target_doc=None, kwargs=None):
 				return False
 
 		return (
-			(abs(doc.delivered_qty) < abs(doc.qty)) or has_unit_price_items
+			(abs(doc.delivered_qty) < abs(doc.qty)) or is_unit_price_row(doc)
 		) and doc.delivered_by_supplier != 1
 
 	def update_item(source, target, source_parent):
 		target.base_amount = (flt(source.qty) - flt(source.delivered_qty)) * flt(source.base_rate)
 		target.amount = (flt(source.qty) - flt(source.delivered_qty)) * flt(source.rate)
-		target.qty = flt(source.qty) - flt(source.delivered_qty) if not has_unit_price_items else 0
+		target.qty = (
+			flt(source.qty) if is_unit_price_row(source) else flt(source.qty) - flt(source.delivered_qty)
+		)
 
 		item = get_item_defaults(target.item_code, source_parent.company)
 		item_group = get_item_group_defaults(target.item_code, source_parent.company)
@@ -1095,6 +1100,12 @@ def make_delivery_note(source_name, target_doc=None, kwargs=None):
 
 @frappe.whitelist()
 def make_sales_invoice(source_name, target_doc=None, ignore_permissions=False):
+	# 0 qty is accepted, as the qty is uncertain for some items
+	has_unit_price_items = frappe.db.get_value("Sales Order", source_name, "has_unit_price_items")
+
+	def is_unit_price_row(source):
+		return has_unit_price_items and source.qty == 0
+
 	def postprocess(source, target):
 		set_missing_values(source, target)
 		# Get the advance paid Journal Entries in Sales Invoice Advance
@@ -1135,7 +1146,7 @@ def make_sales_invoice(source_name, target_doc=None, ignore_permissions=False):
 		target.qty = (
 			target.amount / flt(source.rate)
 			if (source.rate and source.billed_amt)
-			else source.qty - source.returned_qty
+			else (source.qty if is_unit_price_row(source) else source.qty - source.returned_qty)
 		)
 
 		if source_parent.project:
@@ -1148,8 +1159,6 @@ def make_sales_invoice(source_name, target_doc=None, ignore_permissions=False):
 			if cost_center:
 				target.cost_center = cost_center
 
-	# has_unit_price_items = 0 is accepted as the qty uncertain for some items
-	has_unit_price_items = frappe.db.get_value("Sales Order", source_name, "has_unit_price_items")
 	doclist = get_mapped_doc(
 		"Sales Order",
 		source_name,
@@ -1171,9 +1180,10 @@ def make_sales_invoice(source_name, target_doc=None, ignore_permissions=False):
 				},
 				"postprocess": update_item,
 				"condition": lambda doc: (
-					doc.qty and (doc.base_amount == 0 or abs(doc.billed_amt) < abs(doc.amount))
+					True
-				)
+					if is_unit_price_row(doc)
-				or has_unit_price_items,
+					else (doc.qty and (doc.base_amount == 0 or abs(doc.billed_amt) < abs(doc.amount)))
+				),
 			},
 			"Sales Taxes and Charges": {
 				"doctype": "Sales Taxes and Charges",

erpnext/selling/doctype/sales_order/test_sales_order.py

@@ -1983,6 +1983,79 @@ class TestSalesOrder(AccountsTestMixin, FrappeTestCase):
 				self.assertEqual(so.items[0].rate, scenario.get("expected_rate"))
 				self.assertEqual(so.packed_items[0].rate, scenario.get("expected_rate"))
 
+<<<<<<< HEAD
+=======
+	@patch(
+		# this also shadows one (1) call to _get_payment_gateway_controller
+		"erpnext.accounts.doctype.payment_request.payment_request.PaymentRequest.get_payment_url",
+		return_value=None,
+	)
+	def test_sales_order_advance_payment_status(self, mocked_get_payment_url):
+		from erpnext.accounts.doctype.payment_entry.test_payment_entry import get_payment_entry
+		from erpnext.accounts.doctype.payment_request.payment_request import make_payment_request
+
+		# Flow progressing to SI with payment entries "moved" from SO to SI
+		so = make_sales_order(qty=1, rate=100, do_not_submit=True)
+		# no-op; for optical consistency with how a webshop SO would look like
+		so.order_type = "Shopping Cart"
+		so.submit()
+		self.assertEqual(frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Not Requested")
+
+		pr = make_payment_request(
+			dt=so.doctype,
+			dn=so.name,
+			order_type="Shopping Cart",
+			submit_doc=True,
+			return_doc=True,
+			mute_email=True,
+		)
+		self.assertEqual(frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Requested")
+
+		pe = pr.set_as_paid()
+		pr.reload()  # status updated
+		pe.reload()  # references moved to Sales Invoice
+		self.assertEqual(pr.status, "Paid")
+		self.assertEqual(pe.references[0].reference_doctype, "Sales Invoice")
+		self.assertEqual(frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Fully Paid")
+
+		pe.cancel()
+		pr.reload()
+		self.assertEqual(pr.status, "Paid")  # TODO: this might be a bug
+		so.reload()  # reload
+		# regardless, since the references have already "handed-over" to SI,
+		# the SO keeps its historical state at the time of hand over
+		self.assertEqual(frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Fully Paid")
+
+		pr.cancel()
+		self.assertEqual(
+			frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Not Requested"
+		)  # TODO: this might be a bug; handover has happened
+
+		# Flow NOT progressing to SI with payment entries NOT "moved"
+		so = make_sales_order(qty=1, rate=100)
+		self.assertEqual(frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Not Requested")
+
+		pr = make_payment_request(
+			dt=so.doctype, dn=so.name, submit_doc=True, return_doc=True, mute_email=True
+		)
+		self.assertEqual(frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Requested")
+
+		pe = get_payment_entry(so.doctype, so.name).save().submit()
+		self.assertEqual(frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Fully Paid")
+
+		pe.reload()
+		pe.cancel()
+		self.assertEqual(
+			frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Requested"
+		)  # here: reset
+
+		pr.reload()
+		pr.cancel()
+		self.assertEqual(
+			frappe.db.get_value(so.doctype, so.name, "advance_payment_status"), "Not Requested"
+		)  # here: reset
+
+>>>>>>> 0447c7be0a (fix: Treat rows as Unit Price rows only until the qty is 0)
 	def test_pick_list_without_rejected_materials(self):
 		serial_and_batch_item = make_item(
 			"_Test Serial and Batch Item for Rejected Materials",
@@ -2234,7 +2307,7 @@ class TestSalesOrder(AccountsTestMixin, FrappeTestCase):
 		"""
 		Test the flow of a Unit Price SO and DN creation against it until completion.
 		Flow:
-		SO Qty 0 -> Deliver +5 -> Deliver +5 -> Update SO Qty +10 -> SO is 100% delivered
+		SO Qty 0 -> Deliver +5 -> Update SO Qty +10 -> Deliver +5 -> SO is 100% delivered
 		"""
 		so = make_sales_order(qty=0)
 		dn = make_delivery_note(so.name)
@@ -2243,24 +2316,13 @@ class TestSalesOrder(AccountsTestMixin, FrappeTestCase):
 		dn.items[0].qty = 5
 		dn.submit()
 
+		# Test SO impact after DN
 		so.reload()
 		self.assertEqual(so.items[0].delivered_qty, 5)
-		# SO still has qty 0, so delivered % should be unset
 		self.assertFalse(so.per_delivered)
 		self.assertEqual(so.status, "To Deliver and Bill")
 
-		# Test: DN can be made against SO as long SO qty is 0 OR SO qty > delivered qty
+		# Update SO Qty to final qty
-		dn2 = make_delivery_note(so.name)
-		self.assertEqual(dn2.items[0].qty, 0)
-		dn2.items[0].qty = 5
-		dn2.submit()
-
-		so.reload()
-		self.assertEqual(so.items[0].delivered_qty, 10)
-		self.assertFalse(so.per_delivered)
-		self.assertEqual(so.status, "To Deliver and Bill")
-
-		# Update SO Item Qty to 10 after delivery of items
 		first_item_of_so = so.items[0]
 		trans_item = json.dumps(
 			[
@@ -2274,9 +2336,17 @@ class TestSalesOrder(AccountsTestMixin, FrappeTestCase):
 		)
 		update_child_qty_rate("Sales Order", trans_item, so.name)
 
-		# SO should be updated to 100% delivered
+		# Test: DN maps pending qty from SO
+		dn2 = make_delivery_note(so.name)
+
 		so.reload()
 		self.assertEqual(so.items[0].qty, 10)
+		self.assertEqual(dn2.items[0].qty, 5)
+
+		dn2.submit()
+
+		so.reload()
+		self.assertEqual(so.items[0].delivered_qty, 10)
 		self.assertEqual(so.per_delivered, 100.0)
 		self.assertEqual(so.status, "To Bill")
 
@@ -2296,11 +2366,9 @@ class TestSalesOrder(AccountsTestMixin, FrappeTestCase):
 		si.items[0].qty = 5
 		si.submit()
 
-		self.assertEqual(si.grand_total, 500)
-
 		so.reload()
 		self.assertEqual(so.items[0].amount, 0)
-		self.assertEqual(so.items[0].billed_amt, 500)
+		self.assertEqual(so.items[0].billed_amt, si.grand_total)
 		# SO still has qty 0, so billed % should be unset
 		self.assertFalse(so.per_billed)
 		self.assertEqual(so.status, "To Deliver and Bill")