From 933434c3eab5fc42c1ce92d2ada7c1bffeea282a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 29 May 2022 22:09:32 +0530 Subject: [PATCH 1/6] chore: format --- .../quality_inspection_summary/quality_inspection_summary.py | 1 - 1 file changed, 1 deletion(-) diff --git a/erpnext/manufacturing/report/quality_inspection_summary/quality_inspection_summary.py b/erpnext/manufacturing/report/quality_inspection_summary/quality_inspection_summary.py index c324172372..de96a6c032 100644 --- a/erpnext/manufacturing/report/quality_inspection_summary/quality_inspection_summary.py +++ b/erpnext/manufacturing/report/quality_inspection_summary/quality_inspection_summary.py @@ -34,7 +34,6 @@ def get_data(filters): if filters.get(field): query_filters[field] = ("in", filters.get(field)) - query_filters["report_date"] = ["between", [filters.get("from_date"), filters.get("to_date")]] return frappe.get_all( From 85b48fcdb9f0afb79d958c006925088cd1928c21 Mon Sep 17 00:00:00 2001 From: Devin Slauenwhite Date: Sun, 29 May 2022 12:49:09 -0400 Subject: [PATCH 2/6] fix: barcode scan resolve after model is updated (#31058) * fix: resolve row after model is updated. * fix: wait for all fields in the model to be updated. * fix: sider * pref: clear scanned code after capturing value * fix: use frappe.run_serially --- erpnext/public/js/utils/barcode_scanner.js | 88 +++++++++++----------- 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/erpnext/public/js/utils/barcode_scanner.js b/erpnext/public/js/utils/barcode_scanner.js index d378118564..eea91ef5fe 100644 --- a/erpnext/public/js/utils/barcode_scanner.js +++ b/erpnext/public/js/utils/barcode_scanner.js @@ -35,6 +35,7 @@ erpnext.utils.BarcodeScanner = class BarcodeScanner { let me = this; const input = this.scan_barcode_field.value; + this.scan_barcode_field.set_value(""); if (!input) { return; } @@ -55,51 +56,51 @@ erpnext.utils.BarcodeScanner = class BarcodeScanner { return; } - const row = me.update_table(data); - if (row) { - resolve(row); - } - else { - reject(); - } + me.update_table(data).then(row => { + row ? resolve(row) : reject(); + }); }); }); } update_table(data) { - let cur_grid = this.frm.fields_dict[this.items_table_name].grid; + return new Promise(resolve => { + let cur_grid = this.frm.fields_dict[this.items_table_name].grid; - const {item_code, barcode, batch_no, serial_no} = data; + const {item_code, barcode, batch_no, serial_no} = data; - let row = this.get_row_to_modify_on_scan(item_code, batch_no); + let row = this.get_row_to_modify_on_scan(item_code, batch_no); - if (!row) { - if (this.dont_allow_new_row) { - this.show_alert(__("Maximum quantity scanned for item {0}.", [item_code]), "red"); + if (!row) { + if (this.dont_allow_new_row) { + this.show_alert(__("Maximum quantity scanned for item {0}.", [item_code]), "red"); + this.clean_up(); + return; + } + + // add new row if new item/batch is scanned + row = frappe.model.add_child(this.frm.doc, cur_grid.doctype, this.items_table_name); + // trigger any row add triggers defined on child table. + this.frm.script_manager.trigger(`${this.items_table_name}_add`, row.doctype, row.name); + } + + if (this.is_duplicate_serial_no(row, serial_no)) { this.clean_up(); return; } - // add new row if new item/batch is scanned - row = frappe.model.add_child(this.frm.doc, cur_grid.doctype, this.items_table_name); - // trigger any row add triggers defined on child table. - this.frm.script_manager.trigger(`${this.items_table_name}_add`, row.doctype, row.name); - } - - if (this.is_duplicate_serial_no(row, serial_no)) { - this.clean_up(); - return; - } - - this.set_selector_trigger_flag(row, data); - this.set_item(row, item_code).then(qty => { - this.show_scan_message(row.idx, row.item_code, qty); + frappe.run_serially([ + () => this.set_selector_trigger_flag(row, data), + () => this.set_item(row, item_code).then(qty => { + this.show_scan_message(row.idx, row.item_code, qty); + }), + () => this.set_serial_no(row, serial_no), + () => this.set_batch_no(row, batch_no), + () => this.set_barcode(row, barcode), + () => this.clean_up(), + () => resolve(row) + ]); }); - this.set_serial_no(row, serial_no); - this.set_batch_no(row, batch_no); - this.set_barcode(row, barcode); - this.clean_up(); - return row; } // batch and serial selector is reduandant when all info can be added by scan @@ -117,25 +118,24 @@ erpnext.utils.BarcodeScanner = class BarcodeScanner { set_item(row, item_code) { return new Promise(resolve => { - const increment = (value = 1) => { + const increment = async (value = 1) => { const item_data = {item_code: item_code}; item_data[this.qty_field] = Number((row[this.qty_field] || 0)) + Number(value); - frappe.model.set_value(row.doctype, row.name, item_data); + await frappe.model.set_value(row.doctype, row.name, item_data); + return value; }; if (this.prompt_qty) { frappe.prompt(__("Please enter quantity for item {0}", [item_code]), ({value}) => { - increment(value); - resolve(value); + increment(value).then((value) => resolve(value)); }); } else { - increment(); - resolve(); + increment().then((value) => resolve(value)); } }); } - set_serial_no(row, serial_no) { + async set_serial_no(row, serial_no) { if (serial_no && frappe.meta.has_field(row.doctype, this.serial_no_field)) { const existing_serial_nos = row[this.serial_no_field]; let new_serial_nos = ""; @@ -145,19 +145,19 @@ erpnext.utils.BarcodeScanner = class BarcodeScanner { } else { new_serial_nos = serial_no; } - frappe.model.set_value(row.doctype, row.name, this.serial_no_field, new_serial_nos); + await frappe.model.set_value(row.doctype, row.name, this.serial_no_field, new_serial_nos); } } - set_batch_no(row, batch_no) { + async set_batch_no(row, batch_no) { if (batch_no && frappe.meta.has_field(row.doctype, this.batch_no_field)) { - frappe.model.set_value(row.doctype, row.name, this.batch_no_field, batch_no); + await frappe.model.set_value(row.doctype, row.name, this.batch_no_field, batch_no); } } - set_barcode(row, barcode) { + async set_barcode(row, barcode) { if (barcode && frappe.meta.has_field(row.doctype, this.barcode_field)) { - frappe.model.set_value(row.doctype, row.name, this.barcode_field, barcode); + await frappe.model.set_value(row.doctype, row.name, this.barcode_field, barcode); } } From b170cec2fe84f4fc8d0908979194a61f40df0f74 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 29 May 2022 14:32:32 +0530 Subject: [PATCH 3/6] fix(ux): "New Version" button BOM "duplicate" technically creates a new version but that's not intuitive at all. --- erpnext/manufacturing/doctype/bom/bom.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/erpnext/manufacturing/doctype/bom/bom.js b/erpnext/manufacturing/doctype/bom/bom.js index 3d96f9c9c7..d74379881c 100644 --- a/erpnext/manufacturing/doctype/bom/bom.js +++ b/erpnext/manufacturing/doctype/bom/bom.js @@ -93,6 +93,11 @@ frappe.ui.form.on("BOM", { }); } + frm.add_custom_button(__("New Version"), function() { + let new_bom = frappe.model.copy_doc(frm.doc); + frappe.set_route("Form", "BOM", new_bom.name); + }); + if(frm.doc.docstatus==1) { frm.add_custom_button(__("Work Order"), function() { frm.trigger("make_work_order"); From d224bf1d3450ab0d63627f19b7f707074d8a1716 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 30 May 2022 10:33:58 +0530 Subject: [PATCH 4/6] fix: only erase BOM when do_not_explode is set --- erpnext/manufacturing/doctype/bom/bom.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index 220ce1dbd8..3560c32166 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -251,9 +251,8 @@ class BOM(WebsiteGenerator): for item in self.get("items"): self.validate_bom_currency(item) - item.bom_no = "" - if not item.do_not_explode: - item.bom_no = item.bom_no + if item.do_not_explode: + item.bom_no = "" ret = self.get_bom_material_detail( { From 954dac88a85f7760225348d5655c65c71f89825f Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 29 May 2022 14:11:45 +0530 Subject: [PATCH 5/6] fix: allow non-explosive recrusive BOMs Recursion should be allowed as long as child item is not "exploded" further by a BOM. --- erpnext/manufacturing/doctype/bom/bom.py | 44 +++++++++---------- erpnext/manufacturing/doctype/bom/test_bom.py | 33 ++++++-------- 2 files changed, 33 insertions(+), 44 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index 3560c32166..6376359a70 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -22,6 +22,10 @@ from erpnext.stock.get_item_details import get_conversion_factor, get_price_list form_grid_templates = {"items": "templates/form_grid/item_grid.html"} +class BOMRecursionError(frappe.ValidationError): + pass + + class BOMTree: """Full tree representation of a BOM""" @@ -554,35 +558,27 @@ class BOM(WebsiteGenerator): """Check whether recursion occurs in any bom""" def _throw_error(bom_name): - frappe.throw(_("BOM recursion: {0} cannot be parent or child of {0}").format(bom_name)) + frappe.throw( + _("BOM recursion: {1} cannot be parent or child of {0}").format(self.name, bom_name), + exc=BOMRecursionError, + ) bom_list = self.traverse_tree() - child_items = ( - frappe.get_all( - "BOM Item", - fields=["bom_no", "item_code"], - filters={"parent": ("in", bom_list), "parenttype": "BOM"}, - ) - or [] + child_items = frappe.get_all( + "BOM Item", + fields=["bom_no", "item_code"], + filters={"parent": ("in", bom_list), "parenttype": "BOM"}, ) - child_bom = {d.bom_no for d in child_items} - child_items_codes = {d.item_code for d in child_items} + for item in child_items: + if self.name == item.bom_no: + _throw_error(self.name) + if self.item == item.item_code and item.bom_no: + # Same item but with different BOM should not be allowed. + # Same item can appear recursively once as long as it doesn't have BOM. + _throw_error(item.bom_no) - if self.name in child_bom: - _throw_error(self.name) - - if self.item in child_items_codes: - _throw_error(self.item) - - bom_nos = ( - frappe.get_all( - "BOM Item", fields=["parent"], filters={"bom_no": self.name, "parenttype": "BOM"} - ) - or [] - ) - - if self.name in {d.parent for d in bom_nos}: + if self.name in {d.bom_no for d in self.items}: _throw_error(self.name) def traverse_tree(self, bom_list=None): diff --git a/erpnext/manufacturing/doctype/bom/test_bom.py b/erpnext/manufacturing/doctype/bom/test_bom.py index 62fc0724e0..f235e449a3 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -10,7 +10,7 @@ from frappe.tests.utils import FrappeTestCase from frappe.utils import cstr, flt from erpnext.buying.doctype.purchase_order.test_purchase_order import create_purchase_order -from erpnext.manufacturing.doctype.bom.bom import item_query, make_variant_bom +from erpnext.manufacturing.doctype.bom.bom import BOMRecursionError, item_query, make_variant_bom from erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool import update_cost from erpnext.stock.doctype.item.test_item import make_item from erpnext.stock.doctype.stock_reconciliation.test_stock_reconciliation import ( @@ -324,43 +324,36 @@ class TestBOM(FrappeTestCase): def test_bom_recursion_1st_level(self): """BOM should not allow BOM item again in child""" - item_code = "_Test BOM Recursion" - make_item(item_code, {"is_stock_item": 1}) + item_code = make_item(properties={"is_stock_item": 1}).name bom = frappe.new_doc("BOM") bom.item = item_code bom.append("items", frappe._dict(item_code=item_code)) - with self.assertRaises(frappe.ValidationError) as err: + bom.save() + with self.assertRaises(BOMRecursionError): + bom.items[0].bom_no = bom.name bom.save() - self.assertTrue("recursion" in str(err.exception).lower()) - frappe.delete_doc("BOM", bom.name, ignore_missing=True) - def test_bom_recursion_transitive(self): - item1 = "_Test BOM Recursion" - item2 = "_Test BOM Recursion 2" - make_item(item1, {"is_stock_item": 1}) - make_item(item2, {"is_stock_item": 1}) + item1 = make_item(properties={"is_stock_item": 1}).name + item2 = make_item(properties={"is_stock_item": 1}).name bom1 = frappe.new_doc("BOM") bom1.item = item1 bom1.append("items", frappe._dict(item_code=item2)) bom1.save() - bom1.submit() bom2 = frappe.new_doc("BOM") bom2.item = item2 bom2.append("items", frappe._dict(item_code=item1)) + bom2.save() - with self.assertRaises(frappe.ValidationError) as err: + bom2.items[0].bom_no = bom1.name + bom1.items[0].bom_no = bom2.name + + with self.assertRaises(BOMRecursionError): + bom1.save() bom2.save() - bom2.submit() - - self.assertTrue("recursion" in str(err.exception).lower()) - - bom1.cancel() - frappe.delete_doc("BOM", bom1.name, ignore_missing=True, force=True) - frappe.delete_doc("BOM", bom2.name, ignore_missing=True, force=True) def test_bom_with_process_loss_item(self): fg_item_non_whole, fg_item_whole, bom_item = create_process_loss_bom_items() From c02598a51bb38817401e62e305faf82da857ff6d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 30 May 2022 11:12:17 +0530 Subject: [PATCH 6/6] chore: remove framework tests from erpnext Similar tests exist in FW and this is failing because someone updated the translations --- erpnext/tests/test_search.py | 19 ------------------- 1 file changed, 19 deletions(-) delete mode 100644 erpnext/tests/test_search.py diff --git a/erpnext/tests/test_search.py b/erpnext/tests/test_search.py deleted file mode 100644 index 3685828667..0000000000 --- a/erpnext/tests/test_search.py +++ /dev/null @@ -1,19 +0,0 @@ -import unittest - -import frappe -from frappe.contacts.address_and_contact import filter_dynamic_link_doctypes - - -class TestSearch(unittest.TestCase): - # Search for the word "cond", part of the word "conduire" (Lead) in french. - def test_contact_search_in_foreign_language(self): - try: - frappe.local.lang_full_dict = None # reset cached translations - frappe.local.lang = "fr" - output = filter_dynamic_link_doctypes( - "DocType", "cond", "name", 0, 20, {"fieldtype": "HTML", "fieldname": "contact_html"} - ) - result = [["found" for x in y if x == "Lead"] for y in output] - self.assertTrue(["found"] in result) - finally: - frappe.local.lang = "en"