From f57725f8fa016b9826e8fdf2f14dbf1a3d9991f7 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 9 Mar 2022 19:03:07 +0530 Subject: [PATCH 01/36] 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/36] 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/36] 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/36] 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/36] 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/36] 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/36] 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/36] 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/36] 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 1d1e925bcf6066cac03abfb60510e76d0f97f9be Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 30 Mar 2022 13:01:01 +0530 Subject: [PATCH 10/36] 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 11/36] 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 12/36] 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 620575a9012a9759c6285558ac25c6709c4e92cc Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 30 Mar 2022 18:03:52 +0530 Subject: [PATCH 13/36] 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 14/36] 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 15/36] 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 62266b29aa7eccbfcf496508d2d01f11a2259c7f Mon Sep 17 00:00:00 2001 From: Devin Slauenwhite Date: Wed, 30 Mar 2022 14:22:44 -0400 Subject: [PATCH 16/36] 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 4720969ce62c5d2f1a8d3fbb86c6eec391b4a2c4 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 31 Mar 2022 12:14:16 +0530 Subject: [PATCH 17/36] 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 2fece523f6c0cda8025334e4680794b963fb6914 Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 31 Mar 2022 12:55:48 +0530 Subject: [PATCH 18/36] 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 2f63ae2ee9782c879d7b7ef806444f41a16ac4c6 Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Thu, 31 Mar 2022 18:40:09 +0530 Subject: [PATCH 19/36] 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 20/36] 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 21/36] 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 22/36] 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 23/36] 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 24/36] 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 26/36] 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 27/36] 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 28/36] 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 29/36] 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 30/36] 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 31/36] 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 32/36] 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): From 1736ab57ac785b745f37b733691e1f7acb604173 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 28 Mar 2022 09:37:37 +0530 Subject: [PATCH 33/36] feat: Barcode scanning in Stock Reconciliation [skip ci] --- .../stock_reconciliation.js | 37 ++++++++++++++++-- .../stock_reconciliation.json | 39 ++++++++++++++++++- 2 files changed, 71 insertions(+), 5 deletions(-) diff --git a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.js b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.js index 4438acf8118..3e8ac1f19f8 100644 --- a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.js +++ b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.js @@ -55,6 +55,25 @@ frappe.ui.form.on("Stock Reconciliation", { } }, + scan_barcode: function(frm) { + const barcode_scanner = new erpnext.utils.BarcodeScanner({frm:frm}); + barcode_scanner.process_scan(); + }, + + scan_mode: function(frm) { + if (frm.doc.scan_mode) { + frappe.show_alert({ + message: __("Scan mode enabled, existing quantity will not be fetched."), + indicator: "green" + }); + } + }, + + set_warehouse: function(frm) { + let transaction_controller = new erpnext.TransactionController({frm:frm}); + transaction_controller.autofill_warehouse(frm.doc.items, "warehouse", frm.doc.set_warehouse); + }, + get_items: function(frm) { let fields = [ { @@ -148,15 +167,18 @@ frappe.ui.form.on("Stock Reconciliation", { batch_no: d.batch_no }, callback: function(r) { - frappe.model.set_value(cdt, cdn, "qty", r.message.qty); + const row = frappe.model.get_doc(cdt, cdn); + if (!frm.doc.scan_mode) { + frappe.model.set_value(cdt, cdn, "qty", r.message.qty); + } frappe.model.set_value(cdt, cdn, "valuation_rate", r.message.rate); frappe.model.set_value(cdt, cdn, "current_qty", r.message.qty); frappe.model.set_value(cdt, cdn, "current_valuation_rate", r.message.rate); frappe.model.set_value(cdt, cdn, "current_amount", r.message.rate * r.message.qty); - frappe.model.set_value(cdt, cdn, "amount", r.message.rate * r.message.qty); + frappe.model.set_value(cdt, cdn, "amount", row.qty * row.valuation_rate); frappe.model.set_value(cdt, cdn, "current_serial_no", r.message.serial_nos); - if (frm.doc.purpose == "Stock Reconciliation") { + if (frm.doc.purpose == "Stock Reconciliation" && !frm.doc.scan_mode) { frappe.model.set_value(cdt, cdn, "serial_no", r.message.serial_nos); } } @@ -239,7 +261,14 @@ frappe.ui.form.on("Stock Reconciliation Item", { const serial_nos = child.serial_no.trim().split('\n'); frappe.model.set_value(cdt, cdn, "qty", serial_nos.length); } - } + }, + + items_add: function(frm, cdt, cdn) { + var item = frappe.get_doc(cdt, cdn); + if (!item.warehouse && frm.doc.set_warehouse) { + frappe.model.set_value(cdt, cdn, "warehouse", frm.doc.set_warehouse); + } + }, }); diff --git a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.json b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.json index a882a61e5a5..e545b8ea5c3 100644 --- a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.json +++ b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.json @@ -14,6 +14,12 @@ "posting_date", "posting_time", "set_posting_time", + "section_break_8", + "set_warehouse", + "section_break_22", + "scan_barcode", + "column_break_12", + "scan_mode", "sb9", "items", "section_break_9", @@ -139,13 +145,44 @@ { "fieldname": "dimension_col_break", "fieldtype": "Column Break" + }, + { + "fieldname": "section_break_8", + "fieldtype": "Section Break" + }, + { + "fieldname": "scan_barcode", + "fieldtype": "Data", + "label": "Scan Barcode", + "options": "Barcode" + }, + { + "default": "0", + "description": "Disables auto-fetching of existing quantity", + "fieldname": "scan_mode", + "fieldtype": "Check", + "label": "Scan Mode" + }, + { + "fieldname": "set_warehouse", + "fieldtype": "Link", + "label": "Default Warehouse", + "options": "Warehouse" + }, + { + "fieldname": "section_break_22", + "fieldtype": "Section Break" + }, + { + "fieldname": "column_break_12", + "fieldtype": "Column Break" } ], "icon": "fa fa-upload-alt", "idx": 1, "is_submittable": 1, "links": [], - "modified": "2022-02-06 14:28:19.043905", + "modified": "2022-03-27 08:57:47.161959", "modified_by": "Administrator", "module": "Stock", "name": "Stock Reconciliation", From f83a1c19891c76e32757124df209995156158c19 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 2 Apr 2022 13:22:36 +0530 Subject: [PATCH 34/36] fix: dont reset barcode in scan mode --- .../doctype/stock_reconciliation/stock_reconciliation.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.js b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.js index 3e8ac1f19f8..05dd105d99d 100644 --- a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.js +++ b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.js @@ -226,7 +226,7 @@ frappe.ui.form.on("Stock Reconciliation Item", { warehouse: function(frm, cdt, cdn) { var child = locals[cdt][cdn]; - if (child.batch_no) { + if (child.batch_no && !frm.doc.scan_mode) { frappe.model.set_value(child.cdt, child.cdn, "batch_no", ""); } @@ -235,7 +235,7 @@ frappe.ui.form.on("Stock Reconciliation Item", { item_code: function(frm, cdt, cdn) { var child = locals[cdt][cdn]; - if (child.batch_no) { + if (child.batch_no && !frm.doc.scan_mode) { frappe.model.set_value(cdt, cdn, "batch_no", ""); } From deca1aaf063a3a13404ddc7639e14a0a7bf55ff8 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 2 Apr 2022 13:24:37 +0530 Subject: [PATCH 35/36] fix: dont fetch qty in reco if batch is required --- .../stock_reconciliation.py | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py index 07a8566d4a4..5d5a27f1eab 100644 --- a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py +++ b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py @@ -1,6 +1,7 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: GNU General Public License v3. See license.txt +from typing import Optional import frappe from frappe import _, msgprint @@ -706,29 +707,43 @@ def get_itemwise_batch(warehouse, posting_date, company, item_code=None): @frappe.whitelist() def get_stock_balance_for( - item_code, warehouse, posting_date, posting_time, batch_no=None, with_valuation_rate=True + item_code: str, + warehouse: str, + posting_date: str, + posting_time: str, + batch_no: Optional[str] = None, + with_valuation_rate: bool = True, ): frappe.has_permission("Stock Reconciliation", "write", throw=True) - item_dict = frappe.db.get_value("Item", item_code, ["has_serial_no", "has_batch_no"], as_dict=1) + item_dict = frappe.get_cached_value( + "Item", item_code, ["has_serial_no", "has_batch_no"], as_dict=1 + ) if not item_dict: # In cases of data upload to Items table msg = _("Item {} does not exist.").format(item_code) frappe.throw(msg, title=_("Missing")) - serial_nos = "" - with_serial_no = True if item_dict.get("has_serial_no") else False + serial_nos = None + has_serial_no = bool(item_dict.get("has_serial_no")) + has_batch_no = bool(item_dict.get("has_batch_no")) + + if not batch_no and has_batch_no: + # Not enough information to fetch data + return {"qty": 0, "rate": 0, "serial_nos": None} + + # TODO: fetch only selected batch's values data = get_stock_balance( item_code, warehouse, posting_date, posting_time, with_valuation_rate=with_valuation_rate, - with_serial_no=with_serial_no, + with_serial_no=has_serial_no, ) - if with_serial_no: + if has_serial_no: qty, rate, serial_nos = data else: qty, rate = data From e8118fcdf12bd98ff3b2cea67f6aafe6cc11cc4f Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 2 Apr 2022 13:48:35 +0530 Subject: [PATCH 36/36] fix: doctype layout based on expected tab-order --- .../stock_reconciliation_item.json | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/erpnext/stock/doctype/stock_reconciliation_item/stock_reconciliation_item.json b/erpnext/stock/doctype/stock_reconciliation_item/stock_reconciliation_item.json index 6bbba051f98..79c2fcc252c 100644 --- a/erpnext/stock/doctype/stock_reconciliation_item/stock_reconciliation_item.json +++ b/erpnext/stock/doctype/stock_reconciliation_item/stock_reconciliation_item.json @@ -16,15 +16,15 @@ "amount", "allow_zero_valuation_rate", "serial_no_and_batch_section", - "serial_no", - "column_break_11", "batch_no", + "column_break_11", + "serial_no", "section_break_3", "current_qty", - "current_serial_no", + "current_amount", "column_break_9", "current_valuation_rate", - "current_amount", + "current_serial_no", "section_break_14", "quantity_difference", "column_break_16", @@ -181,7 +181,7 @@ ], "istable": 1, "links": [], - "modified": "2021-05-21 12:13:33.041266", + "modified": "2022-04-02 04:19:40.380587", "modified_by": "Administrator", "module": "Stock", "name": "Stock Reconciliation Item", @@ -190,5 +190,6 @@ "quick_entry": 1, "sort_field": "modified", "sort_order": "DESC", + "states": [], "track_changes": 1 } \ No newline at end of file