From f57725f8fa016b9826e8fdf2f14dbf1a3d9991f7 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 9 Mar 2022 19:03:07 +0530 Subject: [PATCH 01/43] 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 0e3955f5a73..c7197347c2d 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 4e92926a525b396173dbc4d6dd476b2ab4874f9b Mon Sep 17 00:00:00 2001 From: Abhinav Raut Date: Fri, 11 Mar 2022 16:44:21 +0530 Subject: [PATCH 02/43] fix: incorrect payable amount for loan closure - Add penalty amount to payable amount for loan closure --- erpnext/loan_management/doctype/loan_repayment/loan_repayment.py | 1 + 1 file changed, 1 insertion(+) diff --git a/erpnext/loan_management/doctype/loan_repayment/loan_repayment.py b/erpnext/loan_management/doctype/loan_repayment/loan_repayment.py index 67c2b1ee14d..63dea6f726a 100644 --- a/erpnext/loan_management/doctype/loan_repayment/loan_repayment.py +++ b/erpnext/loan_management/doctype/loan_repayment/loan_repayment.py @@ -609,5 +609,6 @@ def calculate_amounts(against_loan, posting_date, payment_type=''): amounts['payable_principal_amount'] = amounts['pending_principal_amount'] amounts['interest_amount'] += amounts['unaccrued_interest'] amounts['payable_amount'] = amounts['payable_principal_amount'] + amounts['interest_amount'] + amounts['payable_amount'] = amounts['penalty_amount'] return amounts From 8c76a76154d8976760d19d95f421dd2b0ee238bf Mon Sep 17 00:00:00 2001 From: Abhinav Raut Date: Fri, 11 Mar 2022 16:46:30 +0530 Subject: [PATCH 03/43] fix: incorrect payable amount for loan closure --- .../loan_management/doctype/loan_repayment/loan_repayment.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/loan_management/doctype/loan_repayment/loan_repayment.py b/erpnext/loan_management/doctype/loan_repayment/loan_repayment.py index 63dea6f726a..f7e75768a82 100644 --- a/erpnext/loan_management/doctype/loan_repayment/loan_repayment.py +++ b/erpnext/loan_management/doctype/loan_repayment/loan_repayment.py @@ -609,6 +609,6 @@ def calculate_amounts(against_loan, posting_date, payment_type=''): amounts['payable_principal_amount'] = amounts['pending_principal_amount'] amounts['interest_amount'] += amounts['unaccrued_interest'] amounts['payable_amount'] = amounts['payable_principal_amount'] + amounts['interest_amount'] - amounts['payable_amount'] = amounts['penalty_amount'] + amounts['payable_amount'] += amounts['penalty_amount'] return amounts From 4283a13e5a6a6b9f1e8e1cbcc639646a4e957b36 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 16 Mar 2022 19:45:03 +0530 Subject: [PATCH 04/43] 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 f8c42887fd8..c3cc1e4d572 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 00000000000..e69de29bb2d 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 00000000000..6da808e26d1 --- /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 00000000000..222168be8cf --- /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 00000000000..10db0de9a11 --- /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 00000000000..f74bdc356a7 --- /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 c7197347c2d..fad53f05684 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 05/43] 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 222168be8cf..d89427edc0b 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 10db0de9a11..b08d6f906c2 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 bf5fe2e18de..ec6a76d61c4 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 fad53f05684..16add4fc113 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 06/43] 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 d89427edc0b..38c685a64f1 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 b08d6f906c2..a69b15c5274 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 00000000000..8b3dc520cfa --- /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 07/43] 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 a69b15c5274..7f60d8fc7df 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 8b3dc520cfa..e39b5637c78 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 ec6a76d61c4..0c9816712c2 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 16add4fc113..bc3e82006b3 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 08/43] 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 0c9816712c2..a793ed95354 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 bc3e82006b3..3da8afee156 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 b4c625d6108..c99e88893a6 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 1b2c6a5b78d4ee2e31817eb78bb1f614b672eda4 Mon Sep 17 00:00:00 2001 From: Deepesh Garg <42651287+deepeshgarg007@users.noreply.github.com> Date: Mon, 21 Mar 2022 17:06:23 +0530 Subject: [PATCH 09/43] fix: Code cleanup --- .../loan_management/doctype/loan_repayment/loan_repayment.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/erpnext/loan_management/doctype/loan_repayment/loan_repayment.py b/erpnext/loan_management/doctype/loan_repayment/loan_repayment.py index f7e75768a82..07100001525 100644 --- a/erpnext/loan_management/doctype/loan_repayment/loan_repayment.py +++ b/erpnext/loan_management/doctype/loan_repayment/loan_repayment.py @@ -608,7 +608,6 @@ def calculate_amounts(against_loan, posting_date, payment_type=''): if payment_type == 'Loan Closure': amounts['payable_principal_amount'] = amounts['pending_principal_amount'] amounts['interest_amount'] += amounts['unaccrued_interest'] - amounts['payable_amount'] = amounts['payable_principal_amount'] + amounts['interest_amount'] - amounts['payable_amount'] += amounts['penalty_amount'] + amounts['payable_amount'] = amounts['payable_principal_amount'] + amounts['interest_amount'] + amounts['penalty_amount'] return amounts From 136466d255651ba29be16248e822c2a374114c67 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Tue, 29 Mar 2022 10:47:27 +0530 Subject: [PATCH 10/43] 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 7654aa44dc6..d1e7fb40d12 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 6528218ac31001e04e6b5ebfa0f3d429e296742f Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Tue, 29 Mar 2022 18:43:33 +0530 Subject: [PATCH 11/43] 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 84077f6ac0b..e6a46d0676b 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 72ac1b37ef6..3ad61d4dc24 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 d4cc7c553eaec88edb128da3224cb94650cc12ed Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Wed, 30 Mar 2022 12:43:18 +0530 Subject: [PATCH 12/43] 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 150f68b7bd3..c71ea3648b9 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 efc063de56a..a6d44882eb6 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 1d1e925bcf6066cac03abfb60510e76d0f97f9be Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 30 Mar 2022 13:01:01 +0530 Subject: [PATCH 13/43] 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 f74bdc356a7..52ca9cde1bd 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 cc34d5f6b04..fae72a0f6f7 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 14/43] 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 7f60d8fc7df..172f38d250f 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 ebf00946c91bf03105533d46c85e9b405cc7d62a Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 30 Mar 2022 13:22:29 +0530 Subject: [PATCH 15/43] fix: Semgrep - Explain explicit commits and skip semgrep - Format client side translated string correctly --- .../manufacturing/doctype/bom_update_log/bom_update_log.py | 2 +- .../doctype/bom_update_log/test_bom_update_log.py | 4 +++- .../manufacturing/doctype/bom_update_tool/bom_update_tool.js | 2 +- 3 files changed, 5 insertions(+), 3 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 172f38d250f..ce2774347b2 100644 --- a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py +++ b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py @@ -160,4 +160,4 @@ def run_bom_job( finally: frappe.db.auto_commit_on_many_writes = 0 - frappe.db.commit() + frappe.db.commit() # nosemgrep 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 52ca9cde1bd..d1da18d0ab8 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 @@ -36,7 +36,9 @@ class TestBOMUpdateLog(FrappeTestCase): 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 + + # explicitly commit and restore to original state + frappe.db.commit() # nosemgrep def test_bom_update_log_validate(self): "Test if BOM presence is validated." 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 a793ed95354..7ba6517a4fb 100644 --- a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.js +++ b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.js @@ -78,7 +78,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); frappe.msgprint({ - "message": __(`BOM Updation is queued and may take a few minutes. Check ${log_link} for progress.`), + "message": __("BOM Updation is queued and may take a few minutes. Check {0} for progress.", [log_link]), "title": __("BOM Update Initiated"), "indicator": "blue" }); From 7bece37918734f527e628a70598d8d1e67f88ca9 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Wed, 30 Mar 2022 15:35:56 +0530 Subject: [PATCH 16/43] fix: update `get_cached_value` usage based on changes in definition --- erpnext/accounts/doctype/pricing_rule/pricing_rule.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py index 08cec6a858a..c45b069730e 100644 --- a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py +++ b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py @@ -375,12 +375,12 @@ def get_pricing_rule_for_item(args, price_list_rate=0, doc=None, for_validate=Fa def update_args_for_pricing_rule(args): if not (args.item_group and args.brand): - try: - args.item_group, args.brand = frappe.get_cached_value( - "Item", args.item_code, ["item_group", "brand"] - ) - except frappe.DoesNotExistError: + item = frappe.get_cached_value("Item", args.item_code, ("item_group", "brand")) + if not item: return + + args.item_group, args.brand = item + if not args.item_group: frappe.throw(_("Item Group not mentioned in item master for item {0}").format(args.item_code)) From 89a9afab94e17570ece7850606cdf00a8b84ed06 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 30 Mar 2022 17:00:28 +0530 Subject: [PATCH 17/43] fix: cast array slice index integer while splitting serial_nos array (#30468) (#30496) (cherry picked from commit 3f3717952c94af428c3d4c919aceb6b8cbae7679) Co-authored-by: Anoop --- erpnext/manufacturing/doctype/work_order/work_order.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/manufacturing/doctype/work_order/work_order.py b/erpnext/manufacturing/doctype/work_order/work_order.py index c8c2f9a9320..2ee848c356a 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.py +++ b/erpnext/manufacturing/doctype/work_order/work_order.py @@ -1327,7 +1327,7 @@ def get_serial_nos_for_job_card(row, wo_doc): used_serial_nos.extend(get_serial_nos(d.serial_no)) serial_nos = sorted(list(set(serial_nos) - set(used_serial_nos))) - row.serial_no = "\n".join(serial_nos[0 : row.job_card_qty]) + row.serial_no = "\n".join(serial_nos[0 : cint(row.job_card_qty)]) def validate_operation_data(row): From 620575a9012a9759c6285558ac25c6709c4e92cc Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 30 Mar 2022 18:03:52 +0530 Subject: [PATCH 18/43] fix: Type Annotations, Redundancy, etc. - Renamed public function`update_new_bom` to `update_new_bom_in_bom_items` - Replaced `get_cached_doc` with `get_doc` - Removed click progress bar (drive through update log) - Removed `bom_obj.update_new_bom()`, was redundant. Did same job as `update_new_bom_in_bom_items` - Removed `update_new_bom()` in `bom.py`, unused. - Prettier query formatting - `update_type` annotated as non optional Literal - Removed redundant use of JobTimeoutException - Corrected type annotations in `create_bom_update_log()` --- erpnext/manufacturing/doctype/bom/bom.py | 9 ------ .../doctype/bom_update_log/bom_update_log.py | 31 ++++++++++--------- .../bom_update_tool/bom_update_tool.py | 6 ++-- 3 files changed, 20 insertions(+), 26 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index bf29474c001..8c0112bad0f 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -697,15 +697,6 @@ class BOM(WebsiteGenerator): self.scrap_material_cost = total_sm_cost self.base_scrap_material_cost = base_total_sm_cost - def update_new_bom(self, old_bom, new_bom, rate): - for d in self.get("items"): - if d.bom_no != old_bom: - continue - - d.bom_no = new_bom - d.rate = rate - d.amount = (d.stock_qty or d.qty) * rate - def update_exploded_items(self, save=True): """Update Flat BOM, following will be correct data""" self.get_exploded_items() 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 ce2774347b2..139dcbcdd90 100644 --- a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py +++ b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.py @@ -1,13 +1,11 @@ # Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and contributors # For license information, please see license.txt -from typing import Dict, List, Optional +from typing import Dict, List, Literal, Optional -import click import frappe from frappe import _ from frappe.model.document import Document from frappe.utils import cstr, flt -from rq.timeouts import JobTimeoutException from erpnext.manufacturing.doctype.bom_update_tool.bom_update_tool import update_cost @@ -71,20 +69,17 @@ def replace_bom(boms: Dict) -> None: new_bom = boms.get("new_bom") unit_cost = get_new_bom_unit_cost(new_bom) - update_new_bom(unit_cost, current_bom, new_bom) + update_new_bom_in_bom_items(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) + bom_obj = frappe.get_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() @@ -93,12 +88,16 @@ def replace_bom(boms: Dict) -> None: bom_obj.save_version() -def update_new_bom(unit_cost: float, current_bom: str, new_bom: str) -> None: +def update_new_bom_in_bom_items(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") + ( + 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() @@ -133,7 +132,9 @@ def get_new_bom_unit_cost(new_bom: str) -> float: def run_bom_job( - doc: "BOMUpdateLog", boms: Optional[Dict] = None, update_type: Optional[str] = "Replace BOM" + doc: "BOMUpdateLog", + boms: Optional[Dict[str, str]] = None, + update_type: Literal["Replace BOM", "Update Cost"] = "Replace BOM", ) -> None: try: doc.db_set("status", "In Progress") @@ -151,7 +152,7 @@ def run_bom_job( doc.db_set("status", "Completed") - except (Exception, JobTimeoutException): + except Exception: frappe.db.rollback() error_log = frappe.log_error(message=frappe.get_traceback(), title=_("BOM Update Tool Error")) 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 a9189dd2abe..b0e7da12017 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 TYPE_CHECKING, Dict, Optional, Union +from typing import TYPE_CHECKING, Dict, Literal, Optional, Union if TYPE_CHECKING: from erpnext.manufacturing.doctype.bom_update_log.bom_update_log import BOMUpdateLog @@ -51,9 +51,11 @@ def update_cost() -> None: def create_bom_update_log( - boms: Optional[Dict] = None, update_type: str = "Replace BOM" + boms: Optional[Dict[str, str]] = None, + update_type: Literal["Replace BOM", "Update Cost"] = "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") From a945484af4f69c8b698a2283f4078b99c38df039 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 30 Mar 2022 18:20:54 +0530 Subject: [PATCH 19/43] test: Added test for 2 more validations - Covers full validate function --- .../doctype/bom_update_log/test_bom_update_log.py | 6 ++++++ 1 file changed, 6 insertions(+) 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 d1da18d0ab8..47efea961b4 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 @@ -46,6 +46,12 @@ class TestBOMUpdateLog(FrappeTestCase): with self.assertRaises(BOMMissingError): enqueue_replace_bom(boms={}) + with self.assertRaises(frappe.ValidationError): + enqueue_replace_bom(boms=frappe._dict(current_bom=self.boms.new_bom, new_bom=self.boms.new_bom)) + + with self.assertRaises(frappe.ValidationError): + enqueue_replace_bom(boms=frappe._dict(current_bom=self.boms.new_bom, new_bom="Dummy BOM")) + def test_bom_update_log_queueing(self): "Test if BOM Update Log is created and queued." From 532961fad2a1372edfea902d58d2feb98eace889 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Wed, 30 Mar 2022 19:33:44 +0530 Subject: [PATCH 20/43] fix(India): Tax fetching based on tax category --- erpnext/regional/india/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/erpnext/regional/india/utils.py b/erpnext/regional/india/utils.py index 47e6ae67f4e..48e17516a5d 100644 --- a/erpnext/regional/india/utils.py +++ b/erpnext/regional/india/utils.py @@ -268,6 +268,7 @@ def get_regional_address_details(party_details, doctype, company): if tax_template_by_category: party_details["taxes_and_charges"] = tax_template_by_category + party_details["taxes"] = get_taxes_and_charges(master_doctype, tax_template_by_category) return party_details if not party_details.place_of_supply: @@ -292,7 +293,7 @@ def get_regional_address_details(party_details, doctype, company): return party_details party_details["taxes_and_charges"] = default_tax - party_details.taxes = get_taxes_and_charges(master_doctype, default_tax) + party_details["taxes"] = get_taxes_and_charges(master_doctype, default_tax) return party_details From d93edbc859268fee20be208f84d66b7542bd52ee Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Wed, 30 Mar 2022 21:07:56 +0530 Subject: [PATCH 21/43] fix: make test record --- .../accounts/doctype/account/test_account.py | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/erpnext/accounts/doctype/account/test_account.py b/erpnext/accounts/doctype/account/test_account.py index a6d44882eb6..f9c9173af08 100644 --- a/erpnext/accounts/doctype/account/test_account.py +++ b/erpnext/accounts/doctype/account/test_account.py @@ -264,6 +264,72 @@ class TestAccount(unittest.TestCase): self.assertRaises(frappe.ValidationError, acc.save) +def _make_test_records(verbose=None): + from frappe.test_runner import make_test_objects + + 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"], + ] + + 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 + ], + ) + + return test_objects + + def get_inventory_account(company, warehouse=None): account = None if warehouse: From b981fae5a410785df10e8b06a4681c3ef550d252 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 30 Mar 2022 21:46:15 +0530 Subject: [PATCH 22/43] fix: remove naming series from bin, repost queue (#30497) Removing naming series from: 1. Bin 2. Repost queue These doctypes are not user facing and dont really need naming series. Current implementation of naming makes stock transaction sequential if these documents are to be created during submission. --- erpnext/stock/doctype/bin/bin.json | 6 +++--- .../repost_item_valuation/repost_item_valuation.json | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/erpnext/stock/doctype/bin/bin.json b/erpnext/stock/doctype/bin/bin.json index 56dc71c57e1..d822f4a6095 100644 --- a/erpnext/stock/doctype/bin/bin.json +++ b/erpnext/stock/doctype/bin/bin.json @@ -1,6 +1,6 @@ { "actions": [], - "autoname": "MAT-BIN-.YYYY.-.#####", + "autoname": "hash", "creation": "2013-01-10 16:34:25", "doctype": "DocType", "engine": "InnoDB", @@ -171,11 +171,11 @@ "idx": 1, "in_create": 1, "links": [], - "modified": "2022-01-30 17:04:54.715288", + "modified": "2022-03-30 07:22:23.868602", "modified_by": "Administrator", "module": "Stock", "name": "Bin", - "naming_rule": "Expression (old style)", + "naming_rule": "Random", "owner": "Administrator", "permissions": [ { diff --git a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.json b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.json index 0ba97d59a14..6148e16513c 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.json +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.json @@ -1,6 +1,6 @@ { "actions": [], - "autoname": "REPOST-ITEM-VAL-.######", + "autoname": "hash", "creation": "2022-01-11 15:03:38.273179", "doctype": "DocType", "editable_grid": 1, @@ -177,11 +177,11 @@ "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2022-01-18 10:57:33.450907", + "modified": "2022-03-30 07:22:48.520266", "modified_by": "Administrator", "module": "Stock", "name": "Repost Item Valuation", - "naming_rule": "Expression (old style)", + "naming_rule": "Random", "owner": "Administrator", "permissions": [ { From 62266b29aa7eccbfcf496508d2d01f11a2259c7f Mon Sep 17 00:00:00 2001 From: Devin Slauenwhite Date: Wed, 30 Mar 2022 14:22:44 -0400 Subject: [PATCH 23/43] fix: exchange rate precision --- erpnext/accounts/doctype/payment_entry/payment_entry.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.js b/erpnext/accounts/doctype/payment_entry/payment_entry.js index b2b818a214d..7315ae89365 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.js +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.js @@ -532,7 +532,8 @@ frappe.ui.form.on('Payment Entry', { to_currency: to_currency }, callback: function(r, rt) { - frm.set_value(exchange_rate_field, r.message); + const ex_rate = flt(r.message, frm.get_field(exchange_rate_field).get_precision()); + frm.set_value(exchange_rate_field, ex_rate); } }) }, From 199a6da960c0419a16db59e7c93b2d23405efdc4 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Thu, 31 Mar 2022 11:41:52 +0530 Subject: [PATCH 24/43] perf: skip warehouse validation for non-stock items --- erpnext/controllers/accounts_controller.py | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 3ad61d4dc24..3a20d3f232f 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -1264,25 +1264,13 @@ 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: - stock_items = [ - r[0] - for r in frappe.db.sql( - """ - select name from `tabItem` - where name in (%s) and is_stock_item=1 - """ - % (", ".join(["%s"] * len(item_codes)),), - item_codes, - ) - ] + stock_items = frappe.db.get_values( + "Item", {"name": ["in", item_codes], "is_stock_item": 1}, pluck="name", cache=True + ) - self._stock_items = stock_items return stock_items def set_total_advance_paid(self): From 982a246eecfa0bbe9f1d31c06905227b0671267d Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 31 Mar 2022 11:47:20 +0530 Subject: [PATCH 25/43] fix: Add non-existent Item check and cleanup in `validate_for_items` - Added a validation if invalid item code ia passed via data import/API, etc. - Used DB APIs instead of raw sql - Separated checks into separate functions - Added return types to functions - Better variable naming and removed redundant utils import in function --- erpnext/buying/utils.py | 97 +++++++++++++++++++++++++---------------- 1 file changed, 59 insertions(+), 38 deletions(-) diff --git a/erpnext/buying/utils.py b/erpnext/buying/utils.py index f97cd5e9dd0..e904af0dce3 100644 --- a/erpnext/buying/utils.py +++ b/erpnext/buying/utils.py @@ -3,19 +3,19 @@ import json +from typing import Dict import frappe from frappe import _ -from frappe.utils import cint, cstr, flt +from frappe.utils import cint, cstr, flt, getdate from erpnext.stock.doctype.item.item import get_last_purchase_details, validate_end_of_life -def update_last_purchase_rate(doc, is_submit): +def update_last_purchase_rate(doc, is_submit) -> None: """updates last_purchase_rate in item table for each item""" - import frappe.utils - this_purchase_date = frappe.utils.getdate(doc.get("posting_date") or doc.get("transaction_date")) + this_purchase_date = getdate(doc.get("posting_date") or doc.get("transaction_date")) for d in doc.get("items"): # get last purchase details @@ -41,7 +41,7 @@ def update_last_purchase_rate(doc, is_submit): frappe.db.set_value("Item", d.item_code, "last_purchase_rate", flt(last_purchase_rate)) -def validate_for_items(doc): +def validate_for_items(doc) -> None: items = [] for d in doc.get("items"): if not d.qty: @@ -49,40 +49,11 @@ def validate_for_items(doc): continue frappe.throw(_("Please enter quantity for Item {0}").format(d.item_code)) - # update with latest quantities - bin = frappe.db.sql( - """select projected_qty from `tabBin` where - item_code = %s and warehouse = %s""", - (d.item_code, d.warehouse), - as_dict=1, - ) - - f_lst = { - "projected_qty": bin and flt(bin[0]["projected_qty"]) or 0, - "ordered_qty": 0, - "received_qty": 0, - } - if d.doctype in ("Purchase Receipt Item", "Purchase Invoice Item"): - f_lst.pop("received_qty") - for x in f_lst: - if d.meta.get_field(x): - d.set(x, f_lst[x]) - - item = frappe.db.sql( - """select is_stock_item, - is_sub_contracted_item, end_of_life, disabled from `tabItem` where name=%s""", - d.item_code, - as_dict=1, - )[0] - + set_stock_levels(row=d) # update with latest quantities + item = validate_item_and_get_basic_data(row=d) + validate_stock_item_warehouse(row=d, item=item) validate_end_of_life(d.item_code, item.end_of_life, item.disabled) - # validate stock item - if item.is_stock_item == 1 and d.qty and not d.warehouse and not d.get("delivered_by_supplier"): - frappe.throw( - _("Warehouse is mandatory for stock Item {0} in row {1}").format(d.item_code, d.idx) - ) - items.append(cstr(d.item_code)) if ( @@ -93,7 +64,57 @@ def validate_for_items(doc): frappe.throw(_("Same item cannot be entered multiple times.")) -def check_on_hold_or_closed_status(doctype, docname): +def set_stock_levels(row) -> None: + projected_qty = frappe.db.get_value( + "Bin", + { + "item_code": row.item_code, + "warehouse": row.warehouse, + }, + "projected_qty", + ) + + qty_data = { + "projected_qty": flt(projected_qty), + "ordered_qty": 0, + "received_qty": 0, + } + if row.doctype in ("Purchase Receipt Item", "Purchase Invoice Item"): + qty_data.pop("received_qty") + + for field in qty_data: + if row.meta.get_field(field): + row.set(field, qty_data[field]) + + +def validate_item_and_get_basic_data(row) -> Dict: + item = frappe.db.get_values( + "Item", + filters={"name": row.item_code}, + fieldname=["is_stock_item", "is_sub_contracted_item", "end_of_life", "disabled"], + as_dict=1, + ) + if not item: + frappe.throw(_("Row #{0}: Item {1} does not exist").format(row.idx, frappe.bold(row.item_code))) + + return item[0] + + +def validate_stock_item_warehouse(row, item) -> None: + if ( + item.is_stock_item == 1 + and row.qty + and not row.warehouse + and not row.get("delivered_by_supplier") + ): + frappe.throw( + _("Row #{1}: Warehouse is mandatory for stock Item {0}").format( + frappe.bold(row.item_code), row.idx + ) + ) + + +def check_on_hold_or_closed_status(doctype, docname) -> None: status = frappe.db.get_value(doctype, docname, "status") if status in ("Closed", "On Hold"): From 4720969ce62c5d2f1a8d3fbb86c6eec391b4a2c4 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 31 Mar 2022 12:14:16 +0530 Subject: [PATCH 26/43] fix: Taxes getting overriden from mapped to target doc --- erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js | 2 ++ erpnext/accounts/doctype/sales_invoice/sales_invoice.js | 3 +++ 2 files changed, 5 insertions(+) diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js index 1a398aba2ec..5f6e61090bd 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js @@ -276,6 +276,8 @@ erpnext.accounts.PurchaseInvoice = class PurchaseInvoice extends erpnext.buying. if(this.frm.updating_party_details || this.frm.doc.inter_company_invoice_reference) return; + if (this.frm.doc.__onload && this.frm.doc.__onload.load_after_mapping) return; + erpnext.utils.get_party_details(this.frm, "erpnext.accounts.party.get_party_details", { posting_date: this.frm.doc.posting_date, diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.js b/erpnext/accounts/doctype/sales_invoice/sales_invoice.js index af6a52a6429..6818955c2f1 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.js +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.js @@ -280,6 +280,9 @@ erpnext.accounts.SalesInvoiceController = class SalesInvoiceController extends e } var me = this; if(this.frm.updating_party_details) return; + + if (this.frm.doc.__onload && this.frm.doc.__onload.load_after_mapping) return; + erpnext.utils.get_party_details(this.frm, "erpnext.accounts.party.get_party_details", { posting_date: this.frm.doc.posting_date, From 4623a1bc5777f8bb16a147eae52b9f8e695612af Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Thu, 31 Mar 2022 12:48:00 +0530 Subject: [PATCH 27/43] fix(test): Item MacBook does not exist --- erpnext/assets/doctype/asset/test_asset.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/assets/doctype/asset/test_asset.py b/erpnext/assets/doctype/asset/test_asset.py index f681b3480c2..e759ad0719b 100644 --- a/erpnext/assets/doctype/asset/test_asset.py +++ b/erpnext/assets/doctype/asset/test_asset.py @@ -68,7 +68,7 @@ class TestAsset(AssetSetup): def test_item_exists(self): asset = create_asset(item_code="MacBook", do_not_save=1) - self.assertRaises(frappe.DoesNotExistError, asset.save) + self.assertRaises(frappe.ValidationError, asset.save) def test_validate_item(self): asset = create_asset(item_code="MacBook Pro", do_not_save=1) From 2fece523f6c0cda8025334e4680794b963fb6914 Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 31 Mar 2022 12:55:48 +0530 Subject: [PATCH 28/43] chore: Added BOM std filters and update type in List View --- .../manufacturing/doctype/bom_update_log/bom_update_log.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 38c685a64f1..98c1acb71ce 100644 --- a/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.json +++ b/erpnext/manufacturing/doctype/bom_update_log/bom_update_log.json @@ -20,6 +20,7 @@ "fieldname": "current_bom", "fieldtype": "Link", "in_list_view": 1, + "in_standard_filter": 1, "label": "Current BOM", "options": "BOM" }, @@ -27,6 +28,7 @@ "fieldname": "new_bom", "fieldtype": "Link", "in_list_view": 1, + "in_standard_filter": 1, "label": "New BOM", "options": "BOM" }, @@ -37,6 +39,7 @@ { "fieldname": "update_type", "fieldtype": "Select", + "in_list_view": 1, "label": "Update Type", "options": "Replace BOM\nUpdate Cost" }, @@ -66,7 +69,7 @@ "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2022-03-17 12:51:28.067900", + "modified": "2022-03-31 12:51:44.885102", "modified_by": "Administrator", "module": "Manufacturing", "name": "BOM Update Log", From 93f6346ceaaba582fc84ca090049bda48997e48b Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 31 Mar 2022 13:07:51 +0530 Subject: [PATCH 29/43] fix: (test) change expected exception due to https://github.com/frappe/frappe/pull/16454 --- erpnext/assets/doctype/asset/test_asset.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/assets/doctype/asset/test_asset.py b/erpnext/assets/doctype/asset/test_asset.py index f681b3480c2..e759ad0719b 100644 --- a/erpnext/assets/doctype/asset/test_asset.py +++ b/erpnext/assets/doctype/asset/test_asset.py @@ -68,7 +68,7 @@ class TestAsset(AssetSetup): def test_item_exists(self): asset = create_asset(item_code="MacBook", do_not_save=1) - self.assertRaises(frappe.DoesNotExistError, asset.save) + self.assertRaises(frappe.ValidationError, asset.save) def test_validate_item(self): asset = create_asset(item_code="MacBook Pro", do_not_save=1) From 2f63ae2ee9782c879d7b7ef806444f41a16ac4c6 Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Thu, 31 Mar 2022 18:40:09 +0530 Subject: [PATCH 30/43] feat: minor, pick list item reference on delivery note item table --- .../delivery_note_item/delivery_note_item.json | 12 +++++++++++- erpnext/stock/doctype/pick_list/pick_list.py | 1 + 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/erpnext/stock/doctype/delivery_note_item/delivery_note_item.json b/erpnext/stock/doctype/delivery_note_item/delivery_note_item.json index f1f5d96e628..e2eb2a4bbb2 100644 --- a/erpnext/stock/doctype/delivery_note_item/delivery_note_item.json +++ b/erpnext/stock/doctype/delivery_note_item/delivery_note_item.json @@ -74,6 +74,7 @@ "against_sales_invoice", "si_detail", "dn_detail", + "pick_list_item", "section_break_40", "batch_no", "serial_no", @@ -762,13 +763,22 @@ "fieldtype": "Check", "label": "Grant Commission", "read_only": 1 + }, + { + "fieldname": "pick_list_item", + "fieldtype": "Data", + "hidden": 1, + "label": "Pick List Item", + "no_copy": 1, + "print_hide": 1, + "read_only": 1 } ], "idx": 1, "index_web_pages_for_search": 1, "istable": 1, "links": [], - "modified": "2022-02-24 14:42:20.211085", + "modified": "2022-03-31 18:36:24.671913", "modified_by": "Administrator", "module": "Stock", "name": "Delivery Note Item", diff --git a/erpnext/stock/doctype/pick_list/pick_list.py b/erpnext/stock/doctype/pick_list/pick_list.py index 7061ee1eea4..d3476a88f05 100644 --- a/erpnext/stock/doctype/pick_list/pick_list.py +++ b/erpnext/stock/doctype/pick_list/pick_list.py @@ -534,6 +534,7 @@ def map_pl_locations(pick_list, item_mapper, delivery_note, sales_order=None): dn_item = map_child_doc(source_doc, delivery_note, table_mapper) if dn_item: + dn_item.pick_list_item = location.name dn_item.warehouse = location.warehouse dn_item.qty = flt(location.picked_qty) / (flt(location.conversion_factor) or 1) dn_item.batch_no = location.batch_no From 2f51011f913454c1ca4e96f647af4476c69fbfe3 Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Thu, 31 Mar 2022 18:47:09 +0530 Subject: [PATCH 31/43] test: test case to check pick list name has mapped or not --- erpnext/stock/doctype/pick_list/test_pick_list.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/erpnext/stock/doctype/pick_list/test_pick_list.py b/erpnext/stock/doctype/pick_list/test_pick_list.py index 7496b6b1798..ec5011b93d6 100644 --- a/erpnext/stock/doctype/pick_list/test_pick_list.py +++ b/erpnext/stock/doctype/pick_list/test_pick_list.py @@ -521,6 +521,8 @@ class TestPickList(FrappeTestCase): for dn_item in frappe.get_doc("Delivery Note", dn.name).get("items"): self.assertEqual(dn_item.item_code, "_Test Item") self.assertEqual(dn_item.against_sales_order, sales_order_1.name) + self.assertEqual(dn_item.pick_list_item, pick_list.locations[dn_item.idx - 1].name) + for dn in frappe.get_all( "Delivery Note", filters={"pick_list": pick_list.name, "customer": "_Test Customer 1"}, From bfc34e10840a240a68b976ac8bd4a0e03ca7c153 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Fri, 1 Apr 2022 11:03:16 +0530 Subject: [PATCH 32/43] fix: check for debit credit difference even after round-off adjustment (#30050) --- erpnext/accounts/general_ledger.py | 43 ++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/erpnext/accounts/general_ledger.py b/erpnext/accounts/general_ledger.py index 50f37be27b8..f52e517f730 100644 --- a/erpnext/accounts/general_ledger.py +++ b/erpnext/accounts/general_ledger.py @@ -253,7 +253,7 @@ def save_entries(gl_map, adv_adj, update_outstanding, from_repost=False): if not from_repost: validate_cwip_accounts(gl_map) - round_off_debit_credit(gl_map) + process_debit_credit_difference(gl_map) if gl_map: check_freezing_date(gl_map[0]["posting_date"], adv_adj) @@ -302,12 +302,29 @@ def validate_cwip_accounts(gl_map): ) -def round_off_debit_credit(gl_map): +def process_debit_credit_difference(gl_map): precision = get_field_precision( frappe.get_meta("GL Entry").get_field("debit"), currency=frappe.get_cached_value("Company", gl_map[0].company, "default_currency"), ) + voucher_type = gl_map[0].voucher_type + voucher_no = gl_map[0].voucher_no + allowance = get_debit_credit_allowance(voucher_type, precision) + + debit_credit_diff = get_debit_credit_difference(gl_map, precision) + if abs(debit_credit_diff) > allowance: + raise_debit_credit_not_equal_error(debit_credit_diff, voucher_type, voucher_no) + + elif abs(debit_credit_diff) >= (1.0 / (10**precision)): + make_round_off_gle(gl_map, debit_credit_diff, precision) + + debit_credit_diff = get_debit_credit_difference(gl_map, precision) + if abs(debit_credit_diff) > allowance: + raise_debit_credit_not_equal_error(debit_credit_diff, voucher_type, voucher_no) + + +def get_debit_credit_difference(gl_map, precision): debit_credit_diff = 0.0 for entry in gl_map: entry.debit = flt(entry.debit, precision) @@ -316,20 +333,24 @@ def round_off_debit_credit(gl_map): debit_credit_diff = flt(debit_credit_diff, precision) - if gl_map[0]["voucher_type"] in ("Journal Entry", "Payment Entry"): + return debit_credit_diff + + +def get_debit_credit_allowance(voucher_type, precision): + if voucher_type in ("Journal Entry", "Payment Entry"): allowance = 5.0 / (10**precision) else: allowance = 0.5 - if abs(debit_credit_diff) > allowance: - frappe.throw( - _("Debit and Credit not equal for {0} #{1}. Difference is {2}.").format( - gl_map[0].voucher_type, gl_map[0].voucher_no, debit_credit_diff - ) - ) + return allowance - elif abs(debit_credit_diff) >= (1.0 / (10**precision)): - make_round_off_gle(gl_map, debit_credit_diff, precision) + +def raise_debit_credit_not_equal_error(debit_credit_diff, voucher_type, voucher_no): + frappe.throw( + _("Debit and Credit not equal for {0} #{1}. Difference is {2}.").format( + voucher_type, voucher_no, debit_credit_diff + ) + ) def make_round_off_gle(gl_map, debit_credit_diff, precision): From 257623509d9aaa7fa12be124869ed059bbaa821d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 1 Apr 2022 11:55:19 +0530 Subject: [PATCH 33/43] perf: use cached single docs (#30536) frappe.local is request specific thread local, hence is almost as good as no caching. --- .../doctype/e_commerce_settings/e_commerce_settings.py | 7 +------ .../doctype/education_settings/education_settings.py | 2 +- erpnext/templates/pages/home.py | 8 ++++---- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.py b/erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.py index b5cd067e38f..881d833eb01 100644 --- a/erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.py +++ b/erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.py @@ -146,12 +146,7 @@ def validate_cart_settings(doc=None, method=None): def get_shopping_cart_settings(): - if not getattr(frappe.local, "shopping_cart_settings", None): - frappe.local.shopping_cart_settings = frappe.get_doc( - "E Commerce Settings", "E Commerce Settings" - ) - - return frappe.local.shopping_cart_settings + return frappe.get_cached_doc("E Commerce Settings") @frappe.whitelist(allow_guest=True) diff --git a/erpnext/education/doctype/education_settings/education_settings.py b/erpnext/education/doctype/education_settings/education_settings.py index cde5089e88d..295aa3a4cba 100644 --- a/erpnext/education/doctype/education_settings/education_settings.py +++ b/erpnext/education/doctype/education_settings/education_settings.py @@ -41,4 +41,4 @@ class EducationSettings(Document): def update_website_context(context): - context["lms_enabled"] = frappe.get_doc("Education Settings").enable_lms + context["lms_enabled"] = frappe.get_cached_doc("Education Settings").enable_lms diff --git a/erpnext/templates/pages/home.py b/erpnext/templates/pages/home.py index bca3e560536..47fb89dea31 100644 --- a/erpnext/templates/pages/home.py +++ b/erpnext/templates/pages/home.py @@ -8,7 +8,7 @@ no_cache = 1 def get_context(context): - homepage = frappe.get_doc("Homepage") + homepage = frappe.get_cached_doc("Homepage") for item in homepage.products: route = frappe.db.get_value("Website Item", {"item_code": item.item_code}, "route") @@ -20,10 +20,10 @@ def get_context(context): context.homepage = homepage if homepage.hero_section_based_on == "Homepage Section" and homepage.hero_section: - homepage.hero_section_doc = frappe.get_doc("Homepage Section", homepage.hero_section) + homepage.hero_section_doc = frappe.get_cached_doc("Homepage Section", homepage.hero_section) if homepage.slideshow: - doc = frappe.get_doc("Website Slideshow", homepage.slideshow) + doc = frappe.get_cached_doc("Website Slideshow", homepage.slideshow) context.slideshow = homepage.slideshow context.slideshow_header = doc.header context.slides = doc.slideshow_items @@ -46,7 +46,7 @@ def get_context(context): order_by="section_order asc", ) context.homepage_sections = [ - frappe.get_doc("Homepage Section", name) for name in homepage_sections + frappe.get_cached_doc("Homepage Section", name) for name in homepage_sections ] context.metatags = context.metatags or frappe._dict({}) From 65763275aeac949a2b22a834af08c5462b6e0c8d Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Fri, 1 Apr 2022 13:47:52 +0530 Subject: [PATCH 34/43] fix: convert dates to datetime before comparing in leave days calculation and fix half day edge case (#30538) --- .../leave_application/leave_application.py | 4 ++-- .../test_leave_application.py | 20 ++++++++++++++++--- .../doctype/salary_slip/test_salary_slip.py | 13 +++++++++++- 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index 18c69f7113a..cd6b1686675 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -735,9 +735,9 @@ def get_number_of_leave_days( (Based on the include_holiday setting in Leave Type)""" number_of_days = 0 if cint(half_day) == 1: - if from_date == to_date: + if getdate(from_date) == getdate(to_date): number_of_days = 0.5 - elif half_day_date and half_day_date <= to_date: + elif half_day_date and getdate(from_date) <= getdate(half_day_date) <= getdate(to_date): number_of_days = date_diff(to_date, from_date) + 0.5 else: number_of_days = date_diff(to_date, from_date) + 1 diff --git a/erpnext/hr/doctype/leave_application/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py index f33d0afa4eb..4c39e15c93d 100644 --- a/erpnext/hr/doctype/leave_application/test_leave_application.py +++ b/erpnext/hr/doctype/leave_application/test_leave_application.py @@ -205,7 +205,12 @@ class TestLeaveApplication(unittest.TestCase): # creates separate leave ledger entries frappe.delete_doc_if_exists("Leave Type", "Test Leave Validation", force=1) leave_type = frappe.get_doc( - dict(leave_type_name="Test Leave Validation", doctype="Leave Type", allow_negative=True) + dict( + leave_type_name="Test Leave Validation", + doctype="Leave Type", + allow_negative=True, + include_holiday=True, + ) ).insert() employee = get_employee() @@ -217,8 +222,14 @@ class TestLeaveApplication(unittest.TestCase): # application across allocations # CASE 1: from date has no allocation, to date has an allocation / both dates have allocation + start_date = add_days(year_start, -10) application = make_leave_application( - employee.name, add_days(year_start, -10), add_days(year_start, 3), leave_type.name + employee.name, + start_date, + add_days(year_start, 3), + leave_type.name, + half_day=1, + half_day_date=start_date, ) # 2 separate leave ledger entries @@ -828,6 +839,7 @@ class TestLeaveApplication(unittest.TestCase): leave_type_name="_Test_CF_leave_expiry", is_carry_forward=1, expire_carry_forwarded_leaves_after_days=90, + include_holiday=True, ) leave_type.submit() @@ -840,6 +852,8 @@ class TestLeaveApplication(unittest.TestCase): leave_type=leave_type.name, from_date=add_days(nowdate(), -3), to_date=add_days(nowdate(), 7), + half_day=1, + half_day_date=add_days(nowdate(), -3), description="_Test Reason", company="_Test Company", docstatus=1, @@ -855,7 +869,7 @@ class TestLeaveApplication(unittest.TestCase): self.assertEqual(len(leave_ledger_entry), 2) self.assertEqual(leave_ledger_entry[0].employee, leave_application.employee) self.assertEqual(leave_ledger_entry[0].leave_type, leave_application.leave_type) - self.assertEqual(leave_ledger_entry[0].leaves, -9) + self.assertEqual(leave_ledger_entry[0].leaves, -8.5) self.assertEqual(leave_ledger_entry[1].leaves, -2) def test_leave_application_creation_after_expiry(self): diff --git a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py index e1d1fa1fcb4..dbeadc59004 100644 --- a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py @@ -1290,7 +1290,16 @@ def create_additional_salary(employee, payroll_period, amount): return salary_date -def make_leave_application(employee, from_date, to_date, leave_type, company=None, submit=True): +def make_leave_application( + employee, + from_date, + to_date, + leave_type, + company=None, + half_day=False, + half_day_date=None, + submit=True, +): leave_application = frappe.get_doc( dict( doctype="Leave Application", @@ -1298,6 +1307,8 @@ def make_leave_application(employee, from_date, to_date, leave_type, company=Non leave_type=leave_type, from_date=from_date, to_date=to_date, + half_day=half_day, + half_day_date=half_day_date, company=company or erpnext.get_default_company() or "_Test Company", status="Approved", leave_approver="test@example.com", From 5d14e7860e7c16e39aacd9455aba278afd195fd1 Mon Sep 17 00:00:00 2001 From: Alberto826 <46285948+Alberto826@users.noreply.github.com> Date: Fri, 1 Apr 2022 11:18:00 +0200 Subject: [PATCH 35/43] Fix: Remove trailing slash "/" from route (#30532) Routes with trailing slash "/" causes a redirect to port 8080 in docker host with reverse proxy. The Student Admission Row template has this issue. --- .../student_admission/templates/student_admission_row.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/education/doctype/student_admission/templates/student_admission_row.html b/erpnext/education/doctype/student_admission/templates/student_admission_row.html index 529d65184a8..dc4587bc940 100644 --- a/erpnext/education/doctype/student_admission/templates/student_admission_row.html +++ b/erpnext/education/doctype/student_admission/templates/student_admission_row.html @@ -1,6 +1,6 @@
{% set today = frappe.utils.getdate(frappe.utils.nowdate()) %} - +
Date: Thu, 31 Mar 2022 15:37:14 +0530 Subject: [PATCH 37/43] fix: Handle changes in frappe's get_monthly_goal_graph_data API --- erpnext/setup/doctype/company/company_dashboard.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/setup/doctype/company/company_dashboard.py b/erpnext/setup/doctype/company/company_dashboard.py index ff1e7f1103b..2d073c1d77e 100644 --- a/erpnext/setup/doctype/company/company_dashboard.py +++ b/erpnext/setup/doctype/company/company_dashboard.py @@ -14,7 +14,7 @@ def get_data(): "goal_doctype_link": "company", "goal_field": "base_grand_total", "date_field": "posting_date", - "filter_str": "docstatus = 1 and is_opening != 'Yes'", + "filters": {"docstatus": 1, "is_opening": ("!=", "Yes")}, "aggregation": "sum", }, "fieldname": "company", From 767b827b5951b2b85e8f7f714fa43c96df274f31 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 31 Mar 2022 14:21:01 +0530 Subject: [PATCH 38/43] feat: return batch code while scanning serial no --- .../page/point_of_sale/point_of_sale.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/erpnext/selling/page/point_of_sale/point_of_sale.py b/erpnext/selling/page/point_of_sale/point_of_sale.py index bf629824ad9..400be4c7327 100644 --- a/erpnext/selling/page/point_of_sale/point_of_sale.py +++ b/erpnext/selling/page/point_of_sale/point_of_sale.py @@ -3,6 +3,7 @@ import json +from typing import Dict, Optional import frappe from frappe.utils.nestedset import get_root_of @@ -150,24 +151,34 @@ def get_items(start, page_length, price_list, item_group, pos_profile, search_te @frappe.whitelist() -def search_for_serial_or_batch_or_barcode_number(search_value): +def search_for_serial_or_batch_or_barcode_number(search_value: str) -> Dict[str, Optional[str]]: + # search barcode no barcode_data = frappe.db.get_value( - "Item Barcode", {"barcode": search_value}, ["barcode", "parent as item_code"], as_dict=True + "Item Barcode", + {"barcode": search_value}, + ["barcode", "parent as item_code"], + as_dict=True, ) if barcode_data: return barcode_data # search serial no serial_no_data = frappe.db.get_value( - "Serial No", search_value, ["name as serial_no", "item_code"], as_dict=True + "Serial No", + search_value, + ["name as serial_no", "item_code", "batch_no"], + as_dict=True, ) if serial_no_data: return serial_no_data # search batch no batch_no_data = frappe.db.get_value( - "Batch", search_value, ["name as batch_no", "item as item_code"], as_dict=True + "Batch", + search_value, + ["name as batch_no", "item as item_code"], + as_dict=True, ) if batch_no_data: return batch_no_data From 530f767098ec5769627e40d92a17e5f162901442 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 31 Mar 2022 14:37:30 +0530 Subject: [PATCH 39/43] revert: "fix: ignore circular dependencies on barcode scan" This reverts commit 98468fab18e08de440f2f6731759d221fe08c416. --- erpnext/public/js/controllers/transaction.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/erpnext/public/js/controllers/transaction.js b/erpnext/public/js/controllers/transaction.js index 23c2bd405c1..61963925168 100644 --- a/erpnext/public/js/controllers/transaction.js +++ b/erpnext/public/js/controllers/transaction.js @@ -404,13 +404,11 @@ erpnext.TransactionController = class TransactionController extends erpnext.taxe } barcode(doc, cdt, cdn) { - const d = locals[cdt][cdn]; - if (!d.barcode) { + var d = locals[cdt][cdn]; + if(d.barcode=="" || d.barcode==null) { // barcode cleared, remove item d.item_code = ""; } - // flag required for circular triggers - d._triggerd_from_barcode = true; this.item_code(doc, cdt, cdn); } @@ -431,9 +429,7 @@ erpnext.TransactionController = class TransactionController extends erpnext.taxe this.frm.doc.doctype === 'Delivery Note') { show_batch_dialog = 1; } - if (!item._triggerd_from_barcode) { - item.barcode = null; - } + item.barcode = null; if(item.item_code || item.barcode || item.serial_no) { From 9bf427985fa72ac8f7c5e2c431ff2eb845d74de0 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 31 Mar 2022 14:38:00 +0530 Subject: [PATCH 40/43] fix!: remove barcode field triggers use "scan barcode" field instead for scanning [skip ci] --- erpnext/public/js/controllers/transaction.js | 9 --------- .../stock/doctype/stock_entry/stock_entry.js | 15 --------------- .../stock_reconciliation.js | 18 +----------------- 3 files changed, 1 insertion(+), 41 deletions(-) diff --git a/erpnext/public/js/controllers/transaction.js b/erpnext/public/js/controllers/transaction.js index 61963925168..f948c60010d 100644 --- a/erpnext/public/js/controllers/transaction.js +++ b/erpnext/public/js/controllers/transaction.js @@ -403,15 +403,6 @@ erpnext.TransactionController = class TransactionController extends erpnext.taxe var sms_man = new erpnext.SMSManager(this.frm.doc); } - barcode(doc, cdt, cdn) { - var d = locals[cdt][cdn]; - if(d.barcode=="" || d.barcode==null) { - // barcode cleared, remove item - d.item_code = ""; - } - this.item_code(doc, cdt, cdn); - } - item_code(doc, cdt, cdn) { var me = this; var item = frappe.get_doc(cdt, cdn); diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.js b/erpnext/stock/doctype/stock_entry/stock_entry.js index 1aafcee5bf8..7564bb266d7 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.js +++ b/erpnext/stock/doctype/stock_entry/stock_entry.js @@ -646,21 +646,6 @@ frappe.ui.form.on('Stock Entry Detail', { frm.events.calculate_basic_amount(frm, item); }, - barcode: function(doc, cdt, cdn) { - var d = locals[cdt][cdn]; - if (d.barcode) { - frappe.call({ - method: "erpnext.stock.get_item_details.get_item_code", - args: {"barcode": d.barcode }, - callback: function(r) { - if (!r.exe){ - frappe.model.set_value(cdt, cdn, "item_code", r.message); - } - } - }); - } - }, - uom: function(doc, cdt, cdn) { var d = locals[cdt][cdn]; if(d.uom && d.item_code){ diff --git a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.js b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.js index 84f65a077e0..4438acf8118 100644 --- a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.js +++ b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.js @@ -163,20 +163,7 @@ frappe.ui.form.on("Stock Reconciliation", { }); } }, - set_item_code: function(doc, cdt, cdn) { - var d = frappe.model.get_doc(cdt, cdn); - if (d.barcode) { - frappe.call({ - method: "erpnext.stock.get_item_details.get_item_code", - args: {"barcode": d.barcode }, - callback: function(r) { - if (!r.exe){ - frappe.model.set_value(cdt, cdn, "item_code", r.message); - } - } - }); - } - }, + set_amount_quantity: function(doc, cdt, cdn) { var d = frappe.model.get_doc(cdt, cdn); if (d.qty & d.valuation_rate) { @@ -214,9 +201,6 @@ frappe.ui.form.on("Stock Reconciliation", { }); frappe.ui.form.on("Stock Reconciliation Item", { - barcode: function(frm, cdt, cdn) { - frm.events.set_item_code(frm, cdt, cdn); - }, warehouse: function(frm, cdt, cdn) { var child = locals[cdt][cdn]; From 47f27a51714f999d32fb0bfa21c13a11b1a964c3 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 1 Apr 2022 15:20:40 +0530 Subject: [PATCH 41/43] refactor: move scan api to stock utils; add item_info --- erpnext/public/js/utils/barcode_scanner.js | 4 +- .../page/point_of_sale/point_of_sale.py | 34 +------------ erpnext/stock/tests/test_utils.py | 31 ++++++++++++ erpnext/stock/utils.py | 49 +++++++++++++++++++ 4 files changed, 83 insertions(+), 35 deletions(-) create mode 100644 erpnext/stock/tests/test_utils.py diff --git a/erpnext/public/js/utils/barcode_scanner.js b/erpnext/public/js/utils/barcode_scanner.js index abea5fcb20b..0868a6bd76f 100644 --- a/erpnext/public/js/utils/barcode_scanner.js +++ b/erpnext/public/js/utils/barcode_scanner.js @@ -21,9 +21,7 @@ erpnext.utils.BarcodeScanner = class BarcodeScanner { // batch_no: "LOT12", // present if batch was scanned // serial_no: "987XYZ", // present if serial no was scanned // } - this.scan_api = - opts.scan_api || - "erpnext.selling.page.point_of_sale.point_of_sale.search_for_serial_or_batch_or_barcode_number"; + this.scan_api = opts.scan_api || "erpnext.stock.utils.scan_barcode"; } process_scan() { diff --git a/erpnext/selling/page/point_of_sale/point_of_sale.py b/erpnext/selling/page/point_of_sale/point_of_sale.py index 400be4c7327..99afe813cb9 100644 --- a/erpnext/selling/page/point_of_sale/point_of_sale.py +++ b/erpnext/selling/page/point_of_sale/point_of_sale.py @@ -10,6 +10,7 @@ from frappe.utils.nestedset import get_root_of from erpnext.accounts.doctype.pos_invoice.pos_invoice import get_stock_availability from erpnext.accounts.doctype.pos_profile.pos_profile import get_child_nodes, get_item_groups +from erpnext.stock.utils import scan_barcode def search_by_term(search_term, warehouse, price_list): @@ -152,38 +153,7 @@ def get_items(start, page_length, price_list, item_group, pos_profile, search_te @frappe.whitelist() def search_for_serial_or_batch_or_barcode_number(search_value: str) -> Dict[str, Optional[str]]: - - # search barcode no - barcode_data = frappe.db.get_value( - "Item Barcode", - {"barcode": search_value}, - ["barcode", "parent as item_code"], - as_dict=True, - ) - if barcode_data: - return barcode_data - - # search serial no - serial_no_data = frappe.db.get_value( - "Serial No", - search_value, - ["name as serial_no", "item_code", "batch_no"], - as_dict=True, - ) - if serial_no_data: - return serial_no_data - - # search batch no - batch_no_data = frappe.db.get_value( - "Batch", - search_value, - ["name as batch_no", "item as item_code"], - as_dict=True, - ) - if batch_no_data: - return batch_no_data - - return {} + return scan_barcode(search_value) def get_conditions(search_term): diff --git a/erpnext/stock/tests/test_utils.py b/erpnext/stock/tests/test_utils.py new file mode 100644 index 00000000000..9ee0c9f3b5a --- /dev/null +++ b/erpnext/stock/tests/test_utils.py @@ -0,0 +1,31 @@ +import frappe +from frappe.tests.utils import FrappeTestCase + +from erpnext.stock.doctype.item.test_item import make_item +from erpnext.stock.utils import scan_barcode + + +class TestStockUtilities(FrappeTestCase): + def test_barcode_scanning(self): + simple_item = make_item(properties={"barcodes": [{"barcode": "12399"}]}) + self.assertEqual(scan_barcode("12399")["item_code"], simple_item.name) + + batch_item = make_item(properties={"has_batch_no": 1, "create_new_batch": 1}) + batch = frappe.get_doc(doctype="Batch", item=batch_item.name).insert() + + batch_scan = scan_barcode(batch.name) + self.assertEqual(batch_scan["item_code"], batch_item.name) + self.assertEqual(batch_scan["batch_no"], batch.name) + self.assertEqual(batch_scan["has_batch_no"], 1) + self.assertEqual(batch_scan["has_serial_no"], 0) + + serial_item = make_item(properties={"has_serial_no": 1}) + serial = frappe.get_doc( + doctype="Serial No", item_code=serial_item.name, serial_no=frappe.generate_hash() + ).insert() + + serial_scan = scan_barcode(serial.name) + self.assertEqual(serial_scan["item_code"], serial_item.name) + self.assertEqual(serial_scan["serial_no"], serial.name) + self.assertEqual(serial_scan["has_batch_no"], 0) + self.assertEqual(serial_scan["has_serial_no"], 1) diff --git a/erpnext/stock/utils.py b/erpnext/stock/utils.py index 4f1891fd750..d40218e1439 100644 --- a/erpnext/stock/utils.py +++ b/erpnext/stock/utils.py @@ -3,6 +3,7 @@ import json +from typing import Dict, Optional import frappe from frappe import _ @@ -548,3 +549,51 @@ def check_pending_reposting(posting_date: str, throw_error: bool = True) -> bool ) return bool(reposting_pending) + + +@frappe.whitelist() +def scan_barcode(search_value: str) -> Dict[str, Optional[str]]: + + # search barcode no + barcode_data = frappe.db.get_value( + "Item Barcode", + {"barcode": search_value}, + ["barcode", "parent as item_code"], + as_dict=True, + ) + if barcode_data: + return _update_item_info(barcode_data) + + # search serial no + serial_no_data = frappe.db.get_value( + "Serial No", + search_value, + ["name as serial_no", "item_code", "batch_no"], + as_dict=True, + ) + if serial_no_data: + return _update_item_info(serial_no_data) + + # search batch no + batch_no_data = frappe.db.get_value( + "Batch", + search_value, + ["name as batch_no", "item as item_code"], + as_dict=True, + ) + if batch_no_data: + return _update_item_info(batch_no_data) + + return {} + + +def _update_item_info(scan_result: Dict[str, Optional[str]]) -> Dict[str, Optional[str]]: + if item_code := scan_result.get("item_code"): + if item_info := frappe.get_cached_value( + "Item", + item_code, + ["has_batch_no", "has_serial_no"], + as_dict=True, + ): + scan_result.update(item_info) + return scan_result From 6a069d6efa5a6ae1a8f38415c8c8b6b1799a0ed7 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 1 Apr 2022 16:21:22 +0530 Subject: [PATCH 42/43] feat: dont trigger selector if all info is scanned --- erpnext/public/js/controllers/transaction.js | 6 ++++++ erpnext/public/js/utils/barcode_scanner.js | 20 ++++++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/erpnext/public/js/controllers/transaction.js b/erpnext/public/js/controllers/transaction.js index f948c60010d..a4492e8bddb 100644 --- a/erpnext/public/js/controllers/transaction.js +++ b/erpnext/public/js/controllers/transaction.js @@ -526,6 +526,12 @@ erpnext.TransactionController = class TransactionController extends erpnext.taxe if(!d[k]) d[k] = v; }); + if (d.__disable_batch_serial_selector) { + // reset for future use. + d.__disable_batch_serial_selector = false; + return; + } + if (d.has_batch_no && d.has_serial_no) { d.batch_no = undefined; } diff --git a/erpnext/public/js/utils/barcode_scanner.js b/erpnext/public/js/utils/barcode_scanner.js index 0868a6bd76f..80a463f85c9 100644 --- a/erpnext/public/js/utils/barcode_scanner.js +++ b/erpnext/public/js/utils/barcode_scanner.js @@ -50,14 +50,16 @@ erpnext.utils.BarcodeScanner = class BarcodeScanner { return; } - me.update_table(data.item_code, data.barcode, data.batch_no, data.serial_no); + me.update_table(data); }); } - update_table(item_code, barcode, batch_no, serial_no) { + update_table(data) { let cur_grid = this.frm.fields_dict[this.items_table_name].grid; let row = null; + const {item_code, barcode, batch_no, serial_no} = data; + // Check if batch is scanned and table has batch no field let batch_no_scan = Boolean(batch_no) && frappe.meta.has_field(cur_grid.doctype, this.batch_no_field); @@ -82,6 +84,7 @@ erpnext.utils.BarcodeScanner = class BarcodeScanner { } this.show_scan_message(row.idx, row.item_code); + this.set_selector_trigger_flag(row, data); this.set_item(row, item_code); this.set_serial_no(row, serial_no); this.set_batch_no(row, batch_no); @@ -89,6 +92,19 @@ erpnext.utils.BarcodeScanner = class BarcodeScanner { this.clean_up(); } + // batch and serial selector is reduandant when all info can be added by scan + // this flag on item row is used by transaction.js to avoid triggering selector + set_selector_trigger_flag(row, data) { + const {batch_no, serial_no, has_batch_no, has_serial_no} = data; + + const require_selecting_batch = has_batch_no && !batch_no; + const require_selecting_serial = has_serial_no && !serial_no; + + if (!(require_selecting_batch || require_selecting_serial)) { + row.__disable_batch_serial_selector = true; + } + } + set_item(row, item_code) { const item_data = { item_code: item_code }; item_data[this.qty_field] = (row[this.qty_field] || 0) + 1; From c8ead0a7ab2074b1599474c79b1a1f64650ba829 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 1 Apr 2022 20:25:30 +0530 Subject: [PATCH 43/43] fix: dont send empty serial no in get_item_details [skip ci] --- erpnext/stock/get_item_details.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/erpnext/stock/get_item_details.py b/erpnext/stock/get_item_details.py index f72588e034d..f83f692f642 100644 --- a/erpnext/stock/get_item_details.py +++ b/erpnext/stock/get_item_details.py @@ -167,6 +167,9 @@ def update_stock(args, out): reserved_so = get_so_reservation_for_item(args) out.serial_no = get_serial_no(out, args.serial_no, sales_order=reserved_so) + if not out.serial_no: + out.pop("serial_no", None) + def set_valuation_rate(out, args): if frappe.db.exists("Product Bundle", args.item_code, cache=True):