From d24458ab774571c7c1cdcf5e0b974f0b55eafbfa Mon Sep 17 00:00:00 2001 From: Marica Date: Thu, 24 Mar 2022 12:56:57 +0530 Subject: [PATCH 1/6] fix: (ux) Add is_group=0 filter on website warehouse (#30396) - It does not support group warehouses right now and it is misleading --- erpnext/e_commerce/doctype/website_item/website_item.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/erpnext/e_commerce/doctype/website_item/website_item.js b/erpnext/e_commerce/doctype/website_item/website_item.js index 741e78f4a55..7108cabfb3f 100644 --- a/erpnext/e_commerce/doctype/website_item/website_item.js +++ b/erpnext/e_commerce/doctype/website_item/website_item.js @@ -5,6 +5,12 @@ frappe.ui.form.on('Website Item', { onload: function(frm) { // should never check Private frm.fields_dict["website_image"].df.is_private = 0; + + frm.set_query("website_warehouse", () => { + return { + filters: {"is_group": 0} + }; + }); }, image: function() { From 3d43c437adb596f840f34fe89d4cbcb67957c6fb Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 24 Mar 2022 13:58:23 +0530 Subject: [PATCH 2/6] fix: only validate qty for main non-subassy items --- erpnext/manufacturing/doctype/work_order/work_order.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/erpnext/manufacturing/doctype/work_order/work_order.py b/erpnext/manufacturing/doctype/work_order/work_order.py index 8d7ab85b66a..e832ac9c7f8 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.py +++ b/erpnext/manufacturing/doctype/work_order/work_order.py @@ -457,7 +457,8 @@ class WorkOrder(Document): mr_obj.update_requested_qty([self.material_request_item]) def update_ordered_qty(self): - if self.production_plan and self.production_plan_item: + if self.production_plan and self.production_plan_item \ + and not self.production_plan_sub_assembly_item: qty = frappe.get_value("Production Plan Item", self.production_plan_item, "ordered_qty") or 0.0 if self.docstatus == 1: @@ -644,9 +645,13 @@ class WorkOrder(Document): if not self.qty > 0: frappe.throw(_("Quantity to Manufacture must be greater than 0.")) - if self.production_plan and self.production_plan_item: + if self.production_plan and self.production_plan_item \ + and not self.production_plan_sub_assembly_item: qty_dict = frappe.db.get_value("Production Plan Item", self.production_plan_item, ["planned_qty", "ordered_qty"], as_dict=1) + if not qty_dict: + return + allowance_qty = flt(frappe.db.get_single_value("Manufacturing Settings", "overproduction_percentage_for_work_order"))/100 * qty_dict.get("planned_qty", 0) From 5b1d6055e6eba72a092ae99a9e49cc671c2e8b08 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 24 Mar 2022 14:50:13 +0530 Subject: [PATCH 3/6] fix: subassembly items linked to temporary name Production Plan tables for po_items and sub_assembly_items are prepared client side so both dont exist at time of first save or modifying and hence any "links" created are invalid. This change retains temporary name so it can be relinked server side after naming is performed. Co-Authored-By: Marica --- .../production_plan/production_plan.js | 7 ++++ .../production_plan/production_plan.py | 13 ++++++++ .../production_plan/test_production_plan.py | 33 +++++++++++++++++++ .../production_plan_item.json | 15 +++++++-- erpnext/patches.txt | 2 +- 5 files changed, 66 insertions(+), 4 deletions(-) diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.js b/erpnext/manufacturing/doctype/production_plan/production_plan.js index e8759f55284..59ddf1f0c58 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.js +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.js @@ -2,6 +2,13 @@ // For license information, please see license.txt frappe.ui.form.on('Production Plan', { + + before_save: function(frm) { + // preserve temporary names on production plan item to re-link sub-assembly items + frm.doc.po_items.forEach(item => { + item.temporary_name = item.name; + }); + }, setup: function(frm) { frm.custom_make_buttons = { 'Work Order': 'Work Order / Subcontract PO', diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.py b/erpnext/manufacturing/doctype/production_plan/production_plan.py index 2b6e6968bd3..b88b24fb256 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.py @@ -32,6 +32,7 @@ class ProductionPlan(Document): self.set_pending_qty_in_row_without_reference() self.calculate_total_planned_qty() self.set_status() + self._rename_temporary_references() def set_pending_qty_in_row_without_reference(self): "Set Pending Qty in independent rows (not from SO or MR)." @@ -57,6 +58,18 @@ class ProductionPlan(Document): if not flt(d.planned_qty): frappe.throw(_("Please enter Planned Qty for Item {0} at row {1}").format(d.item_code, d.idx)) + def _rename_temporary_references(self): + """ po_items and sub_assembly_items items are both constructed client side without saving. + + Attempt to fix linkages by using temporary names to map final row names. + """ + new_name_map = {d.temporary_name: d.name for d in self.po_items if d.temporary_name} + actual_names = set(new_name_map.values()) + + for sub_assy in self.sub_assembly_items: + if sub_assy.production_plan_item not in actual_names: + sub_assy.production_plan_item = new_name_map.get(sub_assy.production_plan_item) + @frappe.whitelist() def get_open_sales_orders(self): """ Pull sales orders which are pending to deliver based on criteria selected""" diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index 6425374b1b2..ec497035fe5 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -667,6 +667,39 @@ class TestProductionPlan(FrappeTestCase): wo_doc.submit() self.assertEqual(wo_doc.qty, 0.55) + def test_temporary_name_relinking(self): + + pp = frappe.new_doc("Production Plan") + + # this can not be unittested so mocking data that would be expected + # from client side. + for _ in range(10): + po_item = pp.append("po_items", { + "name": frappe.generate_hash(length=10), + "temporary_name": frappe.generate_hash(length=10), + }) + pp.append("sub_assembly_items", { + "production_plan_item": po_item.temporary_name + }) + pp._rename_temporary_references() + + for po_item, subassy_item in zip(pp.po_items, pp.sub_assembly_items): + self.assertEqual(po_item.name, subassy_item.production_plan_item) + + # bad links should be erased + pp.append("sub_assembly_items", { + "production_plan_item": frappe.generate_hash(length=16) + }) + pp._rename_temporary_references() + self.assertIsNone(pp.sub_assembly_items[-1].production_plan_item) + pp.sub_assembly_items.pop() + + # reattempting on same doc shouldn't change anything + pp._rename_temporary_references() + for po_item, subassy_item in zip(pp.po_items, pp.sub_assembly_items): + self.assertEqual(po_item.name, subassy_item.production_plan_item) + + def create_production_plan(**args): """ sales_order (obj): Sales Order Doc Object diff --git a/erpnext/manufacturing/doctype/production_plan_item/production_plan_item.json b/erpnext/manufacturing/doctype/production_plan_item/production_plan_item.json index f829d57475a..df5862fcac8 100644 --- a/erpnext/manufacturing/doctype/production_plan_item/production_plan_item.json +++ b/erpnext/manufacturing/doctype/production_plan_item/production_plan_item.json @@ -27,7 +27,8 @@ "material_request", "material_request_item", "product_bundle_item", - "item_reference" + "item_reference", + "temporary_name" ], "fields": [ { @@ -204,17 +205,25 @@ "fieldtype": "Data", "hidden": 1, "label": "Item Reference" + }, + { + "fieldname": "temporary_name", + "fieldtype": "Data", + "hidden": 1, + "label": "temporary name" } ], "idx": 1, "istable": 1, "links": [], - "modified": "2021-06-28 18:31:06.822168", + "modified": "2022-03-24 04:54:09.940224", "modified_by": "Administrator", "module": "Manufacturing", "name": "Production Plan Item", + "naming_rule": "Random", "owner": "Administrator", "permissions": [], "sort_field": "modified", - "sort_order": "ASC" + "sort_order": "ASC", + "states": [] } \ No newline at end of file diff --git a/erpnext/patches.txt b/erpnext/patches.txt index 31999126214..35aac549d04 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -360,4 +360,4 @@ erpnext.patches.v14_0.update_batch_valuation_flag erpnext.patches.v14_0.delete_non_profit_doctypes erpnext.patches.v14_0.update_employee_advance_status erpnext.patches.v13_0.add_cost_center_in_loans -erpnext.patches.v13_0.remove_unknown_links_to_prod_plan_items +erpnext.patches.v13_0.remove_unknown_links_to_prod_plan_items # 24-03-2022 From d4ee31dc8f42577d7f3b0c9b368c710edf3e290e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 24 Mar 2022 17:58:22 +0530 Subject: [PATCH 4/6] fix: consider all existing PO items When this is sent from API/client side doesn't send temporary_name it can be flaky. Hence, use all available names for validation. --- .../manufacturing/doctype/production_plan/production_plan.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.py b/erpnext/manufacturing/doctype/production_plan/production_plan.py index b88b24fb256..349f40e9c7b 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.py @@ -64,7 +64,7 @@ class ProductionPlan(Document): Attempt to fix linkages by using temporary names to map final row names. """ new_name_map = {d.temporary_name: d.name for d in self.po_items if d.temporary_name} - actual_names = set(new_name_map.values()) + actual_names = {d.name for d in self.po_items} for sub_assy in self.sub_assembly_items: if sub_assy.production_plan_item not in actual_names: From 10d5fb8cd157719101c5c0cbcdcbd5b7795c83b7 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 24 Mar 2022 23:32:55 +0530 Subject: [PATCH 5/6] test: check printviews for all docs --- erpnext/tests/test_zform_loads.py | 48 ++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/erpnext/tests/test_zform_loads.py b/erpnext/tests/test_zform_loads.py index b6fb6366878..5b82c7bbdd7 100644 --- a/erpnext/tests/test_zform_loads.py +++ b/erpnext/tests/test_zform_loads.py @@ -1,13 +1,14 @@ -""" dumb test to check all function calls on known form loads """ - -import unittest +""" smoak tests to check basic functionality calls on known form loads.""" import frappe from frappe.desk.form.load import getdoc +from frappe.tests.utils import FrappeTestCase, change_settings +from frappe.www.printview import get_html_and_style -class TestFormLoads(unittest.TestCase): +class TestFormLoads(FrappeTestCase): + @change_settings("Print Settings", {"allow_print_for_cancelled": 1}) def test_load(self): erpnext_modules = frappe.get_all("Module Def", filters={"app_name": "erpnext"}, pluck="name") doctypes = frappe.get_all("DocType", {"istable": 0, "issingle": 0, "is_virtual": 0, "module": ("in", erpnext_modules)}, pluck="name") @@ -17,14 +18,35 @@ class TestFormLoads(unittest.TestCase): if not last_doc: continue with self.subTest(msg=f"Loading {doctype} - {last_doc}", doctype=doctype, last_doc=last_doc): - try: - # reset previous response - frappe.response = frappe._dict({"docs":[]}) - frappe.response.docinfo = None + self.assertFormLoad(doctype, last_doc) + self.assertDocPrint(doctype, last_doc) - getdoc(doctype, last_doc) - except Exception as e: - self.fail(f"Failed to load {doctype} - {last_doc}: {e}") + def assertFormLoad(self, doctype, docname): + # reset previous response + frappe.response = frappe._dict({"docs":[]}) + frappe.response.docinfo = None - self.assertTrue(frappe.response.docs, msg=f"expected document in reponse, found: {frappe.response.docs}") - self.assertTrue(frappe.response.docinfo, msg=f"expected docinfo in reponse, found: {frappe.response.docinfo}") + try: + getdoc(doctype, docname) + except Exception as e: + self.fail(f"Failed to load {doctype}-{docname}: {e}") + + self.assertTrue(frappe.response.docs, msg=f"expected document in reponse, found: {frappe.response.docs}") + self.assertTrue(frappe.response.docinfo, msg=f"expected docinfo in reponse, found: {frappe.response.docinfo}") + + def assertDocPrint(self, doctype, docname): + doc = frappe.get_doc(doctype, docname) + doc.set("__onload", frappe._dict()) + doc.run_method("onload") + + messages_before = frappe.get_message_log() + ret = get_html_and_style(doc=doc.as_json(), print_format="Standard", no_letterhead=1) + messages_after = frappe.get_message_log() + + if len(messages_after) > len(messages_before): + new_messages = messages_after[len(messages_before):] + self.fail("Print view showing error/warnings: \n" + + "\n".join(str(msg) for msg in new_messages)) + + # html should exist + self.assertTrue(bool(ret["html"])) From 788d4927570afc6725758399ae3c4589a26f9dc7 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 25 Mar 2022 00:01:23 +0530 Subject: [PATCH 6/6] test: basic item and wh capacity dashboard tests --- erpnext/stock/dashboard/item_dashboard.py | 11 ++++------- erpnext/stock/doctype/item/test_item.py | 6 ++++++ .../doctype/putaway_rule/test_putaway_rule.py | 15 +++++++++++++++ 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/erpnext/stock/dashboard/item_dashboard.py b/erpnext/stock/dashboard/item_dashboard.py index 57d78a21e14..75aa1d36c7b 100644 --- a/erpnext/stock/dashboard/item_dashboard.py +++ b/erpnext/stock/dashboard/item_dashboard.py @@ -40,18 +40,15 @@ def get_data(item_code=None, warehouse=None, item_group=None, filters=filters, order_by=sort_by + ' ' + sort_order, limit_start=start, - limit_page_length='21') + limit_page_length=21) precision = cint(frappe.db.get_single_value("System Settings", "float_precision")) for item in items: item.update({ - 'item_name': frappe.get_cached_value( - "Item", item.item_code, 'item_name'), - 'disable_quick_entry': frappe.get_cached_value( - "Item", item.item_code, 'has_batch_no') - or frappe.get_cached_value( - "Item", item.item_code, 'has_serial_no'), + 'item_name': frappe.get_cached_value("Item", item.item_code, 'item_name'), + 'disable_quick_entry': frappe.get_cached_value( "Item", item.item_code, 'has_batch_no') + or frappe.get_cached_value( "Item", item.item_code, 'has_serial_no'), 'projected_qty': flt(item.projected_qty, precision), 'reserved_qty': flt(item.reserved_qty, precision), 'reserved_qty_for_production': flt(item.reserved_qty_for_production, precision), diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index 112420ff7b7..05e6e76e21d 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -685,6 +685,12 @@ class TestItem(FrappeTestCase): # standalone return make_purchase_receipt(is_return=True, qty=-1, **typical_args) + def test_item_dashboard(self): + from erpnext.stock.dashboard.item_dashboard import get_data + + self.assertTrue(get_data(item_code="_Test Item")) + self.assertTrue(get_data(warehouse="_Test Warehouse - _TC")) + self.assertTrue(get_data(item_group="All Item Groups")) def set_item_variant_settings(fields): diff --git a/erpnext/stock/doctype/putaway_rule/test_putaway_rule.py b/erpnext/stock/doctype/putaway_rule/test_putaway_rule.py index 4e8d71fe5e4..0ec812c655a 100644 --- a/erpnext/stock/doctype/putaway_rule/test_putaway_rule.py +++ b/erpnext/stock/doctype/putaway_rule/test_putaway_rule.py @@ -396,6 +396,21 @@ class TestPutawayRule(FrappeTestCase): rule_1.delete() rule_2.delete() + def test_warehouse_capacity_dashbord(self): + from erpnext.stock.dashboard.warehouse_capacity_dashboard import get_data + + item = "_Rice" + rule = create_putaway_rule(item_code=item, warehouse=self.warehouse_1, capacity=500, + uom="Kg") + + capacities = get_data(warehouse=self.warehouse_1) + for capacity in capacities: + if capacity.item_code == item and capacity.warehouse == self.warehouse_1: + self.assertEqual(capacity.stock_capacity, 500) + + get_data(warehouse=self.warehouse_1) + rule.delete() + def create_putaway_rule(**args): args = frappe._dict(args) putaway = frappe.new_doc("Putaway Rule")