From f57725f8fa016b9826e8fdf2f14dbf1a3d9991f7 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 9 Mar 2022 19:03:07 +0530 Subject: [PATCH 01/54] refactor: Add exception handling in background job within BOM Update Tool --- .../bom_update_tool/bom_update_tool.py | 41 ++++++++++++------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py index 0e3955f5a7..c7197347c2 100644 --- a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py +++ b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py @@ -94,20 +94,31 @@ def update_latest_price_in_all_boms(): update_cost() def replace_bom(args): - frappe.db.auto_commit_on_many_writes = 1 - args = frappe._dict(args) - - doc = frappe.get_doc("BOM Update Tool") - doc.current_bom = args.current_bom - doc.new_bom = args.new_bom - doc.replace_bom() - - frappe.db.auto_commit_on_many_writes = 0 + try: + frappe.db.auto_commit_on_many_writes = 1 + args = frappe._dict(args) + doc = frappe.get_doc("BOM Update Tool") + doc.current_bom = args.current_bom + doc.new_bom = args.new_bom + doc.replace_bom() + except Exception: + frappe.log_error( + msg=frappe.get_traceback(), + title=_("BOM Update Tool Error") + ) + finally: + frappe.db.auto_commit_on_many_writes = 0 def update_cost(): - frappe.db.auto_commit_on_many_writes = 1 - bom_list = get_boms_in_bottom_up_order() - for bom in bom_list: - frappe.get_doc("BOM", bom).update_cost(update_parent=False, from_child_bom=True) - - frappe.db.auto_commit_on_many_writes = 0 + try: + frappe.db.auto_commit_on_many_writes = 1 + bom_list = get_boms_in_bottom_up_order() + for bom in bom_list: + frappe.get_doc("BOM", bom).update_cost(update_parent=False, from_child_bom=True) + except Exception: + frappe.log_error( + msg=frappe.get_traceback(), + title=_("BOM Update Tool Error") + ) + finally: + frappe.db.auto_commit_on_many_writes = 0 From 4283a13e5a6a6b9f1e8e1cbcc639646a4e957b36 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 16 Mar 2022 19:45:03 +0530 Subject: [PATCH 02/54] feat: BOM Update Log - Created BOM Update Log that will handle queued job status and failures - Moved validation and BG job to thus new doctype - BOM Update Tool only works as an endpoint --- erpnext/hooks.py | 2 +- .../doctype/bom_update_log/__init__.py | 0 .../doctype/bom_update_log/bom_update_log.js | 8 ++ .../bom_update_log/bom_update_log.json | 101 +++++++++++++++ .../doctype/bom_update_log/bom_update_log.py | 117 ++++++++++++++++++ .../bom_update_log/test_bom_update_log.py | 9 ++ .../bom_update_tool/bom_update_tool.py | 61 +++------ 7 files changed, 254 insertions(+), 44 deletions(-) create mode 100644 erpnext/manufacturing/doctype/bom_update_log/__init__.py create mode 100644 erpnext/manufacturing/doctype/bom_update_log/bom_update_log.js create mode 100644 erpnext/manufacturing/doctype/bom_update_log/bom_update_log.json create mode 100644 erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py create mode 100644 erpnext/manufacturing/doctype/bom_update_log/test_bom_update_log.py diff --git a/erpnext/hooks.py b/erpnext/hooks.py index f8c42887fd..c3cc1e4d57 100644 --- a/erpnext/hooks.py +++ b/erpnext/hooks.py @@ -371,7 +371,7 @@ scheduler_events = { ], "daily_long": [ "erpnext.setup.doctype.email_digest.email_digest.send", - "erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool.update_latest_price_in_all_boms", + "erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool.auto_update_latest_price_in_all_boms", "erpnext.hr.doctype.leave_ledger_entry.leave_ledger_entry.process_expired_allocation", "erpnext.hr.utils.generate_leave_encashment", "erpnext.hr.utils.allocate_earned_leaves", diff --git a/erpnext/manufacturing/doctype/bom_update_log/__init__.py b/erpnext/manufacturing/doctype/bom_update_log/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.js b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.js new file mode 100644 index 0000000000..6da808e26d --- /dev/null +++ b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.js @@ -0,0 +1,8 @@ +// Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and contributors +// For license information, please see license.txt + +frappe.ui.form.on('BOM Update Log', { + // refresh: function(frm) { + + // } +}); diff --git a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.json b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.json new file mode 100644 index 0000000000..222168be8c --- /dev/null +++ b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.json @@ -0,0 +1,101 @@ +{ + "actions": [], + "autoname": "BOM-UPDT-LOG-.#####", + "creation": "2022-03-16 14:23:35.210155", + "description": "BOM Update Tool Log with job status maintained", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "current_bom", + "new_bom", + "column_break_3", + "update_type", + "status", + "amended_from" + ], + "fields": [ + { + "fieldname": "current_bom", + "fieldtype": "Link", + "in_list_view": 1, + "label": "Current BOM", + "options": "BOM", + "reqd": 1 + }, + { + "fieldname": "new_bom", + "fieldtype": "Link", + "in_list_view": 1, + "label": "New BOM", + "options": "BOM", + "reqd": 1 + }, + { + "fieldname": "column_break_3", + "fieldtype": "Column Break" + }, + { + "fieldname": "update_type", + "fieldtype": "Select", + "label": "Update Type", + "options": "Replace BOM\nUpdate Cost" + }, + { + "fieldname": "status", + "fieldtype": "Select", + "label": "Status", + "options": "Queued\nIn Progress\nCompleted\nFailed" + }, + { + "fieldname": "amended_from", + "fieldtype": "Link", + "label": "Amended From", + "no_copy": 1, + "options": "BOM Update Log", + "print_hide": 1, + "read_only": 1 + } + ], + "in_create": 1, + "index_web_pages_for_search": 1, + "is_submittable": 1, + "links": [], + "modified": "2022-03-16 18:25:49.833836", + "modified_by": "Administrator", + "module": "Manufacturing", + "name": "BOM Update Log", + "naming_rule": "Expression (old style)", + "owner": "Administrator", + "permissions": [ + { + "create": 1, + "delete": 1, + "email": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "System Manager", + "share": 1, + "submit": 1, + "write": 1 + }, + { + "create": 1, + "email": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "Manufacturing Manager", + "share": 1, + "submit": 1, + "write": 1 + } + ], + "sort_field": "modified", + "sort_order": "DESC", + "states": [], + "track_changes": 1 +} \ No newline at end of file diff --git a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py new file mode 100644 index 0000000000..10db0de9a1 --- /dev/null +++ b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py @@ -0,0 +1,117 @@ +# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and contributors +# For license information, please see license.txt + +import frappe +from frappe import _ +from frappe.model.document import Document +from frappe.utils import cstr + +from erpnext.manufacturing.doctype.bom.bom import get_boms_in_bottom_up_order + +from rq.timeouts import JobTimeoutException + + +class BOMMissingError(frappe.ValidationError): pass + +class BOMUpdateLog(Document): + def validate(self): + self.validate_boms_are_specified() + self.validate_same_bom() + self.validate_bom_items() + self.status = "Queued" + + def validate_boms_are_specified(self): + if self.update_type == "Replace BOM" and not (self.current_bom and self.new_bom): + frappe.throw( + msg=_("Please mention the Current and New BOM for replacement."), + title=_("Mandatory"), exc=BOMMissingError + ) + + def validate_same_bom(self): + if cstr(self.current_bom) == cstr(self.new_bom): + frappe.throw(_("Current BOM and New BOM can not be same")) + + def validate_bom_items(self): + current_bom_item = frappe.db.get_value("BOM", self.current_bom, "item") + new_bom_item = frappe.db.get_value("BOM", self.new_bom, "item") + + if current_bom_item != new_bom_item: + frappe.throw(_("The selected BOMs are not for the same item")) + + def on_submit(self): + if frappe.flags.in_test: + return + + if self.update_type == "Replace BOM": + boms = { + "current_bom": self.current_bom, + "new_bom": self.new_bom + } + frappe.enqueue( + method="erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool.replace_bom", + boms=boms, doc=self, timeout=40000 + ) + else: + frappe.enqueue( + method="erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool.update_cost_queue", + doc=self, timeout=40000 + ) + +def replace_bom(boms, doc): + try: + doc.db_set("status", "In Progress") + if not frappe.flags.in_test: + frappe.db.commit() + + frappe.db.auto_commit_on_many_writes = 1 + + args = frappe._dict(boms) + doc = frappe.get_doc("BOM Update Tool") + doc.current_bom = args.current_bom + doc.new_bom = args.new_bom + doc.replace_bom() + + doc.db_set("status", "Completed") + + except (Exception, JobTimeoutException): + frappe.db.rollback() + frappe.log_error( + msg=frappe.get_traceback(), + title=_("BOM Update Tool Error") + ) + doc.db_set("status", "Failed") + + finally: + frappe.db.auto_commit_on_many_writes = 0 + frappe.db.commit() + +def update_cost_queue(doc): + try: + doc.db_set("status", "In Progress") + if not frappe.flags.in_test: + frappe.db.commit() + + frappe.db.auto_commit_on_many_writes = 1 + + bom_list = get_boms_in_bottom_up_order() + for bom in bom_list: + frappe.get_doc("BOM", bom).update_cost(update_parent=False, from_child_bom=True) + + doc.db_set("status", "Completed") + + except (Exception, JobTimeoutException): + frappe.db.rollback() + frappe.log_error( + msg=frappe.get_traceback(), + title=_("BOM Update Tool Error") + ) + doc.db_set("status", "Failed") + + finally: + frappe.db.auto_commit_on_many_writes = 0 + frappe.db.commit() + +def update_cost(): + bom_list = get_boms_in_bottom_up_order() + for bom in bom_list: + frappe.get_doc("BOM", bom).update_cost(update_parent=False, from_child_bom=True) \ No newline at end of file diff --git a/erpnext/manufacturing/doctype/bom_update_log/test_bom_update_log.py b/erpnext/manufacturing/doctype/bom_update_log/test_bom_update_log.py new file mode 100644 index 0000000000..f74bdc356a --- /dev/null +++ b/erpnext/manufacturing/doctype/bom_update_log/test_bom_update_log.py @@ -0,0 +1,9 @@ +# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors +# See license.txt + +# import frappe +from frappe.tests.utils import FrappeTestCase + + +class TestBOMUpdateLog(FrappeTestCase): + pass diff --git a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py index c7197347c2..fad53f0568 100644 --- a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py +++ b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py @@ -10,13 +10,11 @@ from frappe import _ from frappe.model.document import Document from frappe.utils import cstr, flt -from erpnext.manufacturing.doctype.bom.bom import get_boms_in_bottom_up_order +from erpnext.manufacturing.doctype.bom_update_log.bom_update_log import update_cost class BOMUpdateTool(Document): def replace_bom(self): - self.validate_bom() - unit_cost = get_new_bom_unit_cost(self.new_bom) self.update_new_bom(unit_cost) @@ -42,14 +40,6 @@ class BOMUpdateTool(Document): except Exception: frappe.log_error(frappe.get_traceback()) - def validate_bom(self): - if cstr(self.current_bom) == cstr(self.new_bom): - frappe.throw(_("Current BOM and New BOM can not be same")) - - if frappe.db.get_value("BOM", self.current_bom, "item") \ - != frappe.db.get_value("BOM", self.new_bom, "item"): - frappe.throw(_("The selected BOMs are not for the same item")) - def update_new_bom(self, unit_cost): frappe.db.sql("""update `tabBOM Item` set bom_no=%s, rate=%s, amount=stock_qty*%s where bom_no = %s and docstatus < 2 and parenttype='BOM'""", @@ -81,44 +71,29 @@ def enqueue_replace_bom(args): if isinstance(args, str): args = json.loads(args) - frappe.enqueue("erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool.replace_bom", args=args, timeout=40000) + create_bom_update_log(boms=args) frappe.msgprint(_("Queued for replacing the BOM. It may take a few minutes.")) + @frappe.whitelist() def enqueue_update_cost(): - frappe.enqueue("erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool.update_cost", timeout=40000) + create_bom_update_log(update_type="Update Cost") frappe.msgprint(_("Queued for updating latest price in all Bill of Materials. It may take a few minutes.")) -def update_latest_price_in_all_boms(): + +def auto_update_latest_price_in_all_boms(): + "Called via hooks.py." if frappe.db.get_single_value("Manufacturing Settings", "update_bom_costs_automatically"): update_cost() -def replace_bom(args): - try: - frappe.db.auto_commit_on_many_writes = 1 - args = frappe._dict(args) - doc = frappe.get_doc("BOM Update Tool") - doc.current_bom = args.current_bom - doc.new_bom = args.new_bom - doc.replace_bom() - except Exception: - frappe.log_error( - msg=frappe.get_traceback(), - title=_("BOM Update Tool Error") - ) - finally: - frappe.db.auto_commit_on_many_writes = 0 - -def update_cost(): - try: - frappe.db.auto_commit_on_many_writes = 1 - bom_list = get_boms_in_bottom_up_order() - for bom in bom_list: - frappe.get_doc("BOM", bom).update_cost(update_parent=False, from_child_bom=True) - except Exception: - frappe.log_error( - msg=frappe.get_traceback(), - title=_("BOM Update Tool Error") - ) - finally: - frappe.db.auto_commit_on_many_writes = 0 +def create_bom_update_log(boms=None, update_type="Replace BOM"): + "Creates a BOM Update Log that handles the background job." + current_bom = boms.get("current_bom") if boms else None + new_bom = boms.get("new_bom") if boms else None + log_doc = frappe.get_doc({ + "doctype": "BOM Update Log", + "current_bom": current_bom, + "new_bom": new_bom, + "update_type": update_type + }) + log_doc.submit() \ No newline at end of file From cff91558d4f380cc7566d009ea85ccba36976f69 Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 17 Mar 2022 12:32:37 +0530 Subject: [PATCH 03/54] chore: Polish error handling and code sepration - Added Typing - Moved all job business logic to bom update log - Added `run_bom_job` that handles errors and runs either of two methods - UX: Replace button disabled until both inputs are filled - Show log creation message on UI for correctness - APIs return log document as result - Converted raw sql to QB --- .../bom_update_log/bom_update_log.json | 8 +- .../doctype/bom_update_log/bom_update_log.py | 146 ++++++++++++------ .../bom_update_tool/bom_update_tool.js | 43 +++++- .../bom_update_tool/bom_update_tool.py | 108 ++++--------- 4 files changed, 171 insertions(+), 134 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.json b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.json index 222168be8c..d89427edc0 100644 --- a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.json +++ b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.json @@ -20,16 +20,14 @@ "fieldtype": "Link", "in_list_view": 1, "label": "Current BOM", - "options": "BOM", - "reqd": 1 + "options": "BOM" }, { "fieldname": "new_bom", "fieldtype": "Link", "in_list_view": 1, "label": "New BOM", - "options": "BOM", - "reqd": 1 + "options": "BOM" }, { "fieldname": "column_break_3", @@ -61,7 +59,7 @@ "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2022-03-16 18:25:49.833836", + "modified": "2022-03-17 12:21:16.156437", "modified_by": "Administrator", "module": "Manufacturing", "name": "BOM Update Log", diff --git a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py index 10db0de9a1..b08d6f906c 100644 --- a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py +++ b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py @@ -1,23 +1,27 @@ # Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and contributors # For license information, please see license.txt +from typing import Dict, List, Optional +import click import frappe from frappe import _ from frappe.model.document import Document -from frappe.utils import cstr - -from erpnext.manufacturing.doctype.bom.bom import get_boms_in_bottom_up_order - +from frappe.utils import cstr, flt from rq.timeouts import JobTimeoutException +from erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool import update_cost -class BOMMissingError(frappe.ValidationError): pass + +class BOMMissingError(frappe.ValidationError): + pass class BOMUpdateLog(Document): def validate(self): - self.validate_boms_are_specified() - self.validate_same_bom() - self.validate_bom_items() + if self.update_type == "Replace BOM": + self.validate_boms_are_specified() + self.validate_same_bom() + self.validate_bom_items() + self.status = "Queued" def validate_boms_are_specified(self): @@ -48,16 +52,88 @@ class BOMUpdateLog(Document): "new_bom": self.new_bom } frappe.enqueue( - method="erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool.replace_bom", - boms=boms, doc=self, timeout=40000 + method="erpnext.manufacturing.doctype.bom_update_log.bom_update_log.run_bom_job", + doc=self, boms=boms, timeout=40000 ) else: frappe.enqueue( - method="erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool.update_cost_queue", - doc=self, timeout=40000 + method="erpnext.manufacturing.doctype.bom_update_log.bom_update_log.run_bom_job", + doc=self, update_type="Update Cost", timeout=40000 ) -def replace_bom(boms, doc): +def replace_bom(boms: Dict) -> None: + """Replace current BOM with new BOM in parent BOMs.""" + current_bom = boms.get("current_bom") + new_bom = boms.get("new_bom") + + unit_cost = get_new_bom_unit_cost(new_bom) + update_new_bom(unit_cost, current_bom, new_bom) + + frappe.cache().delete_key('bom_children') + parent_boms = get_parent_boms(new_bom) + + with click.progressbar(parent_boms) as parent_boms: + pass + for bom in parent_boms: + bom_obj = frappe.get_cached_doc('BOM', bom) + # this is only used for versioning and we do not want + # to make separate db calls by using load_doc_before_save + # which proves to be expensive while doing bulk replace + bom_obj._doc_before_save = bom_obj + bom_obj.update_new_bom(unit_cost, current_bom, new_bom) + bom_obj.update_exploded_items() + bom_obj.calculate_cost() + bom_obj.update_parent_cost() + bom_obj.db_update() + if bom_obj.meta.get('track_changes') and not bom_obj.flags.ignore_version: + bom_obj.save_version() + +def update_new_bom(unit_cost: float, current_bom: str, new_bom: str) -> None: + bom_item = frappe.qb.DocType("BOM Item") + frappe.qb.update(bom_item).set( + bom_item.bom_no, new_bom + ).set( + bom_item.rate, unit_cost + ).set( + bom_item.amount, (bom_item.stock_qty * unit_cost) + ).where( + (bom_item.bom_no == current_bom) + & (bom_item.docstatus < 2) + & (bom_item.parenttype == "BOM") + ).run() + +def get_parent_boms(new_bom: str, bom_list: Optional[List] = None) -> List: + bom_list = bom_list or [] + bom_item = frappe.qb.DocType("BOM Item") + + parents = frappe.qb.from_(bom_item).select( + bom_item.parent + ).where( + (bom_item.bom_no == new_bom) + & (bom_item.docstatus <2) + & (bom_item.parenttype == "BOM") + ).run(as_dict=True) + + for d in parents: + if new_bom == d.parent: + frappe.throw(_("BOM recursion: {0} cannot be child of {1}").format(new_bom, d.parent)) + + bom_list.append(d.parent) + get_parent_boms(d.parent, bom_list) + + return list(set(bom_list)) + +def get_new_bom_unit_cost(new_bom: str) -> float: + bom = frappe.qb.DocType("BOM") + new_bom_unitcost = frappe.qb.from_(bom).select( + bom.total_cost / bom.quantity + ).where( + bom.name == new_bom + ).run() + + return flt(new_bom_unitcost[0][0]) + +def run_bom_job(doc: "BOMUpdateLog", boms: Optional[Dict] = None, update_type: Optional[str] = "Replace BOM") -> None: try: doc.db_set("status", "In Progress") if not frappe.flags.in_test: @@ -65,18 +141,19 @@ def replace_bom(boms, doc): frappe.db.auto_commit_on_many_writes = 1 - args = frappe._dict(boms) - doc = frappe.get_doc("BOM Update Tool") - doc.current_bom = args.current_bom - doc.new_bom = args.new_bom - doc.replace_bom() + boms = frappe._dict(boms or {}) + + if update_type == "Replace BOM": + replace_bom(boms) + else: + update_cost() doc.db_set("status", "Completed") except (Exception, JobTimeoutException): frappe.db.rollback() frappe.log_error( - msg=frappe.get_traceback(), + message=frappe.get_traceback(), title=_("BOM Update Tool Error") ) doc.db_set("status", "Failed") @@ -84,34 +161,3 @@ def replace_bom(boms, doc): finally: frappe.db.auto_commit_on_many_writes = 0 frappe.db.commit() - -def update_cost_queue(doc): - try: - doc.db_set("status", "In Progress") - if not frappe.flags.in_test: - frappe.db.commit() - - frappe.db.auto_commit_on_many_writes = 1 - - bom_list = get_boms_in_bottom_up_order() - for bom in bom_list: - frappe.get_doc("BOM", bom).update_cost(update_parent=False, from_child_bom=True) - - doc.db_set("status", "Completed") - - except (Exception, JobTimeoutException): - frappe.db.rollback() - frappe.log_error( - msg=frappe.get_traceback(), - title=_("BOM Update Tool Error") - ) - doc.db_set("status", "Failed") - - finally: - frappe.db.auto_commit_on_many_writes = 0 - frappe.db.commit() - -def update_cost(): - bom_list = get_boms_in_bottom_up_order() - for bom in bom_list: - frappe.get_doc("BOM", bom).update_cost(update_parent=False, from_child_bom=True) \ No newline at end of file diff --git a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.js b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.js index bf5fe2e18d..ec6a76d61c 100644 --- a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.js +++ b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.js @@ -20,30 +20,63 @@ frappe.ui.form.on('BOM Update Tool', { refresh: function(frm) { frm.disable_save(); + frm.events.disable_button(frm, "replace"); }, - replace: function(frm) { + disable_button: (frm, field, disable=true) => { + frm.get_field(field).input.disabled = disable; + }, + + current_bom: (frm) => { + if (frm.doc.current_bom && frm.doc.new_bom){ + frm.events.disable_button(frm, "replace", false); + } + }, + + new_bom: (frm) => { + if (frm.doc.current_bom && frm.doc.new_bom){ + frm.events.disable_button(frm, "replace", false); + } + }, + + replace: (frm) => { if (frm.doc.current_bom && frm.doc.new_bom) { frappe.call({ method: "erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool.enqueue_replace_bom", freeze: true, args: { - args: { + boms: { "current_bom": frm.doc.current_bom, "new_bom": frm.doc.new_bom } + }, + callback: result => { + if (result && result.message && !result.exc) { + frm.events.confirm_job_start(frm, result.message); + } } }); } }, - update_latest_price_in_all_boms: function() { + update_latest_price_in_all_boms: (frm) => { frappe.call({ method: "erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool.enqueue_update_cost", freeze: true, - callback: function() { - frappe.msgprint(__("Latest price updated in all BOMs")); + callback: result => { + if (result && result.message && !result.exc) { + frm.events.confirm_job_start(frm, result.message); + } } }); + }, + + confirm_job_start: (frm, log_data) => { + let log_link = frappe.utils.get_form_link("BOM Update Log", log_data.name, true) + frappe.msgprint({ + "message": __(`BOM Updation is queued and may take a few minutes. Check ${log_link} for progress.`), + "title": __("BOM Update Initiated"), + "indicator": "blue" + }); } }); diff --git a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py index fad53f0568..16add4fc11 100644 --- a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py +++ b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py @@ -1,99 +1,59 @@ -# Copyright (c) 2017, Frappe Technologies Pvt. Ltd. and contributors +# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and contributors # For license information, please see license.txt - import json +from typing import Dict, List, Optional, TYPE_CHECKING, Union + +if TYPE_CHECKING: + from erpnext.manufacturing.doctype.bom_update_log.bom_update_log import BOMUpdateLog -import click import frappe from frappe import _ from frappe.model.document import Document from frappe.utils import cstr, flt -from erpnext.manufacturing.doctype.bom_update_log.bom_update_log import update_cost +from erpnext.manufacturing.doctype.bom.bom import get_boms_in_bottom_up_order class BOMUpdateTool(Document): - def replace_bom(self): - unit_cost = get_new_bom_unit_cost(self.new_bom) - self.update_new_bom(unit_cost) - - frappe.cache().delete_key('bom_children') - bom_list = self.get_parent_boms(self.new_bom) - - with click.progressbar(bom_list) as bom_list: - pass - for bom in bom_list: - try: - bom_obj = frappe.get_cached_doc('BOM', bom) - # this is only used for versioning and we do not want - # to make separate db calls by using load_doc_before_save - # which proves to be expensive while doing bulk replace - bom_obj._doc_before_save = bom_obj - bom_obj.update_new_bom(self.current_bom, self.new_bom, unit_cost) - bom_obj.update_exploded_items() - bom_obj.calculate_cost() - bom_obj.update_parent_cost() - bom_obj.db_update() - if bom_obj.meta.get('track_changes') and not bom_obj.flags.ignore_version: - bom_obj.save_version() - except Exception: - frappe.log_error(frappe.get_traceback()) - - def update_new_bom(self, unit_cost): - frappe.db.sql("""update `tabBOM Item` set bom_no=%s, - rate=%s, amount=stock_qty*%s where bom_no = %s and docstatus < 2 and parenttype='BOM'""", - (self.new_bom, unit_cost, unit_cost, self.current_bom)) - - def get_parent_boms(self, bom, bom_list=None): - if bom_list is None: - bom_list = [] - data = frappe.db.sql("""SELECT DISTINCT parent FROM `tabBOM Item` - WHERE bom_no = %s AND docstatus < 2 AND parenttype='BOM'""", bom) - - for d in data: - if self.new_bom == d[0]: - frappe.throw(_("BOM recursion: {0} cannot be child of {1}").format(bom, self.new_bom)) - - bom_list.append(d[0]) - self.get_parent_boms(d[0], bom_list) - - return list(set(bom_list)) - -def get_new_bom_unit_cost(bom): - new_bom_unitcost = frappe.db.sql("""SELECT `total_cost`/`quantity` - FROM `tabBOM` WHERE name = %s""", bom) - - return flt(new_bom_unitcost[0][0]) if new_bom_unitcost else 0 + pass @frappe.whitelist() -def enqueue_replace_bom(args): - if isinstance(args, str): - args = json.loads(args) - - create_bom_update_log(boms=args) - frappe.msgprint(_("Queued for replacing the BOM. It may take a few minutes.")) +def enqueue_replace_bom(boms: Optional[Union[Dict, str]] = None, args: Optional[Union[Dict, str]] = None) -> "BOMUpdateLog": + """Returns a BOM Update Log (that queues a job) for BOM Replacement.""" + boms = boms or args + if isinstance(boms, str): + boms = json.loads(boms) + update_log = create_bom_update_log(boms=boms) + return update_log @frappe.whitelist() -def enqueue_update_cost(): - create_bom_update_log(update_type="Update Cost") - frappe.msgprint(_("Queued for updating latest price in all Bill of Materials. It may take a few minutes.")) +def enqueue_update_cost() -> "BOMUpdateLog": + """Returns a BOM Update Log (that queues a job) for BOM Cost Updation.""" + update_log = create_bom_update_log(update_type="Update Cost") + return update_log -def auto_update_latest_price_in_all_boms(): - "Called via hooks.py." +def auto_update_latest_price_in_all_boms() -> None: + """Called via hooks.py.""" if frappe.db.get_single_value("Manufacturing Settings", "update_bom_costs_automatically"): update_cost() -def create_bom_update_log(boms=None, update_type="Replace BOM"): - "Creates a BOM Update Log that handles the background job." - current_bom = boms.get("current_bom") if boms else None - new_bom = boms.get("new_bom") if boms else None - log_doc = frappe.get_doc({ +def update_cost() -> None: + """Updates Cost for all BOMs from bottom to top.""" + bom_list = get_boms_in_bottom_up_order() + for bom in bom_list: + frappe.get_doc("BOM", bom).update_cost(update_parent=False, from_child_bom=True) + +def create_bom_update_log(boms: Optional[Dict] = None, update_type: str = "Replace BOM") -> "BOMUpdateLog": + """Creates a BOM Update Log that handles the background job.""" + boms = boms or {} + current_bom = boms.get("current_bom") + new_bom = boms.get("new_bom") + return frappe.get_doc({ "doctype": "BOM Update Log", "current_bom": current_bom, "new_bom": new_bom, - "update_type": update_type - }) - log_doc.submit() \ No newline at end of file + "update_type": update_type, + }).submit() \ No newline at end of file From 8aff75f8e8f6cf885f0e59ead89b8596d6f56c0a Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 17 Mar 2022 12:58:09 +0530 Subject: [PATCH 04/54] feat: List View indicators for Log and Error Log link in log --- .../doctype/bom_update_log/bom_update_log.json | 9 ++++++++- .../doctype/bom_update_log/bom_update_log.py | 4 +++- .../doctype/bom_update_log/bom_update_log_list.js | 13 +++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 erpnext/manufacturing/doctype/bom_update_log/bom_update_log_list.js diff --git a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.json b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.json index d89427edc0..38c685a64f 100644 --- a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.json +++ b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.json @@ -12,6 +12,7 @@ "column_break_3", "update_type", "status", + "error_log", "amended_from" ], "fields": [ @@ -53,13 +54,19 @@ "options": "BOM Update Log", "print_hide": 1, "read_only": 1 + }, + { + "fieldname": "error_log", + "fieldtype": "Link", + "label": "Error Log", + "options": "Error Log" } ], "in_create": 1, "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2022-03-17 12:21:16.156437", + "modified": "2022-03-17 12:51:28.067900", "modified_by": "Administrator", "module": "Manufacturing", "name": "BOM Update Log", diff --git a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py index b08d6f906c..a69b15c527 100644 --- a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py +++ b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py @@ -152,11 +152,13 @@ def run_bom_job(doc: "BOMUpdateLog", boms: Optional[Dict] = None, update_type: O except (Exception, JobTimeoutException): frappe.db.rollback() - frappe.log_error( + error_log = frappe.log_error( message=frappe.get_traceback(), title=_("BOM Update Tool Error") ) + doc.db_set("status", "Failed") + doc.db_set("error_log", error_log.name) finally: frappe.db.auto_commit_on_many_writes = 0 diff --git a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log_list.js b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log_list.js new file mode 100644 index 0000000000..8b3dc520cf --- /dev/null +++ b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log_list.js @@ -0,0 +1,13 @@ +frappe.listview_settings['BOM Update Log'] = { + add_fields: ["status"], + get_indicator: function(doc) { + let status_map = { + "Queued": "orange", + "In Progress": "blue", + "Completed": "green", + "Failed": "red" + } + + return [__(doc.status), status_map[doc.status], "status,=," + doc.status]; + } +}; \ No newline at end of file From 3e3af95712b5241a243a5b6169be2fc888bb4c39 Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 17 Mar 2022 15:03:20 +0530 Subject: [PATCH 05/54] fix: Sider and Linter --- .../doctype/bom_update_log/bom_update_log.py | 56 +++++++++---------- .../bom_update_log/bom_update_log_list.js | 2 +- .../bom_update_tool/bom_update_tool.js | 6 +- .../bom_update_tool/bom_update_tool.py | 2 +- 4 files changed, 33 insertions(+), 33 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py index a69b15c527..7f60d8fc7d 100644 --- a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py +++ b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py @@ -1,8 +1,8 @@ # Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and contributors # For license information, please see license.txt from typing import Dict, List, Optional -import click +import click import frappe from frappe import _ from frappe.model.document import Document @@ -89,39 +89,39 @@ def replace_bom(boms: Dict) -> None: bom_obj.save_version() def update_new_bom(unit_cost: float, current_bom: str, new_bom: str) -> None: - bom_item = frappe.qb.DocType("BOM Item") - frappe.qb.update(bom_item).set( - bom_item.bom_no, new_bom - ).set( - bom_item.rate, unit_cost - ).set( - bom_item.amount, (bom_item.stock_qty * unit_cost) - ).where( - (bom_item.bom_no == current_bom) - & (bom_item.docstatus < 2) - & (bom_item.parenttype == "BOM") - ).run() + bom_item = frappe.qb.DocType("BOM Item") + frappe.qb.update(bom_item).set( + bom_item.bom_no, new_bom + ).set( + bom_item.rate, unit_cost + ).set( + bom_item.amount, (bom_item.stock_qty * unit_cost) + ).where( + (bom_item.bom_no == current_bom) + & (bom_item.docstatus < 2) + & (bom_item.parenttype == "BOM") + ).run() def get_parent_boms(new_bom: str, bom_list: Optional[List] = None) -> List: - bom_list = bom_list or [] - bom_item = frappe.qb.DocType("BOM Item") + bom_list = bom_list or [] + bom_item = frappe.qb.DocType("BOM Item") - parents = frappe.qb.from_(bom_item).select( - bom_item.parent - ).where( - (bom_item.bom_no == new_bom) - & (bom_item.docstatus <2) - & (bom_item.parenttype == "BOM") - ).run(as_dict=True) + parents = frappe.qb.from_(bom_item).select( + bom_item.parent + ).where( + (bom_item.bom_no == new_bom) + & (bom_item.docstatus <2) + & (bom_item.parenttype == "BOM") + ).run(as_dict=True) - for d in parents: - if new_bom == d.parent: - frappe.throw(_("BOM recursion: {0} cannot be child of {1}").format(new_bom, d.parent)) + for d in parents: + if new_bom == d.parent: + frappe.throw(_("BOM recursion: {0} cannot be child of {1}").format(new_bom, d.parent)) - bom_list.append(d.parent) - get_parent_boms(d.parent, bom_list) + bom_list.append(d.parent) + get_parent_boms(d.parent, bom_list) - return list(set(bom_list)) + return list(set(bom_list)) def get_new_bom_unit_cost(new_bom: str) -> float: bom = frappe.qb.DocType("BOM") diff --git a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log_list.js b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log_list.js index 8b3dc520cf..e39b5637c7 100644 --- a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log_list.js +++ b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log_list.js @@ -6,7 +6,7 @@ frappe.listview_settings['BOM Update Log'] = { "In Progress": "blue", "Completed": "green", "Failed": "red" - } + }; return [__(doc.status), status_map[doc.status], "status,=," + doc.status]; } diff --git a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.js b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.js index ec6a76d61c..0c9816712c 100644 --- a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.js +++ b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.js @@ -28,13 +28,13 @@ frappe.ui.form.on('BOM Update Tool', { }, current_bom: (frm) => { - if (frm.doc.current_bom && frm.doc.new_bom){ + if (frm.doc.current_bom && frm.doc.new_bom) { frm.events.disable_button(frm, "replace", false); } }, new_bom: (frm) => { - if (frm.doc.current_bom && frm.doc.new_bom){ + if (frm.doc.current_bom && frm.doc.new_bom) { frm.events.disable_button(frm, "replace", false); } }, @@ -72,7 +72,7 @@ frappe.ui.form.on('BOM Update Tool', { }, confirm_job_start: (frm, log_data) => { - let log_link = frappe.utils.get_form_link("BOM Update Log", log_data.name, true) + let log_link = frappe.utils.get_form_link("BOM Update Log", log_data.name, true); frappe.msgprint({ "message": __(`BOM Updation is queued and may take a few minutes. Check ${log_link} for progress.`), "title": __("BOM Update Initiated"), diff --git a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py index 16add4fc11..bc3e82006b 100644 --- a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py +++ b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py @@ -2,7 +2,7 @@ # For license information, please see license.txt import json -from typing import Dict, List, Optional, TYPE_CHECKING, Union +from typing import TYPE_CHECKING, Dict, Optional, Union if TYPE_CHECKING: from erpnext.manufacturing.doctype.bom_update_log.bom_update_log import BOMUpdateLog From f3715ab38260f21f5be8c6f9bdfcf8a02c051556 Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 17 Mar 2022 17:43:12 +0530 Subject: [PATCH 06/54] fix: Test, Sider and Added button to access log from Tool --- .../doctype/bom_update_tool/bom_update_tool.js | 4 ++++ .../doctype/bom_update_tool/bom_update_tool.py | 2 -- .../bom_update_tool/test_bom_update_tool.py | 16 +++++++++------- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.js b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.js index 0c9816712c..a793ed9535 100644 --- a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.js +++ b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.js @@ -21,6 +21,10 @@ frappe.ui.form.on('BOM Update Tool', { refresh: function(frm) { frm.disable_save(); frm.events.disable_button(frm, "replace"); + + frm.add_custom_button(__("View BOM Update Log"), () => { + frappe.set_route("List", "BOM Update Log"); + }); }, disable_button: (frm, field, disable=true) => { diff --git a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py index bc3e82006b..3da8afee15 100644 --- a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py +++ b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py @@ -8,9 +8,7 @@ if TYPE_CHECKING: from erpnext.manufacturing.doctype.bom_update_log.bom_update_log import BOMUpdateLog import frappe -from frappe import _ from frappe.model.document import Document -from frappe.utils import cstr, flt from erpnext.manufacturing.doctype.bom.bom import get_boms_in_bottom_up_order diff --git a/erpnext/manufacturing/doctype/bom_update_tool/test_bom_update_tool.py b/erpnext/manufacturing/doctype/bom_update_tool/test_bom_update_tool.py index b4c625d610..c99e88893a 100644 --- a/erpnext/manufacturing/doctype/bom_update_tool/test_bom_update_tool.py +++ b/erpnext/manufacturing/doctype/bom_update_tool/test_bom_update_tool.py @@ -5,6 +5,7 @@ import frappe from frappe.tests.utils import FrappeTestCase from erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool import update_cost +from erpnext.manufacturing.doctype.bom_update_log.bom_update_log import replace_bom from erpnext.manufacturing.doctype.production_plan.test_production_plan import make_bom from erpnext.stock.doctype.item.test_item import create_item @@ -18,18 +19,19 @@ class TestBOMUpdateTool(FrappeTestCase): bom_doc.items[1].item_code = "_Test Item" bom_doc.insert() - update_tool = frappe.get_doc("BOM Update Tool") - update_tool.current_bom = current_bom - update_tool.new_bom = bom_doc.name - update_tool.replace_bom() + boms = frappe._dict( + current_bom=current_bom, + new_bom=bom_doc.name + ) + replace_bom(boms) self.assertFalse(frappe.db.sql("select name from `tabBOM Item` where bom_no=%s", current_bom)) self.assertTrue(frappe.db.sql("select name from `tabBOM Item` where bom_no=%s", bom_doc.name)) # reverse, as it affects other testcases - update_tool.current_bom = bom_doc.name - update_tool.new_bom = current_bom - update_tool.replace_bom() + boms.current_bom = bom_doc.name + boms.new_bom = current_bom + replace_bom(boms) def test_bom_cost(self): for item in ["BOM Cost Test Item 1", "BOM Cost Test Item 2", "BOM Cost Test Item 3"]: From a787ebb7325294eb95cca1c91b6257bd9cdab88f Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 23 Mar 2022 14:59:27 +0530 Subject: [PATCH 07/54] fix: Dont set `idx` while adding WO items to Stock Entry - `idx` must be computed by base document's `self.append()` function, so do not set it --- erpnext/stock/doctype/stock_entry/stock_entry.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py index 99cf4de5de..26d59fdab2 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/stock_entry.py @@ -1603,8 +1603,11 @@ class StockEntry(StockController): se_child.is_scrap_item = item_row.get("is_scrap_item", 0) se_child.is_process_loss = item_row.get("is_process_loss", 0) - for field in ["idx", "po_detail", "original_item", "expense_account", - "description", "item_name", "serial_no", "batch_no", "allow_zero_valuation_rate"]: + for field in [ + "po_detail", "original_item", "expense_account", + "description", "item_name", "serial_no", + "batch_no", "allow_zero_valuation_rate" + ]: if item_row.get(field): se_child.set(field, item_row.get(field)) From 500870b2b0eca0407f06198d497bc7031950bab9 Mon Sep 17 00:00:00 2001 From: rahib-hassan Date: Mon, 28 Mar 2022 15:25:01 +0530 Subject: [PATCH 08/54] fix: enable row deletion in reference table --- erpnext/accounts/doctype/payment_order/payment_order.js | 1 - 1 file changed, 1 deletion(-) diff --git a/erpnext/accounts/doctype/payment_order/payment_order.js b/erpnext/accounts/doctype/payment_order/payment_order.js index 9074defa57..7d85d89c45 100644 --- a/erpnext/accounts/doctype/payment_order/payment_order.js +++ b/erpnext/accounts/doctype/payment_order/payment_order.js @@ -12,7 +12,6 @@ frappe.ui.form.on('Payment Order', { }); frm.set_df_property('references', 'cannot_add_rows', true); - frm.set_df_property('references', 'cannot_delete_rows', true); }, refresh: function(frm) { if (frm.doc.docstatus == 0) { From 639d380c1f333ba2162aacd62511d2593db156bc Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 28 Mar 2022 21:00:00 +0530 Subject: [PATCH 09/54] chore: Remove redundant idx query and value setting - idx can be removed from `select_columns` as it is already in the main query - setting idx to '' is not required as it is not used further --- erpnext/manufacturing/doctype/bom/bom.py | 4 ++-- erpnext/stock/doctype/stock_entry/stock_entry.py | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index bf29474c00..9b6cf46c11 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -1025,7 +1025,7 @@ def get_bom_items_as_dict( query = query.format( table="BOM Scrap Item", where_conditions="", - select_columns=", bom_item.idx, item.description, is_process_loss", + select_columns=", item.description, is_process_loss", is_stock_item=is_stock_item, qty_field="stock_qty", ) @@ -1038,7 +1038,7 @@ def get_bom_items_as_dict( is_stock_item=is_stock_item, qty_field="stock_qty" if fetch_qty_in_stock_uom else "qty", select_columns=""", bom_item.uom, bom_item.conversion_factor, bom_item.source_warehouse, - bom_item.idx, bom_item.operation, bom_item.include_item_in_manufacturing, bom_item.sourced_by_supplier, + bom_item.operation, bom_item.include_item_in_manufacturing, bom_item.sourced_by_supplier, bom_item.description, bom_item.base_rate as rate """, ) items = frappe.db.sql(query, {"qty": qty, "bom": bom, "company": company}, as_dict=True) diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py index eaf273051e..2545956b6b 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/stock_entry.py @@ -1382,7 +1382,6 @@ class StockEntry(StockController): if self.purpose != "Send to Subcontractor" and self.purpose in ["Manufacture", "Repack"]: scrap_item_dict = self.get_bom_scrap_material(self.fg_completed_qty) for item in scrap_item_dict.values(): - item.idx = "" if self.pro_doc and self.pro_doc.scrap_warehouse: item["to_warehouse"] = self.pro_doc.scrap_warehouse From 136466d255651ba29be16248e822c2a374114c67 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Tue, 29 Mar 2022 10:47:27 +0530 Subject: [PATCH 10/54] fix(asset): do not validate warehouse on asset purchase --- erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py index 7654aa44dc..d1e7fb40d1 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py @@ -226,7 +226,7 @@ class PurchaseInvoice(BuyingController): def validate_warehouse(self, for_validate=True): if self.update_stock and for_validate: for d in self.get('items'): - if not d.warehouse: + if not d.warehouse and not d.is_fixed_asset: frappe.throw(_("Row No {0}: Warehouse is required. Please set a Default Warehouse for Item {1} and Company {2}"). format(d.idx, d.item_code, self.company), exc=WarehouseMissingError) From 76dce2eddc69a1df8277028510a9dbe6ab4acd7d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 29 Mar 2022 11:21:03 +0530 Subject: [PATCH 11/54] fix: use `name` for links not `item_code` (#30462) --- .../report/item_wise_sales_history/item_wise_sales_history.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/selling/report/item_wise_sales_history/item_wise_sales_history.py b/erpnext/selling/report/item_wise_sales_history/item_wise_sales_history.py index da0098409b..12ca7b3ff8 100644 --- a/erpnext/selling/report/item_wise_sales_history/item_wise_sales_history.py +++ b/erpnext/selling/report/item_wise_sales_history/item_wise_sales_history.py @@ -177,11 +177,11 @@ def get_customer_details(): def get_item_details(): - details = frappe.db.get_all("Item", fields=["item_code", "item_name", "item_group"]) + details = frappe.db.get_all("Item", fields=["name", "item_name", "item_group"]) item_details = {} for d in details: item_details.setdefault( - d.item_code, frappe._dict({"item_name": d.item_name, "item_group": d.item_group}) + d.name, frappe._dict({"item_name": d.item_name, "item_group": d.item_group}) ) return item_details From d29e314c812e89fbb68090b5eca8fba1229af687 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 29 Mar 2022 11:47:28 +0530 Subject: [PATCH 12/54] ci: pin click for black (#30464) [skip ci] --- .pre-commit-config.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index cba6759f61..dc3011f050 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -30,6 +30,7 @@ repos: rev: 9cb0a69f4d0030cdf687eddf314468b39ed54119 hooks: - id: black + additional_dependencies: ['click==8.0.4'] - repo: https://github.com/timothycrosley/isort rev: 5.9.1 From c122882884d8a4d2f504eb53bef9003333bd393c Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Tue, 29 Mar 2022 13:36:00 +0530 Subject: [PATCH 13/54] fix: credit limit validation in delivery note --- erpnext/stock/doctype/delivery_note/delivery_note.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/erpnext/stock/doctype/delivery_note/delivery_note.py b/erpnext/stock/doctype/delivery_note/delivery_note.py index 69e052bb81..0e68e85806 100644 --- a/erpnext/stock/doctype/delivery_note/delivery_note.py +++ b/erpnext/stock/doctype/delivery_note/delivery_note.py @@ -280,8 +280,11 @@ class DeliveryNote(SellingController): ) if bypass_credit_limit_check_at_sales_order: - validate_against_credit_limit = True - extra_amount = self.base_grand_total + for d in self.get("items"): + if not d.against_sales_invoice: + validate_against_credit_limit = True + extra_amount = self.base_grand_total + break else: for d in self.get("items"): if not (d.against_sales_order or d.against_sales_invoice): From fa3b953cf7ee9d4c5fa8330bb56499efa9339113 Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 29 Mar 2022 13:54:14 +0530 Subject: [PATCH 14/54] test: idx mapping correctness --- .../doctype/work_order/test_work_order.py | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/erpnext/manufacturing/doctype/work_order/test_work_order.py b/erpnext/manufacturing/doctype/work_order/test_work_order.py index 3721704840..9209cbf755 100644 --- a/erpnext/manufacturing/doctype/work_order/test_work_order.py +++ b/erpnext/manufacturing/doctype/work_order/test_work_order.py @@ -1114,6 +1114,32 @@ class TestWorkOrder(FrappeTestCase): except frappe.MandatoryError: self.fail("Batch generation causing failing in Work Order") + @change_settings("Manufacturing Settings", {"backflush_raw_materials_based_on": "Material Transferred for Manufacture"}) + def test_manufacture_entry_mapped_idx_with_exploded_bom(self): + """Test if WO containing BOM with partial exploded items and scrap items, maps idx correctly.""" + test_stock_entry.make_stock_entry( + item_code="_Test Item", + target="_Test Warehouse - _TC", + basic_rate=5000.0, + qty=2, + ) + test_stock_entry.make_stock_entry( + item_code="_Test Item Home Desktop 100", + target="_Test Warehouse - _TC", + basic_rate=1000.0, + qty=2, + ) + + wo_order = make_wo_order_test_record( + qty=1, + use_multi_level_bom=1, + skip_transfer=1, + ) + + ste_manu = frappe.get_doc(make_stock_entry(wo_order.name, "Manufacture", 1)) + + for index, row in enumerate(ste_manu.get("items"), start=1): + self.assertEqual(index, row.idx) def update_job_card(job_card, jc_qty=None): employee = frappe.db.get_value("Employee", {"status": "Active"}, "name") From b12fe0f15bcc5d71c16d5bebc4d494518b6220b4 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 29 Mar 2022 13:54:26 +0530 Subject: [PATCH 15/54] fix: dont check for failed repost while freezing (#30472) [skip ci] --- erpnext/stock/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/stock/utils.py b/erpnext/stock/utils.py index 741646dfff..4f1891fd75 100644 --- a/erpnext/stock/utils.py +++ b/erpnext/stock/utils.py @@ -526,7 +526,7 @@ def check_pending_reposting(posting_date: str, throw_error: bool = True) -> bool filters = { "docstatus": 1, - "status": ["in", ["Queued", "In Progress", "Failed"]], + "status": ["in", ["Queued", "In Progress"]], "posting_date": ["<=", posting_date], } From b80fac03afe99d3c0b9c0f09d6eb34573c5692cf Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 29 Mar 2022 17:19:22 +0530 Subject: [PATCH 16/54] fix: validate 0 transfer qty in stock entry (#30476) --- .../stock/doctype/stock_entry/stock_entry.py | 6 ++- .../doctype/stock_entry/test_stock_entry.py | 48 ++++--------------- 2 files changed, 13 insertions(+), 41 deletions(-) diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py index bc54f7f84e..c0a8e14dbf 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/stock_entry.py @@ -225,12 +225,16 @@ class StockEntry(StockController): def set_transfer_qty(self): for item in self.get("items"): if not flt(item.qty): - frappe.throw(_("Row {0}: Qty is mandatory").format(item.idx)) + frappe.throw(_("Row {0}: Qty is mandatory").format(item.idx), title=_("Zero quantity")) if not flt(item.conversion_factor): frappe.throw(_("Row {0}: UOM Conversion Factor is mandatory").format(item.idx)) item.transfer_qty = flt( flt(item.qty) * flt(item.conversion_factor), self.precision("transfer_qty", item) ) + if not flt(item.transfer_qty): + frappe.throw( + _("Row {0}: Qty in Stock UOM can not be zero.").format(item.idx), title=_("Zero quantity") + ) def update_cost_in_project(self): if self.work_order and not frappe.db.get_value( diff --git a/erpnext/stock/doctype/stock_entry/test_stock_entry.py b/erpnext/stock/doctype/stock_entry/test_stock_entry.py index aeedcd1847..3ccd3420e3 100644 --- a/erpnext/stock/doctype/stock_entry/test_stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/test_stock_entry.py @@ -51,7 +51,6 @@ class TestStockEntry(FrappeTestCase): def tearDown(self): frappe.db.rollback() frappe.set_user("Administrator") - frappe.db.set_value("Manufacturing Settings", None, "material_consumption", "0") def test_fifo(self): frappe.db.set_value("Stock Settings", None, "allow_negative_stock", 1) @@ -767,13 +766,12 @@ class TestStockEntry(FrappeTestCase): fg_cost, flt(rm_cost + bom_operation_cost + work_order.additional_operating_cost, 2) ) + @change_settings("Manufacturing Settings", {"material_consumption": 1}) def test_work_order_manufacture_with_material_consumption(self): from erpnext.manufacturing.doctype.work_order.work_order import ( make_stock_entry as _make_stock_entry, ) - frappe.db.set_value("Manufacturing Settings", None, "material_consumption", "1") - bom_no = frappe.db.get_value("BOM", {"item": "_Test FG Item", "is_default": 1, "docstatus": 1}) work_order = frappe.new_doc("Work Order") @@ -983,43 +981,6 @@ class TestStockEntry(FrappeTestCase): repack.insert() self.assertRaises(frappe.ValidationError, repack.submit) - # def test_material_consumption(self): - # frappe.db.set_value("Manufacturing Settings", None, "backflush_raw_materials_based_on", "BOM") - # frappe.db.set_value("Manufacturing Settings", None, "material_consumption", "0") - - # from erpnext.manufacturing.doctype.work_order.work_order \ - # import make_stock_entry as _make_stock_entry - # bom_no = frappe.db.get_value("BOM", {"item": "_Test FG Item 2", - # "is_default": 1, "docstatus": 1}) - - # work_order = frappe.new_doc("Work Order") - # work_order.update({ - # "company": "_Test Company", - # "fg_warehouse": "_Test Warehouse 1 - _TC", - # "production_item": "_Test FG Item 2", - # "bom_no": bom_no, - # "qty": 4.0, - # "stock_uom": "_Test UOM", - # "wip_warehouse": "_Test Warehouse - _TC", - # "additional_operating_cost": 1000, - # "use_multi_level_bom": 1 - # }) - # work_order.insert() - # work_order.submit() - - # make_stock_entry(item_code="_Test Serialized Item With Series", target="_Test Warehouse - _TC", qty=50, basic_rate=100) - # make_stock_entry(item_code="_Test Item 2", target="_Test Warehouse - _TC", qty=50, basic_rate=20) - - # item_quantity = { - # '_Test Item': 2.0, - # '_Test Item 2': 12.0, - # '_Test Serialized Item With Series': 6.0 - # } - - # stock_entry = frappe.get_doc(_make_stock_entry(work_order.name, "Material Consumption for Manufacture", 2)) - # for d in stock_entry.get('items'): - # self.assertEqual(item_quantity.get(d.item_code), d.qty) - def test_customer_provided_parts_se(self): create_item( "CUST-0987", is_customer_provided_item=1, customer="_Test Customer", is_purchase_item=0 @@ -1358,6 +1319,13 @@ class TestStockEntry(FrappeTestCase): issue.reload() # reload because reposting current voucher updates rate self.assertEqual(issue.value_difference, -30) + def test_transfer_qty_validation(self): + se = make_stock_entry(item_code="_Test Item", do_not_save=True, qty=0.001, rate=100) + se.items[0].uom = "Kg" + se.items[0].conversion_factor = 0.002 + + self.assertRaises(frappe.ValidationError, se.save) + def make_serialized_item(**args): args = frappe._dict(args) From 6528218ac31001e04e6b5ebfa0f3d429e296742f Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Tue, 29 Mar 2022 18:43:33 +0530 Subject: [PATCH 17/54] perf: skip warehouse validation for non-stock items --- erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py | 3 ++- erpnext/controllers/accounts_controller.py | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py index 84077f6ac0..e6a46d0676 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py @@ -249,8 +249,9 @@ class PurchaseInvoice(BuyingController): def validate_warehouse(self, for_validate=True): if self.update_stock and for_validate: + stock_items = self.get_stock_items() for d in self.get("items"): - if not d.warehouse and not d.is_fixed_asset: + if not d.warehouse and d.item_code in stock_items: frappe.throw( _( "Row No {0}: Warehouse is required. Please set a Default Warehouse for Item {1} and Company {2}" diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 72ac1b37ef..3ad61d4dc2 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -1264,6 +1264,9 @@ class AccountsController(TransactionBase): return get_company_default(self.company, fieldname, ignore_validation=ignore_validation) def get_stock_items(self): + if hasattr(self, "_stock_items") and self._stock_items: + return self._stock_items + stock_items = [] item_codes = list(set(item.item_code for item in self.get("items"))) if item_codes: @@ -1279,6 +1282,7 @@ class AccountsController(TransactionBase): ) ] + self._stock_items = stock_items return stock_items def set_total_advance_paid(self): From b5ad626d23fc4246e5665b3df78d9eca181b0d4f Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 30 Mar 2022 10:22:05 +0530 Subject: [PATCH 18/54] fix: Linter --- erpnext/manufacturing/doctype/work_order/test_work_order.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/erpnext/manufacturing/doctype/work_order/test_work_order.py b/erpnext/manufacturing/doctype/work_order/test_work_order.py index 9209cbf755..8934f9c4e0 100644 --- a/erpnext/manufacturing/doctype/work_order/test_work_order.py +++ b/erpnext/manufacturing/doctype/work_order/test_work_order.py @@ -1114,7 +1114,10 @@ class TestWorkOrder(FrappeTestCase): except frappe.MandatoryError: self.fail("Batch generation causing failing in Work Order") - @change_settings("Manufacturing Settings", {"backflush_raw_materials_based_on": "Material Transferred for Manufacture"}) + @change_settings( + "Manufacturing Settings", + {"backflush_raw_materials_based_on": "Material Transferred for Manufacture"}, + ) def test_manufacture_entry_mapped_idx_with_exploded_bom(self): """Test if WO containing BOM with partial exploded items and scrap items, maps idx correctly.""" test_stock_entry.make_stock_entry( @@ -1141,6 +1144,7 @@ class TestWorkOrder(FrappeTestCase): for index, row in enumerate(ste_manu.get("items"), start=1): self.assertEqual(index, row.idx) + def update_job_card(job_card, jc_qty=None): employee = frappe.db.get_value("Employee", {"status": "Active"}, "name") job_card_doc = frappe.get_doc("Job Card", job_card) From fe962547301d617cd082e3fcd1c6013f5c6dcd20 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 30 Mar 2022 12:34:36 +0530 Subject: [PATCH 19/54] chore: skip style checks in sider [skip ci] --- .flake8 | 1 + 1 file changed, 1 insertion(+) diff --git a/.flake8 b/.flake8 index 4ff8840324..4b852abd7c 100644 --- a/.flake8 +++ b/.flake8 @@ -31,6 +31,7 @@ ignore = E124, # closing bracket, irritating while writing QB code E131, # continuation line unaligned for hanging indent E123, # closing bracket does not match indentation of opening bracket's line + E101, # ensured by use of black max-line-length = 200 exclude=.github/helper/semgrep_rules From d4cc7c553eaec88edb128da3224cb94650cc12ed Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Wed, 30 Mar 2022 12:43:18 +0530 Subject: [PATCH 20/54] fix: Account currency validation --- erpnext/accounts/doctype/account/account.py | 4 +- .../accounts/doctype/account/test_account.py | 76 ++++--------------- 2 files changed, 19 insertions(+), 61 deletions(-) diff --git a/erpnext/accounts/doctype/account/account.py b/erpnext/accounts/doctype/account/account.py index 150f68b7bd..c71ea3648b 100644 --- a/erpnext/accounts/doctype/account/account.py +++ b/erpnext/accounts/doctype/account/account.py @@ -204,7 +204,9 @@ class Account(NestedSet): if not self.account_currency: self.account_currency = frappe.get_cached_value("Company", self.company, "default_currency") - elif self.account_currency != frappe.db.get_value("Account", self.name, "account_currency"): + gl_currency = frappe.db.get_value("GL Entry", {"account": self.name}, "account_currency") + + if gl_currency and self.account_currency != gl_currency: if frappe.db.get_value("GL Entry", {"account": self.name}): frappe.throw(_("Currency can not be changed after making entries using some other currency")) diff --git a/erpnext/accounts/doctype/account/test_account.py b/erpnext/accounts/doctype/account/test_account.py index efc063de56..a6d44882eb 100644 --- a/erpnext/accounts/doctype/account/test_account.py +++ b/erpnext/accounts/doctype/account/test_account.py @@ -241,71 +241,27 @@ class TestAccount(unittest.TestCase): for doc in to_delete: frappe.delete_doc("Account", doc) + def test_validate_account_currency(self): + from erpnext.accounts.doctype.journal_entry.test_journal_entry import make_journal_entry -def _make_test_records(verbose=None): - from frappe.test_runner import make_test_objects + if not frappe.db.get_value("Account", "Test Currency Account - _TC"): + acc = frappe.new_doc("Account") + acc.account_name = "Test Currency Account" + acc.parent_account = "Tax Assets - _TC" + acc.company = "_Test Company" + acc.insert() + else: + acc = frappe.get_doc("Account", "Test Currency Account - _TC") - accounts = [ - # [account_name, parent_account, is_group] - ["_Test Bank", "Bank Accounts", 0, "Bank", None], - ["_Test Bank USD", "Bank Accounts", 0, "Bank", "USD"], - ["_Test Bank EUR", "Bank Accounts", 0, "Bank", "EUR"], - ["_Test Cash", "Cash In Hand", 0, "Cash", None], - ["_Test Account Stock Expenses", "Direct Expenses", 1, None, None], - ["_Test Account Shipping Charges", "_Test Account Stock Expenses", 0, "Chargeable", None], - ["_Test Account Customs Duty", "_Test Account Stock Expenses", 0, "Tax", None], - ["_Test Account Insurance Charges", "_Test Account Stock Expenses", 0, "Chargeable", None], - ["_Test Account Stock Adjustment", "_Test Account Stock Expenses", 0, "Stock Adjustment", None], - ["_Test Employee Advance", "Current Liabilities", 0, None, None], - ["_Test Account Tax Assets", "Current Assets", 1, None, None], - ["_Test Account VAT", "_Test Account Tax Assets", 0, "Tax", None], - ["_Test Account Service Tax", "_Test Account Tax Assets", 0, "Tax", None], - ["_Test Account Reserves and Surplus", "Current Liabilities", 0, None, None], - ["_Test Account Cost for Goods Sold", "Expenses", 0, None, None], - ["_Test Account Excise Duty", "_Test Account Tax Assets", 0, "Tax", None], - ["_Test Account Education Cess", "_Test Account Tax Assets", 0, "Tax", None], - ["_Test Account S&H Education Cess", "_Test Account Tax Assets", 0, "Tax", None], - ["_Test Account CST", "Direct Expenses", 0, "Tax", None], - ["_Test Account Discount", "Direct Expenses", 0, None, None], - ["_Test Write Off", "Indirect Expenses", 0, None, None], - ["_Test Exchange Gain/Loss", "Indirect Expenses", 0, None, None], - ["_Test Account Sales", "Direct Income", 0, None, None], - # related to Account Inventory Integration - ["_Test Account Stock In Hand", "Current Assets", 0, None, None], - # fixed asset depreciation - ["_Test Fixed Asset", "Current Assets", 0, "Fixed Asset", None], - ["_Test Accumulated Depreciations", "Current Assets", 0, "Accumulated Depreciation", None], - ["_Test Depreciations", "Expenses", 0, None, None], - ["_Test Gain/Loss on Asset Disposal", "Expenses", 0, None, None], - # Receivable / Payable Account - ["_Test Receivable", "Current Assets", 0, "Receivable", None], - ["_Test Payable", "Current Liabilities", 0, "Payable", None], - ["_Test Receivable USD", "Current Assets", 0, "Receivable", "USD"], - ["_Test Payable USD", "Current Liabilities", 0, "Payable", "USD"], - ] + self.assertEqual(acc.account_currency, "INR") - for company, abbr in [ - ["_Test Company", "_TC"], - ["_Test Company 1", "_TC1"], - ["_Test Company with perpetual inventory", "TCP1"], - ]: - test_objects = make_test_objects( - "Account", - [ - { - "doctype": "Account", - "account_name": account_name, - "parent_account": parent_account + " - " + abbr, - "company": company, - "is_group": is_group, - "account_type": account_type, - "account_currency": currency, - } - for account_name, parent_account, is_group, account_type, currency in accounts - ], + # Make a JV against this account + make_journal_entry( + "Test Currency Account - _TC", "Miscellaneous Expenses - _TC", 100, submit=True ) - return test_objects + acc.account_currency = "USD" + self.assertRaises(frappe.ValidationError, acc.save) def get_inventory_account(company, warehouse=None): From 62c57199f2894cfe7ecd93c8d2ffffafa2bb7e8b Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 30 Mar 2022 12:50:37 +0530 Subject: [PATCH 21/54] chore: remove ui test config [skip ci] --- .github/workflows/ui-tests.yml | 117 --------------------------------- 1 file changed, 117 deletions(-) delete mode 100644 .github/workflows/ui-tests.yml diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml deleted file mode 100644 index ab6a53b5d9..0000000000 --- a/.github/workflows/ui-tests.yml +++ /dev/null @@ -1,117 +0,0 @@ -name: UI - -on: - pull_request: - paths-ignore: - - '**.md' - workflow_dispatch: - -concurrency: - group: ui-develop-${{ github.event.number }} - cancel-in-progress: true - -jobs: - test: - runs-on: ubuntu-latest - timeout-minutes: 60 - - strategy: - fail-fast: false - - name: UI Tests (Cypress) - - services: - mysql: - image: mariadb:10.3 - env: - MYSQL_ALLOW_EMPTY_PASSWORD: YES - ports: - - 3306:3306 - options: --health-cmd="mysqladmin ping" --health-interval=5s --health-timeout=2s --health-retries=3 - - steps: - - name: Clone - uses: actions/checkout@v2 - - - name: Setup Python - uses: actions/setup-python@v2 - with: - python-version: 3.8 - - - uses: actions/setup-node@v2 - with: - node-version: 14 - check-latest: true - - - name: Add to Hosts - run: | - echo "127.0.0.1 test_site" | sudo tee -a /etc/hosts - - - name: Cache pip - uses: actions/cache@v2 - with: - path: ~/.cache/pip - key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }} - restore-keys: | - ${{ runner.os }}-pip- - ${{ runner.os }}- - - - name: Cache node modules - uses: actions/cache@v2 - env: - cache-name: cache-node-modules - with: - path: ~/.npm - key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }} - restore-keys: | - ${{ runner.os }}-build-${{ env.cache-name }}- - ${{ runner.os }}-build- - ${{ runner.os }}- - - - name: Get yarn cache directory path - id: yarn-cache-dir-path - run: echo "::set-output name=dir::$(yarn cache dir)" - - - uses: actions/cache@v2 - id: yarn-cache - with: - path: ${{ steps.yarn-cache-dir-path.outputs.dir }} - key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }} - restore-keys: | - ${{ runner.os }}-yarn- - - - name: Cache cypress binary - uses: actions/cache@v2 - with: - path: ~/.cache - key: ${{ runner.os }}-cypress- - restore-keys: | - ${{ runner.os }}-cypress- - ${{ runner.os }}- - - - name: Install - run: bash ${GITHUB_WORKSPACE}/.github/helper/install.sh - env: - DB: mariadb - TYPE: ui - - - name: Site Setup - run: cd ~/frappe-bench/ && bench --site test_site execute erpnext.setup.utils.before_tests - - - name: cypress pre-requisites - run: cd ~/frappe-bench/apps/frappe && yarn add cypress-file-upload@^5 @testing-library/cypress@^8 --no-lockfile - - - - name: Build Assets - run: cd ~/frappe-bench/ && bench build - env: - CI: Yes - - - name: UI Tests - run: cd ~/frappe-bench/ && bench --site test_site run-ui-tests erpnext --headless - env: - CYPRESS_RECORD_KEY: 60a8e3bf-08f5-45b1-9269-2b207d7d30cd - - - name: Show bench console if tests failed - if: ${{ failure() }} - run: cat ~/frappe-bench/bench_run_logs.txt From 1d1e925bcf6066cac03abfb60510e76d0f97f9be Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 30 Mar 2022 13:01:01 +0530 Subject: [PATCH 22/54] test: API hit via BOM Update Tool - test creation of log and it's impact --- .../bom_update_log/test_bom_update_log.py | 83 ++++++++++++++++++- .../bom_update_tool/test_bom_update_tool.py | 2 + 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom_update_log/test_bom_update_log.py b/erpnext/manufacturing/doctype/bom_update_log/test_bom_update_log.py index f74bdc356a..52ca9cde1b 100644 --- a/erpnext/manufacturing/doctype/bom_update_log/test_bom_update_log.py +++ b/erpnext/manufacturing/doctype/bom_update_log/test_bom_update_log.py @@ -1,9 +1,88 @@ # Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # See license.txt -# import frappe +import frappe from frappe.tests.utils import FrappeTestCase +from erpnext.manufacturing.doctype.bom_update_log.bom_update_log import ( + BOMMissingError, + run_bom_job, +) +from erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool import enqueue_replace_bom + +test_records = frappe.get_test_records("BOM") + class TestBOMUpdateLog(FrappeTestCase): - pass + "Test BOM Update Tool Operations via BOM Update Log." + + def setUp(self): + bom_doc = frappe.copy_doc(test_records[0]) + bom_doc.items[1].item_code = "_Test Item" + bom_doc.insert() + + self.boms = frappe._dict( + current_bom="BOM-_Test Item Home Desktop Manufactured-001", + new_bom=bom_doc.name, + ) + + self.new_bom_doc = bom_doc + + def tearDown(self): + frappe.db.rollback() + + if self._testMethodName == "test_bom_update_log_completion": + # clear logs and delete BOM created via setUp + frappe.db.delete("BOM Update Log") + self.new_bom_doc.cancel() + self.new_bom_doc.delete() + frappe.db.commit() # explicitly commit and restore to original state + + def test_bom_update_log_validate(self): + "Test if BOM presence is validated." + + with self.assertRaises(BOMMissingError): + enqueue_replace_bom(boms={}) + + def test_bom_update_log_queueing(self): + "Test if BOM Update Log is created and queued." + + log = enqueue_replace_bom( + boms=self.boms, + ) + + self.assertEqual(log.docstatus, 1) + self.assertEqual(log.status, "Queued") + + def test_bom_update_log_completion(self): + "Test if BOM Update Log handles job completion correctly." + + log = enqueue_replace_bom( + boms=self.boms, + ) + + # Explicitly commits log, new bom (setUp) and replacement impact. + # Is run via background jobs IRL + run_bom_job( + doc=log, + boms=self.boms, + update_type="Replace BOM", + ) + log.reload() + + self.assertEqual(log.status, "Completed") + + # teardown (undo replace impact) due to commit + boms = frappe._dict( + current_bom=self.boms.new_bom, + new_bom=self.boms.current_bom, + ) + log2 = enqueue_replace_bom( + boms=self.boms, + ) + run_bom_job( # Explicitly commits + doc=log2, + boms=boms, + update_type="Replace BOM", + ) + self.assertEqual(log2.status, "Completed") diff --git a/erpnext/manufacturing/doctype/bom_update_tool/test_bom_update_tool.py b/erpnext/manufacturing/doctype/bom_update_tool/test_bom_update_tool.py index cc34d5f6b0..fae72a0f6f 100644 --- a/erpnext/manufacturing/doctype/bom_update_tool/test_bom_update_tool.py +++ b/erpnext/manufacturing/doctype/bom_update_tool/test_bom_update_tool.py @@ -13,6 +13,8 @@ test_records = frappe.get_test_records("BOM") class TestBOMUpdateTool(FrappeTestCase): + "Test major functions run via BOM Update Tool." + def test_replace_bom(self): current_bom = "BOM-_Test Item Home Desktop Manufactured-001" From 79495679e209a31a1865b7d4bd1bfc42c4813403 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 30 Mar 2022 13:08:58 +0530 Subject: [PATCH 23/54] fix: Auto format `bom_update_log.py` --- .../doctype/bom_update_log/bom_update_log.py | 68 +++++++++---------- 1 file changed, 33 insertions(+), 35 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py index 7f60d8fc7d..172f38d250 100644 --- a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py +++ b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py @@ -15,6 +15,7 @@ from erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool import update class BOMMissingError(frappe.ValidationError): pass + class BOMUpdateLog(Document): def validate(self): if self.update_type == "Replace BOM": @@ -28,7 +29,8 @@ class BOMUpdateLog(Document): if self.update_type == "Replace BOM" and not (self.current_bom and self.new_bom): frappe.throw( msg=_("Please mention the Current and New BOM for replacement."), - title=_("Mandatory"), exc=BOMMissingError + title=_("Mandatory"), + exc=BOMMissingError, ) def validate_same_bom(self): @@ -47,20 +49,22 @@ class BOMUpdateLog(Document): return if self.update_type == "Replace BOM": - boms = { - "current_bom": self.current_bom, - "new_bom": self.new_bom - } + boms = {"current_bom": self.current_bom, "new_bom": self.new_bom} frappe.enqueue( method="erpnext.manufacturing.doctype.bom_update_log.bom_update_log.run_bom_job", - doc=self, boms=boms, timeout=40000 + doc=self, + boms=boms, + timeout=40000, ) else: frappe.enqueue( method="erpnext.manufacturing.doctype.bom_update_log.bom_update_log.run_bom_job", - doc=self, update_type="Update Cost", timeout=40000 + doc=self, + update_type="Update Cost", + timeout=40000, ) + def replace_bom(boms: Dict) -> None: """Replace current BOM with new BOM in parent BOMs.""" current_bom = boms.get("current_bom") @@ -69,13 +73,13 @@ def replace_bom(boms: Dict) -> None: unit_cost = get_new_bom_unit_cost(new_bom) update_new_bom(unit_cost, current_bom, new_bom) - frappe.cache().delete_key('bom_children') + frappe.cache().delete_key("bom_children") parent_boms = get_parent_boms(new_bom) with click.progressbar(parent_boms) as parent_boms: pass for bom in parent_boms: - bom_obj = frappe.get_cached_doc('BOM', bom) + bom_obj = frappe.get_cached_doc("BOM", bom) # this is only used for versioning and we do not want # to make separate db calls by using load_doc_before_save # which proves to be expensive while doing bulk replace @@ -85,34 +89,29 @@ def replace_bom(boms: Dict) -> None: bom_obj.calculate_cost() bom_obj.update_parent_cost() bom_obj.db_update() - if bom_obj.meta.get('track_changes') and not bom_obj.flags.ignore_version: + if bom_obj.meta.get("track_changes") and not bom_obj.flags.ignore_version: bom_obj.save_version() + def update_new_bom(unit_cost: float, current_bom: str, new_bom: str) -> None: bom_item = frappe.qb.DocType("BOM Item") - frappe.qb.update(bom_item).set( - bom_item.bom_no, new_bom - ).set( - bom_item.rate, unit_cost - ).set( + frappe.qb.update(bom_item).set(bom_item.bom_no, new_bom).set(bom_item.rate, unit_cost).set( bom_item.amount, (bom_item.stock_qty * unit_cost) ).where( - (bom_item.bom_no == current_bom) - & (bom_item.docstatus < 2) - & (bom_item.parenttype == "BOM") + (bom_item.bom_no == current_bom) & (bom_item.docstatus < 2) & (bom_item.parenttype == "BOM") ).run() + def get_parent_boms(new_bom: str, bom_list: Optional[List] = None) -> List: bom_list = bom_list or [] bom_item = frappe.qb.DocType("BOM Item") - parents = frappe.qb.from_(bom_item).select( - bom_item.parent - ).where( - (bom_item.bom_no == new_bom) - & (bom_item.docstatus <2) - & (bom_item.parenttype == "BOM") - ).run(as_dict=True) + parents = ( + frappe.qb.from_(bom_item) + .select(bom_item.parent) + .where((bom_item.bom_no == new_bom) & (bom_item.docstatus < 2) & (bom_item.parenttype == "BOM")) + .run(as_dict=True) + ) for d in parents: if new_bom == d.parent: @@ -123,17 +122,19 @@ def get_parent_boms(new_bom: str, bom_list: Optional[List] = None) -> List: return list(set(bom_list)) + def get_new_bom_unit_cost(new_bom: str) -> float: bom = frappe.qb.DocType("BOM") - new_bom_unitcost = frappe.qb.from_(bom).select( - bom.total_cost / bom.quantity - ).where( - bom.name == new_bom - ).run() + new_bom_unitcost = ( + frappe.qb.from_(bom).select(bom.total_cost / bom.quantity).where(bom.name == new_bom).run() + ) return flt(new_bom_unitcost[0][0]) -def run_bom_job(doc: "BOMUpdateLog", boms: Optional[Dict] = None, update_type: Optional[str] = "Replace BOM") -> None: + +def run_bom_job( + doc: "BOMUpdateLog", boms: Optional[Dict] = None, update_type: Optional[str] = "Replace BOM" +) -> None: try: doc.db_set("status", "In Progress") if not frappe.flags.in_test: @@ -152,10 +153,7 @@ def run_bom_job(doc: "BOMUpdateLog", boms: Optional[Dict] = None, update_type: O except (Exception, JobTimeoutException): frappe.db.rollback() - error_log = frappe.log_error( - message=frappe.get_traceback(), - title=_("BOM Update Tool Error") - ) + error_log = frappe.log_error(message=frappe.get_traceback(), title=_("BOM Update Tool Error")) doc.db_set("status", "Failed") doc.db_set("error_log", error_log.name) From 8ab226a2fc9056491b9abf59b44be46414bd7a89 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 30 Mar 2022 13:20:54 +0530 Subject: [PATCH 24/54] test: add class to uniquely id item dashboard (#30487) --- erpnext/stock/dashboard/item_dashboard.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/stock/dashboard/item_dashboard.html b/erpnext/stock/dashboard/item_dashboard.html index 99698ba69a..b7a786e50f 100644 --- a/erpnext/stock/dashboard/item_dashboard.html +++ b/erpnext/stock/dashboard/item_dashboard.html @@ -1,4 +1,4 @@ -
+