fix: Negative Qty and Rates in SO/PO (#38252)

* fix: Don't allow negative qty in SO/PO

* fix: Type casting for safe comparisons

* fix: Grammar in error message

* fix: Negative rates should be allowed via Update Items in SO/PO

* fix: Use `non_negative` property in docfield & emove code validation

(cherry picked from commit b9f5a1c)

erpnext/buying/doctype/purchase_order/test_purchase_order.py

@@ -16,7 +16,7 @@
 	make_purchase_invoice as make_pi_from_po,
 )
 from erpnext.buying.doctype.purchase_order.purchase_order import make_purchase_receipt
-from erpnext.controllers.accounts_controller import update_child_qty_rate
+from erpnext.controllers.accounts_controller import InvalidQtyError, update_child_qty_rate
 from erpnext.manufacturing.doctype.blanket_order.test_blanket_order import make_blanket_order
 from erpnext.stock.doctype.item.test_item import make_item
 from erpnext.stock.doctype.material_request.material_request import make_purchase_order
@@ -27,6 +27,21 @@
 
 
 class TestPurchaseOrder(FrappeTestCase):
+	def test_purchase_order_qty(self):
+		po = create_purchase_order(qty=1, do_not_save=True)
+		po.append(
+			"items",
+			{
+				"item_code": "_Test Item",
+				"qty": -1,
+				"rate": 10,
+			},
+		)
+		self.assertRaises(frappe.NonNegativeError, po.save)
+
+		po.items[1].qty = 0
+		self.assertRaises(InvalidQtyError, po.save)
+
 	def test_make_purchase_receipt(self):
 		po = create_purchase_order(do_not_submit=True)
 		self.assertRaises(frappe.ValidationError, make_purchase_receipt, po.name)

erpnext/buying/doctype/purchase_order_item/purchase_order_item.json

@@ -214,6 +214,7 @@
    "fieldtype": "Float",
    "in_list_view": 1,
    "label": "Quantity",
+   "non_negative": 1,
    "oldfieldname": "qty",
    "oldfieldtype": "Currency",
    "print_width": "60px",
@@ -917,7 +918,7 @@
  "index_web_pages_for_search": 1,
  "istable": 1,
  "links": [],
- "modified": "2023-11-14 18:34:27.267382",
+ "modified": "2023-11-24 13:24:41.298416",
  "modified_by": "Administrator",
  "module": "Buying",
  "name": "Purchase Order Item",

erpnext/controllers/accounts_controller.py

@@ -71,6 +71,10 @@ class AccountMissingError(frappe.ValidationError):
 	pass
 
 
+class InvalidQtyError(frappe.ValidationError):
+	pass
+
+
 force_item_fields = (
 	"item_group",
 	"brand",
@@ -910,10 +914,16 @@ def get_value_in_transaction_currency(self, account_currency, args, field):
 			return flt(args.get(field, 0) / self.get("conversion_rate", 1))
 
 	def validate_qty_is_not_zero(self):
-		if self.doctype != "Purchase Receipt":
-			for item in self.items:
-				if not item.qty:
-					frappe.throw(_("Item quantity can not be zero"))
+		if self.doctype == "Purchase Receipt":
+			return
+
+		for item in self.items:
+			if not flt(item.qty):
+				frappe.throw(
+					msg=_("Row #{0}: Item quantity cannot be zero").format(item.idx),
+					title=_("Invalid Quantity"),
+					exc=InvalidQtyError,
+				)
 
 	def validate_account_currency(self, account, account_currency=None):
 		valid_currency = [self.company_currency]
@@ -3139,16 +3149,19 @@ def validate_fg_item_for_subcontracting(new_data, is_new):
 		conv_fac_precision = child_item.precision("conversion_factor") or 2
 		qty_precision = child_item.precision("qty") or 2
 
-		if flt(child_item.billed_amt, rate_precision) > flt(
-			flt(d.get("rate"), rate_precision) * flt(d.get("qty"), qty_precision), rate_precision
-		):
+		# Amount cannot be lesser than billed amount, except for negative amounts
+		row_rate = flt(d.get("rate"), rate_precision)
+		amount_below_billed_amt = flt(child_item.billed_amt, rate_precision) > flt(
+			row_rate * flt(d.get("qty"), qty_precision), rate_precision
+		)
+		if amount_below_billed_amt and row_rate > 0.0:
 			frappe.throw(
 				_("Row #{0}: Cannot set Rate if amount is greater than billed amount for Item {1}.").format(
 					child_item.idx, child_item.item_code
 				)
 			)
 		else:
-			child_item.rate = flt(d.get("rate"), rate_precision)
+			child_item.rate = row_rate
 
 		if d.get("conversion_factor"):
 			if child_item.stock_uom == child_item.uom:

erpnext/selling/doctype/sales_order/test_sales_order.py

@@ -51,6 +51,35 @@ def tearDownClass(cls) -> None:
 	def tearDown(self):
 		frappe.set_user("Administrator")
 
+	def test_sales_order_with_negative_rate(self):
+		"""
+		Test if negative rate is allowed in Sales Order via doc submission and update items
+		"""
+		so = make_sales_order(qty=1, rate=100, do_not_save=True)
+		so.append("items", {"item_code": "_Test Item", "qty": 1, "rate": -10})
+		so.save()
+		so.submit()
+
+		first_item = so.get("items")[0]
+		second_item = so.get("items")[1]
+		trans_item = json.dumps(
+			[
+				{
+					"item_code": first_item.item_code,
+					"rate": first_item.rate,
+					"qty": first_item.qty,
+					"docname": first_item.name,
+				},
+				{
+					"item_code": second_item.item_code,
+					"rate": -20,
+					"qty": second_item.qty,
+					"docname": second_item.name,
+				},
+			]
+		)
+		update_child_qty_rate("Sales Order", trans_item, so.name)
+
 	def test_make_material_request(self):
 		so = make_sales_order(do_not_submit=True)
 

erpnext/selling/doctype/sales_order_item/sales_order_item.json

@@ -200,6 +200,7 @@
    "fieldtype": "Float",
    "in_list_view": 1,
    "label": "Quantity",
+   "non_negative": 1,
    "oldfieldname": "qty",
    "oldfieldtype": "Currency",
    "print_width": "100px",
@@ -895,7 +896,7 @@
  "idx": 1,
  "istable": 1,
  "links": [],
- "modified": "2023-11-14 18:37:12.787893",
+ "modified": "2023-11-24 13:24:55.756320",
  "modified_by": "Administrator",
  "module": "Selling",
  "name": "Sales Order Item",