From 61314248bb733191f58cf033e2b0312bcf1de481 Mon Sep 17 00:00:00 2001
From: Saqib <nextchamp.saqib@gmail.com>
Date: Tue, 15 Sep 2020 11:14:31 +0530
Subject: [PATCH] feat: uom in update items for SO & PO (#22869)

* feat: uom in update items for SO & PO

* fix: supplied items doesn't updates on uom change

* chore: add uom and conv factor change test

* fix: test

* fix: purchase order updates are not tracked

* fix: fetch conversion factor on uom change

* fix: codacy

Co-authored-by: Marica <maricadsouza221197@gmail.com>
---
 .../purchase_order/purchase_order.json        |  5 ++--
 .../purchase_order/test_purchase_order.py     | 27 +++++++++++++++---
 erpnext/controllers/accounts_controller.py    | 21 ++++++++++----
 erpnext/public/js/utils.js                    | 28 +++++++++++++++++++
 .../doctype/sales_order/test_sales_order.py   | 24 ++++++++++++----
 5 files changed, 89 insertions(+), 16 deletions(-)

diff --git a/erpnext/buying/doctype/purchase_order/purchase_order.json b/erpnext/buying/doctype/purchase_order/purchase_order.json
index 4201e0b6353..d1063b1503d 100644
--- a/erpnext/buying/doctype/purchase_order/purchase_order.json
+++ b/erpnext/buying/doctype/purchase_order/purchase_order.json
@@ -1084,7 +1084,7 @@
  "idx": 105,
  "is_submittable": 1,
  "links": [],
- "modified": "2020-07-31 14:13:44.610190",
+ "modified": "2020-09-14 14:36:12.418690",
  "modified_by": "Administrator",
  "module": "Buying",
  "name": "Purchase Order",
@@ -1135,5 +1135,6 @@
  "sort_field": "modified",
  "sort_order": "DESC",
  "timeline_field": "supplier",
- "title_field": "supplier"
+ "title_field": "supplier",
+ "track_changes": 1
 }
\ No newline at end of file
diff --git a/erpnext/buying/doctype/purchase_order/test_purchase_order.py b/erpnext/buying/doctype/purchase_order/test_purchase_order.py
index 813286f7fa4..69817a46756 100644
--- a/erpnext/buying/doctype/purchase_order/test_purchase_order.py
+++ b/erpnext/buying/doctype/purchase_order/test_purchase_order.py
@@ -89,7 +89,7 @@ class TestPurchaseOrder(unittest.TestCase):
 		frappe.db.set_value("Accounts Settings", None, "over_billing_allowance", 0)
 
 
-	def test_update_child_qty_rate(self):
+	def test_update_child(self):
 		mr = make_material_request(qty=10)
 		po = make_purchase_order(mr.name)
 		po.supplier = "_Test Supplier"
@@ -119,7 +119,7 @@ class TestPurchaseOrder(unittest.TestCase):
 		self.assertEqual(get_ordered_qty(), existing_ordered_qty + 3)
 
 
-	def test_add_new_item_in_update_child_qty_rate(self):
+	def test_update_child_adding_new_item(self):
 		po = create_purchase_order(do_not_save=1)
 		po.items[0].qty = 4
 		po.save()
@@ -145,7 +145,7 @@ class TestPurchaseOrder(unittest.TestCase):
 		self.assertEqual(po.status, 'To Receive and Bill')
 
 
-	def test_remove_item_in_update_child_qty_rate(self):
+	def test_update_child_removing_item(self):
 		po = create_purchase_order(do_not_save=1)
 		po.items[0].qty = 4
 		po.save()
@@ -185,7 +185,7 @@ class TestPurchaseOrder(unittest.TestCase):
 		self.assertEquals(len(po.get('items')), 1)
 		self.assertEqual(po.status, 'To Receive and Bill')
 
-	def test_update_child_qty_rate_perm(self):
+	def test_update_child_perm(self):
 		po = create_purchase_order(item_code= "_Test Item", qty=4)
 
 		user = 'test@example.com'
@@ -202,6 +202,25 @@ class TestPurchaseOrder(unittest.TestCase):
 		self.assertRaises(frappe.ValidationError, update_child_qty_rate,'Purchase Order', trans_item, po.name)
 		frappe.set_user("Administrator")
 
+	def test_update_child_uom_conv_factor_change(self):
+		po = create_purchase_order(item_code="_Test FG Item", is_subcontracted="Yes")
+		total_reqd_qty = sum([d.get("required_qty") for d in po.as_dict().get("supplied_items")])
+
+		trans_item = json.dumps([{
+			'item_code': po.get("items")[0].item_code,
+			'rate': po.get("items")[0].rate,
+			'qty': po.get("items")[0].qty,
+			'uom': "_Test UOM 1",
+			'conversion_factor': 2,
+			'docname': po.get("items")[0].name
+		}])
+		update_child_qty_rate('Purchase Order', trans_item, po.name)
+		po.reload()
+
+		total_reqd_qty_after_change = sum([d.get("required_qty") for d in po.as_dict().get("supplied_items")])
+
+		self.assertEqual(total_reqd_qty_after_change, 2 * total_reqd_qty)
+
 	def test_update_qty(self):
 		po = create_purchase_order()
 
diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py
index 5244777558a..9093cd570a2 100644
--- a/erpnext/controllers/accounts_controller.py
+++ b/erpnext/controllers/accounts_controller.py
@@ -1169,8 +1169,9 @@ def set_sales_order_defaults(parent_doctype, parent_doctype_name, child_docname,
 	child_item.item_name = item.item_name
 	child_item.description = item.description
 	child_item.delivery_date = trans_item.get('delivery_date') or p_doc.delivery_date
-	child_item.conversion_factor = flt(trans_item.get('conversion_factor')) or get_conversion_factor(item.item_code, item.stock_uom).get("conversion_factor") or 1.0
-	child_item.uom = item.stock_uom
+	child_item.uom = trans_item.get("uom") or item.stock_uom
+	conversion_factor = flt(get_conversion_factor(item.item_code, child_item.uom).get("conversion_factor"))
+	child_item.conversion_factor = flt(trans_item.get('conversion_factor')) or conversion_factor
 	child_item.warehouse = get_item_warehouse(item, p_doc, overwrite_warehouse=True)
 	if not child_item.warehouse:
 		frappe.throw(_("Cannot find {} for item {}. Please set the same in Item Master or Stock Settings.")
@@ -1189,8 +1190,9 @@ def set_purchase_order_defaults(parent_doctype, parent_doctype_name, child_docna
 	child_item.item_name = item.item_name
 	child_item.description = item.description
 	child_item.schedule_date = trans_item.get('schedule_date') or p_doc.schedule_date
-	child_item.conversion_factor = flt(trans_item.get('conversion_factor')) or get_conversion_factor(item.item_code, item.stock_uom).get("conversion_factor") or 1.0
-	child_item.uom = item.stock_uom
+	child_item.uom = trans_item.get("uom") or item.stock_uom
+	conversion_factor = flt(get_conversion_factor(item.item_code, child_item.uom).get("conversion_factor"))
+	child_item.conversion_factor = flt(trans_item.get('conversion_factor')) or conversion_factor
 	child_item.base_rate = 1 # Initiallize value will update in parent validation
 	child_item.base_amount = 1 # Initiallize value will update in parent validation
 	return child_item
@@ -1282,6 +1284,7 @@ def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, chil
 			prev_rate, new_rate = flt(child_item.get("rate")), flt(d.get("rate"))
 			prev_qty, new_qty = flt(child_item.get("qty")), flt(d.get("qty"))
 			prev_con_fac, new_con_fac = flt(child_item.get("conversion_factor")), flt(d.get("conversion_factor"))
+			prev_uom, new_uom = child_item.get("uom"), d.get("uom")
 
 			if parent_doctype == 'Sales Order':
 				prev_date, new_date = child_item.get("delivery_date"), d.get("delivery_date")
@@ -1290,9 +1293,10 @@ def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, chil
 
 			rate_unchanged = prev_rate == new_rate
 			qty_unchanged = prev_qty == new_qty
+			uom_unchanged = prev_uom == new_uom
 			conversion_factor_unchanged = prev_con_fac == new_con_fac
 			date_unchanged = prev_date == new_date if prev_date and new_date else False # in case of delivery note etc
-			if rate_unchanged and qty_unchanged and conversion_factor_unchanged and date_unchanged:
+			if rate_unchanged and qty_unchanged and conversion_factor_unchanged and uom_unchanged and date_unchanged:
 				continue
 
 		validate_quantity(child_item, d)
@@ -1311,6 +1315,11 @@ def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, chil
 				child_item.conversion_factor = 1
 			else:
 				child_item.conversion_factor = flt(d.get('conversion_factor'))
+		
+		if d.get("uom"):
+			child_item.uom = d.get("uom")
+			conversion_factor = flt(get_conversion_factor(child_item.item_code, child_item.uom).get("conversion_factor"))
+			child_item.conversion_factor = flt(d.get('conversion_factor')) or conversion_factor
 
 		if d.get("delivery_date") and parent_doctype == 'Sales Order':
 			child_item.delivery_date = d.get('delivery_date')
@@ -1377,6 +1386,8 @@ def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, chil
 		parent.update_receiving_percentage()
 		if parent.is_subcontracted == "Yes":
 			parent.update_reserved_qty_for_subcontract()
+			parent.create_raw_materials_supplied("supplied_items")
+			parent.save()
 	else:
 		parent.update_reserved_qty()
 		parent.update_project()
diff --git a/erpnext/public/js/utils.js b/erpnext/public/js/utils.js
index bcab0d84c62..87982f14a6e 100755
--- a/erpnext/public/js/utils.js
+++ b/erpnext/public/js/utils.js
@@ -466,6 +466,33 @@ erpnext.utils.update_child_items = function(opts) {
 		read_only: 0,
 		disabled: 0,
 		label: __('Item Code')
+	}, {
+		fieldtype:'Link',
+		fieldname:'uom',
+		options: 'UOM',
+		read_only: 0,
+		label: __('UOM'),
+		reqd: 1,
+		onchange: function () {
+			frappe.call({
+				method: "erpnext.stock.get_item_details.get_conversion_factor",
+				args: { item_code: this.doc.item_code, uom: this.value },
+				callback: r => {
+					if(!r.exc) {
+						if (this.doc.conversion_factor == r.message.conversion_factor) return;
+						
+						const docname = this.doc.docname;
+						dialog.fields_dict.trans_items.df.data.some(doc => {
+							if (doc.docname == docname) {
+								doc.conversion_factor = r.message.conversion_factor;
+								dialog.fields_dict.trans_items.grid.refresh();
+								return true;
+							}
+						})
+					}
+				}
+			});
+		}
 	}, {
 		fieldtype:'Float',
 		fieldname:"qty",
@@ -546,6 +573,7 @@ erpnext.utils.update_child_items = function(opts) {
 			"conversion_factor": d.conversion_factor,
 			"qty": d.qty,
 			"rate": d.rate,
+			"uom": d.uom
 		});
 		this.data = dialog.fields_dict.trans_items.df.data;
 		dialog.fields_dict.trans_items.grid.refresh();
diff --git a/erpnext/selling/doctype/sales_order/test_sales_order.py b/erpnext/selling/doctype/sales_order/test_sales_order.py
index 34aaf083959..735b071f443 100644
--- a/erpnext/selling/doctype/sales_order/test_sales_order.py
+++ b/erpnext/selling/doctype/sales_order/test_sales_order.py
@@ -318,7 +318,7 @@ class TestSalesOrder(unittest.TestCase):
 		self.assertEqual(get_reserved_qty("_Test Item Home Desktop 100"),
 			existing_reserved_qty_item2 + 20)
 
-	def test_add_new_item_in_update_child_qty_rate(self):
+	def test_update_child_adding_new_item(self):
 		so = make_sales_order(item_code= "_Test Item", qty=4)
 		create_dn_against_so(so.name, 4)
 		make_sales_invoice(so.name)
@@ -338,7 +338,7 @@ class TestSalesOrder(unittest.TestCase):
 		self.assertEqual(so.get("items")[-1].amount, 1400)
 		self.assertEqual(so.status, 'To Deliver and Bill')
 
-	def test_remove_item_in_update_child_qty_rate(self):
+	def test_update_child_removing_item(self):
 		so = make_sales_order(**{
 			"item_list": [{
 				"item_code": '_Test Item',
@@ -381,7 +381,7 @@ class TestSalesOrder(unittest.TestCase):
 		self.assertEqual(so.status, 'To Deliver and Bill')
 
 
-	def test_update_child_qty_rate(self):
+	def test_update_child(self):
 		so = make_sales_order(item_code= "_Test Item", qty=4)
 		create_dn_against_so(so.name, 4)
 		make_sales_invoice(so.name)
@@ -402,7 +402,7 @@ class TestSalesOrder(unittest.TestCase):
 		trans_item = json.dumps([{'item_code' : '_Test Item', 'rate' : 200, 'qty' : 2, 'docname': so.items[0].name}])
 		self.assertRaises(frappe.ValidationError, update_child_qty_rate,'Sales Order', trans_item, so.name)
 
-	def test_update_child_qty_rate_perm(self):
+	def test_update_child_perm(self):
 		so = make_sales_order(item_code= "_Test Item", qty=4)
 
 		user = 'test@example.com'
@@ -454,7 +454,7 @@ class TestSalesOrder(unittest.TestCase):
 		workflow.is_active = 0
 		workflow.save()
 
-	def test_update_child_qty_rate_product_bundle(self):
+	def test_update_child_product_bundle(self):
 		# test Update Items with product bundle
 		if not frappe.db.exists("Item", "_Product Bundle Item"):
 			bundle_item = make_item("_Product Bundle Item", {"is_stock_item": 0})
@@ -474,6 +474,20 @@ class TestSalesOrder(unittest.TestCase):
 		so.reload()
 		self.assertEqual(so.packed_items[0].qty, 4)
 
+		# test uom and conversion factor change
+		update_uom_conv_factor = json.dumps([{
+			'item_code': so.get("items")[0].item_code,
+			'rate': so.get("items")[0].rate,
+			'qty': so.get("items")[0].qty,
+			'uom': "_Test UOM 1",
+			'conversion_factor': 2,
+			'docname': so.get("items")[0].name
+		}])
+		update_child_qty_rate('Sales Order', update_uom_conv_factor, so.name)
+
+		so.reload()
+		self.assertEqual(so.packed_items[0].qty, 8)
+
 	def test_warehouse_user(self):
 		frappe.permissions.add_user_permission("Warehouse", "_Test Warehouse 1 - _TC", "test@example.com")
 		frappe.permissions.add_user_permission("Warehouse", "_Test Warehouse 2 - _TC1", "test2@example.com")
