Merge pull request #38530 from bosue/duplicate_zero_quantity_check2

refactor: Consolidate duplicate zero-quantity Items checks for transactions.

erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py

@@ -14,7 +14,7 @@
 from erpnext.buying.doctype.purchase_order.purchase_order import get_mapped_purchase_invoice
 from erpnext.buying.doctype.purchase_order.test_purchase_order import create_purchase_order
 from erpnext.buying.doctype.supplier.test_supplier import create_supplier
-from erpnext.controllers.accounts_controller import get_payment_terms
+from erpnext.controllers.accounts_controller import InvalidQtyError, get_payment_terms
 from erpnext.controllers.buying_controller import QtyMismatchError
 from erpnext.exceptions import InvalidCurrency
 from erpnext.projects.doctype.project.test_project import make_project
@@ -51,6 +51,16 @@ def tearDownClass(self):
 	def tearDown(self):
 		frappe.db.rollback()
 
+	def test_purchase_invoice_qty(self):
+		pi = make_purchase_invoice(qty=0, do_not_save=True)
+		with self.assertRaises(InvalidQtyError):
+			pi.save()
+
+		# No error with qty=1
+		pi.items[0].qty = 1
+		pi.save()
+		self.assertEqual(pi.items[0].qty, 1)
+
 	def test_purchase_invoice_received_qty(self):
 		"""
 		1. Test if received qty is validated against accepted + rejected
@@ -2114,7 +2124,7 @@ def make_purchase_invoice(**args):
 	bundle_id = None
 	if args.get("batch_no") or args.get("serial_no"):
 		batches = {}
-		qty = args.qty or 5
+		qty = args.qty if args.qty is not None else 5
 		item_code = args.item or args.item_code or "_Test Item"
 		if args.get("batch_no"):
 			batches = frappe._dict({args.batch_no: qty})
@@ -2143,7 +2153,7 @@ def make_purchase_invoice(**args):
 			"item_code": args.item or args.item_code or "_Test Item",
 			"item_name": args.item_name,
 			"warehouse": args.warehouse or "_Test Warehouse - _TC",
-			"qty": args.qty or 5,
+			"qty": args.qty if args.qty is not None else 5,
 			"received_qty": args.received_qty or 0,
 			"rejected_qty": args.rejected_qty or 0,
 			"rate": args.rate or 50,

erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py

@@ -23,7 +23,7 @@
 from erpnext.assets.doctype.asset_depreciation_schedule.asset_depreciation_schedule import (
 	get_depr_schedule,
 )
-from erpnext.controllers.accounts_controller import update_invoice_status
+from erpnext.controllers.accounts_controller import InvalidQtyError, update_invoice_status
 from erpnext.controllers.taxes_and_totals import get_itemised_tax_breakup_data
 from erpnext.exceptions import InvalidAccountCurrency, InvalidCurrency
 from erpnext.selling.doctype.customer.test_customer import get_customer_dict
@@ -72,6 +72,16 @@ def setUpClass(self):
 	def tearDownClass(self):
 		unlink_payment_on_cancel_of_invoice(0)
 
+	def test_sales_invoice_qty(self):
+		si = create_sales_invoice(qty=0, do_not_save=True)
+		with self.assertRaises(InvalidQtyError):
+			si.save()
+
+		# No error with qty=1
+		si.items[0].qty = 1
+		si.save()
+		self.assertEqual(si.items[0].qty, 1)
+
 	def test_timestamp_change(self):
 		w = frappe.copy_doc(test_records[0])
 		w.docstatus = 0
@@ -3636,7 +3646,7 @@ def create_sales_invoice(**args):
 	bundle_id = None
 	if si.update_stock and (args.get("batch_no") or args.get("serial_no")):
 		batches = {}
-		qty = args.qty or 1
+		qty = args.qty if args.qty is not None else 1
 		item_code = args.item or args.item_code or "_Test Item"
 		if args.get("batch_no"):
 			batches = frappe._dict({args.batch_no: qty})
@@ -3668,7 +3678,7 @@ def create_sales_invoice(**args):
 			"description": args.description or "_Test Item",
 			"warehouse": args.warehouse or "_Test Warehouse - _TC",
 			"target_warehouse": args.target_warehouse,
-			"qty": args.qty or 1,
+			"qty": args.qty if args.qty is not None else 1,
 			"uom": args.uom or "Nos",
 			"stock_uom": args.uom or "Nos",
 			"rate": args.rate if args.get("rate") is not None else 100,

erpnext/buying/doctype/purchase_order/test_purchase_order.py

@@ -29,6 +29,8 @@
 class TestPurchaseOrder(FrappeTestCase):
 	def test_purchase_order_qty(self):
 		po = create_purchase_order(qty=1, do_not_save=True)
+
+		# NonNegativeError with qty=-1
 		po.append(
 			"items",
 			{
@@ -39,9 +41,15 @@ def test_purchase_order_qty(self):
 		)
 		self.assertRaises(frappe.NonNegativeError, po.save)
 
+		# InvalidQtyError with qty=0
 		po.items[1].qty = 0
 		self.assertRaises(InvalidQtyError, po.save)
 
+		# No error with qty=1
+		po.items[1].qty = 1
+		po.save()
+		self.assertEqual(po.items[1].qty, 1)
+
 	def test_make_purchase_receipt(self):
 		po = create_purchase_order(do_not_submit=True)
 		self.assertRaises(frappe.ValidationError, make_purchase_receipt, po.name)
@@ -1108,7 +1116,7 @@ def create_purchase_order(**args):
 				"item_code": args.item or args.item_code or "_Test Item",
 				"warehouse": args.warehouse or "_Test Warehouse - _TC",
 				"from_warehouse": args.from_warehouse,
-				"qty": args.qty or 10,
+				"qty": args.qty if args.qty is not None else 10,
 				"rate": args.rate or 500,
 				"schedule_date": add_days(nowdate(), 1),
 				"include_exploded_items": args.get("include_exploded_items", 1),

erpnext/buying/doctype/request_for_quotation/request_for_quotation.py

@@ -65,6 +65,7 @@ class RequestforQuotation(BuyingController):
 	def validate(self):
 		self.validate_duplicate_supplier()
 		self.validate_supplier_list()
+		super(RequestforQuotation, self).validate_qty_is_not_zero()
 		validate_for_items(self)
 		super(RequestforQuotation, self).set_qty_as_per_stock_uom()
 		self.update_email_id()

erpnext/buying/doctype/request_for_quotation/test_request_for_quotation.py

@@ -14,13 +14,24 @@
 	get_pdf,
 	make_supplier_quotation_from_rfq,
 )
+from erpnext.controllers.accounts_controller import InvalidQtyError
 from erpnext.crm.doctype.opportunity.opportunity import make_request_for_quotation as make_rfq
 from erpnext.crm.doctype.opportunity.test_opportunity import make_opportunity
 from erpnext.stock.doctype.item.test_item import make_item
 from erpnext.templates.pages.rfq import check_supplier_has_docname_access
 
 
 class TestRequestforQuotation(FrappeTestCase):
+	def test_rfq_qty(self):
+		rfq = make_request_for_quotation(qty=0, do_not_save=True)
+		with self.assertRaises(InvalidQtyError):
+			rfq.save()
+
+		# No error with qty=1
+		rfq.items[0].qty = 1
+		rfq.save()
+		self.assertEqual(rfq.items[0].qty, 1)
+
 	def test_quote_status(self):
 		rfq = make_request_for_quotation()
 
@@ -161,14 +172,17 @@ def make_request_for_quotation(**args) -> "RequestforQuotation":
 			"description": "_Test Item",
 			"uom": args.uom or "_Test UOM",
 			"stock_uom": args.stock_uom or "_Test UOM",
-			"qty": args.qty or 5,
+			"qty": args.qty if args.qty is not None else 5,
 			"conversion_factor": args.conversion_factor or 1.0,
 			"warehouse": args.warehouse or "_Test Warehouse - _TC",
 			"schedule_date": nowdate(),
 		},
 	)
 
-	rfq.submit()
+	if not args.do_not_save:
+		rfq.insert()
+		if not args.do_not_submit:
+			rfq.submit()
 
 	return rfq
 

erpnext/buying/doctype/supplier_quotation/test_supplier_quotation.py

@@ -5,8 +5,21 @@
 import frappe
 from frappe.tests.utils import FrappeTestCase
 
+from erpnext.controllers.accounts_controller import InvalidQtyError
+
 
 class TestPurchaseOrder(FrappeTestCase):
+	def test_supplier_quotation_qty(self):
+		sq = frappe.copy_doc(test_records[0])
+		sq.items[0].qty = 0
+		with self.assertRaises(InvalidQtyError):
+			sq.save()
+
+		# No error with qty=1
+		sq.items[0].qty = 1
+		sq.save()
+		self.assertEqual(sq.items[0].qty, 1)
+
 	def test_make_purchase_order(self):
 		from erpnext.buying.doctype.supplier_quotation.supplier_quotation import make_purchase_order
 

erpnext/buying/utils.py

@@ -44,11 +44,6 @@ def update_last_purchase_rate(doc, is_submit) -> None:
 def validate_for_items(doc) -> None:
 	items = []
 	for d in doc.get("items"):
-		if not d.qty:
-			if doc.doctype == "Purchase Receipt" and d.rejected_qty:
-				continue
-			frappe.throw(_("Please enter quantity for Item {0}").format(d.item_code))
-
 		set_stock_levels(row=d)  # update with latest quantities
 		item = validate_item_and_get_basic_data(row=d)
 		validate_stock_item_warehouse(row=d, item=item)

erpnext/controllers/accounts_controller.py

@@ -975,13 +975,15 @@ def get_value_in_transaction_currency(self, account_currency, args, field):
 			return flt(args.get(field, 0) / (args.get("conversion_rate") or self.get("conversion_rate", 1)))
 
 	def validate_qty_is_not_zero(self):
-		if self.doctype == "Purchase Receipt":
-			return
-
 		for item in self.items:
+			if self.doctype == "Purchase Receipt" and item.rejected_qty:
+				continue
+
 			if not flt(item.qty):
 				frappe.throw(
-					msg=_("Row #{0}: Item quantity cannot be zero").format(item.idx),
+					msg=_("Row #{0}: Quantity for Item {1} cannot be zero.").format(
+						item.idx, frappe.bold(item.item_code)
+					),
 					title=_("Invalid Quantity"),
 					exc=InvalidQtyError,
 				)
@@ -3097,7 +3099,7 @@ def get_new_child_item(item_row):
 	def validate_quantity(child_item, new_data):
 		if not flt(new_data.get("qty")):
 			frappe.throw(
-				_("Row # {0}: Quantity for Item {1} cannot be zero").format(
+				_("Row #{0}: Quantity for Item {1} cannot be zero.").format(
 					new_data.get("idx"), frappe.bold(new_data.get("item_code"))
 				),
 				title=_("Invalid Qty"),

erpnext/controllers/selling_controller.py

@@ -295,9 +295,6 @@ def throw_message(idx, item_name, rate, ref_rate_field):
 	def get_item_list(self):
 		il = []
 		for d in self.get("items"):
-			if d.qty is None:
-				frappe.throw(_("Row {0}: Qty is mandatory").format(d.idx))
-
 			if self.has_product_bundle(d.item_code):
 				for p in self.get("packed_items"):
 					if p.parent_detail_docname == d.name and p.parent_item == d.item_code:

erpnext/manufacturing/doctype/bom/test_bom.py

@@ -27,6 +27,28 @@
 
 
 class TestBOM(FrappeTestCase):
+	@timeout
+	def test_bom_qty(self):
+		from erpnext.stock.doctype.item.test_item import make_item
+
+		# No error.
+		bom = frappe.new_doc("BOM")
+		item = make_item(properties={"is_stock_item": 1})
+		bom.item = fg_item.item_code
+		bom.quantity = 1
+		bom.append(
+			"items",
+			{
+				"item_code": bom_item.item_code,
+				"qty": 0,
+				"uom": bom_item.stock_uom,
+				"stock_uom": bom_item.stock_uom,
+				"rate": 100.0,
+			},
+		)
+		bom.save()
+		self.assertEqual(bom.items[0].qty, 0)
+
 	@timeout
 	def test_get_items(self):
 		from erpnext.manufacturing.doctype.bom.bom import get_bom_items_as_dict

erpnext/selling/doctype/quotation/test_quotation.py

@@ -5,10 +5,22 @@
 from frappe.tests.utils import FrappeTestCase
 from frappe.utils import add_days, add_months, flt, getdate, nowdate
 
+from erpnext.controllers.accounts_controller import InvalidQtyError
+
 test_dependencies = ["Product Bundle"]
 
 
 class TestQuotation(FrappeTestCase):
+	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_make_quotation_without_terms(self):
 		quotation = make_quotation(do_not_save=1)
 		self.assertFalse(quotation.get("payment_schedule"))
@@ -629,7 +641,7 @@ def make_quotation(**args):
 			{
 				"item_code": args.item or args.item_code or "_Test Item",
 				"warehouse": args.warehouse,
-				"qty": args.qty or 10,
+				"qty": args.qty if args.qty is not None else 10,
 				"uom": args.uom or None,
 				"rate": args.rate or 100,
 			},

erpnext/selling/doctype/sales_order/test_sales_order.py

@@ -9,7 +9,7 @@
 from frappe.tests.utils import FrappeTestCase, change_settings
 from frappe.utils import add_days, flt, getdate, nowdate, today
 
-from erpnext.controllers.accounts_controller import update_child_qty_rate
+from erpnext.controllers.accounts_controller import InvalidQtyError, update_child_qty_rate
 from erpnext.maintenance.doctype.maintenance_schedule.test_maintenance_schedule import (
 	make_maintenance_schedule,
 )
@@ -80,6 +80,29 @@ def test_sales_order_with_negative_rate(self):
 		)
 		update_child_qty_rate("Sales Order", trans_item, so.name)
 
+	def test_sales_order_qty(self):
+		so = make_sales_order(qty=1, do_not_save=True)
+
+		# NonNegativeError with qty=-1
+		so.append(
+			"items",
+			{
+				"item_code": "_Test Item",
+				"qty": -1,
+				"rate": 10,
+			},
+		)
+		self.assertRaises(frappe.NonNegativeError, so.save)
+
+		# InvalidQtyError with qty=0
+		so.items[1].qty = 0
+		self.assertRaises(InvalidQtyError, so.save)
+
+		# No error with qty=1
+		so.items[1].qty = 1
+		so.save()
+		self.assertEqual(so.items[0].qty, 1)
+
 	def test_make_material_request(self):
 		so = make_sales_order(do_not_submit=True)
 
@@ -2015,7 +2038,7 @@ def make_sales_order(**args):
 			{
 				"item_code": args.item or args.item_code or "_Test Item",
 				"warehouse": args.warehouse,
-				"qty": args.qty or 10,
+				"qty": args.qty if args.qty is not None else 10,
 				"uom": args.uom or None,
 				"price_list_rate": args.price_list_rate or None,
 				"discount_percentage": args.discount_percentage or None,

erpnext/stock/doctype/delivery_note/test_delivery_note.py

@@ -10,6 +10,7 @@
 
 from erpnext.accounts.doctype.account.test_account import get_inventory_account
 from erpnext.accounts.utils import get_balance_on
+from erpnext.controllers.accounts_controller import InvalidQtyError
 from erpnext.selling.doctype.product_bundle.test_product_bundle import make_product_bundle
 from erpnext.selling.doctype.sales_order.test_sales_order import (
 	automatically_fetch_payment_terms,
@@ -42,6 +43,16 @@
 
 
 class TestDeliveryNote(FrappeTestCase):
+	def test_delivery_note_qty(self):
+		dn = create_delivery_note(qty=0, do_not_save=True)
+		with self.assertRaises(InvalidQtyError):
+			dn.save()
+
+		# No error with qty=1
+		dn.items[0].qty = 1
+		dn.save()
+		self.assertEqual(dn.items[0].qty, 1)
+
 	def test_over_billing_against_dn(self):
 		frappe.db.set_single_value("Stock Settings", "allow_negative_stock", 1)
 
@@ -1558,7 +1569,7 @@ def create_delivery_note(**args):
 		if dn.is_return:
 			type_of_transaction = "Inward"
 
-		qty = args.get("qty") or 1
+		qty = args.qty if args.get("qty") is not None else 1
 		qty *= -1 if type_of_transaction == "Outward" else 1
 		batches = {}
 		if args.get("batch_no"):
@@ -1586,7 +1597,7 @@ def create_delivery_note(**args):
 		{
 			"item_code": args.item or args.item_code or "_Test Item",
 			"warehouse": args.warehouse or "_Test Warehouse - _TC",
-			"qty": args.qty or 1,
+			"qty": args.qty if args.get("qty") is not None else 1,
 			"rate": args.rate if args.get("rate") is not None else 100,
 			"conversion_factor": 1.0,
 			"serial_and_batch_bundle": bundle_id,