From ad31e02616786480cfbadeb39fcaf357e68b4133 Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 30 Mar 2023 21:03:03 +0530 Subject: [PATCH 01/45] feat: Store Party bank details in party records (Customer/Supplier/Employee/Shareholder) --- .../bank_transaction/bank_transaction.json | 29 +++++++++++++++-- .../bank_transaction/bank_transaction.py | 11 +++++++ .../doctype/shareholder/shareholder.json | 32 +++++++++++++++++-- erpnext/buying/doctype/supplier/supplier.json | 30 ++++++++++++++++- .../selling/doctype/customer/customer.json | 25 ++++++++++++++- erpnext/setup/doctype/employee/employee.json | 15 +++++++-- 6 files changed, 132 insertions(+), 10 deletions(-) diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.json b/erpnext/accounts/doctype/bank_transaction/bank_transaction.json index 768d2f0fa45..1543fdb894f 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.json +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.json @@ -33,7 +33,11 @@ "unallocated_amount", "party_section", "party_type", - "party" + "party", + "column_break_3czf", + "bank_party_name", + "bank_party_no", + "bank_party_iban" ], "fields": [ { @@ -202,11 +206,30 @@ "fieldtype": "Data", "label": "Transaction Type", "length": 50 + }, + { + "fieldname": "column_break_3czf", + "fieldtype": "Column Break" + }, + { + "fieldname": "bank_party_name", + "fieldtype": "Data", + "label": "Party Name (Bank Statement)" + }, + { + "fieldname": "bank_party_no", + "fieldtype": "Data", + "label": "Party Account No. (Bank Statement)" + }, + { + "fieldname": "bank_party_iban", + "fieldtype": "Data", + "label": "Party IBAN (Bank Statement)" } ], "is_submittable": 1, "links": [], - "modified": "2022-05-29 18:36:50.475964", + "modified": "2023-03-30 15:30:46.485683", "modified_by": "Administrator", "module": "Accounts", "name": "Bank Transaction", @@ -260,4 +283,4 @@ "states": [], "title_field": "bank_account", "track_changes": 1 -} +} \ No newline at end of file diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py index fcbaf329f56..676c71910bf 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py @@ -8,6 +8,17 @@ from erpnext.controllers.status_updater import StatusUpdater class BankTransaction(StatusUpdater): + # TODO + # On BT save: + # - Match by account no/iban in Customer/Supplier/Employee + # - Match by Party Name + # - If match found, set party type and party name. + + # On submit/update after submit + # - Create/Update a Bank Party Map record + # - User can edit after submit. + # - If changes in party/party name after submit, edit bank party map (which should edit all transactions with same account no/iban/bank party name) + def after_insert(self): self.unallocated_amount = abs(flt(self.withdrawal) - flt(self.deposit)) diff --git a/erpnext/accounts/doctype/shareholder/shareholder.json b/erpnext/accounts/doctype/shareholder/shareholder.json index e94aea94b75..2be2a2fc427 100644 --- a/erpnext/accounts/doctype/shareholder/shareholder.json +++ b/erpnext/accounts/doctype/shareholder/shareholder.json @@ -1,4 +1,5 @@ { + "actions": [], "autoname": "naming_series:", "creation": "2017-12-25 16:50:53.878430", "doctype": "DocType", @@ -19,7 +20,11 @@ "contact_html", "section_break_3", "share_balance", - "contact_list" + "contact_list", + "bank_details_section", + "bank_account_no", + "column_break_tyo0", + "iban" ], "fields": [ { @@ -109,13 +114,33 @@ "hidden": 1, "label": "Contact List", "read_only": 1 + }, + { + "fieldname": "bank_details_section", + "fieldtype": "Section Break", + "label": "Bank Details" + }, + { + "fieldname": "bank_account_no", + "fieldtype": "Data", + "label": "Bank Account No" + }, + { + "fieldname": "column_break_tyo0", + "fieldtype": "Column Break" + }, + { + "fieldname": "iban", + "fieldtype": "Data", + "label": "IBAN" } ], - "modified": "2019-11-17 23:24:11.395882", + "links": [], + "modified": "2023-03-30 16:00:55.087823", "modified_by": "Administrator", "module": "Accounts", "name": "Shareholder", - "name_case": "Title Case", + "naming_rule": "By \"Naming Series\" field", "owner": "Administrator", "permissions": [ { @@ -158,6 +183,7 @@ "search_fields": "folio_no", "sort_field": "modified", "sort_order": "DESC", + "states": [], "title_field": "title", "track_changes": 1 } \ No newline at end of file diff --git a/erpnext/buying/doctype/supplier/supplier.json b/erpnext/buying/doctype/supplier/supplier.json index 1bf7f589e23..a80dcfe5388 100644 --- a/erpnext/buying/doctype/supplier/supplier.json +++ b/erpnext/buying/doctype/supplier/supplier.json @@ -52,6 +52,11 @@ "supplier_primary_address", "primary_address", "accounting_tab", + "bank_details_section", + "bank_account_no", + "column_break_n8mz", + "iban", + "section_break_ow3k", "payment_terms", "accounts", "settings_tab", @@ -445,6 +450,29 @@ { "fieldname": "column_break_59", "fieldtype": "Column Break" + }, + { + "fieldname": "bank_details_section", + "fieldtype": "Section Break", + "label": "Bank Details" + }, + { + "fieldname": "bank_account_no", + "fieldtype": "Data", + "label": "Bank Account No" + }, + { + "fieldname": "column_break_n8mz", + "fieldtype": "Column Break" + }, + { + "fieldname": "iban", + "fieldtype": "Data", + "label": "IBAN" + }, + { + "fieldname": "section_break_ow3k", + "fieldtype": "Section Break" } ], "icon": "fa fa-user", @@ -457,7 +485,7 @@ "link_fieldname": "party" } ], - "modified": "2023-02-18 11:05:50.592270", + "modified": "2023-03-30 15:50:40.241257", "modified_by": "Administrator", "module": "Buying", "name": "Supplier", diff --git a/erpnext/selling/doctype/customer/customer.json b/erpnext/selling/doctype/customer/customer.json index c133cd3152d..5dc6a72da85 100644 --- a/erpnext/selling/doctype/customer/customer.json +++ b/erpnext/selling/doctype/customer/customer.json @@ -61,6 +61,10 @@ "tax_category", "tax_withholding_category", "accounting_tab", + "bank_details_section", + "bank_account_no", + "column_break_xtwg", + "iban", "credit_limit_section", "payment_terms", "credit_limits", @@ -555,6 +559,25 @@ { "fieldname": "column_break_54", "fieldtype": "Column Break" + }, + { + "fieldname": "bank_details_section", + "fieldtype": "Section Break", + "label": "Bank Details" + }, + { + "fieldname": "bank_account_no", + "fieldtype": "Data", + "label": "Bank Account No" + }, + { + "fieldname": "column_break_xtwg", + "fieldtype": "Column Break" + }, + { + "fieldname": "iban", + "fieldtype": "Data", + "label": "IBAN" } ], "icon": "fa fa-user", @@ -568,7 +591,7 @@ "link_fieldname": "party" } ], - "modified": "2023-02-18 11:04:46.343527", + "modified": "2023-03-30 15:45:44.387975", "modified_by": "Administrator", "module": "Selling", "name": "Customer", diff --git a/erpnext/setup/doctype/employee/employee.json b/erpnext/setup/doctype/employee/employee.json index 99693d90918..6cb4292226c 100644 --- a/erpnext/setup/doctype/employee/employee.json +++ b/erpnext/setup/doctype/employee/employee.json @@ -78,7 +78,9 @@ "salary_mode", "bank_details_section", "bank_name", + "column_break_heye", "bank_ac_no", + "iban", "personal_details", "marital_status", "family_background", @@ -804,17 +806,26 @@ { "fieldname": "column_break_104", "fieldtype": "Column Break" + }, + { + "fieldname": "column_break_heye", + "fieldtype": "Column Break" + }, + { + "depends_on": "eval:doc.salary_mode == 'Bank'", + "fieldname": "iban", + "fieldtype": "Data", + "label": "IBAN" } ], "icon": "fa fa-user", "idx": 24, "image_field": "image", "links": [], - "modified": "2022-09-13 10:27:14.579197", + "modified": "2023-03-30 15:57:05.174592", "modified_by": "Administrator", "module": "Setup", "name": "Employee", - "name_case": "Title Case", "naming_rule": "By \"Naming Series\" field", "owner": "Administrator", "permissions": [ From e7745033dfc819bae534cc78a2987210c1fbcdf9 Mon Sep 17 00:00:00 2001 From: marination Date: Fri, 31 Mar 2023 16:11:00 +0530 Subject: [PATCH 02/45] feat: Party auto-matcher from Bank Transaction data - Created Bank Party Mapper - Created class to auto match by account/iban or party name/description(fuzzy) - Automatch and set in transaction or create mapper - `rapidfuzz` introduced --- .../doctype/bank_party_mapper/__init__.py | 0 .../bank_party_mapper/bank_party_mapper.js | 8 + .../bank_party_mapper/bank_party_mapper.json | 80 +++++++++ .../bank_party_mapper/bank_party_mapper.py | 9 + .../test_bank_party_mapper.py | 9 + .../bank_transaction/auto_match_party.py | 157 ++++++++++++++++++ .../bank_transaction/bank_transaction.json | 14 +- .../bank_transaction/bank_transaction.py | 35 +++- pyproject.toml | 1 + 9 files changed, 301 insertions(+), 12 deletions(-) create mode 100644 erpnext/accounts/doctype/bank_party_mapper/__init__.py create mode 100644 erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.js create mode 100644 erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.json create mode 100644 erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.py create mode 100644 erpnext/accounts/doctype/bank_party_mapper/test_bank_party_mapper.py create mode 100644 erpnext/accounts/doctype/bank_transaction/auto_match_party.py diff --git a/erpnext/accounts/doctype/bank_party_mapper/__init__.py b/erpnext/accounts/doctype/bank_party_mapper/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.js b/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.js new file mode 100644 index 00000000000..d11d8040b26 --- /dev/null +++ b/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.js @@ -0,0 +1,8 @@ +// Copyright (c) 2023, Frappe Technologies Pvt. Ltd. and contributors +// For license information, please see license.txt + +// frappe.ui.form.on("Bank Party Mapper", { +// refresh(frm) { + +// }, +// }); diff --git a/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.json b/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.json new file mode 100644 index 00000000000..4f5f6bfa886 --- /dev/null +++ b/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.json @@ -0,0 +1,80 @@ +{ + "actions": [], + "creation": "2023-03-31 10:48:20.249481", + "default_view": "List", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "party_type", + "party", + "column_break_wbna", + "bank_party_name_desc", + "bank_party_account_number", + "bank_party_iban" + ], + "fields": [ + { + "fieldname": "bank_party_account_number", + "fieldtype": "Data", + "in_list_view": 1, + "label": "Party Account No. (Bank Statement)" + }, + { + "fieldname": "bank_party_iban", + "fieldtype": "Data", + "in_list_view": 1, + "label": "Party IBAN (Bank Statement)" + }, + { + "fieldname": "column_break_wbna", + "fieldtype": "Column Break" + }, + { + "fieldname": "party_type", + "fieldtype": "Link", + "in_list_view": 1, + "in_standard_filter": 1, + "label": "Party Type", + "options": "DocType" + }, + { + "fieldname": "party", + "fieldtype": "Dynamic Link", + "in_list_view": 1, + "in_standard_filter": 1, + "label": "Party", + "options": "party_type" + }, + { + "fieldname": "bank_party_name_desc", + "fieldtype": "Small Text", + "label": "Party Name/Desc (Bank Statement)" + } + ], + "index_web_pages_for_search": 1, + "links": [], + "modified": "2023-04-03 10:11:31.384383", + "modified_by": "Administrator", + "module": "Accounts", + "name": "Bank Party Mapper", + "owner": "Administrator", + "permissions": [ + { + "create": 1, + "delete": 1, + "email": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "System Manager", + "share": 1, + "write": 1 + } + ], + "sort_field": "modified", + "sort_order": "DESC", + "states": [], + "track_changes": 1 +} \ No newline at end of file diff --git a/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.py b/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.py new file mode 100644 index 00000000000..d3a9a5e586c --- /dev/null +++ b/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.py @@ -0,0 +1,9 @@ +# Copyright (c) 2023, Frappe Technologies Pvt. Ltd. and contributors +# For license information, please see license.txt + +# import frappe +from frappe.model.document import Document + + +class BankPartyMapper(Document): + pass diff --git a/erpnext/accounts/doctype/bank_party_mapper/test_bank_party_mapper.py b/erpnext/accounts/doctype/bank_party_mapper/test_bank_party_mapper.py new file mode 100644 index 00000000000..c05b23f1a56 --- /dev/null +++ b/erpnext/accounts/doctype/bank_party_mapper/test_bank_party_mapper.py @@ -0,0 +1,9 @@ +# Copyright (c) 2023, Frappe Technologies Pvt. Ltd. and Contributors +# See license.txt + +# import frappe +from frappe.tests.utils import FrappeTestCase + + +class TestBankPartyMapper(FrappeTestCase): + pass diff --git a/erpnext/accounts/doctype/bank_transaction/auto_match_party.py b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py new file mode 100644 index 00000000000..7354fa0928a --- /dev/null +++ b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py @@ -0,0 +1,157 @@ +import frappe +from rapidfuzz import fuzz, process + + +class AutoMatchParty: + def __init__(self, **kwargs) -> None: + self.__dict__.update(kwargs) + + def get(self, key): + return self.__dict__.get(key, None) + + def match(self): + result = AutoMatchbyAccountIBAN( + bank_party_account_number=self.bank_party_account_number, + bank_party_iban=self.bank_party_iban, + deposit=self.deposit, + ).match() + + if not result: + result = AutoMatchbyPartyDescription( + bank_party_name=self.bank_party_name, description=self.description, deposit=self.deposit + ).match() + + return result + + +class AutoMatchbyAccountIBAN: + def __init__(self, **kwargs) -> None: + self.__dict__.update(kwargs) + + def get(self, key): + return self.__dict__.get(key, None) + + def match(self): + if not (self.bank_party_account_number or self.bank_party_iban): + return None + + result = self.match_account_in_bank_party_mapper() + if not result: + result = self.match_account_in_party() + + return result + + def match_account_in_bank_party_mapper(self): + filter_field = ( + "bank_party_account_number" if self.bank_party_account_number else "bank_party_iban" + ) + result = frappe.db.get_value( + "Bank Party Mapper", + filters={filter_field: self.get(filter_field)}, + fieldname=["party_type", "party"], + ) + if result: + party_type, party = result + return (party_type, party, None) + + return result + + def match_account_in_party(self): + # If not check if there is a match in Customer/Supplier/Employee + filter_field = "bank_account_no" if self.bank_party_account_number else "iban" + transaction_field = ( + "bank_party_account_number" if self.bank_party_account_number else "bank_party_iban" + ) + result = None + + parties = ["Supplier", "Employee", "Customer"] # most -> least likely to receive + if self.deposit > 0: + parties = ["Customer", "Supplier", "Employee"] # most -> least likely to pay + + for party in parties: + party_name = frappe.db.get_value( + party, filters={filter_field: self.get(transaction_field)}, fieldname=["name"] + ) + if party_name: + result = (party, party_name, {transaction_field: self.get(transaction_field)}) + break + + return result + + +class AutoMatchbyPartyDescription: + def __init__(self, **kwargs) -> None: + self.__dict__.update(kwargs) + + def get(self, key): + return self.__dict__.get(key, None) + + def match(self): + # Match by Customer, Supplier or Employee Name + # search bank party mapper by party and then description + # fuzzy search by customer/supplier & employee + if not (self.bank_party_name or self.description): + return None + + result = self.match_party_name_desc_in_bank_party_mapper() + + if not result: + result = self.match_party_name_desc_in_party() + + return result + + def match_party_name_desc_in_bank_party_mapper(self): + """Check if match exists for party name or description in Bank Party Mapper""" + result = None + # TODO: or filters + if self.bank_party_name: + result = frappe.db.get_value( + "Bank Party Mapper", + filters={"bank_party_name_desc": self.bank_party_name}, + fieldname=["party_type", "party"], + ) + + if not result and self.description: + result = frappe.db.get_value( + "Bank Party Mapper", + filters={"bank_party_name_desc": self.description}, + fieldname=["party_type", "party"], + ) + + result = result + (None,) if result else result + + return result + + def match_party_name_desc_in_party(self): + """Fuzzy search party name and/or description against parties in the system""" + result = None + + parties = ["Supplier", "Employee", "Customer"] # most-least likely to receive + if frappe.utils.flt(self.deposit) > 0.0: + parties = ["Customer", "Supplier", "Employee"] # most-least likely to pay + + for party in parties: + name_field = party.lower() + "_name" + filters = {"status": "Active"} if party == "Employee" else {"disabled": 0} + + names = frappe.get_all(party, filters=filters, pluck=name_field) + + for field in ["bank_party_name", "description"]: + if not result and self.get(field): + result = self.fuzzy_search_and_return_result(party, names, field) + if result: + break + + return result + + def fuzzy_search_and_return_result(self, party, names, field): + result = process.extractOne(query=self.get(field), choices=names, scorer=fuzz.token_set_ratio) + + if result: + party_name, score, index = result + if score > 75: + return (party, party_name, {"bank_party_name_desc": self.get(field)}) + else: + return None + + return result diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.json b/erpnext/accounts/doctype/bank_transaction/bank_transaction.json index 1543fdb894f..4139a9f6d5a 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.json +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.json @@ -36,7 +36,7 @@ "party", "column_break_3czf", "bank_party_name", - "bank_party_no", + "bank_party_account_number", "bank_party_iban" ], "fields": [ @@ -216,20 +216,20 @@ "fieldtype": "Data", "label": "Party Name (Bank Statement)" }, - { - "fieldname": "bank_party_no", - "fieldtype": "Data", - "label": "Party Account No. (Bank Statement)" - }, { "fieldname": "bank_party_iban", "fieldtype": "Data", "label": "Party IBAN (Bank Statement)" + }, + { + "fieldname": "bank_party_account_number", + "fieldtype": "Data", + "label": "Party Account No. (Bank Statement)" } ], "is_submittable": 1, "links": [], - "modified": "2023-03-30 15:30:46.485683", + "modified": "2023-03-31 10:45:30.671309", "modified_by": "Administrator", "module": "Accounts", "name": "Bank Transaction", diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py index 676c71910bf..745450423bd 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py @@ -9,11 +9,6 @@ from erpnext.controllers.status_updater import StatusUpdater class BankTransaction(StatusUpdater): # TODO - # On BT save: - # - Match by account no/iban in Customer/Supplier/Employee - # - Match by Party Name - # - If match found, set party type and party name. - # On submit/update after submit # - Create/Update a Bank Party Map record # - User can edit after submit. @@ -22,6 +17,12 @@ class BankTransaction(StatusUpdater): def after_insert(self): self.unallocated_amount = abs(flt(self.withdrawal) - flt(self.deposit)) + def on_update(self): + if self.party_type and self.party: + return + + self.auto_set_party() + def on_submit(self): self.clear_linked_payment_entries() self.set_status() @@ -157,6 +158,30 @@ class BankTransaction(StatusUpdater): payment_entry.payment_document, payment_entry.payment_entry, clearance_date, self ) + def auto_set_party(self): + # TODO: check if enabled + from erpnext.accounts.doctype.bank_transaction.auto_match_party import AutoMatchParty + + result = AutoMatchParty( + bank_party_account_number=self.bank_party_account_number, + bank_party_iban=self.bank_party_iban, + bank_party_name=self.bank_party_name, + description=self.description, + deposit=self.deposit, + ).match() + + if result: + self.party_type, self.party, mapper = result + + if not mapper: + return + + mapper_doc = frappe.get_doc( + {"doctype": "Bank Party Mapper", "party_type": self.party_type, "party": self.party} + ) + mapper_doc.update(mapper) + mapper_doc.insert() + @frappe.whitelist() def get_doctypes_for_bank_reconciliation(): diff --git a/pyproject.toml b/pyproject.toml index 0718e5b4a16..e5bc884729b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,6 +12,7 @@ dependencies = [ "pycountry~=20.7.3", "Unidecode~=1.2.0", "barcodenumber~=0.5.0", + "rapidfuzz~=2.15.0", # integration dependencies "gocardless-pro~=1.22.0", From 3a898289b0bcf867263e1b94a90846a1bcaa756a Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 3 Apr 2023 16:11:00 +0530 Subject: [PATCH 03/45] chore: Single query with or filter to search Party Mapper by name/desc --- .../bank_transaction/auto_match_party.py | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/erpnext/accounts/doctype/bank_transaction/auto_match_party.py b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py index 7354fa0928a..01b674340a2 100644 --- a/erpnext/accounts/doctype/bank_transaction/auto_match_party.py +++ b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py @@ -1,4 +1,5 @@ import frappe +from frappe.utils import flt from rapidfuzz import fuzz, process @@ -65,7 +66,7 @@ class AutoMatchbyAccountIBAN: result = None parties = ["Supplier", "Employee", "Customer"] # most -> least likely to receive - if self.deposit > 0: + if flt(self.deposit) > 0: parties = ["Customer", "Supplier", "Employee"] # most -> least likely to pay for party in parties: @@ -103,22 +104,27 @@ class AutoMatchbyPartyDescription: def match_party_name_desc_in_bank_party_mapper(self): """Check if match exists for party name or description in Bank Party Mapper""" result = None - # TODO: or filters + or_filters = [] + if self.bank_party_name: - result = frappe.db.get_value( - "Bank Party Mapper", - filters={"bank_party_name_desc": self.bank_party_name}, - fieldname=["party_type", "party"], - ) + or_filters.append(["bank_party_name_desc", self.bank_party_name]) - if not result and self.description: - result = frappe.db.get_value( - "Bank Party Mapper", - filters={"bank_party_name_desc": self.description}, - fieldname=["party_type", "party"], - ) + if self.description: + or_filters.append(["bank_party_name_desc", self.description]) - result = result + (None,) if result else result + mapper_res = frappe.get_all( + "Bank Party Mapper", + or_filters=or_filters, + fields=["party_type", "party"], + limit_page_length=1, + ) + if mapper_res: + mapper_res = mapper_res[0] + result = ( + mapper_res["party_type"], + mapper_res["party"], + None, + ) return result @@ -127,7 +133,7 @@ class AutoMatchbyPartyDescription: result = None parties = ["Supplier", "Employee", "Customer"] # most-least likely to receive - if frappe.utils.flt(self.deposit) > 0.0: + if flt(self.deposit) > 0.0: parties = ["Customer", "Supplier", "Employee"] # most-least likely to pay for party in parties: From 37c1331aba62ee85a79286d58a71461bb483be54 Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 4 Apr 2023 14:03:35 +0530 Subject: [PATCH 04/45] fix: Don't set description as key in Mapper doc if matched by description - Description is volatile and will keep changing - It will lead to multiple Bank Party Mapper docs for the same party that will never be referenced again - Parts of the descripton keep changing which is why it will never match a mapper record - If matched by desc, dont create mapper record. --- .../doctype/bank_transaction/auto_match_party.py | 12 +++++++++--- .../doctype/bank_transaction/bank_transaction.py | 6 +++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/erpnext/accounts/doctype/bank_transaction/auto_match_party.py b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py index 01b674340a2..3fbdc5f36b0 100644 --- a/erpnext/accounts/doctype/bank_transaction/auto_match_party.py +++ b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py @@ -107,10 +107,10 @@ class AutoMatchbyPartyDescription: or_filters = [] if self.bank_party_name: - or_filters.append(["bank_party_name_desc", self.bank_party_name]) + or_filters.append({"bank_party_name_desc": self.bank_party_name}) if self.description: - or_filters.append(["bank_party_name_desc", self.description]) + or_filters.append({"bank_party_name_desc": self.description}) mapper_res = frappe.get_all( "Bank Party Mapper", @@ -156,7 +156,13 @@ class AutoMatchbyPartyDescription: if result: party_name, score, index = result if score > 75: - return (party, party_name, {"bank_party_name_desc": self.get(field)}) + # Dont set description as a key in Bank Party Mapper due to its volatility + mapper = {"bank_party_name_desc": self.get(field)} if field == "bank_party_name" else None + return ( + party, + party_name, + mapper, + ) else: return None diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py index 745450423bd..118705d0d3d 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py @@ -18,9 +18,6 @@ class BankTransaction(StatusUpdater): self.unallocated_amount = abs(flt(self.withdrawal) - flt(self.deposit)) def on_update(self): - if self.party_type and self.party: - return - self.auto_set_party() def on_submit(self): @@ -162,6 +159,9 @@ class BankTransaction(StatusUpdater): # TODO: check if enabled from erpnext.accounts.doctype.bank_transaction.auto_match_party import AutoMatchParty + if self.party_type and self.party: + return + result = AutoMatchParty( bank_party_account_number=self.bank_party_account_number, bank_party_iban=self.bank_party_iban, From 27ce789023c98f4cd99086db9f5fd8d1f3ee3a30 Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 4 Apr 2023 19:27:01 +0530 Subject: [PATCH 05/45] feat: Manually Update/Correct Party in Bank Transaction - On updating bank trans.n party after submit, the corresponding mapper doc will be updated too - The mapper doc in turn will update all linked bank transactions that do not have this updated value - Added Bank Party Mapper hidden link in Bank Transaction - Rename field in BPM to `Party Name` as it does not hold description data - If a BT matches with a BPM record, link that record in the BT --- .../bank_party_mapper/bank_party_mapper.js | 12 ++++---- .../bank_party_mapper/bank_party_mapper.json | 10 +++---- .../bank_party_mapper/bank_party_mapper.py | 19 ++++++++++-- .../bank_transaction/auto_match_party.py | 30 ++++++++----------- .../bank_transaction/bank_transaction.json | 15 ++++++++-- .../bank_transaction/bank_transaction.py | 19 ++++++++++++ 6 files changed, 73 insertions(+), 32 deletions(-) diff --git a/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.js b/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.js index d11d8040b26..b13e46a0e76 100644 --- a/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.js +++ b/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.js @@ -1,8 +1,10 @@ // Copyright (c) 2023, Frappe Technologies Pvt. Ltd. and contributors // For license information, please see license.txt -// frappe.ui.form.on("Bank Party Mapper", { -// refresh(frm) { - -// }, -// }); +frappe.ui.form.on("Bank Party Mapper", { + refresh(frm) { + if (!frm.is_new()) { + frm.set_intro(__("Please avoid editing unless you are absolutely certain.")); + } + }, +}); diff --git a/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.json b/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.json index 4f5f6bfa886..ddc79f24a0c 100644 --- a/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.json +++ b/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.json @@ -9,7 +9,7 @@ "party_type", "party", "column_break_wbna", - "bank_party_name_desc", + "bank_party_name", "bank_party_account_number", "bank_party_iban" ], @@ -47,14 +47,14 @@ "options": "party_type" }, { - "fieldname": "bank_party_name_desc", - "fieldtype": "Small Text", - "label": "Party Name/Desc (Bank Statement)" + "fieldname": "bank_party_name", + "fieldtype": "Data", + "label": "Party Name (Bank Statement)" } ], "index_web_pages_for_search": 1, "links": [], - "modified": "2023-04-03 10:11:31.384383", + "modified": "2023-04-04 14:27:23.450456", "modified_by": "Administrator", "module": "Accounts", "name": "Bank Party Mapper", diff --git a/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.py b/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.py index d3a9a5e586c..02e36b08fe4 100644 --- a/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.py +++ b/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.py @@ -1,9 +1,24 @@ # Copyright (c) 2023, Frappe Technologies Pvt. Ltd. and contributors # For license information, please see license.txt -# import frappe +import frappe from frappe.model.document import Document class BankPartyMapper(Document): - pass + def on_update(self): + self.update_party_in_linked_transactions() + + def update_party_in_linked_transactions(self): + if self.is_new(): + return + + # Set updated party values in other linked bank transactions + bank_transaction = frappe.qb.DocType("Bank Transaction") + + frappe.qb.update(bank_transaction).set("party_type", self.party_type).set( + "party", self.party + ).where( + (bank_transaction.bank_party_mapper == self.name) + & ((bank_transaction.party_type != self.party_type) | (bank_transaction.party != self.party)) + ).run() diff --git a/erpnext/accounts/doctype/bank_transaction/auto_match_party.py b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py index 3fbdc5f36b0..9707f79a2ac 100644 --- a/erpnext/accounts/doctype/bank_transaction/auto_match_party.py +++ b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py @@ -49,11 +49,11 @@ class AutoMatchbyAccountIBAN: result = frappe.db.get_value( "Bank Party Mapper", filters={filter_field: self.get(filter_field)}, - fieldname=["party_type", "party"], + fieldname=["party_type", "party", "name"], ) if result: - party_type, party = result - return (party_type, party, None) + party_type, party, mapper_name = result + return (party_type, party, {"mapper_name": mapper_name}) return result @@ -89,33 +89,29 @@ class AutoMatchbyPartyDescription: def match(self): # Match by Customer, Supplier or Employee Name - # search bank party mapper by party and then description + # search bank party mapper by party # fuzzy search by customer/supplier & employee if not (self.bank_party_name or self.description): return None - result = self.match_party_name_desc_in_bank_party_mapper() + result = self.match_party_name_in_bank_party_mapper() if not result: result = self.match_party_name_desc_in_party() return result - def match_party_name_desc_in_bank_party_mapper(self): - """Check if match exists for party name or description in Bank Party Mapper""" + def match_party_name_in_bank_party_mapper(self): + """Check if match exists for party name in Bank Party Mapper""" result = None - or_filters = [] - if self.bank_party_name: - or_filters.append({"bank_party_name_desc": self.bank_party_name}) - - if self.description: - or_filters.append({"bank_party_name_desc": self.description}) + if not self.bank_party_name: + return mapper_res = frappe.get_all( "Bank Party Mapper", - or_filters=or_filters, - fields=["party_type", "party"], + filters={"bank_party_name": self.bank_party_name}, + fields=["party_type", "party", "name"], limit_page_length=1, ) if mapper_res: @@ -123,7 +119,7 @@ class AutoMatchbyPartyDescription: result = ( mapper_res["party_type"], mapper_res["party"], - None, + {"mapper_name": mapper_res["name"]}, ) return result @@ -157,7 +153,7 @@ class AutoMatchbyPartyDescription: party_name, score, index = result if score > 75: # Dont set description as a key in Bank Party Mapper due to its volatility - mapper = {"bank_party_name_desc": self.get(field)} if field == "bank_party_name" else None + mapper = {"bank_party_name": self.get(field)} if field == "bank_party_name" else None return ( party, party_name, diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.json b/erpnext/accounts/doctype/bank_transaction/bank_transaction.json index 4139a9f6d5a..d3dc5b5845b 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.json +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.json @@ -37,7 +37,8 @@ "column_break_3czf", "bank_party_name", "bank_party_account_number", - "bank_party_iban" + "bank_party_iban", + "bank_party_mapper" ], "fields": [ { @@ -214,7 +215,7 @@ { "fieldname": "bank_party_name", "fieldtype": "Data", - "label": "Party Name (Bank Statement)" + "label": "Party Name/Account Holder (Bank Statement)" }, { "fieldname": "bank_party_iban", @@ -225,11 +226,19 @@ "fieldname": "bank_party_account_number", "fieldtype": "Data", "label": "Party Account No. (Bank Statement)" + }, + { + "fieldname": "bank_party_mapper", + "fieldtype": "Link", + "hidden": 1, + "label": "Bank Party Mapper", + "options": "Bank Party Mapper", + "read_only": 1 } ], "is_submittable": 1, "links": [], - "modified": "2023-03-31 10:45:30.671309", + "modified": "2023-04-04 15:47:20.620006", "modified_by": "Administrator", "module": "Accounts", "name": "Bank Transaction", diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py index 118705d0d3d..a93882a5b32 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py @@ -35,6 +35,8 @@ class BankTransaction(StatusUpdater): self.update_allocations() self._saving_flag = False + self.set_in_bank_party_mapper() + def on_cancel(self): self.clear_linked_payment_entries(for_cancel=True) self.set_status(update=True) @@ -176,11 +178,28 @@ class BankTransaction(StatusUpdater): if not mapper: return + if mapper.get("mapper_name"): + # Transaction matched with a Bank party Mapper record + self.bank_party_mapper = mapper.get("mapper_name") # Link mapper to Bank Transaction + return + mapper_doc = frappe.get_doc( {"doctype": "Bank Party Mapper", "party_type": self.party_type, "party": self.party} ) mapper_doc.update(mapper) mapper_doc.insert() + self.bank_party_mapper = mapper_doc.name # Link mapper to Bank Transaction + + def set_in_bank_party_mapper(self): + """Set in Bank Party Mapper if Party Type & Party are manually changed after submit.""" + doc_before_update = self.get_doc_before_save() + party_type_changed = self.party_type and (doc_before_update.party_type != self.party_type) + party_changed = self.party and (doc_before_update.party != self.party) + + if (party_type_changed or party_changed) and self.bank_party_mapper: + mapper_doc = frappe.get_doc("Bank Party Mapper", self.bank_party_mapper) + mapper_doc.update({"party_type": self.party_type, "party": self.party}) + mapper_doc.save() @frappe.whitelist() From 33604550ce237c23d2d17345e2c041fc183a9b54 Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 4 Apr 2023 19:42:25 +0530 Subject: [PATCH 06/45] chore: Perform automatch on submit - misc: Clearer naming --- .../doctype/bank_transaction/bank_transaction.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py index a93882a5b32..6a094915b75 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py @@ -17,13 +17,12 @@ class BankTransaction(StatusUpdater): def after_insert(self): self.unallocated_amount = abs(flt(self.withdrawal) - flt(self.deposit)) - def on_update(self): - self.auto_set_party() - def on_submit(self): self.clear_linked_payment_entries() self.set_status() + self.auto_set_party() + _saving_flag = False # nosemgrep: frappe-semgrep-rules.rules.frappe-modifying-but-not-comitting @@ -35,7 +34,7 @@ class BankTransaction(StatusUpdater): self.update_allocations() self._saving_flag = False - self.set_in_bank_party_mapper() + self.update_automatch_bank_party_mapper() def on_cancel(self): self.clear_linked_payment_entries(for_cancel=True) @@ -190,8 +189,8 @@ class BankTransaction(StatusUpdater): mapper_doc.insert() self.bank_party_mapper = mapper_doc.name # Link mapper to Bank Transaction - def set_in_bank_party_mapper(self): - """Set in Bank Party Mapper if Party Type & Party are manually changed after submit.""" + def update_automatch_bank_party_mapper(self): + """Update Bank Party Mapper if Party Type & Party are manually changed after submit.""" doc_before_update = self.get_doc_before_save() party_type_changed = self.party_type and (doc_before_update.party_type != self.party_type) party_changed = self.party and (doc_before_update.party != self.party) From aea4315435cecd8c531eda5c4bf305b4792ca692 Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 4 Apr 2023 19:56:21 +0530 Subject: [PATCH 07/45] chore: Make auto matching party configurable - Checkbox in Accounts settings "Enable Automatic Party Matching" - Check before invoking automatching methods - misc: Remove TODO comments --- .../accounts_settings/accounts_settings.json | 18 ++++++++++++++++-- .../bank_transaction/bank_transaction.py | 13 ++++--------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/erpnext/accounts/doctype/accounts_settings/accounts_settings.json b/erpnext/accounts/doctype/accounts_settings/accounts_settings.json index c0eed18ad1b..259612e0e47 100644 --- a/erpnext/accounts/doctype/accounts_settings/accounts_settings.json +++ b/erpnext/accounts/doctype/accounts_settings/accounts_settings.json @@ -59,7 +59,9 @@ "frozen_accounts_modifier", "report_settings_sb", "tab_break_dpet", - "show_balance_in_coa" + "show_balance_in_coa", + "banking_tab", + "enable_party_matching" ], "fields": [ { @@ -368,6 +370,18 @@ "fieldname": "book_tax_discount_loss", "fieldtype": "Check", "label": "Book Tax Loss on Early Payment Discount" + }, + { + "fieldname": "banking_tab", + "fieldtype": "Tab Break", + "label": "Banking" + }, + { + "default": "0", + "description": "Auto match and set the Party in Bank Transactions", + "fieldname": "enable_party_matching", + "fieldtype": "Check", + "label": "Enable Automatic Party Matching" } ], "icon": "icon-cog", @@ -375,7 +389,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2023-03-28 09:50:20.375233", + "modified": "2023-04-04 16:20:41.330039", "modified_by": "Administrator", "module": "Accounts", "name": "Accounts Settings", diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py index 6a094915b75..25385a03ca0 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py @@ -8,12 +8,6 @@ from erpnext.controllers.status_updater import StatusUpdater class BankTransaction(StatusUpdater): - # TODO - # On submit/update after submit - # - Create/Update a Bank Party Map record - # - User can edit after submit. - # - If changes in party/party name after submit, edit bank party map (which should edit all transactions with same account no/iban/bank party name) - def after_insert(self): self.unallocated_amount = abs(flt(self.withdrawal) - flt(self.deposit)) @@ -21,7 +15,8 @@ class BankTransaction(StatusUpdater): self.clear_linked_payment_entries() self.set_status() - self.auto_set_party() + if frappe.db.get_single_value("Accounts Settings", "enable_party_matching"): + self.auto_set_party() _saving_flag = False @@ -34,7 +29,8 @@ class BankTransaction(StatusUpdater): self.update_allocations() self._saving_flag = False - self.update_automatch_bank_party_mapper() + if frappe.db.get_single_value("Accounts Settings", "enable_party_matching"): + self.update_automatch_bank_party_mapper() def on_cancel(self): self.clear_linked_payment_entries(for_cancel=True) @@ -157,7 +153,6 @@ class BankTransaction(StatusUpdater): ) def auto_set_party(self): - # TODO: check if enabled from erpnext.accounts.doctype.bank_transaction.auto_match_party import AutoMatchParty if self.party_type and self.party: From d7bc1928045d4e89e9acc61b6a71da25537b19a5 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 5 Apr 2023 15:28:47 +0530 Subject: [PATCH 08/45] fix: Match by both Account No and IBAN & other cleanups - A BT could have both account and iban, and a Supplier could have only IBAN set - In this case, matching by either (only account) gives no match - Match by Account OR IBAN, use `or_filters` - If matched, set both account no. and IBAN in Bank Party Mapper - Explain AutoMatchParty - Add type hints to return values - Use `set_value` to set values in BT after matching since its an after submit event --- .../bank_transaction/auto_match_party.py | 96 +++++++++++++------ .../bank_transaction/bank_transaction.py | 28 +++--- 2 files changed, 82 insertions(+), 42 deletions(-) diff --git a/erpnext/accounts/doctype/bank_transaction/auto_match_party.py b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py index 9707f79a2ac..2a2aff1eef6 100644 --- a/erpnext/accounts/doctype/bank_transaction/auto_match_party.py +++ b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py @@ -1,16 +1,40 @@ +from typing import Tuple, Union + import frappe from frappe.utils import flt from rapidfuzz import fuzz, process class AutoMatchParty: + """ + Matches by Account/IBAN and then by Party Name/Description sequentially. + Returns when a result is obtained. + + Result (if present) is of the form: (Party Type, Party, Mapper,) + + Mapper(if present) is one of the forms: + 1. {"mapper_name": }: Indicates that an existing Bank Party Mapper matched against + the transaction and the same must be linked in the Bank Transaction. + + 2. {"bank_party_account_number": , "bank_party_iban": } : Indicates that a match was + found in Customer/Supplier/Employee by account details. A Bank Party Mapper is now created + mapping the Party to the Account No./IBAN + + 3. {"bank_party_name": }: Indicates that a match was found in + Customer/Supplier/Employee by party name. A Bank Party Mapper is now created mapping the Party + to the Party Name (Bank Statement). If matched by Description, no mapper is created as + description is not a static key. + + Mapper data is used either to create a new Bank Party Mapper or link an existing mapper to a transaction. + """ + def __init__(self, **kwargs) -> None: self.__dict__.update(kwargs) def get(self, key): return self.__dict__.get(key, None) - def match(self): + def match(self) -> Union[Tuple, None]: result = AutoMatchbyAccountIBAN( bank_party_account_number=self.bank_party_account_number, bank_party_iban=self.bank_party_iban, @@ -42,27 +66,30 @@ class AutoMatchbyAccountIBAN: return result - def match_account_in_bank_party_mapper(self): - filter_field = ( - "bank_party_account_number" if self.bank_party_account_number else "bank_party_iban" - ) - result = frappe.db.get_value( + def match_account_in_bank_party_mapper(self) -> Union[Tuple, None]: + """Check for a IBAN/Account No. match in Bank Party Mapper""" + result = None + or_filters = {} + if self.bank_party_account_number: + or_filters["bank_party_account_number"] = self.bank_party_account_number + + if self.bank_party_iban: + or_filters["bank_party_iban"] = self.bank_party_iban + + mapper = frappe.db.get_all( "Bank Party Mapper", - filters={filter_field: self.get(filter_field)}, - fieldname=["party_type", "party", "name"], + or_filters=or_filters, + fields=["party_type", "party", "name"], + limit_page_length=1, ) - if result: - party_type, party, mapper_name = result - return (party_type, party, {"mapper_name": mapper_name}) + if mapper: + data = mapper[0] + return (data["party_type"], data["party"], {"mapper_name": data["name"]}) return result - def match_account_in_party(self): - # If not check if there is a match in Customer/Supplier/Employee - filter_field = "bank_account_no" if self.bank_party_account_number else "iban" - transaction_field = ( - "bank_party_account_number" if self.bank_party_account_number else "bank_party_iban" - ) + def match_account_in_party(self) -> Union[Tuple, None]: + """Check if there is a IBAN/Account No. match in Customer/Supplier/Employee""" result = None parties = ["Supplier", "Employee", "Customer"] # most -> least likely to receive @@ -70,11 +97,26 @@ class AutoMatchbyAccountIBAN: parties = ["Customer", "Supplier", "Employee"] # most -> least likely to pay for party in parties: - party_name = frappe.db.get_value( - party, filters={filter_field: self.get(transaction_field)}, fieldname=["name"] + or_filters = {} + if self.bank_party_account_number: + acc_no_field = "bank_ac_no" if party == "Employee" else "bank_account_no" + or_filters[acc_no_field] = self.bank_party_account_number + + if self.bank_party_iban: + or_filters["iban"] = self.bank_party_iban + + party_result = frappe.db.get_all( + party, or_filters=or_filters, pluck="name", limit_page_length=1 ) - if party_name: - result = (party, party_name, {transaction_field: self.get(transaction_field)}) + if party_result: + result = ( + party, + party_result[0], + { + "bank_party_account_number": self.get("bank_party_account_number"), + "bank_party_iban": self.get("bank_party_iban"), + }, + ) break return result @@ -87,7 +129,7 @@ class AutoMatchbyPartyDescription: def get(self, key): return self.__dict__.get(key, None) - def match(self): + def match(self) -> Union[Tuple, None]: # Match by Customer, Supplier or Employee Name # search bank party mapper by party # fuzzy search by customer/supplier & employee @@ -101,10 +143,9 @@ class AutoMatchbyPartyDescription: return result - def match_party_name_in_bank_party_mapper(self): + def match_party_name_in_bank_party_mapper(self) -> Union[Tuple, None]: """Check if match exists for party name in Bank Party Mapper""" result = None - if not self.bank_party_name: return @@ -116,7 +157,7 @@ class AutoMatchbyPartyDescription: ) if mapper_res: mapper_res = mapper_res[0] - result = ( + return ( mapper_res["party_type"], mapper_res["party"], {"mapper_name": mapper_res["name"]}, @@ -124,10 +165,9 @@ class AutoMatchbyPartyDescription: return result - def match_party_name_desc_in_party(self): + def match_party_name_desc_in_party(self) -> Union[Tuple, None]: """Fuzzy search party name and/or description against parties in the system""" result = None - parties = ["Supplier", "Employee", "Customer"] # most-least likely to receive if flt(self.deposit) > 0.0: parties = ["Customer", "Supplier", "Employee"] # most-least likely to pay @@ -146,7 +186,7 @@ class AutoMatchbyPartyDescription: return result - def fuzzy_search_and_return_result(self, party, names, field): + def fuzzy_search_and_return_result(self, party, names, field) -> Union[Tuple, None]: result = process.extractOne(query=self.get(field), choices=names, scorer=fuzz.token_set_ratio) if result: diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py index 25385a03ca0..cf3a6e648a3 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py @@ -167,22 +167,22 @@ class BankTransaction(StatusUpdater): ).match() if result: - self.party_type, self.party, mapper = result + party_type, party, mapper = result + to_update = {"party_type": party_type, "party": party} - if not mapper: - return + if mapper and mapper.get("mapper_name"): + # Transaction matched with an existing Bank party Mapper record + to_update["bank_party_mapper"] = mapper.get("mapper_name") + elif mapper: + # Make new Mapper record to remember match + mapper_doc = frappe.get_doc( + {"doctype": "Bank Party Mapper", "party_type": party_type, "party": party} + ) + mapper_doc.update(mapper) + mapper_doc.insert() + to_update["bank_party_mapper"] = mapper_doc.name - if mapper.get("mapper_name"): - # Transaction matched with a Bank party Mapper record - self.bank_party_mapper = mapper.get("mapper_name") # Link mapper to Bank Transaction - return - - mapper_doc = frappe.get_doc( - {"doctype": "Bank Party Mapper", "party_type": self.party_type, "party": self.party} - ) - mapper_doc.update(mapper) - mapper_doc.insert() - self.bank_party_mapper = mapper_doc.name # Link mapper to Bank Transaction + frappe.db.set_value("Bank Transaction", self.name, field=to_update) def update_automatch_bank_party_mapper(self): """Update Bank Party Mapper if Party Type & Party are manually changed after submit.""" From 7ed8f59dc89f02562a56d1281219506214a440b3 Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 10 Apr 2023 22:11:00 +0530 Subject: [PATCH 09/45] test: Match by Account No, IBAN, Party Name, Desc and match correction --- .../bank_transaction/test_auto_match_party.py | 193 ++++++++++++++++++ 1 file changed, 193 insertions(+) create mode 100644 erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py diff --git a/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py b/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py new file mode 100644 index 00000000000..86181c865d7 --- /dev/null +++ b/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py @@ -0,0 +1,193 @@ +# Copyright (c) 2023, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + +import frappe +from frappe.tests.utils import FrappeTestCase +from frappe.utils import nowdate + +from erpnext.accounts.doctype.bank_transaction.test_bank_transaction import create_bank_account + + +class TestAutoMatchParty(FrappeTestCase): + @classmethod + def setUpClass(cls): + create_bank_account() + frappe.db.set_single_value("Accounts Settings", "enable_party_matching", 1) + return super().setUpClass() + + @classmethod + def tearDownClass(cls): + frappe.db.set_single_value("Accounts Settings", "enable_party_matching", 0) + + def test_match_by_account_number(self): + """Test if transaction matches with existing (Bank Party Mapper) or new match.""" + create_supplier_for_match(account_no="000000003716541159") + doc = create_bank_transaction( + withdrawal=1200, + transaction_id="562213b0ca1bf838dab8f2c6a39bbc3b", + account_no="000000003716541159", + iban="DE02000000003716541159", + ) + + self.assertEqual(doc.party_type, "Supplier") + self.assertEqual(doc.party, "John Doe & Co.") + self.assertTrue(doc.bank_party_mapper) + + # Check if Bank Party Mapper is created to remember mapping + bank_party_mapper = frappe.get_doc("Bank Party Mapper", doc.bank_party_mapper) + self.assertEqual(bank_party_mapper.party, "John Doe & Co.") + self.assertEqual(bank_party_mapper.bank_party_account_number, "000000003716541159") + self.assertEqual(bank_party_mapper.bank_party_iban, "DE02000000003716541159") + + # Check if created mapping is used for quick match + doc_2 = create_bank_transaction( + withdrawal=500, + transaction_id="602413b8ji8bf838fub8f2c6a39bah7y", + account_no="000000003716541159", + ) + self.assertEqual(doc_2.party, "John Doe & Co.") + self.assertEqual(doc_2.bank_party_mapper, bank_party_mapper.name) + + def test_match_by_iban(self): + create_supplier_for_match(iban="DE02000000003716541159") + doc = create_bank_transaction( + withdrawal=1200, + transaction_id="c5455a224602afaa51592a9d9250600d", + account_no="000000003716541159", + iban="DE02000000003716541159", + ) + + self.assertEqual(doc.party_type, "Supplier") + self.assertEqual(doc.party, "John Doe & Co.") + self.assertTrue(doc.bank_party_mapper) + + bank_party_mapper = frappe.get_doc("Bank Party Mapper", doc.bank_party_mapper) + self.assertEqual(bank_party_mapper.party, "John Doe & Co.") + self.assertEqual(bank_party_mapper.bank_party_account_number, "000000003716541159") + self.assertEqual(bank_party_mapper.bank_party_iban, "DE02000000003716541159") + + def test_match_by_party_name(self): + create_supplier_for_match(supplier_name="Jackson Ella W.") + doc = create_bank_transaction( + withdrawal=1200, + transaction_id="1f6f661f347ff7b1ea588665f473adb1", + party_name="Ella Jackson", + iban="DE04000000003716545346", + ) + self.assertEqual(doc.party_type, "Supplier") + self.assertEqual(doc.party, "Jackson Ella W.") + self.assertTrue(doc.bank_party_mapper) + + bank_party_mapper = frappe.get_doc("Bank Party Mapper", doc.bank_party_mapper) + self.assertEqual(bank_party_mapper.party, "Jackson Ella W.") + self.assertEqual(bank_party_mapper.bank_party_name, "Ella Jackson") + self.assertEqual(bank_party_mapper.bank_party_iban, None) + + # Check if created mapping is used for quick match + doc_2 = create_bank_transaction( + withdrawal=500, + transaction_id="578313b8ji8bf838fub8f2c6a39bah7y", + party_name="Ella Jackson", + account_no="000000004316531152", + ) + self.assertEqual(doc_2.party, "Jackson Ella W.") + self.assertEqual(doc_2.bank_party_mapper, bank_party_mapper.name) + + def test_match_by_description(self): + create_supplier_for_match(supplier_name="Microsoft") + doc = create_bank_transaction( + description="Auftraggeber: microsoft payments Buchungstext: msft ..e3006b5hdy. ref. j375979555927627/5536", + withdrawal=1200, + transaction_id="8df880a2d09c3bed3fea358ca5168c5a", + party_name="", + ) + self.assertEqual(doc.party_type, "Supplier") + self.assertEqual(doc.party, "Microsoft") + self.assertFalse(doc.bank_party_mapper) + + def test_correct_match_after_submit(self): + """Correct wrong mapping after submit. Test impact.""" + # Similar named suppliers + create_supplier_for_match(supplier_name="Amazon") + create_supplier_for_match(supplier_name="Amazing Co.") + + # Bank Transactions actually from "Amazon" that match better with "Amazing Co." + doc = create_bank_transaction( + description="visa06323202 amzn.com/bill 7,88eur1,5324711959 90.22. 1,62 87861003", + withdrawal=24.85, + transaction_id="3a1da4ee2dc5a980138d36ef5297cbd9", + party_name="Amazn Co.", + ) + doc_2 = create_bank_transaction( + description="visa61268005 amzn.com/bill 22,345eur1,7654711959 89.23. 1,64 61268005", + withdrawal=80, + transaction_id="584314e459b00f792bfd569267efba6e", + party_name="Amazn Co.", + ) + + self.assertEqual(doc.party_type, "Supplier") + self.assertEqual(doc.party, "Amazing Co.") + self.assertTrue(doc.bank_party_mapper) + self.assertTrue(doc_2.bank_party_mapper, doc.bank_party_mapper) + + bank_party_mapper = frappe.get_doc("Bank Party Mapper", doc.bank_party_mapper) + self.assertEqual(bank_party_mapper.party, "Amazing Co.") + self.assertEqual(bank_party_mapper.bank_party_name, "Amazn Co.") + + # User corrects the value after submit to "Amazon" + doc.party = "Amazon" + doc.save() + bank_party_mapper.reload() + doc_2.reload() + + # Mapping is edited and all transactions with this mapping are updated + self.assertEqual(bank_party_mapper.party, "Amazon") + self.assertEqual(bank_party_mapper.bank_party_name, "Amazn Co.") + self.assertEqual(doc_2.party, "Amazon") + + +def create_supplier_for_match(supplier_name="John Doe & Co.", account_no=None, iban=None): + if frappe.db.exists("Supplier", supplier_name): + frappe.db.set_value("Supplier", supplier_name, {"bank_account_no": account_no, "iban": iban}) + return + + frappe.get_doc( + { + "doctype": "Supplier", + "supplier_name": supplier_name, + "supplier_group": "Services", + "supplier_type": "Company", + "bank_account_no": account_no, + "iban": iban, + } + ).insert() + + +def create_bank_transaction( + description=None, + withdrawal=0, + deposit=0, + transaction_id=None, + party_name=None, + account_no=None, + iban=None, +): + doc = frappe.get_doc( + { + "doctype": "Bank Transaction", + "description": description or "1512567 BG/000002918 OPSKATTUZWXXX AT776000000098709837 Herr G", + "date": nowdate(), + "withdrawal": withdrawal, + "deposit": deposit, + "currency": "INR", + "bank_account": "Checking Account - Citi Bank", + "transaction_id": transaction_id, + "bank_party_name": party_name, + "bank_party_account_number": account_no, + "bank_party_iban": iban, + } + ).insert() + doc.submit() + doc.reload() + + return doc From 430b247dfc0e4f681fe47d540fd24cd0b40f76d1 Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 11 Apr 2023 01:33:08 +0530 Subject: [PATCH 10/45] fix: Remove bank details fields from Shareholder --- .../doctype/shareholder/shareholder.json | 27 ++----------------- 1 file changed, 2 insertions(+), 25 deletions(-) diff --git a/erpnext/accounts/doctype/shareholder/shareholder.json b/erpnext/accounts/doctype/shareholder/shareholder.json index 2be2a2fc427..e80b05720e0 100644 --- a/erpnext/accounts/doctype/shareholder/shareholder.json +++ b/erpnext/accounts/doctype/shareholder/shareholder.json @@ -20,11 +20,7 @@ "contact_html", "section_break_3", "share_balance", - "contact_list", - "bank_details_section", - "bank_account_no", - "column_break_tyo0", - "iban" + "contact_list" ], "fields": [ { @@ -114,29 +110,10 @@ "hidden": 1, "label": "Contact List", "read_only": 1 - }, - { - "fieldname": "bank_details_section", - "fieldtype": "Section Break", - "label": "Bank Details" - }, - { - "fieldname": "bank_account_no", - "fieldtype": "Data", - "label": "Bank Account No" - }, - { - "fieldname": "column_break_tyo0", - "fieldtype": "Column Break" - }, - { - "fieldname": "iban", - "fieldtype": "Data", - "label": "IBAN" } ], "links": [], - "modified": "2023-03-30 16:00:55.087823", + "modified": "2023-04-10 22:02:20.406087", "modified_by": "Administrator", "module": "Accounts", "name": "Shareholder", From dbf7a479b66bc31890b4a957564c7dfdfad49517 Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 9 May 2023 20:36:07 +0530 Subject: [PATCH 11/45] fix: Use existing bank fields to match by bank account no/IBAN - Remove newly added fields in Party doctypes to store bank details - Use Bank Account's fields to match against account no/iban - For employee, if Bank Account does not exist, find in Employee doctype against account no/iban --- .../bank_transaction/auto_match_party.py | 15 ++++++++-- erpnext/buying/doctype/supplier/supplier.json | 30 +------------------ .../selling/doctype/customer/customer.json | 25 +--------------- 3 files changed, 14 insertions(+), 56 deletions(-) diff --git a/erpnext/accounts/doctype/bank_transaction/auto_match_party.py b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py index 2a2aff1eef6..e43d9beb05d 100644 --- a/erpnext/accounts/doctype/bank_transaction/auto_match_party.py +++ b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py @@ -99,15 +99,24 @@ class AutoMatchbyAccountIBAN: for party in parties: or_filters = {} if self.bank_party_account_number: - acc_no_field = "bank_ac_no" if party == "Employee" else "bank_account_no" - or_filters[acc_no_field] = self.bank_party_account_number + or_filters["bank_account_no"] = self.bank_party_account_number if self.bank_party_iban: or_filters["iban"] = self.bank_party_iban party_result = frappe.db.get_all( - party, or_filters=or_filters, pluck="name", limit_page_length=1 + "Bank Account", or_filters=or_filters, pluck="name", limit_page_length=1 ) + + if party == "Employee" and not party_result: + # Search in Bank Accounts first for Employee, and then Employee record + if "bank_account_no" in or_filters: + or_filters["bank_ac_no"] = or_filters.pop("bank_account_no") + + party_result = frappe.db.get_all( + party, or_filters=or_filters, pluck="name", limit_page_length=1 + ) + if party_result: result = ( party, diff --git a/erpnext/buying/doctype/supplier/supplier.json b/erpnext/buying/doctype/supplier/supplier.json index a80dcfe5388..f0097898f65 100644 --- a/erpnext/buying/doctype/supplier/supplier.json +++ b/erpnext/buying/doctype/supplier/supplier.json @@ -52,11 +52,6 @@ "supplier_primary_address", "primary_address", "accounting_tab", - "bank_details_section", - "bank_account_no", - "column_break_n8mz", - "iban", - "section_break_ow3k", "payment_terms", "accounts", "settings_tab", @@ -450,29 +445,6 @@ { "fieldname": "column_break_59", "fieldtype": "Column Break" - }, - { - "fieldname": "bank_details_section", - "fieldtype": "Section Break", - "label": "Bank Details" - }, - { - "fieldname": "bank_account_no", - "fieldtype": "Data", - "label": "Bank Account No" - }, - { - "fieldname": "column_break_n8mz", - "fieldtype": "Column Break" - }, - { - "fieldname": "iban", - "fieldtype": "Data", - "label": "IBAN" - }, - { - "fieldname": "section_break_ow3k", - "fieldtype": "Section Break" } ], "icon": "fa fa-user", @@ -485,7 +457,7 @@ "link_fieldname": "party" } ], - "modified": "2023-03-30 15:50:40.241257", + "modified": "2023-05-09 15:34:13.408932", "modified_by": "Administrator", "module": "Buying", "name": "Supplier", diff --git a/erpnext/selling/doctype/customer/customer.json b/erpnext/selling/doctype/customer/customer.json index 5dc6a72da85..72a1594b2d2 100644 --- a/erpnext/selling/doctype/customer/customer.json +++ b/erpnext/selling/doctype/customer/customer.json @@ -61,10 +61,6 @@ "tax_category", "tax_withholding_category", "accounting_tab", - "bank_details_section", - "bank_account_no", - "column_break_xtwg", - "iban", "credit_limit_section", "payment_terms", "credit_limits", @@ -559,25 +555,6 @@ { "fieldname": "column_break_54", "fieldtype": "Column Break" - }, - { - "fieldname": "bank_details_section", - "fieldtype": "Section Break", - "label": "Bank Details" - }, - { - "fieldname": "bank_account_no", - "fieldtype": "Data", - "label": "Bank Account No" - }, - { - "fieldname": "column_break_xtwg", - "fieldtype": "Column Break" - }, - { - "fieldname": "iban", - "fieldtype": "Data", - "label": "IBAN" } ], "icon": "fa fa-user", @@ -591,7 +568,7 @@ "link_fieldname": "party" } ], - "modified": "2023-03-30 15:45:44.387975", + "modified": "2023-05-09 15:38:40.255193", "modified_by": "Administrator", "module": "Selling", "name": "Customer", From 4a14e9ea4e8f131913f7a0ede725db9a7603e85a Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 17 May 2023 14:23:44 +0530 Subject: [PATCH 12/45] fix: Tests --- .../bank_transaction/auto_match_party.py | 2 +- .../bank_transaction/test_auto_match_party.py | 40 ++++++++++++++++--- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/erpnext/accounts/doctype/bank_transaction/auto_match_party.py b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py index e43d9beb05d..79d52c65650 100644 --- a/erpnext/accounts/doctype/bank_transaction/auto_match_party.py +++ b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py @@ -105,7 +105,7 @@ class AutoMatchbyAccountIBAN: or_filters["iban"] = self.bank_party_iban party_result = frappe.db.get_all( - "Bank Account", or_filters=or_filters, pluck="name", limit_page_length=1 + "Bank Account", or_filters=or_filters, pluck="party", limit_page_length=1 ) if party == "Employee" and not party_result: diff --git a/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py b/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py index 86181c865d7..2f94516ac29 100644 --- a/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py +++ b/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py @@ -146,22 +146,50 @@ class TestAutoMatchParty(FrappeTestCase): self.assertEqual(doc_2.party, "Amazon") -def create_supplier_for_match(supplier_name="John Doe & Co.", account_no=None, iban=None): - if frappe.db.exists("Supplier", supplier_name): - frappe.db.set_value("Supplier", supplier_name, {"bank_account_no": account_no, "iban": iban}) +def create_supplier_for_match(supplier_name="John Doe & Co.", iban=None, account_no=None): + if frappe.db.exists("Supplier", {"supplier_name": supplier_name}): + # Update related Bank Account details + if not (iban or account_no): + return + + frappe.db.set_value( + dt="Bank Account", + dn={"party": supplier_name}, + field={"iban": iban, "bank_account_no": account_no}, + ) return - frappe.get_doc( + # Create Supplier and Bank Account for the same + supplier = frappe.get_doc( { "doctype": "Supplier", "supplier_name": supplier_name, "supplier_group": "Services", "supplier_type": "Company", - "bank_account_no": account_no, - "iban": iban, } ).insert() + if not frappe.db.exists("Bank", "TestBank"): + frappe.get_doc( + { + "doctype": "Bank", + "bank_name": "TestBank", + } + ).insert(ignore_if_duplicate=True) + + if not frappe.db.exists("Bank Account", supplier.name + " - " + "TestBank"): + frappe.get_doc( + { + "doctype": "Bank Account", + "account_name": supplier.name, + "bank": "TestBank", + "iban": iban, + "bank_account_no": account_no, + "party_type": "Supplier", + "party": supplier.name, + } + ).insert() + def create_bank_transaction( description=None, From 4364fb9628854a27942304e55c26f62d916cd46c Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 17 May 2023 19:45:03 +0530 Subject: [PATCH 13/45] feat: Optional Fuzzy Matching & Skip Matches for multiple similar matches - Fuzzy matching can be enabled optionally in the settings - If a query gets multiple matches with the same score, do not set a party as it is an extremely close call - misc: Add 'cancelled' status to Bank transaction - Test for skipping matching with extremely close matches --- .../accounts_settings/accounts_settings.json | 13 +++- .../bank_transaction/auto_match_party.py | 77 ++++++++++++++----- .../bank_transaction/bank_transaction.json | 4 +- .../bank_transaction/test_auto_match_party.py | 18 +++++ 4 files changed, 88 insertions(+), 24 deletions(-) diff --git a/erpnext/accounts/doctype/accounts_settings/accounts_settings.json b/erpnext/accounts/doctype/accounts_settings/accounts_settings.json index ff07de34671..05f1169d96a 100644 --- a/erpnext/accounts/doctype/accounts_settings/accounts_settings.json +++ b/erpnext/accounts/doctype/accounts_settings/accounts_settings.json @@ -64,7 +64,8 @@ "tab_break_dpet", "show_balance_in_coa", "banking_tab", - "enable_party_matching" + "enable_party_matching", + "enable_fuzzy_matching" ], "fields": [ { @@ -404,6 +405,14 @@ "fieldname": "enable_party_matching", "fieldtype": "Check", "label": "Enable Automatic Party Matching" + }, + { + "default": "0", + "depends_on": "enable_party_matching", + "description": "Approximately match the description/party name against parties", + "fieldname": "enable_fuzzy_matching", + "fieldtype": "Check", + "label": "Enable Fuzzy Matching" } ], "icon": "icon-cog", @@ -411,7 +420,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2023-04-21 13:11:37.130743", + "modified": "2023-05-17 12:20:04.107641", "modified_by": "Administrator", "module": "Accounts", "name": "Accounts Settings", diff --git a/erpnext/accounts/doctype/bank_transaction/auto_match_party.py b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py index 79d52c65650..753f0c1171a 100644 --- a/erpnext/accounts/doctype/bank_transaction/auto_match_party.py +++ b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py @@ -35,13 +35,15 @@ class AutoMatchParty: return self.__dict__.get(key, None) def match(self) -> Union[Tuple, None]: + result = None result = AutoMatchbyAccountIBAN( bank_party_account_number=self.bank_party_account_number, bank_party_iban=self.bank_party_iban, deposit=self.deposit, ).match() - if not result: + fuzzy_matching_enabled = frappe.db.get_single_value("Accounts Settings", "enable_fuzzy_matching") + if not result and fuzzy_matching_enabled: result = AutoMatchbyPartyDescription( bank_party_name=self.bank_party_name, description=self.description, deposit=self.deposit ).match() @@ -184,31 +186,66 @@ class AutoMatchbyPartyDescription: for party in parties: name_field = party.lower() + "_name" filters = {"status": "Active"} if party == "Employee" else {"disabled": 0} - names = frappe.get_all(party, filters=filters, pluck=name_field) for field in ["bank_party_name", "description"]: - if not result and self.get(field): - result = self.fuzzy_search_and_return_result(party, names, field) - if result: - break + if not self.get(field): + continue + + result, skip = self.fuzzy_search_and_return_result(party, names, field) + if result or skip: + break + + if result or skip: + # We skip if: + # If it was hard to distinguish between close matches and so match is None + # OR if the right match was found + break return result def fuzzy_search_and_return_result(self, party, names, field) -> Union[Tuple, None]: - result = process.extractOne(query=self.get(field), choices=names, scorer=fuzz.token_set_ratio) + skip = False - if result: - party_name, score, index = result - if score > 75: - # Dont set description as a key in Bank Party Mapper due to its volatility - mapper = {"bank_party_name": self.get(field)} if field == "bank_party_name" else None - return ( - party, - party_name, - mapper, - ) - else: - return None + result = process.extract(query=self.get(field), choices=names, scorer=fuzz.token_set_ratio) + party_name, skip = self.process_fuzzy_result(result) - return result + if not party_name: + return None, skip + + # Dont set description as a key in Bank Party Mapper due to its volatility + mapper = {"bank_party_name": self.get(field)} if field == "bank_party_name" else None + return ( + party, + party_name, + mapper, + ), skip + + def process_fuzzy_result(self, result: Union[list, None]): + """ + If there are multiple valid close matches return None as result may be faulty. + Return the result only if one accurate match stands out. + + Returns: Result, Skip (whether or not to continue matching) + """ + PARTY, SCORE, CUTOFF = 0, 1, 80 + + if not result or not len(result): + return None, False + + first_result = result[0] + + if len(result) == 1: + return (result[0][PARTY] if first_result[SCORE] > CUTOFF else None), True + + second_result = result[1] + + if first_result[SCORE] > CUTOFF: + # If multiple matches with the same score, return None but discontinue matching + # Matches were found but were too closes to distinguish between + if first_result[SCORE] == second_result[SCORE]: + return None, True + + return first_result[PARTY], True + else: + return None, False diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.json b/erpnext/accounts/doctype/bank_transaction/bank_transaction.json index d3dc5b5845b..e7de71aef40 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.json +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.json @@ -68,7 +68,7 @@ "fieldtype": "Select", "in_standard_filter": 1, "label": "Status", - "options": "\nPending\nSettled\nUnreconciled\nReconciled" + "options": "\nPending\nSettled\nUnreconciled\nReconciled\nCancelled" }, { "fieldname": "bank_account", @@ -238,7 +238,7 @@ ], "is_submittable": 1, "links": [], - "modified": "2023-04-04 15:47:20.620006", + "modified": "2023-05-17 14:56:10.547480", "modified_by": "Administrator", "module": "Accounts", "name": "Bank Transaction", diff --git a/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py b/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py index 2f94516ac29..8c6dc9d7668 100644 --- a/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py +++ b/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py @@ -13,11 +13,13 @@ class TestAutoMatchParty(FrappeTestCase): def setUpClass(cls): create_bank_account() frappe.db.set_single_value("Accounts Settings", "enable_party_matching", 1) + frappe.db.set_single_value("Accounts Settings", "enable_fuzzy_matching", 1) return super().setUpClass() @classmethod def tearDownClass(cls): frappe.db.set_single_value("Accounts Settings", "enable_party_matching", 0) + frappe.db.set_single_value("Accounts Settings", "enable_fuzzy_matching", 0) def test_match_by_account_number(self): """Test if transaction matches with existing (Bank Party Mapper) or new match.""" @@ -145,6 +147,22 @@ class TestAutoMatchParty(FrappeTestCase): self.assertEqual(bank_party_mapper.bank_party_name, "Amazn Co.") self.assertEqual(doc_2.party, "Amazon") + def test_skip_match_if_multiple_close_results(self): + create_supplier_for_match(supplier_name="Adithya Medical & General Stores") + create_supplier_for_match(supplier_name="Adithya Medical And General Stores") + + doc = create_bank_transaction( + description="Paracetamol Consignment, SINV-0009", + withdrawal=24.85, + transaction_id="3a1da4ee2dc5a980138d56ef3460cbd9", + party_name="Adithya Medical & General", + ) + + # Mapping is skipped as both Supplier names have the same match score + self.assertEqual(doc.party_type, None) + self.assertEqual(doc.party, None) + self.assertFalse(doc.bank_party_mapper) + def create_supplier_for_match(supplier_name="John Doe & Co.", iban=None, account_no=None): if frappe.db.exists("Supplier", {"supplier_name": supplier_name}): From 2e52a63b0df0f20ff774131623580377ce57d451 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Sun, 4 Jun 2023 19:20:28 +0530 Subject: [PATCH 14/45] feat: Accounting Ledger Preview --- .../doctype/sales_invoice/sales_invoice.js | 1 + erpnext/controllers/stock_controller.py | 13 +++++++ .../public/js/controllers/stock_controller.js | 36 ++++++++++++++++++- 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.js b/erpnext/accounts/doctype/sales_invoice/sales_invoice.js index 8cb29505eb2..1ef0c51cbac 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.js +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.js @@ -88,6 +88,7 @@ erpnext.accounts.SalesInvoiceController = class SalesInvoiceController extends e } this.show_general_ledger(); + this.show_ledger_preview(); if(doc.update_stock) this.show_stock_ledger(); diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index befde71775a..30dcc7ffa7f 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -15,6 +15,7 @@ from erpnext.accounts.general_ledger import ( make_reverse_gl_entries, process_gl_map, ) +from erpnext.accounts.report.general_ledger.general_ledger import get_columns from erpnext.accounts.utils import get_fiscal_year from erpnext.controllers.accounts_controller import AccountsController from erpnext.stock import get_warehouse_account_map @@ -824,6 +825,18 @@ class StockController(AccountsController): gl_entries.append(self.get_gl_dict(gl_entry, item=item)) +@frappe.whitelist() +def show_ledger_preview(company, doctype, docname): + filters = {"company": company} + doc = frappe.get_doc(doctype, docname) + columns = get_columns(filters) + data = doc.get_gl_entries() + return { + "columns": columns, + "data": data, + } + + def repost_required_for_queue(doc: StockController) -> bool: """check if stock document contains repeated item-warehouse with queue based valuation. diff --git a/erpnext/public/js/controllers/stock_controller.js b/erpnext/public/js/controllers/stock_controller.js index d346357a8f8..919ffda52fc 100644 --- a/erpnext/public/js/controllers/stock_controller.js +++ b/erpnext/public/js/controllers/stock_controller.js @@ -66,7 +66,7 @@ erpnext.stock.StockController = class StockController extends frappe.ui.form.Con } show_general_ledger() { - var me = this; + let me = this; if(this.frm.doc.docstatus > 0) { cur_frm.add_custom_button(__('Accounting Ledger'), function() { frappe.route_options = { @@ -81,4 +81,38 @@ erpnext.stock.StockController = class StockController extends frappe.ui.form.Con }, __("View")); } } + + show_ledger_preview() { + let me = this + if(this.frm.doc.docstatus == 0) { + cur_frm.add_custom_button(__('Accounting Ledger Preview'), function() { + frappe.call({ + "method": "erpnext.controllers.stock_controller.show_ledger_preview", + "args": { + "company": me.frm.doc.company, + "doctype": me.frm.doc.doctype, + "docname": me.frm.doc.name + }, + "callback": function(response) { + me.get_datatable(response); + } + }) + }, __("View")); + } + } + + get_datatable(response) { + const datatable_options = { + columns: response.columns, + data: response.data, + dynamicRowHeight: true, + checkboxColumn: false, + inlineFilters: true, + }; + + this.datatable = new frappe.DataTable( + this.frm.page.main.parent, + datatable_options + ); + } }; From 752a92bd8be6d25325b01e4780f1b37b2195da87 Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 6 Jun 2023 18:59:07 +0530 Subject: [PATCH 15/45] chore: Remove Bank Party Mapper implementation - Matching by Acc No/IBAN can easily happen with Bank Accounts. It's not a tedious query - Historical lookups for Party Name/Desc match are very tricky. The user could have manually set a match and we would not know. Also this leaves the Bank Party Mapper only useful for Party Name/Desc lookups, which feels excessive. - We want to reduce the number of places the same data is stored and reduce confusion - The Party Name/Desc will optionally happen fuzzily, or not at all - There will be no Mapper lookups --- .../doctype/bank_party_mapper/__init__.py | 0 .../bank_party_mapper/bank_party_mapper.js | 10 -- .../bank_party_mapper/bank_party_mapper.json | 80 ----------- .../bank_party_mapper/bank_party_mapper.py | 24 ---- .../test_bank_party_mapper.py | 9 -- .../bank_transaction/auto_match_party.py | 135 ++++-------------- .../bank_transaction/bank_transaction.json | 13 +- .../bank_transaction/bank_transaction.py | 34 +---- .../bank_transaction/test_auto_match_party.py | 79 ---------- 9 files changed, 37 insertions(+), 347 deletions(-) delete mode 100644 erpnext/accounts/doctype/bank_party_mapper/__init__.py delete mode 100644 erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.js delete mode 100644 erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.json delete mode 100644 erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.py delete mode 100644 erpnext/accounts/doctype/bank_party_mapper/test_bank_party_mapper.py diff --git a/erpnext/accounts/doctype/bank_party_mapper/__init__.py b/erpnext/accounts/doctype/bank_party_mapper/__init__.py deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.js b/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.js deleted file mode 100644 index b13e46a0e76..00000000000 --- a/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.js +++ /dev/null @@ -1,10 +0,0 @@ -// Copyright (c) 2023, Frappe Technologies Pvt. Ltd. and contributors -// For license information, please see license.txt - -frappe.ui.form.on("Bank Party Mapper", { - refresh(frm) { - if (!frm.is_new()) { - frm.set_intro(__("Please avoid editing unless you are absolutely certain.")); - } - }, -}); diff --git a/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.json b/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.json deleted file mode 100644 index ddc79f24a0c..00000000000 --- a/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.json +++ /dev/null @@ -1,80 +0,0 @@ -{ - "actions": [], - "creation": "2023-03-31 10:48:20.249481", - "default_view": "List", - "doctype": "DocType", - "editable_grid": 1, - "engine": "InnoDB", - "field_order": [ - "party_type", - "party", - "column_break_wbna", - "bank_party_name", - "bank_party_account_number", - "bank_party_iban" - ], - "fields": [ - { - "fieldname": "bank_party_account_number", - "fieldtype": "Data", - "in_list_view": 1, - "label": "Party Account No. (Bank Statement)" - }, - { - "fieldname": "bank_party_iban", - "fieldtype": "Data", - "in_list_view": 1, - "label": "Party IBAN (Bank Statement)" - }, - { - "fieldname": "column_break_wbna", - "fieldtype": "Column Break" - }, - { - "fieldname": "party_type", - "fieldtype": "Link", - "in_list_view": 1, - "in_standard_filter": 1, - "label": "Party Type", - "options": "DocType" - }, - { - "fieldname": "party", - "fieldtype": "Dynamic Link", - "in_list_view": 1, - "in_standard_filter": 1, - "label": "Party", - "options": "party_type" - }, - { - "fieldname": "bank_party_name", - "fieldtype": "Data", - "label": "Party Name (Bank Statement)" - } - ], - "index_web_pages_for_search": 1, - "links": [], - "modified": "2023-04-04 14:27:23.450456", - "modified_by": "Administrator", - "module": "Accounts", - "name": "Bank Party Mapper", - "owner": "Administrator", - "permissions": [ - { - "create": 1, - "delete": 1, - "email": 1, - "export": 1, - "print": 1, - "read": 1, - "report": 1, - "role": "System Manager", - "share": 1, - "write": 1 - } - ], - "sort_field": "modified", - "sort_order": "DESC", - "states": [], - "track_changes": 1 -} \ No newline at end of file diff --git a/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.py b/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.py deleted file mode 100644 index 02e36b08fe4..00000000000 --- a/erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.py +++ /dev/null @@ -1,24 +0,0 @@ -# Copyright (c) 2023, Frappe Technologies Pvt. Ltd. and contributors -# For license information, please see license.txt - -import frappe -from frappe.model.document import Document - - -class BankPartyMapper(Document): - def on_update(self): - self.update_party_in_linked_transactions() - - def update_party_in_linked_transactions(self): - if self.is_new(): - return - - # Set updated party values in other linked bank transactions - bank_transaction = frappe.qb.DocType("Bank Transaction") - - frappe.qb.update(bank_transaction).set("party_type", self.party_type).set( - "party", self.party - ).where( - (bank_transaction.bank_party_mapper == self.name) - & ((bank_transaction.party_type != self.party_type) | (bank_transaction.party != self.party)) - ).run() diff --git a/erpnext/accounts/doctype/bank_party_mapper/test_bank_party_mapper.py b/erpnext/accounts/doctype/bank_party_mapper/test_bank_party_mapper.py deleted file mode 100644 index c05b23f1a56..00000000000 --- a/erpnext/accounts/doctype/bank_party_mapper/test_bank_party_mapper.py +++ /dev/null @@ -1,9 +0,0 @@ -# Copyright (c) 2023, Frappe Technologies Pvt. Ltd. and Contributors -# See license.txt - -# import frappe -from frappe.tests.utils import FrappeTestCase - - -class TestBankPartyMapper(FrappeTestCase): - pass diff --git a/erpnext/accounts/doctype/bank_transaction/auto_match_party.py b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py index 753f0c1171a..5d94a08f2f0 100644 --- a/erpnext/accounts/doctype/bank_transaction/auto_match_party.py +++ b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py @@ -10,22 +10,7 @@ class AutoMatchParty: Matches by Account/IBAN and then by Party Name/Description sequentially. Returns when a result is obtained. - Result (if present) is of the form: (Party Type, Party, Mapper,) - - Mapper(if present) is one of the forms: - 1. {"mapper_name": }: Indicates that an existing Bank Party Mapper matched against - the transaction and the same must be linked in the Bank Transaction. - - 2. {"bank_party_account_number": , "bank_party_iban": } : Indicates that a match was - found in Customer/Supplier/Employee by account details. A Bank Party Mapper is now created - mapping the Party to the Account No./IBAN - - 3. {"bank_party_name": }: Indicates that a match was found in - Customer/Supplier/Employee by party name. A Bank Party Mapper is now created mapping the Party - to the Party Name (Bank Statement). If matched by Description, no mapper is created as - description is not a static key. - - Mapper data is used either to create a new Bank Party Mapper or link an existing mapper to a transaction. + Result (if present) is of the form: (Party Type, Party,) """ def __init__(self, **kwargs) -> None: @@ -44,7 +29,7 @@ class AutoMatchParty: fuzzy_matching_enabled = frappe.db.get_single_value("Accounts Settings", "enable_fuzzy_matching") if not result and fuzzy_matching_enabled: - result = AutoMatchbyPartyDescription( + result = AutoMatchbyPartyNameDescription( bank_party_name=self.bank_party_name, description=self.description, deposit=self.deposit ).match() @@ -62,50 +47,16 @@ class AutoMatchbyAccountIBAN: if not (self.bank_party_account_number or self.bank_party_iban): return None - result = self.match_account_in_bank_party_mapper() - if not result: - result = self.match_account_in_party() - - return result - - def match_account_in_bank_party_mapper(self) -> Union[Tuple, None]: - """Check for a IBAN/Account No. match in Bank Party Mapper""" - result = None - or_filters = {} - if self.bank_party_account_number: - or_filters["bank_party_account_number"] = self.bank_party_account_number - - if self.bank_party_iban: - or_filters["bank_party_iban"] = self.bank_party_iban - - mapper = frappe.db.get_all( - "Bank Party Mapper", - or_filters=or_filters, - fields=["party_type", "party", "name"], - limit_page_length=1, - ) - if mapper: - data = mapper[0] - return (data["party_type"], data["party"], {"mapper_name": data["name"]}) - + result = self.match_account_in_party() return result def match_account_in_party(self) -> Union[Tuple, None]: """Check if there is a IBAN/Account No. match in Customer/Supplier/Employee""" result = None - - parties = ["Supplier", "Employee", "Customer"] # most -> least likely to receive - if flt(self.deposit) > 0: - parties = ["Customer", "Supplier", "Employee"] # most -> least likely to pay + parties = get_parties_in_order(self.deposit) + or_filters = self.get_or_filters() for party in parties: - or_filters = {} - if self.bank_party_account_number: - or_filters["bank_account_no"] = self.bank_party_account_number - - if self.bank_party_iban: - or_filters["iban"] = self.bank_party_iban - party_result = frappe.db.get_all( "Bank Account", or_filters=or_filters, pluck="party", limit_page_length=1 ) @@ -123,17 +74,23 @@ class AutoMatchbyAccountIBAN: result = ( party, party_result[0], - { - "bank_party_account_number": self.get("bank_party_account_number"), - "bank_party_iban": self.get("bank_party_iban"), - }, ) break return result + def get_or_filters(self) -> dict: + or_filters = {} + if self.bank_party_account_number: + or_filters["bank_account_no"] = self.bank_party_account_number -class AutoMatchbyPartyDescription: + if self.bank_party_iban: + or_filters["iban"] = self.bank_party_iban + + return or_filters + + +class AutoMatchbyPartyNameDescription: def __init__(self, **kwargs) -> None: self.__dict__.update(kwargs) @@ -141,52 +98,21 @@ class AutoMatchbyPartyDescription: return self.__dict__.get(key, None) def match(self) -> Union[Tuple, None]: - # Match by Customer, Supplier or Employee Name - # search bank party mapper by party # fuzzy search by customer/supplier & employee if not (self.bank_party_name or self.description): return None - result = self.match_party_name_in_bank_party_mapper() - - if not result: - result = self.match_party_name_desc_in_party() - - return result - - def match_party_name_in_bank_party_mapper(self) -> Union[Tuple, None]: - """Check if match exists for party name in Bank Party Mapper""" - result = None - if not self.bank_party_name: - return - - mapper_res = frappe.get_all( - "Bank Party Mapper", - filters={"bank_party_name": self.bank_party_name}, - fields=["party_type", "party", "name"], - limit_page_length=1, - ) - if mapper_res: - mapper_res = mapper_res[0] - return ( - mapper_res["party_type"], - mapper_res["party"], - {"mapper_name": mapper_res["name"]}, - ) - + result = self.match_party_name_desc_in_party() return result def match_party_name_desc_in_party(self) -> Union[Tuple, None]: """Fuzzy search party name and/or description against parties in the system""" result = None - parties = ["Supplier", "Employee", "Customer"] # most-least likely to receive - if flt(self.deposit) > 0.0: - parties = ["Customer", "Supplier", "Employee"] # most-least likely to pay + parties = get_parties_in_order(self.deposit) for party in parties: - name_field = party.lower() + "_name" filters = {"status": "Active"} if party == "Employee" else {"disabled": 0} - names = frappe.get_all(party, filters=filters, pluck=name_field) + names = frappe.get_all(party, filters=filters, pluck=party.lower() + "_name") for field in ["bank_party_name", "description"]: if not self.get(field): @@ -197,8 +123,7 @@ class AutoMatchbyPartyDescription: break if result or skip: - # We skip if: - # If it was hard to distinguish between close matches and so match is None + # Skip If: It was hard to distinguish between close matches and so match is None # OR if the right match was found break @@ -206,19 +131,15 @@ class AutoMatchbyPartyDescription: def fuzzy_search_and_return_result(self, party, names, field) -> Union[Tuple, None]: skip = False - result = process.extract(query=self.get(field), choices=names, scorer=fuzz.token_set_ratio) party_name, skip = self.process_fuzzy_result(result) if not party_name: return None, skip - # Dont set description as a key in Bank Party Mapper due to its volatility - mapper = {"bank_party_name": self.get(field)} if field == "bank_party_name" else None return ( party, party_name, - mapper, ), skip def process_fuzzy_result(self, result: Union[list, None]): @@ -226,7 +147,7 @@ class AutoMatchbyPartyDescription: If there are multiple valid close matches return None as result may be faulty. Return the result only if one accurate match stands out. - Returns: Result, Skip (whether or not to continue matching) + Returns: Result, Skip (whether or not to discontinue matching) """ PARTY, SCORE, CUTOFF = 0, 1, 80 @@ -234,18 +155,24 @@ class AutoMatchbyPartyDescription: return None, False first_result = result[0] - if len(result) == 1: - return (result[0][PARTY] if first_result[SCORE] > CUTOFF else None), True + return (first_result[PARTY] if first_result[SCORE] > CUTOFF else None), True second_result = result[1] - if first_result[SCORE] > CUTOFF: # If multiple matches with the same score, return None but discontinue matching - # Matches were found but were too closes to distinguish between + # Matches were found but were too close to distinguish between if first_result[SCORE] == second_result[SCORE]: return None, True return first_result[PARTY], True else: return None, False + + +def get_parties_in_order(deposit: float) -> list: + parties = ["Supplier", "Employee", "Customer"] # most -> least likely to receive + if flt(deposit) > 0: + parties = ["Customer", "Supplier", "Employee"] # most -> least likely to pay + + return parties diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.json b/erpnext/accounts/doctype/bank_transaction/bank_transaction.json index e7de71aef40..b32022e6fd8 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.json +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.json @@ -37,8 +37,7 @@ "column_break_3czf", "bank_party_name", "bank_party_account_number", - "bank_party_iban", - "bank_party_mapper" + "bank_party_iban" ], "fields": [ { @@ -226,19 +225,11 @@ "fieldname": "bank_party_account_number", "fieldtype": "Data", "label": "Party Account No. (Bank Statement)" - }, - { - "fieldname": "bank_party_mapper", - "fieldtype": "Link", - "hidden": 1, - "label": "Bank Party Mapper", - "options": "Bank Party Mapper", - "read_only": 1 } ], "is_submittable": 1, "links": [], - "modified": "2023-05-17 14:56:10.547480", + "modified": "2023-06-06 13:58:12.821411", "modified_by": "Administrator", "module": "Accounts", "name": "Bank Transaction", diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py index 04c3013a002..f82337fbd77 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py @@ -29,9 +29,6 @@ class BankTransaction(StatusUpdater): self.update_allocations() self._saving_flag = False - if frappe.db.get_single_value("Accounts Settings", "enable_party_matching"): - self.update_automatch_bank_party_mapper() - def on_cancel(self): self.clear_linked_payment_entries(for_cancel=True) self.set_status(update=True) @@ -167,33 +164,10 @@ class BankTransaction(StatusUpdater): ).match() if result: - party_type, party, mapper = result - to_update = {"party_type": party_type, "party": party} - - if mapper and mapper.get("mapper_name"): - # Transaction matched with an existing Bank party Mapper record - to_update["bank_party_mapper"] = mapper.get("mapper_name") - elif mapper: - # Make new Mapper record to remember match - mapper_doc = frappe.get_doc( - {"doctype": "Bank Party Mapper", "party_type": party_type, "party": party} - ) - mapper_doc.update(mapper) - mapper_doc.insert() - to_update["bank_party_mapper"] = mapper_doc.name - - frappe.db.set_value("Bank Transaction", self.name, field=to_update) - - def update_automatch_bank_party_mapper(self): - """Update Bank Party Mapper if Party Type & Party are manually changed after submit.""" - doc_before_update = self.get_doc_before_save() - party_type_changed = self.party_type and (doc_before_update.party_type != self.party_type) - party_changed = self.party and (doc_before_update.party != self.party) - - if (party_type_changed or party_changed) and self.bank_party_mapper: - mapper_doc = frappe.get_doc("Bank Party Mapper", self.bank_party_mapper) - mapper_doc.update({"party_type": self.party_type, "party": self.party}) - mapper_doc.save() + party_type, party = result + frappe.db.set_value( + "Bank Transaction", self.name, field={"party_type": party_type, "party": party} + ) @frappe.whitelist() diff --git a/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py b/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py index 8c6dc9d7668..a498545f2a0 100644 --- a/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py +++ b/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py @@ -22,7 +22,6 @@ class TestAutoMatchParty(FrappeTestCase): frappe.db.set_single_value("Accounts Settings", "enable_fuzzy_matching", 0) def test_match_by_account_number(self): - """Test if transaction matches with existing (Bank Party Mapper) or new match.""" create_supplier_for_match(account_no="000000003716541159") doc = create_bank_transaction( withdrawal=1200, @@ -33,22 +32,6 @@ class TestAutoMatchParty(FrappeTestCase): self.assertEqual(doc.party_type, "Supplier") self.assertEqual(doc.party, "John Doe & Co.") - self.assertTrue(doc.bank_party_mapper) - - # Check if Bank Party Mapper is created to remember mapping - bank_party_mapper = frappe.get_doc("Bank Party Mapper", doc.bank_party_mapper) - self.assertEqual(bank_party_mapper.party, "John Doe & Co.") - self.assertEqual(bank_party_mapper.bank_party_account_number, "000000003716541159") - self.assertEqual(bank_party_mapper.bank_party_iban, "DE02000000003716541159") - - # Check if created mapping is used for quick match - doc_2 = create_bank_transaction( - withdrawal=500, - transaction_id="602413b8ji8bf838fub8f2c6a39bah7y", - account_no="000000003716541159", - ) - self.assertEqual(doc_2.party, "John Doe & Co.") - self.assertEqual(doc_2.bank_party_mapper, bank_party_mapper.name) def test_match_by_iban(self): create_supplier_for_match(iban="DE02000000003716541159") @@ -61,12 +44,6 @@ class TestAutoMatchParty(FrappeTestCase): self.assertEqual(doc.party_type, "Supplier") self.assertEqual(doc.party, "John Doe & Co.") - self.assertTrue(doc.bank_party_mapper) - - bank_party_mapper = frappe.get_doc("Bank Party Mapper", doc.bank_party_mapper) - self.assertEqual(bank_party_mapper.party, "John Doe & Co.") - self.assertEqual(bank_party_mapper.bank_party_account_number, "000000003716541159") - self.assertEqual(bank_party_mapper.bank_party_iban, "DE02000000003716541159") def test_match_by_party_name(self): create_supplier_for_match(supplier_name="Jackson Ella W.") @@ -78,22 +55,6 @@ class TestAutoMatchParty(FrappeTestCase): ) self.assertEqual(doc.party_type, "Supplier") self.assertEqual(doc.party, "Jackson Ella W.") - self.assertTrue(doc.bank_party_mapper) - - bank_party_mapper = frappe.get_doc("Bank Party Mapper", doc.bank_party_mapper) - self.assertEqual(bank_party_mapper.party, "Jackson Ella W.") - self.assertEqual(bank_party_mapper.bank_party_name, "Ella Jackson") - self.assertEqual(bank_party_mapper.bank_party_iban, None) - - # Check if created mapping is used for quick match - doc_2 = create_bank_transaction( - withdrawal=500, - transaction_id="578313b8ji8bf838fub8f2c6a39bah7y", - party_name="Ella Jackson", - account_no="000000004316531152", - ) - self.assertEqual(doc_2.party, "Jackson Ella W.") - self.assertEqual(doc_2.bank_party_mapper, bank_party_mapper.name) def test_match_by_description(self): create_supplier_for_match(supplier_name="Microsoft") @@ -107,46 +68,6 @@ class TestAutoMatchParty(FrappeTestCase): self.assertEqual(doc.party, "Microsoft") self.assertFalse(doc.bank_party_mapper) - def test_correct_match_after_submit(self): - """Correct wrong mapping after submit. Test impact.""" - # Similar named suppliers - create_supplier_for_match(supplier_name="Amazon") - create_supplier_for_match(supplier_name="Amazing Co.") - - # Bank Transactions actually from "Amazon" that match better with "Amazing Co." - doc = create_bank_transaction( - description="visa06323202 amzn.com/bill 7,88eur1,5324711959 90.22. 1,62 87861003", - withdrawal=24.85, - transaction_id="3a1da4ee2dc5a980138d36ef5297cbd9", - party_name="Amazn Co.", - ) - doc_2 = create_bank_transaction( - description="visa61268005 amzn.com/bill 22,345eur1,7654711959 89.23. 1,64 61268005", - withdrawal=80, - transaction_id="584314e459b00f792bfd569267efba6e", - party_name="Amazn Co.", - ) - - self.assertEqual(doc.party_type, "Supplier") - self.assertEqual(doc.party, "Amazing Co.") - self.assertTrue(doc.bank_party_mapper) - self.assertTrue(doc_2.bank_party_mapper, doc.bank_party_mapper) - - bank_party_mapper = frappe.get_doc("Bank Party Mapper", doc.bank_party_mapper) - self.assertEqual(bank_party_mapper.party, "Amazing Co.") - self.assertEqual(bank_party_mapper.bank_party_name, "Amazn Co.") - - # User corrects the value after submit to "Amazon" - doc.party = "Amazon" - doc.save() - bank_party_mapper.reload() - doc_2.reload() - - # Mapping is edited and all transactions with this mapping are updated - self.assertEqual(bank_party_mapper.party, "Amazon") - self.assertEqual(bank_party_mapper.bank_party_name, "Amazn Co.") - self.assertEqual(doc_2.party, "Amazon") - def test_skip_match_if_multiple_close_results(self): create_supplier_for_match(supplier_name="Adithya Medical & General Stores") create_supplier_for_match(supplier_name="Adithya Medical And General Stores") From eb1db5eaa3e3d28351b4f906b7e91b03da7ae83c Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 7 Jun 2023 11:54:16 +0530 Subject: [PATCH 16/45] chore: Remove instances of `bank_party_mapper` and use `new_doc` --- .../bank_transaction/test_auto_match_party.py | 49 ++++++++----------- 1 file changed, 20 insertions(+), 29 deletions(-) diff --git a/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py b/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py index a498545f2a0..36ef1fca074 100644 --- a/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py +++ b/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py @@ -66,7 +66,6 @@ class TestAutoMatchParty(FrappeTestCase): ) self.assertEqual(doc.party_type, "Supplier") self.assertEqual(doc.party, "Microsoft") - self.assertFalse(doc.bank_party_mapper) def test_skip_match_if_multiple_close_results(self): create_supplier_for_match(supplier_name="Adithya Medical & General Stores") @@ -82,7 +81,6 @@ class TestAutoMatchParty(FrappeTestCase): # Mapping is skipped as both Supplier names have the same match score self.assertEqual(doc.party_type, None) self.assertEqual(doc.party, None) - self.assertFalse(doc.bank_party_mapper) def create_supplier_for_match(supplier_name="John Doe & Co.", iban=None, account_no=None): @@ -99,35 +97,26 @@ def create_supplier_for_match(supplier_name="John Doe & Co.", iban=None, account return # Create Supplier and Bank Account for the same - supplier = frappe.get_doc( - { - "doctype": "Supplier", - "supplier_name": supplier_name, - "supplier_group": "Services", - "supplier_type": "Company", - } - ).insert() + supplier = frappe.new_doc("Supplier") + supplier.supplier_name = supplier_name + supplier.supplier_group = "Services" + supplier.supplier_type = "Company" + supplier.insert() if not frappe.db.exists("Bank", "TestBank"): - frappe.get_doc( - { - "doctype": "Bank", - "bank_name": "TestBank", - } - ).insert(ignore_if_duplicate=True) + bank = frappe.new_doc("Bank") + bank.bank_name = "TestBank" + bank.insert(ignore_if_duplicate=True) if not frappe.db.exists("Bank Account", supplier.name + " - " + "TestBank"): - frappe.get_doc( - { - "doctype": "Bank Account", - "account_name": supplier.name, - "bank": "TestBank", - "iban": iban, - "bank_account_no": account_no, - "party_type": "Supplier", - "party": supplier.name, - } - ).insert() + bank_account = frappe.new_doc("Bank Account") + bank_account.account_name = supplier.name + bank_account.bank = "TestBank" + bank_account.iban = iban + bank_account.bank_account_no = account_no + bank_account.party_type = "Supplier" + bank_account.party = supplier.name + bank_account.insert() def create_bank_transaction( @@ -139,7 +128,8 @@ def create_bank_transaction( account_no=None, iban=None, ): - doc = frappe.get_doc( + doc = frappe.new_doc("Bank Transaction") + doc.update( { "doctype": "Bank Transaction", "description": description or "1512567 BG/000002918 OPSKATTUZWXXX AT776000000098709837 Herr G", @@ -153,7 +143,8 @@ def create_bank_transaction( "bank_party_account_number": account_no, "bank_party_iban": iban, } - ).insert() + ) + doc.insert() doc.submit() doc.reload() From 5155d5bfb2d8909d76f6515fdae1cb0230d27a67 Mon Sep 17 00:00:00 2001 From: DaizyModi Date: Wed, 7 Jun 2023 12:05:17 +0530 Subject: [PATCH 17/45] chore: remove whitelisting for methods not accessed from UI --- erpnext/accounts/doctype/bank_account/bank_account.py | 1 - erpnext/assets/doctype/asset_maintenance/asset_maintenance.py | 1 - .../supplier_scorecard_period/supplier_scorecard_period.py | 1 - .../doctype/plaid_settings/plaid_settings.py | 1 - erpnext/stock/get_item_details.py | 1 - 5 files changed, 5 deletions(-) diff --git a/erpnext/accounts/doctype/bank_account/bank_account.py b/erpnext/accounts/doctype/bank_account/bank_account.py index b91f0f91371..363a2776aa7 100644 --- a/erpnext/accounts/doctype/bank_account/bank_account.py +++ b/erpnext/accounts/doctype/bank_account/bank_account.py @@ -70,7 +70,6 @@ def make_bank_account(doctype, docname): return doc -@frappe.whitelist() def get_party_bank_account(party_type, party): return frappe.db.get_value(party_type, party, "default_bank_account") diff --git a/erpnext/assets/doctype/asset_maintenance/asset_maintenance.py b/erpnext/assets/doctype/asset_maintenance/asset_maintenance.py index 83031415ec3..641d35fa04c 100644 --- a/erpnext/assets/doctype/asset_maintenance/asset_maintenance.py +++ b/erpnext/assets/doctype/asset_maintenance/asset_maintenance.py @@ -42,7 +42,6 @@ class AssetMaintenance(Document): maintenance_log.db_set("maintenance_status", "Cancelled") -@frappe.whitelist() def assign_tasks(asset_maintenance_name, assign_to_member, maintenance_task, next_due_date): team_member = frappe.db.get_value("User", assign_to_member, "email") args = { diff --git a/erpnext/buying/doctype/supplier_scorecard_period/supplier_scorecard_period.py b/erpnext/buying/doctype/supplier_scorecard_period/supplier_scorecard_period.py index a8b76db0931..1967df2a26b 100644 --- a/erpnext/buying/doctype/supplier_scorecard_period/supplier_scorecard_period.py +++ b/erpnext/buying/doctype/supplier_scorecard_period/supplier_scorecard_period.py @@ -99,7 +99,6 @@ def import_string_path(path): return mod -@frappe.whitelist() def make_supplier_scorecard(source_name, target_doc=None): def update_criteria_fields(obj, target, source_parent): target.max_score, target.formula = frappe.db.get_value( diff --git a/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py b/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py index e57a30a88e1..61d2acefae4 100644 --- a/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py +++ b/erpnext/erpnext_integrations/doctype/plaid_settings/plaid_settings.py @@ -161,7 +161,6 @@ def add_account_subtype(account_subtype): frappe.throw(frappe.get_traceback()) -@frappe.whitelist() def sync_transactions(bank, bank_account): """Sync transactions based on the last integration date as the start date, after sync is completed add the transaction date of the oldest transaction as the last integration date.""" diff --git a/erpnext/stock/get_item_details.py b/erpnext/stock/get_item_details.py index 64650bc2018..4f85ac054d0 100644 --- a/erpnext/stock/get_item_details.py +++ b/erpnext/stock/get_item_details.py @@ -191,7 +191,6 @@ def process_string_args(args): return args -@frappe.whitelist() def get_item_code(barcode=None, serial_no=None): if barcode: item_code = frappe.db.get_value("Item Barcode", {"barcode": barcode}, fieldname=["parent"]) From 0bd4de450431b2992b08389570a1526f187f0672 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Wed, 7 Jun 2023 22:33:35 +0530 Subject: [PATCH 18/45] fix: Remove special treatment for P&L Accounts --- erpnext/accounts/utils.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index 0ee06e8239c..a5cb3247627 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -237,11 +237,6 @@ def get_balance_on( if not (frappe.flags.ignore_account_permission or ignore_account_permission): acc.check_permission("read") - if report_type == "Profit and Loss": - # for pl accounts, get balance within a fiscal year - cond.append( - "posting_date >= '%s' and voucher_type != 'Period Closing Voucher'" % year_start_date - ) # different filter for group and ledger - improved performance if acc.is_group: cond.append( From e30c3eafefca33afd91e2e473750a1582a3b88ad Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Mon, 12 Jun 2023 11:46:51 +0530 Subject: [PATCH 19/45] fix: Stock ledger preview --- .../doctype/sales_invoice/sales_invoice.json | 32 ++++++++- erpnext/controllers/buying_controller.py | 1 + erpnext/controllers/stock_controller.py | 69 +++++++++++++++++-- .../public/js/controllers/stock_controller.js | 15 ++-- 4 files changed, 104 insertions(+), 13 deletions(-) diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.json b/erpnext/accounts/doctype/sales_invoice/sales_invoice.json index 6a65b30ceb0..15be2e71baa 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.json +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.json @@ -211,7 +211,12 @@ "is_discounted", "remarks", "repost_required", - "connections_tab" + "connections_tab", + "ledger_preview", + "accounting_ledger_section", + "accounting_ledger_preview_html", + "stock_ledger_section", + "stock_ledger_preview_html" ], "fields": [ { @@ -2142,6 +2147,29 @@ "fieldname": "use_company_roundoff_cost_center", "fieldtype": "Check", "label": "Use Company default Cost Center for Round off" + }, + { + "fieldname": "ledger_preview", + "fieldtype": "Tab Break", + "label": "Ledger Preview" + }, + { + "fieldname": "accounting_ledger_section", + "fieldtype": "Section Break", + "label": "Accounting Ledger" + }, + { + "fieldname": "accounting_ledger_preview_html", + "fieldtype": "HTML" + }, + { + "fieldname": "stock_ledger_section", + "fieldtype": "Section Break", + "label": "Stock Ledger" + }, + { + "fieldname": "stock_ledger_preview_html", + "fieldtype": "HTML" } ], "icon": "fa fa-file-text", @@ -2154,7 +2182,7 @@ "link_fieldname": "consolidated_invoice" } ], - "modified": "2023-04-28 14:15:59.901154", + "modified": "2023-06-11 11:18:14.024258", "modified_by": "Administrator", "module": "Accounts", "name": "Sales Invoice", diff --git a/erpnext/controllers/buying_controller.py b/erpnext/controllers/buying_controller.py index e15b61287eb..efddae7e70b 100644 --- a/erpnext/controllers/buying_controller.py +++ b/erpnext/controllers/buying_controller.py @@ -511,6 +511,7 @@ class BuyingController(SubcontractingController): if self.get("is_old_subcontracting_flow"): self.make_sl_entries_for_supplier_warehouse(sl_entries) + self.make_sl_entries( sl_entries, allow_negative_stock=allow_negative_stock, diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index 30dcc7ffa7f..a81c03615ee 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -15,7 +15,6 @@ from erpnext.accounts.general_ledger import ( make_reverse_gl_entries, process_gl_map, ) -from erpnext.accounts.report.general_ledger.general_ledger import get_columns from erpnext.accounts.utils import get_fiscal_year from erpnext.controllers.accounts_controller import AccountsController from erpnext.stock import get_warehouse_account_map @@ -827,16 +826,76 @@ class StockController(AccountsController): @frappe.whitelist() def show_ledger_preview(company, doctype, docname): + from erpnext.accounts.report.general_ledger.general_ledger import get_columns as get_gl_columns + from erpnext.stock.report.stock_ledger.stock_ledger import get_columns as get_sl_columns + + frappe.db.savepoint("show_ledger_preview") + filters = {"company": company} doc = frappe.get_doc(doctype, docname) - columns = get_columns(filters) - data = doc.get_gl_entries() + + datatable_sl_columns = [] + datatable_sl_data = [] + + if doc.update_stock or doc.doctype in ("Purchase Receipt", "Delivery Note"): + sl_columns = get_sl_columns(filters) + doc.docstatus = 1 + doc.update_stock_ledger() + sl_entries = get_sl_entries_for_preview(doc.doctype, doc.name) + datatable_sl_columns = get_columns(sl_columns) + datatable_sl_data = get_data(sl_columns, sl_entries) + + doc.docstatus = 1 + gl_columns = get_gl_columns(filters) + doc.make_gl_entries() + gl_data = get_gl_entries_for_preview(doc.doctype, doc.name) + + datatable_gl_columns = get_columns(gl_columns) + datatable_gl_data = get_data(gl_columns, gl_data) + + frappe.db.rollback(save_point="show_ledger_preview") + return { - "columns": columns, - "data": data, + "gl_columns": datatable_gl_columns, + "gl_data": datatable_gl_data, + "sl_columns": datatable_sl_columns, + "sl_data": datatable_sl_data, } +def get_sl_entries_for_preview(doctype, docname): + return frappe.get_all( + "Stock Ledger Entry", filters={"voucher_type": doctype, "voucher_no": docname}, fields=["*"] + ) + + +def get_gl_entries_for_preview(doctype, docname): + return frappe.get_all( + "GL Entry", filters={"voucher_type": doctype, "voucher_no": docname}, fields=["*"] + ) + + +def get_columns(raw_columns): + return [ + {"name": d.get("label"), "editable": False, "width": 100} + for d in raw_columns + if not d.get("hidden") + ] + + +def get_data(raw_columns, raw_data): + datatable_data = [] + for row in raw_data: + data_row = [] + for column in raw_columns: + if not column.get("hidden"): + data_row.append(row.get(column.get("fieldname"))) + + datatable_data.append(data_row) + + return datatable_data + + def repost_required_for_queue(doc: StockController) -> bool: """check if stock document contains repeated item-warehouse with queue based valuation. diff --git a/erpnext/public/js/controllers/stock_controller.js b/erpnext/public/js/controllers/stock_controller.js index 919ffda52fc..0ef2e6eb697 100644 --- a/erpnext/public/js/controllers/stock_controller.js +++ b/erpnext/public/js/controllers/stock_controller.js @@ -94,24 +94,27 @@ erpnext.stock.StockController = class StockController extends frappe.ui.form.Con "docname": me.frm.doc.name }, "callback": function(response) { - me.get_datatable(response); + console.log(response.message); + me.get_datatable(response.message.gl_columns, response.message.gl_data, me.frm.get_field("accounting_ledger_preview_html").wrapper); + me.get_datatable(response.message.sl_columns, response.message.sl_data, me.frm.get_field("stock_ledger_preview_html").wrapper); + me.frm.scroll_to_field("accounting_ledger_preview_html"); } }) }, __("View")); } } - get_datatable(response) { + get_datatable(columns, data, wrapper) { const datatable_options = { - columns: response.columns, - data: response.data, + columns: columns, + data: data, dynamicRowHeight: true, checkboxColumn: false, inlineFilters: true, }; - this.datatable = new frappe.DataTable( - this.frm.page.main.parent, + new frappe.DataTable( + wrapper, datatable_options ); } From 011ac131cfdbb7ae75dcebc92412c43a3de8cd92 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Mon, 12 Jun 2023 18:42:49 +0530 Subject: [PATCH 20/45] fix: Add column values --- erpnext/controllers/stock_controller.py | 133 +++++++++++++----- .../public/js/controllers/stock_controller.js | 1 - 2 files changed, 95 insertions(+), 39 deletions(-) diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index a81c03615ee..3277721126d 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -826,60 +826,118 @@ class StockController(AccountsController): @frappe.whitelist() def show_ledger_preview(company, doctype, docname): - from erpnext.accounts.report.general_ledger.general_ledger import get_columns as get_gl_columns - from erpnext.stock.report.stock_ledger.stock_ledger import get_columns as get_sl_columns - frappe.db.savepoint("show_ledger_preview") filters = {"company": company} doc = frappe.get_doc(doctype, docname) - datatable_sl_columns = [] - datatable_sl_data = [] - - if doc.update_stock or doc.doctype in ("Purchase Receipt", "Delivery Note"): - sl_columns = get_sl_columns(filters) - doc.docstatus = 1 - doc.update_stock_ledger() - sl_entries = get_sl_entries_for_preview(doc.doctype, doc.name) - datatable_sl_columns = get_columns(sl_columns) - datatable_sl_data = get_data(sl_columns, sl_entries) - - doc.docstatus = 1 - gl_columns = get_gl_columns(filters) - doc.make_gl_entries() - gl_data = get_gl_entries_for_preview(doc.doctype, doc.name) - - datatable_gl_columns = get_columns(gl_columns) - datatable_gl_data = get_data(gl_columns, gl_data) + sl_columns, sl_data = get_stock_ledger_preview(doc, filters) + gl_columns, gl_data = get_accounting_ledger_preview(doc, filters) frappe.db.rollback(save_point="show_ledger_preview") return { - "gl_columns": datatable_gl_columns, - "gl_data": datatable_gl_data, - "sl_columns": datatable_sl_columns, - "sl_data": datatable_sl_data, + "gl_columns": gl_columns, + "gl_data": gl_data, + "sl_columns": sl_columns, + "sl_data": sl_data, } -def get_sl_entries_for_preview(doctype, docname): +def get_accounting_ledger_preview(doc, filters): + from erpnext.accounts.report.general_ledger.general_ledger import get_columns as get_gl_columns + + gl_columns, gl_data = [], [] + fields = [ + "posting_date", + "account", + "debit", + "credit", + "against", + "party", + "party_type", + "against_voucher_type", + "against_voucher", + ] + + doc.docstatus = 1 + doc.make_gl_entries() + columns = get_gl_columns(filters) + gl_entries = get_gl_entries_for_preview(doc.doctype, doc.name, fields) + + gl_columns = get_columns(columns, fields) + gl_data = get_data(fields, gl_entries) + + return gl_columns, gl_data + + +def get_stock_ledger_preview(doc, filters): + from erpnext.stock.report.stock_ledger.stock_ledger import get_columns as get_sl_columns + + sl_columns, sl_data = [], [] + fields = [ + "item_code", + "stock_uom", + "actual_qty", + "qty_after_transaction", + "warehouse", + "incoming_rate", + "valuation_rate", + "stock_value", + "stock_value_difference", + ] + columns_fields = [ + "item_code", + "stock_uom", + "in_qty", + "out_qty", + "qty_after_transaction", + "warehouse", + "incoming_rate", + "valuation_rate", + "stock_value", + "stock_value_difference", + ] + + if doc.update_stock or doc.doctype in ("Purchase Receipt", "Delivery Note"): + doc.docstatus = 1 + doc.update_stock_ledger() + columns = get_sl_columns(filters) + sl_entries = get_sl_entries_for_preview(doc.doctype, doc.name, fields) + + sl_columns = get_columns(columns, columns_fields) + sl_data = get_data(columns_fields, sl_entries) + + return sl_columns, sl_data + + +def get_sl_entries_for_preview(doctype, docname, fields): + sl_entries = frappe.get_all( + "Stock Ledger Entry", filters={"voucher_type": doctype, "voucher_no": docname}, fields=fields + ) + + for entry in sl_entries: + if entry.actual_qty > 0: + entry["in_qty"] = entry.actual_qty + entry["out_qty"] = 0 + else: + entry["out_qty"] = abs(entry.actual_qty) + entry["in_qty"] = 0 + + return sl_entries + + +def get_gl_entries_for_preview(doctype, docname, fields): return frappe.get_all( - "Stock Ledger Entry", filters={"voucher_type": doctype, "voucher_no": docname}, fields=["*"] + "GL Entry", filters={"voucher_type": doctype, "voucher_no": docname}, fields=fields ) -def get_gl_entries_for_preview(doctype, docname): - return frappe.get_all( - "GL Entry", filters={"voucher_type": doctype, "voucher_no": docname}, fields=["*"] - ) - - -def get_columns(raw_columns): +def get_columns(raw_columns, fields): return [ - {"name": d.get("label"), "editable": False, "width": 100} + {"name": d.get("label"), "editable": False, "width": 110} for d in raw_columns - if not d.get("hidden") + if not d.get("hidden") and d.get("fieldname") in fields ] @@ -888,8 +946,7 @@ def get_data(raw_columns, raw_data): for row in raw_data: data_row = [] for column in raw_columns: - if not column.get("hidden"): - data_row.append(row.get(column.get("fieldname"))) + data_row.append(row.get(column) or "") datatable_data.append(data_row) diff --git a/erpnext/public/js/controllers/stock_controller.js b/erpnext/public/js/controllers/stock_controller.js index 0ef2e6eb697..0a14ed79359 100644 --- a/erpnext/public/js/controllers/stock_controller.js +++ b/erpnext/public/js/controllers/stock_controller.js @@ -94,7 +94,6 @@ erpnext.stock.StockController = class StockController extends frappe.ui.form.Con "docname": me.frm.doc.name }, "callback": function(response) { - console.log(response.message); me.get_datatable(response.message.gl_columns, response.message.gl_data, me.frm.get_field("accounting_ledger_preview_html").wrapper); me.get_datatable(response.message.sl_columns, response.message.sl_data, me.frm.get_field("stock_ledger_preview_html").wrapper); me.frm.scroll_to_field("accounting_ledger_preview_html"); From 4fbff2095447f12e6941d39b156b50fa4f1c7b80 Mon Sep 17 00:00:00 2001 From: Smit Vora Date: Mon, 19 Jun 2023 21:14:42 +0530 Subject: [PATCH 21/45] fix: make credit note and debit note exclusive (#35781) --- erpnext/accounts/doctype/sales_invoice/sales_invoice.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.json b/erpnext/accounts/doctype/sales_invoice/sales_invoice.json index 7b68dd41d93..ab4aab3da29 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.json +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.json @@ -320,6 +320,7 @@ }, { "default": "0", + "depends_on": "eval: !doc.is_debit_note", "fieldname": "is_return", "fieldtype": "Check", "hide_days": 1, @@ -1960,6 +1961,7 @@ }, { "default": "0", + "depends_on": "eval: !doc.is_return", "description": "Issue a debit note with 0 qty against an existing Sales Invoice", "fieldname": "is_debit_note", "fieldtype": "Check", @@ -2155,7 +2157,7 @@ "link_fieldname": "consolidated_invoice" } ], - "modified": "2023-06-03 16:22:16.219333", + "modified": "2023-06-19 16:02:05.309332", "modified_by": "Administrator", "module": "Accounts", "name": "Sales Invoice", From 0d125885836f0ae5015195401dd41653314fddfe Mon Sep 17 00:00:00 2001 From: Anand Baburajan Date: Tue, 20 Jun 2023 12:06:27 +0530 Subject: [PATCH 22/45] fix: date and finance book fixes in fixed asset register (#35751) * fix: handle finance books properly and show all assets by default in fixed asset register * chore: rename value to depr amount * chore: get asset value for correct fb properly * chore: rename include_default_book_entries to include_default_book_assets --- .../fixed_asset_register.js | 119 +++++++------- .../fixed_asset_register.py | 146 +++++++++++------- 2 files changed, 147 insertions(+), 118 deletions(-) diff --git a/erpnext/assets/report/fixed_asset_register/fixed_asset_register.js b/erpnext/assets/report/fixed_asset_register/fixed_asset_register.js index 4f7b8361077..b788a32d6ab 100644 --- a/erpnext/assets/report/fixed_asset_register/fixed_asset_register.js +++ b/erpnext/assets/report/fixed_asset_register/fixed_asset_register.js @@ -19,56 +19,6 @@ frappe.query_reports["Fixed Asset Register"] = { options: "\nIn Location\nDisposed", default: 'In Location' }, - { - "fieldname":"filter_based_on", - "label": __("Period Based On"), - "fieldtype": "Select", - "options": ["Fiscal Year", "Date Range"], - "default": "Fiscal Year", - "reqd": 1 - }, - { - "fieldname":"from_date", - "label": __("Start Date"), - "fieldtype": "Date", - "default": frappe.datetime.add_months(frappe.datetime.nowdate(), -12), - "depends_on": "eval: doc.filter_based_on == 'Date Range'", - "reqd": 1 - }, - { - "fieldname":"to_date", - "label": __("End Date"), - "fieldtype": "Date", - "default": frappe.datetime.nowdate(), - "depends_on": "eval: doc.filter_based_on == 'Date Range'", - "reqd": 1 - }, - { - "fieldname":"from_fiscal_year", - "label": __("Start Year"), - "fieldtype": "Link", - "options": "Fiscal Year", - "default": frappe.defaults.get_user_default("fiscal_year"), - "depends_on": "eval: doc.filter_based_on == 'Fiscal Year'", - "reqd": 1 - }, - { - "fieldname":"to_fiscal_year", - "label": __("End Year"), - "fieldtype": "Link", - "options": "Fiscal Year", - "default": frappe.defaults.get_user_default("fiscal_year"), - "depends_on": "eval: doc.filter_based_on == 'Fiscal Year'", - "reqd": 1 - }, - { - "fieldname":"date_based_on", - "label": __("Date Based On"), - "fieldtype": "Select", - "options": ["Purchase Date", "Available For Use Date"], - "default": "Purchase Date", - "reqd": 1 - }, { fieldname:"asset_category", label: __("Asset Category"), @@ -89,22 +39,67 @@ frappe.query_reports["Fixed Asset Register"] = { default: "--Select a group--", reqd: 1 }, - { - fieldname:"finance_book", - label: __("Finance Book"), - fieldtype: "Link", - options: "Finance Book", - depends_on: "eval: doc.filter_by_finance_book == 1", - }, - { - fieldname:"filter_by_finance_book", - label: __("Filter by Finance Book"), - fieldtype: "Check" - }, { fieldname:"only_existing_assets", label: __("Only existing assets"), fieldtype: "Check" }, + { + fieldname:"finance_book", + label: __("Finance Book"), + fieldtype: "Link", + options: "Finance Book", + }, + { + "fieldname": "include_default_book_assets", + "label": __("Include Default Book Assets"), + "fieldtype": "Check", + "default": 1 + }, + { + "fieldname":"filter_based_on", + "label": __("Period Based On"), + "fieldtype": "Select", + "options": ["--Select a period--", "Fiscal Year", "Date Range"], + "default": "--Select a period--", + }, + { + "fieldname":"from_date", + "label": __("Start Date"), + "fieldtype": "Date", + "default": frappe.datetime.add_months(frappe.datetime.nowdate(), -12), + "depends_on": "eval: doc.filter_based_on == 'Date Range'", + }, + { + "fieldname":"to_date", + "label": __("End Date"), + "fieldtype": "Date", + "default": frappe.datetime.nowdate(), + "depends_on": "eval: doc.filter_based_on == 'Date Range'", + }, + { + "fieldname":"from_fiscal_year", + "label": __("Start Year"), + "fieldtype": "Link", + "options": "Fiscal Year", + "default": frappe.defaults.get_user_default("fiscal_year"), + "depends_on": "eval: doc.filter_based_on == 'Fiscal Year'", + }, + { + "fieldname":"to_fiscal_year", + "label": __("End Year"), + "fieldtype": "Link", + "options": "Fiscal Year", + "default": frappe.defaults.get_user_default("fiscal_year"), + "depends_on": "eval: doc.filter_based_on == 'Fiscal Year'", + }, + { + "fieldname":"date_based_on", + "label": __("Date Based On"), + "fieldtype": "Select", + "options": ["Purchase Date", "Available For Use Date"], + "default": "Purchase Date", + "depends_on": "eval: doc.filter_based_on == 'Date Range' || doc.filter_based_on == 'Fiscal Year'", + }, ] }; diff --git a/erpnext/assets/report/fixed_asset_register/fixed_asset_register.py b/erpnext/assets/report/fixed_asset_register/fixed_asset_register.py index 984b3fd9820..cb61914c255 100644 --- a/erpnext/assets/report/fixed_asset_register/fixed_asset_register.py +++ b/erpnext/assets/report/fixed_asset_register/fixed_asset_register.py @@ -2,9 +2,11 @@ # For license information, please see license.txt +from itertools import chain + import frappe from frappe import _ -from frappe.query_builder.functions import Sum +from frappe.query_builder.functions import IfNull, Sum from frappe.utils import cstr, flt, formatdate, getdate from erpnext.accounts.report.financial_statements import ( @@ -13,7 +15,6 @@ from erpnext.accounts.report.financial_statements import ( validate_fiscal_year, ) from erpnext.assets.doctype.asset.asset import get_asset_value_after_depreciation -from erpnext.assets.doctype.asset.depreciation import get_depreciation_accounts def execute(filters=None): @@ -64,11 +65,9 @@ def get_conditions(filters): def get_data(filters): - data = [] conditions = get_conditions(filters) - depreciation_amount_map = get_finance_book_value_map(filters) pr_supplier_map = get_purchase_receipt_supplier_map() pi_supplier_map = get_purchase_invoice_supplier_map() @@ -102,20 +101,27 @@ def get_data(filters): ] assets_record = frappe.db.get_all("Asset", filters=conditions, fields=fields) - assets_linked_to_fb = None + assets_linked_to_fb = get_assets_linked_to_fb(filters) - if filters.filter_by_finance_book: - assets_linked_to_fb = frappe.db.get_all( - doctype="Asset Finance Book", - filters={"finance_book": filters.finance_book or ("is", "not set")}, - pluck="parent", - ) + company_fb = frappe.get_cached_value("Company", filters.company, "default_finance_book") + + if filters.include_default_book_assets and company_fb: + finance_book = company_fb + elif filters.finance_book: + finance_book = filters.finance_book + else: + finance_book = None + + depreciation_amount_map = get_asset_depreciation_amount_map(filters, finance_book) for asset in assets_record: if assets_linked_to_fb and asset.asset_id not in assets_linked_to_fb: continue - asset_value = get_asset_value_after_depreciation(asset.asset_id, filters.finance_book) + asset_value = get_asset_value_after_depreciation( + asset.asset_id, finance_book + ) or get_asset_value_after_depreciation(asset.asset_id) + row = { "asset_id": asset.asset_id, "asset_name": asset.asset_name, @@ -126,7 +132,7 @@ def get_data(filters): or pi_supplier_map.get(asset.purchase_invoice), "gross_purchase_amount": asset.gross_purchase_amount, "opening_accumulated_depreciation": asset.opening_accumulated_depreciation, - "depreciated_amount": get_depreciation_amount_of_asset(asset, depreciation_amount_map, filters), + "depreciated_amount": get_depreciation_amount_of_asset(asset, depreciation_amount_map), "available_for_use_date": asset.available_for_use_date, "location": asset.location, "asset_category": asset.asset_category, @@ -140,14 +146,23 @@ def get_data(filters): def prepare_chart_data(data, filters): labels_values_map = {} - date_field = frappe.scrub(filters.date_based_on) + if filters.filter_based_on not in ("Date Range", "Fiscal Year"): + filters_filter_based_on = "Date Range" + date_field = "purchase_date" + filters_from_date = min(data, key=lambda a: a.get(date_field)).get(date_field) + filters_to_date = max(data, key=lambda a: a.get(date_field)).get(date_field) + else: + filters_filter_based_on = filters.filter_based_on + date_field = frappe.scrub(filters.date_based_on) + filters_from_date = filters.from_date + filters_to_date = filters.to_date period_list = get_period_list( filters.from_fiscal_year, filters.to_fiscal_year, - filters.from_date, - filters.to_date, - filters.filter_based_on, + filters_from_date, + filters_to_date, + filters_filter_based_on, "Monthly", company=filters.company, ignore_fiscal_year=True, @@ -184,59 +199,78 @@ def prepare_chart_data(data, filters): } -def get_depreciation_amount_of_asset(asset, depreciation_amount_map, filters): - if asset.calculate_depreciation: - depr_amount = depreciation_amount_map.get(asset.asset_id) or 0.0 - else: - depr_amount = get_manual_depreciation_amount_of_asset(asset, filters) +def get_assets_linked_to_fb(filters): + afb = frappe.qb.DocType("Asset Finance Book") - return flt(depr_amount, 2) - - -def get_finance_book_value_map(filters): - date = filters.to_date if filters.filter_based_on == "Date Range" else filters.year_end_date - - return frappe._dict( - frappe.db.sql( - """ Select - ads.asset, SUM(depreciation_amount) - FROM `tabAsset Depreciation Schedule` ads, `tabDepreciation Schedule` ds - WHERE - ds.parent = ads.name - AND ifnull(ads.finance_book, '')=%s - AND ads.docstatus=1 - AND ds.parentfield='depreciation_schedule' - AND ds.schedule_date<=%s - AND ds.journal_entry IS NOT NULL - GROUP BY ads.asset""", - (cstr(filters.finance_book or ""), date), - ) + query = frappe.qb.from_(afb).select( + afb.parent, ) + if filters.include_default_book_assets: + company_fb = frappe.get_cached_value("Company", filters.company, "default_finance_book") -def get_manual_depreciation_amount_of_asset(asset, filters): + if filters.finance_book and company_fb and cstr(filters.finance_book) != cstr(company_fb): + frappe.throw( + _("To use a different finance book, please uncheck 'Include Default Book Entries'") + ) + + query = query.where( + (afb.finance_book.isin([cstr(filters.finance_book), cstr(company_fb), ""])) + | (afb.finance_book.isnull()) + ) + else: + query = query.where( + (afb.finance_book.isin([cstr(filters.finance_book), ""])) | (afb.finance_book.isnull()) + ) + + assets_linked_to_fb = list(chain(*query.run(as_list=1))) + + return assets_linked_to_fb + + +def get_depreciation_amount_of_asset(asset, depreciation_amount_map): + return depreciation_amount_map.get(asset.asset_id) or 0.0 + + +def get_asset_depreciation_amount_map(filters, finance_book): date = filters.to_date if filters.filter_based_on == "Date Range" else filters.year_end_date - (_, _, depreciation_expense_account) = get_depreciation_accounts(asset) - + asset = frappe.qb.DocType("Asset") gle = frappe.qb.DocType("GL Entry") + aca = frappe.qb.DocType("Asset Category Account") + company = frappe.qb.DocType("Company") - result = ( + query = ( frappe.qb.from_(gle) - .select(Sum(gle.debit)) - .where(gle.against_voucher == asset.asset_id) - .where(gle.account == depreciation_expense_account) + .join(asset) + .on(gle.against_voucher == asset.name) + .join(aca) + .on((aca.parent == asset.asset_category) & (aca.company_name == asset.company)) + .join(company) + .on(company.name == asset.company) + .select(asset.name.as_("asset"), Sum(gle.debit).as_("depreciation_amount")) + .where( + gle.account == IfNull(aca.depreciation_expense_account, company.depreciation_expense_account) + ) .where(gle.debit != 0) .where(gle.is_cancelled == 0) - .where(gle.posting_date <= date) - ).run() + .where(asset.docstatus == 1) + .groupby(asset.name) + ) - if result and result[0] and result[0][0]: - depr_amount = result[0][0] + if finance_book: + query = query.where( + (gle.finance_book.isin([cstr(finance_book), ""])) | (gle.finance_book.isnull()) + ) else: - depr_amount = 0 + query = query.where((gle.finance_book.isin([""])) | (gle.finance_book.isnull())) - return depr_amount + if filters.filter_based_on in ("Date Range", "Fiscal Year"): + query = query.where(gle.posting_date <= date) + + asset_depr_amount_map = query.run() + + return dict(asset_depr_amount_map) def get_purchase_receipt_supplier_map(): From 1b33afd69987f04c1c92e236c8eeb86bf9e3e76d Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 20 Jun 2023 07:19:59 +0530 Subject: [PATCH 23/45] fix: for zero bal accounts, dr/cr only on currency that has balance --- .../exchange_rate_revaluation.py | 48 +++++++++++++------ 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.py b/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.py index 5d239c91f71..4926006c965 100644 --- a/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.py +++ b/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.py @@ -373,6 +373,24 @@ class ExchangeRateRevaluation(Document): "credit": 0, } ) + + journal_entry_accounts.append(journal_account) + + journal_entry_accounts.append( + { + "account": unrealized_exchange_gain_loss_account, + "balance": get_balance_on(unrealized_exchange_gain_loss_account), + "debit": 0, + "credit": 0, + "debit_in_account_currency": abs(d.gain_loss) if d.gain_loss < 0 else 0, + "credit_in_account_currency": abs(d.gain_loss) if d.gain_loss > 0 else 0, + "cost_center": erpnext.get_default_cost_center(self.company), + "exchange_rate": 1, + "reference_type": "Exchange Rate Revaluation", + "reference_name": self.name, + } + ) + elif d.get("balance_in_base_currency") and not d.get("new_balance_in_base_currency"): # Base currency has balance dr_or_cr = "credit" if d.get("balance_in_base_currency") > 0 else "debit" @@ -388,22 +406,22 @@ class ExchangeRateRevaluation(Document): } ) - journal_entry_accounts.append(journal_account) + journal_entry_accounts.append(journal_account) - journal_entry_accounts.append( - { - "account": unrealized_exchange_gain_loss_account, - "balance": get_balance_on(unrealized_exchange_gain_loss_account), - "debit": abs(self.gain_loss_booked) if self.gain_loss_booked < 0 else 0, - "credit": abs(self.gain_loss_booked) if self.gain_loss_booked > 0 else 0, - "debit_in_account_currency": abs(self.gain_loss_booked) if self.gain_loss_booked < 0 else 0, - "credit_in_account_currency": self.gain_loss_booked if self.gain_loss_booked > 0 else 0, - "cost_center": erpnext.get_default_cost_center(self.company), - "exchange_rate": 1, - "reference_type": "Exchange Rate Revaluation", - "reference_name": self.name, - } - ) + journal_entry_accounts.append( + { + "account": unrealized_exchange_gain_loss_account, + "balance": get_balance_on(unrealized_exchange_gain_loss_account), + "debit": abs(d.gain_loss) if d.gain_loss < 0 else 0, + "credit": abs(d.gain_loss) if d.gain_loss > 0 else 0, + "debit_in_account_currency": 0, + "credit_in_account_currency": 0, + "cost_center": erpnext.get_default_cost_center(self.company), + "exchange_rate": 1, + "reference_type": "Exchange Rate Revaluation", + "reference_name": self.name, + } + ) journal_entry.set("accounts", journal_entry_accounts) journal_entry.set_total_debit_credit() From 9d04af9ecc865b77e9408f621eed0c4d933aa543 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 20 Jun 2023 07:21:51 +0530 Subject: [PATCH 24/45] refactor: allow higher precision for new exchange rate --- .../exchange_rate_revaluation_account.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/exchange_rate_revaluation_account/exchange_rate_revaluation_account.json b/erpnext/accounts/doctype/exchange_rate_revaluation_account/exchange_rate_revaluation_account.json index 2968359a0d0..0a7d0579b15 100644 --- a/erpnext/accounts/doctype/exchange_rate_revaluation_account/exchange_rate_revaluation_account.json +++ b/erpnext/accounts/doctype/exchange_rate_revaluation_account/exchange_rate_revaluation_account.json @@ -92,6 +92,7 @@ "fieldtype": "Float", "in_list_view": 1, "label": "New Exchange Rate", + "precision": "9", "reqd": 1 }, { @@ -147,7 +148,7 @@ ], "istable": 1, "links": [], - "modified": "2022-12-29 19:38:52.915295", + "modified": "2023-06-20 07:21:40.743460", "modified_by": "Administrator", "module": "Accounts", "name": "Exchange Rate Revaluation Account", From 4567474418d159e6334c7b69c3a7f7f6ccc9741d Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 20 Jun 2023 07:23:51 +0530 Subject: [PATCH 25/45] refactor: allow '0' rounding allowance --- .../exchange_rate_revaluation/exchange_rate_revaluation.js | 2 +- .../exchange_rate_revaluation/exchange_rate_revaluation.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.js b/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.js index f51b90d8f6a..1ef5c837402 100644 --- a/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.js +++ b/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.js @@ -37,7 +37,7 @@ frappe.ui.form.on('Exchange Rate Revaluation', { validate_rounding_loss: function(frm) { let allowance = frm.doc.rounding_loss_allowance; - if (!(allowance > 0 && allowance < 1)) { + if (!(allowance >= 0 && allowance < 1)) { frappe.throw(__("Rounding Loss Allowance should be between 0 and 1")); } }, diff --git a/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.py b/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.py index 4926006c965..598db642f33 100644 --- a/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.py +++ b/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.py @@ -22,7 +22,7 @@ class ExchangeRateRevaluation(Document): self.set_total_gain_loss() def validate_rounding_loss_allowance(self): - if not (self.rounding_loss_allowance > 0 and self.rounding_loss_allowance < 1): + if not (self.rounding_loss_allowance >= 0 and self.rounding_loss_allowance < 1): frappe.throw(_("Rounding Loss Allowance should be between 0 and 1")) def set_total_gain_loss(self): From 6694175a517dc942f056e5b9b9918e55419f85b0 Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 20 Jun 2023 07:29:21 +0530 Subject: [PATCH 26/45] refactor: higher precision for rounding loss and allow '0' --- .../exchange_rate_revaluation.json | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.json b/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.json index 2310d1272cd..79428d591b4 100644 --- a/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.json +++ b/erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.json @@ -100,15 +100,16 @@ }, { "default": "0.05", - "description": "Only values between 0 and 1 are allowed. \nEx: If allowance is set at 0.07, accounts that have balance of 0.07 in either of the currencies will be considered as zero balance account", + "description": "Only values between [0,1) are allowed. Like {0.00, 0.04, 0.09, ...}\nEx: If allowance is set at 0.07, accounts that have balance of 0.07 in either of the currencies will be considered as zero balance account", "fieldname": "rounding_loss_allowance", "fieldtype": "Float", - "label": "Rounding Loss Allowance" + "label": "Rounding Loss Allowance", + "precision": "9" } ], "is_submittable": 1, "links": [], - "modified": "2023-06-12 21:02:09.818208", + "modified": "2023-06-20 07:29:06.972434", "modified_by": "Administrator", "module": "Accounts", "name": "Exchange Rate Revaluation", From df090cbe873d827f5fee64feee20a68b6eab15ce Mon Sep 17 00:00:00 2001 From: Anand Baburajan Date: Tue, 20 Jun 2023 13:03:15 +0530 Subject: [PATCH 27/45] chore: minor typo in fixed asset register (#35801) chore: renaming entries to assets --- .../report/fixed_asset_register/fixed_asset_register.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/erpnext/assets/report/fixed_asset_register/fixed_asset_register.py b/erpnext/assets/report/fixed_asset_register/fixed_asset_register.py index cb61914c255..f810819b4fc 100644 --- a/erpnext/assets/report/fixed_asset_register/fixed_asset_register.py +++ b/erpnext/assets/report/fixed_asset_register/fixed_asset_register.py @@ -210,9 +210,7 @@ def get_assets_linked_to_fb(filters): company_fb = frappe.get_cached_value("Company", filters.company, "default_finance_book") if filters.finance_book and company_fb and cstr(filters.finance_book) != cstr(company_fb): - frappe.throw( - _("To use a different finance book, please uncheck 'Include Default Book Entries'") - ) + frappe.throw(_("To use a different finance book, please uncheck 'Include Default Book Assets'")) query = query.where( (afb.finance_book.isin([cstr(filters.finance_book), cstr(company_fb), ""])) From a627d2a38cc6b194694066da256dda28d3c7ddba Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Tue, 20 Jun 2023 15:55:18 +0530 Subject: [PATCH 28/45] fix: keyerror while checking the stock balance report --- erpnext/stock/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/stock/utils.py b/erpnext/stock/utils.py index 402f998677d..02444064c17 100644 --- a/erpnext/stock/utils.py +++ b/erpnext/stock/utils.py @@ -475,7 +475,7 @@ def add_additional_uom_columns(columns, result, include_uom, conversion_factors) for row_idx, row in enumerate(result): for convertible_col, data in convertible_column_map.items(): - conversion_factor = conversion_factors[row.get("item_code")] or 1 + conversion_factor = conversion_factors.get(row.get("item_code")) or 1.0 for_type = data.for_type value_before_conversion = row.get(convertible_col) if for_type == "rate": From 32965f1af9403b20b8d783c6e8f609a565b2864c Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Tue, 20 Jun 2023 16:27:23 +0530 Subject: [PATCH 29/45] fix: stock error for service item --- erpnext/e_commerce/variant_selector/utils.py | 1 + erpnext/templates/generators/item/item_configure.js | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/erpnext/e_commerce/variant_selector/utils.py b/erpnext/e_commerce/variant_selector/utils.py index 1a3e7379281..4466c457436 100644 --- a/erpnext/e_commerce/variant_selector/utils.py +++ b/erpnext/e_commerce/variant_selector/utils.py @@ -162,6 +162,7 @@ def get_next_attribute_and_values(item_code, selected_attributes): product_info = get_item_variant_price_dict(exact_match[0], cart_settings) if product_info: + product_info["is_stock_item"] = frappe.get_cached_value("Item", exact_match[0], "is_stock_item") product_info["allow_items_not_in_stock"] = cint(cart_settings.allow_items_not_in_stock) else: product_info = None diff --git a/erpnext/templates/generators/item/item_configure.js b/erpnext/templates/generators/item/item_configure.js index 613c967e3d6..9beba3fd01e 100644 --- a/erpnext/templates/generators/item/item_configure.js +++ b/erpnext/templates/generators/item/item_configure.js @@ -219,7 +219,8 @@ class ItemConfigure { : '' } - ${available_qty === 0 ? '(' + __('Out of Stock') + ')' : ''} + ${available_qty === 0 && product_info && product_info?.is_stock_item + ? '(' + __('Out of Stock') + ')' : ''} @@ -236,7 +237,8 @@ class ItemConfigure { `; /* eslint-disable indent */ - if (!product_info?.allow_items_not_in_stock && available_qty === 0) { + if (!product_info?.allow_items_not_in_stock && available_qty === 0 + && product_info && product_info?.is_stock_item) { item_add_to_cart = ''; } From 96c5c7b1dfb953e665c2683ac68ad67b55474bcb Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Thu, 8 Jun 2023 17:08:22 +0530 Subject: [PATCH 30/45] fix: don't allow to make reposting entry for closing stock balance period --- .../repost_item_valuation.py | 41 ++++++++++++++++++- .../test_repost_item_valuation.py | 30 ++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py index d5fc710625a..27066b825c1 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py @@ -13,6 +13,7 @@ from frappe.utils.user import get_users_with_role from rq.timeouts import JobTimeoutException import erpnext +from erpnext.accounts.general_ledger import validate_accounting_period from erpnext.accounts.utils import get_future_stock_vouchers, repost_gle_for_stock_vouchers from erpnext.stock.stock_ledger import ( get_affected_transactions, @@ -44,11 +45,49 @@ class RepostItemValuation(Document): self.validate_accounts_freeze() def validate_period_closing_voucher(self): + # Period Closing Voucher year_end_date = self.get_max_year_end_date(self.company) if year_end_date and getdate(self.posting_date) <= getdate(year_end_date): - msg = f"Due to period closing, you cannot repost item valuation before {year_end_date}" + date = frappe.format(year_end_date, "Date") + msg = f"Due to period closing, you cannot repost item valuation before {date}" frappe.throw(_(msg)) + # Accounting Period + if self.voucher_type: + validate_accounting_period( + [ + frappe._dict( + { + "posting_date": self.posting_date, + "company": self.company, + "voucher_type": self.voucher_type, + } + ) + ] + ) + + # Closing Stock Balance + closing_stock = self.get_closing_stock_balance() + if closing_stock and closing_stock[0].name: + name = get_link_to_form("Closing Stock Balance", closing_stock[0].name) + to_date = frappe.format(closing_stock[0].to_date, "Date") + msg = f"Due to closing stock balance {name}, you cannot repost item valuation before {to_date}" + frappe.throw(_(msg)) + + def get_closing_stock_balance(self): + filters = { + "company": self.company, + "status": "Completed", + "docstatus": 1, + "to_date": (">=", self.posting_date), + } + + for field in ["warehouse", "item_code"]: + if self.get(field): + filters.update({field: ("in", ["", self.get(field)])}) + + return frappe.get_all("Closing Stock Balance", fields=["name", "to_date"], filters=filters) + @staticmethod def get_max_year_end_date(company): data = frappe.get_all( diff --git a/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py b/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py index 9c4d997b316..1853f45f583 100644 --- a/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/test_repost_item_valuation.py @@ -392,3 +392,33 @@ class TestRepostItemValuation(FrappeTestCase, StockTestMixin): pr.cancel() self.assertTrue(pr.docstatus == 2) self.assertTrue(frappe.db.exists("Repost Item Valuation", {"voucher_no": pr.name})) + + def test_repost_item_valuation_for_closing_stock_balance(self): + from erpnext.stock.doctype.closing_stock_balance.closing_stock_balance import ( + prepare_closing_stock_balance, + ) + + doc = frappe.new_doc("Closing Stock Balance") + doc.company = "_Test Company" + doc.from_date = today() + doc.to_date = today() + doc.submit() + + prepare_closing_stock_balance(doc.name) + doc.load_from_db() + self.assertEqual(doc.docstatus, 1) + self.assertEqual(doc.status, "Completed") + + riv = frappe.new_doc("Repost Item Valuation") + riv.update( + { + "item_code": "_Test Item", + "warehouse": "_Test Warehouse - _TC", + "based_on": "Item and Warehouse", + "posting_date": today(), + "posting_time": "00:01:00", + } + ) + + self.assertRaises(frappe.ValidationError, riv.save) + doc.cancel() From ad758b8d8511853934b7cfcfde42eddeff9a333c Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 21 Jun 2023 14:19:02 +0530 Subject: [PATCH 31/45] fix: no permission for accounts settings on payment reconciliation --- .../payment_reconciliation.js | 40 ++++++++++--------- .../payment_reconciliation.py | 4 ++ 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.js b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.js index 22836776345..89fa15172f1 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.js +++ b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.js @@ -85,25 +85,29 @@ erpnext.accounts.PaymentReconciliationController = class PaymentReconciliationCo // check for any running reconciliation jobs if (this.frm.doc.receivable_payable_account) { - frappe.db.get_single_value("Accounts Settings", "auto_reconcile_payments").then((enabled) => { - if(enabled) { - this.frm.call({ - 'method': "erpnext.accounts.doctype.process_payment_reconciliation.process_payment_reconciliation.is_any_doc_running", - "args": { - for_filter: { - company: this.frm.doc.company, - party_type: this.frm.doc.party_type, - party: this.frm.doc.party, - receivable_payable_account: this.frm.doc.receivable_payable_account + this.frm.call({ + doc: this.frm.doc, + method: 'is_auto_process_enabled', + callback: (r) => { + if (r.message) { + this.frm.call({ + 'method': "erpnext.accounts.doctype.process_payment_reconciliation.process_payment_reconciliation.is_any_doc_running", + "args": { + for_filter: { + company: this.frm.doc.company, + party_type: this.frm.doc.party_type, + party: this.frm.doc.party, + receivable_payable_account: this.frm.doc.receivable_payable_account + } } - } - }).then(r => { - if (r.message) { - let doc_link = frappe.utils.get_form_link("Process Payment Reconciliation", r.message, true); - let msg = __("Payment Reconciliation Job: {0} is running for this party. Can't reconcile now.", [doc_link]); - this.frm.dashboard.add_comment(msg, "yellow"); - } - }); + }).then(r => { + if (r.message) { + let doc_link = frappe.utils.get_form_link("Process Payment Reconciliation", r.message, true); + let msg = __("Payment Reconciliation Job: {0} is running for this party. Can't reconcile now.", [doc_link]); + this.frm.dashboard.add_comment(msg, "yellow"); + } + }); + } } }); } diff --git a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py index 2c8faecf4b1..2e4e3b0e078 100644 --- a/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py +++ b/erpnext/accounts/doctype/payment_reconciliation/payment_reconciliation.py @@ -252,6 +252,10 @@ class PaymentReconciliation(Document): return difference_amount + @frappe.whitelist() + def is_auto_process_enabled(self): + return frappe.db.get_single_value("Accounts Settings", "auto_reconcile_payments") + @frappe.whitelist() def calculate_difference_on_allocation_change(self, payment_entry, invoice, allocated_amount): invoice_exchange_map = self.get_invoice_exchange_map(invoice, payment_entry) From 39a1f4a4c1dbe95ca784f307fd2c6d66b0ec4175 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 21 Jun 2023 15:38:06 +0530 Subject: [PATCH 32/45] fix: issue of asset value_after_depreciation field getting updated twice if workflow is enabled in Journal Entry (backport #35821) (#35826) * Fixes issue of asset value_after_depreciation field getting updated twice if workflow is enabled in Journal Entry (#35821) * Fixes issue of asset value_after_depreciation field getting updated twice if workflow is enabled in Journal Entry * chore: remove unnecessary line break * chore: formatting --------- Co-authored-by: Anand Baburajan (cherry picked from commit 000ebe447991185e4d5a4050b066fa0781575e65) # Conflicts: # erpnext/assets/doctype/asset/depreciation.py * chore: resolve conflicts --------- Co-authored-by: saeedkola Co-authored-by: Anand Baburajan --- erpnext/assets/doctype/asset/depreciation.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/erpnext/assets/doctype/asset/depreciation.py b/erpnext/assets/doctype/asset/depreciation.py index bfef57e4947..259568a24b1 100644 --- a/erpnext/assets/doctype/asset/depreciation.py +++ b/erpnext/assets/doctype/asset/depreciation.py @@ -159,15 +159,15 @@ def make_depreciation_entry(asset_depr_schedule_name, date=None): je.flags.ignore_permissions = True je.flags.planned_depr_entry = True je.save() - if not je.meta.get_workflow(): - je.submit() d.db_set("journal_entry", je.name) - idx = cint(asset_depr_schedule_doc.finance_book_id) - row = asset.get("finance_books")[idx - 1] - row.value_after_depreciation -= d.depreciation_amount - row.db_update() + if not je.meta.get_workflow(): + je.submit() + idx = cint(asset_depr_schedule_doc.finance_book_id) + row = asset.get("finance_books")[idx - 1] + row.value_after_depreciation -= d.depreciation_amount + row.db_update() asset.db_set("depr_entry_posting_status", "Successful") From dcfc86e3afa53e393c4dec74fa3f7d3a01be2aa4 Mon Sep 17 00:00:00 2001 From: phot0n Date: Wed, 21 Jun 2023 16:49:54 +0530 Subject: [PATCH 33/45] fix: use correct fieldname for purchase receipt column in item_wise_purcchase_register report --- .../item_wise_purchase_register.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/report/item_wise_purchase_register/item_wise_purchase_register.py b/erpnext/accounts/report/item_wise_purchase_register/item_wise_purchase_register.py index d34c21348c8..924c14bdb94 100644 --- a/erpnext/accounts/report/item_wise_purchase_register/item_wise_purchase_register.py +++ b/erpnext/accounts/report/item_wise_purchase_register/item_wise_purchase_register.py @@ -87,7 +87,7 @@ def _execute(filters=None, additional_table_columns=None, additional_query_colum "project": d.project, "company": d.company, "purchase_order": d.purchase_order, - "purchase_receipt": d.purchase_receipt, + "purchase_receipt": purchase_receipt, "expense_account": expense_account, "stock_qty": d.stock_qty, "stock_uom": d.stock_uom, @@ -241,7 +241,7 @@ def get_columns(additional_table_columns, filters): }, { "label": _("Purchase Receipt"), - "fieldname": "Purchase Receipt", + "fieldname": "purchase_receipt", "fieldtype": "Link", "options": "Purchase Receipt", "width": 100, From ed76ee3e161454af1eafa310cd6b207dd20e19fc Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Wed, 21 Jun 2023 17:15:46 +0530 Subject: [PATCH 34/45] fix: Move ledger display to dialog --- .../doctype/sales_invoice/sales_invoice.json | 32 ++--------------- .../public/js/controllers/stock_controller.js | 35 +++++++++++++++++-- 2 files changed, 34 insertions(+), 33 deletions(-) diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.json b/erpnext/accounts/doctype/sales_invoice/sales_invoice.json index 15be2e71baa..61c10d9cd58 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.json +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.json @@ -211,12 +211,7 @@ "is_discounted", "remarks", "repost_required", - "connections_tab", - "ledger_preview", - "accounting_ledger_section", - "accounting_ledger_preview_html", - "stock_ledger_section", - "stock_ledger_preview_html" + "connections_tab" ], "fields": [ { @@ -2147,29 +2142,6 @@ "fieldname": "use_company_roundoff_cost_center", "fieldtype": "Check", "label": "Use Company default Cost Center for Round off" - }, - { - "fieldname": "ledger_preview", - "fieldtype": "Tab Break", - "label": "Ledger Preview" - }, - { - "fieldname": "accounting_ledger_section", - "fieldtype": "Section Break", - "label": "Accounting Ledger" - }, - { - "fieldname": "accounting_ledger_preview_html", - "fieldtype": "HTML" - }, - { - "fieldname": "stock_ledger_section", - "fieldtype": "Section Break", - "label": "Stock Ledger" - }, - { - "fieldname": "stock_ledger_preview_html", - "fieldtype": "HTML" } ], "icon": "fa fa-file-text", @@ -2182,7 +2154,7 @@ "link_fieldname": "consolidated_invoice" } ], - "modified": "2023-06-11 11:18:14.024258", + "modified": "2023-06-21 16:02:18.988799", "modified_by": "Administrator", "module": "Accounts", "name": "Sales Invoice", diff --git a/erpnext/public/js/controllers/stock_controller.js b/erpnext/public/js/controllers/stock_controller.js index 0a14ed79359..ff593485be2 100644 --- a/erpnext/public/js/controllers/stock_controller.js +++ b/erpnext/public/js/controllers/stock_controller.js @@ -94,15 +94,44 @@ erpnext.stock.StockController = class StockController extends frappe.ui.form.Con "docname": me.frm.doc.name }, "callback": function(response) { - me.get_datatable(response.message.gl_columns, response.message.gl_data, me.frm.get_field("accounting_ledger_preview_html").wrapper); - me.get_datatable(response.message.sl_columns, response.message.sl_data, me.frm.get_field("stock_ledger_preview_html").wrapper); - me.frm.scroll_to_field("accounting_ledger_preview_html"); + me.make_dialog(response.message); } }) }, __("View")); } } + make_dialog(data) { + let me = this; + let gl_columns = data.gl_columns; + let gl_data = data.gl_data; + let sl_columns = data.sl_columns; + let sl_data = data.sl_data; + + let dialog = new frappe.ui.Dialog({ + "size": "extra-large", + "title": __("Ledger Preview"), + "fields": [ + { + "fieldtype": "HTML", + "fieldname": "accounting_ledger_preview_html", + "label": __("Accounting Ledger"), + }, + { + "fieldtype": "HTML", + "fieldname": "stock_ledger_preview_html", + "label": __("Stock Ledger"), + } + ] + }); + + setTimeout(function() { + me.get_datatable(gl_columns, gl_data, dialog.get_field("accounting_ledger_preview_html").wrapper); + }, 200); + + dialog.show(); + } + get_datatable(columns, data, wrapper) { const datatable_options = { columns: columns, From 756dbe7ce89838a9cbff2e13e03c67f6f7c6e0ff Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Wed, 21 Jun 2023 17:40:48 +0530 Subject: [PATCH 35/45] refactor: return against rejected warehouse --- .../controllers/sales_and_purchase_return.py | 13 +++++- .../purchase_receipt/purchase_receipt.js | 46 +++++++++++++++++-- .../purchase_receipt/purchase_receipt.py | 7 +++ .../purchase_receipt/test_purchase_receipt.py | 27 +++++++++++ 4 files changed, 88 insertions(+), 5 deletions(-) diff --git a/erpnext/controllers/sales_and_purchase_return.py b/erpnext/controllers/sales_and_purchase_return.py index 818c7894b7d..954668055e1 100644 --- a/erpnext/controllers/sales_and_purchase_return.py +++ b/erpnext/controllers/sales_and_purchase_return.py @@ -320,7 +320,9 @@ def get_returned_qty_map_for_row(return_against, party, row_name, doctype): return data[0] -def make_return_doc(doctype: str, source_name: str, target_doc=None): +def make_return_doc( + doctype: str, source_name: str, target_doc=None, return_against_rejected_qty=False +): from frappe.model.mapper import get_mapped_doc company = frappe.db.get_value("Delivery Note", source_name, "company") @@ -471,7 +473,7 @@ def make_return_doc(doctype: str, source_name: str, target_doc=None): target_doc.qty = -1 * flt(source_doc.qty - (returned_qty_map.get("qty") or 0)) - if hasattr(target_doc, "stock_qty"): + if hasattr(target_doc, "stock_qty") and not return_against_rejected_qty: target_doc.stock_qty = -1 * flt( source_doc.stock_qty - (returned_qty_map.get("stock_qty") or 0) ) @@ -490,6 +492,13 @@ def make_return_doc(doctype: str, source_name: str, target_doc=None): target_doc.rejected_warehouse = source_doc.rejected_warehouse target_doc.purchase_receipt_item = source_doc.name + if doctype == "Purchase Receipt" and return_against_rejected_qty: + target_doc.qty = -1 * flt(source_doc.rejected_qty - (returned_qty_map.get("qty") or 0)) + target_doc.rejected_qty = 0.0 + target_doc.rejected_warehouse = "" + target_doc.warehouse = source_doc.rejected_warehouse + target_doc.received_qty = target_doc.qty + elif doctype == "Purchase Invoice": returned_qty_map = get_returned_qty_map_for_row( source_parent.name, source_parent.supplier, source_doc.name, doctype diff --git a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.js b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.js index 312c166f8b7..dc3e0d99351 100644 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.js +++ b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.js @@ -209,10 +209,43 @@ erpnext.stock.PurchaseReceiptController = class PurchaseReceiptController extend } make_purchase_return() { - frappe.model.open_mapped_doc({ - method: "erpnext.stock.doctype.purchase_receipt.purchase_receipt.make_purchase_return", - frm: cur_frm + let me = this; + + let has_rejected_items = cur_frm.doc.items.filter((item) => { + if (item.rejected_qty > 0) { + return true; + } }) + + if (has_rejected_items && has_rejected_items.length > 0) { + frappe.prompt([ + { + label: __("Return Qty from Rejected Warehouse"), + fieldtype: "Check", + fieldname: "return_for_rejected_warehouse", + default: 1 + }, + ], function(values){ + if (values.return_for_rejected_warehouse) { + frappe.call({ + method: "erpnext.stock.doctype.purchase_receipt.purchase_receipt.make_purchase_return_against_rejected_warehouse", + args: { + source_name: cur_frm.doc.name + }, + callback: function(r) { + if(r.message) { + frappe.model.sync(r.message); + frappe.set_route("Form", r.message.doctype, r.message.name); + } + } + }) + } else { + cur_frm.cscript._make_purchase_return(); + } + }, __("Return Qty"), __("Make Return Entry")); + } else { + cur_frm.cscript._make_purchase_return(); + } } close_purchase_receipt() { @@ -322,6 +355,13 @@ frappe.ui.form.on('Purchase Receipt Item', { }, }); +cur_frm.cscript._make_purchase_return = function() { + frappe.model.open_mapped_doc({ + method: "erpnext.stock.doctype.purchase_receipt.purchase_receipt.make_purchase_return", + frm: cur_frm + }); +} + cur_frm.cscript['Make Stock Entry'] = function() { frappe.model.open_mapped_doc({ method: "erpnext.stock.doctype.purchase_receipt.purchase_receipt.make_stock_entry", diff --git a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py index 387f0313804..0b5dc05c3ab 100644 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py @@ -1136,6 +1136,13 @@ def get_returned_qty_map(purchase_receipt): return returned_qty_map +@frappe.whitelist() +def make_purchase_return_against_rejected_warehouse(source_name): + from erpnext.controllers.sales_and_purchase_return import make_return_doc + + return make_return_doc("Purchase Receipt", source_name, return_against_rejected_qty=True) + + @frappe.whitelist() def make_purchase_return(source_name, target_doc=None): from erpnext.controllers.sales_and_purchase_return import make_return_doc diff --git a/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py index ddc055656f2..19867225876 100644 --- a/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py @@ -1827,6 +1827,33 @@ class TestPurchaseReceipt(FrappeTestCase): self.assertEqual(abs(data["stock_value_difference"]), 400.00) + def test_return_from_rejected_warehouse(self): + from erpnext.stock.doctype.purchase_receipt.purchase_receipt import ( + make_purchase_return_against_rejected_warehouse, + ) + + item_code = "_Test Item Return from Rejected Warehouse" + create_item(item_code) + + warehouse = create_warehouse("_Test Warehouse Return Qty Warehouse") + rejected_warehouse = create_warehouse("_Test Rejected Warehouse Return Qty Warehouse") + + # Step 1: Create Purchase Receipt with valuation rate 100 + pr = make_purchase_receipt( + item_code=item_code, + warehouse=warehouse, + qty=10, + rate=100, + rejected_qty=2, + rejected_warehouse=rejected_warehouse, + ) + + pr_return = make_purchase_return_against_rejected_warehouse(pr.name) + self.assertEqual(pr_return.items[0].warehouse, rejected_warehouse) + self.assertEqual(pr_return.items[0].qty, 2.0 * -1) + self.assertEqual(pr_return.items[0].rejected_qty, 0.0) + self.assertEqual(pr_return.items[0].rejected_warehouse, "") + def prepare_data_for_internal_transfer(): from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_internal_supplier From 41b9e928680a5dca42e10c383d0b9301482540fb Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Wed, 21 Jun 2023 21:52:44 +0530 Subject: [PATCH 36/45] fix: incorrect cost center error in bank reconciliation --- .../bank_reconciliation_tool/bank_reconciliation_tool.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py b/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py index c4a23a640c3..0eef3e9a67b 100644 --- a/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py +++ b/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py @@ -10,6 +10,7 @@ from frappe.model.document import Document from frappe.query_builder.custom import ConstantColumn from frappe.utils import cint, flt +from erpnext import get_default_cost_center from erpnext.accounts.doctype.bank_transaction.bank_transaction import get_total_allocated_amount from erpnext.accounts.report.bank_reconciliation_statement.bank_reconciliation_statement import ( get_amounts_not_reflected_in_system, @@ -140,6 +141,9 @@ def create_journal_entry_bts( second_account ) ) + + company = frappe.get_value("Account", company_account, "company") + accounts = [] # Multi Currency? accounts.append( @@ -149,6 +153,7 @@ def create_journal_entry_bts( "debit_in_account_currency": bank_transaction.withdrawal, "party_type": party_type, "party": party, + "cost_center": get_default_cost_center(company), } ) @@ -158,11 +163,10 @@ def create_journal_entry_bts( "bank_account": bank_transaction.bank_account, "credit_in_account_currency": bank_transaction.withdrawal, "debit_in_account_currency": bank_transaction.deposit, + "cost_center": get_default_cost_center(company), } ) - company = frappe.get_value("Account", company_account, "company") - journal_entry_dict = { "voucher_type": entry_type, "company": company, From b4db25dd188134dba30e22977c0fa4665ba749be Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Thu, 22 Jun 2023 12:40:02 +0530 Subject: [PATCH 37/45] refactor: increase precision for current exc rate in ERR --- .../exchange_rate_revaluation_account.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/exchange_rate_revaluation_account/exchange_rate_revaluation_account.json b/erpnext/accounts/doctype/exchange_rate_revaluation_account/exchange_rate_revaluation_account.json index 0a7d0579b15..fd2d931315c 100644 --- a/erpnext/accounts/doctype/exchange_rate_revaluation_account/exchange_rate_revaluation_account.json +++ b/erpnext/accounts/doctype/exchange_rate_revaluation_account/exchange_rate_revaluation_account.json @@ -73,6 +73,7 @@ "fieldname": "current_exchange_rate", "fieldtype": "Float", "label": "Current Exchange Rate", + "precision": "9", "read_only": 1 }, { @@ -148,7 +149,7 @@ ], "istable": 1, "links": [], - "modified": "2023-06-20 07:21:40.743460", + "modified": "2023-06-22 12:39:56.446722", "modified_by": "Administrator", "module": "Accounts", "name": "Exchange Rate Revaluation Account", From 0e68da5a2ab2855635cd292ea478d727a0ab7601 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 22 Jun 2023 15:43:32 +0530 Subject: [PATCH 38/45] feat: Show stock ledger preview --- .../doctype/payment_entry/payment_entry.js | 1 + .../purchase_invoice/purchase_invoice.js | 4 +- .../doctype/sales_invoice/sales_invoice.js | 7 +- erpnext/controllers/stock_controller.py | 34 +++++++-- .../public/js/controllers/stock_controller.js | 65 ---------------- erpnext/public/js/erpnext.bundle.js | 1 + erpnext/public/js/utils/ledger_preview.js | 76 +++++++++++++++++++ .../doctype/delivery_note/delivery_note.js | 3 + .../purchase_receipt/purchase_receipt.js | 4 + 9 files changed, 119 insertions(+), 76 deletions(-) create mode 100644 erpnext/public/js/utils/ledger_preview.js diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.js b/erpnext/accounts/doctype/payment_entry/payment_entry.js index 9f55ba1167f..bac84db2315 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.js +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.js @@ -155,6 +155,7 @@ frappe.ui.form.on('Payment Entry', { frm.events.hide_unhide_fields(frm); frm.events.set_dynamic_labels(frm); frm.events.show_general_ledger(frm); + erpnext.accounts.ledger_preview.show_accounting_ledger_preview(frm); }, validate_company: (frm) => { diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js index ab7884d5209..6a558ca606b 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js @@ -54,9 +54,11 @@ erpnext.accounts.PurchaseInvoice = class PurchaseInvoice extends erpnext.buying. hide_fields(this.frm.doc); // Show / Hide button this.show_general_ledger(); + erpnext.accounts.ledger_preview.show_accounting_ledger_preview(this.frm); - if(doc.update_stock==1 && doc.docstatus==1) { + if(doc.update_stock==1) { this.show_stock_ledger(); + erpnext.accounts.ledger_preview.show_stock_ledger_preview(this.frm); } if(!doc.is_return && doc.docstatus == 1 && doc.outstanding_amount != 0){ diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.js b/erpnext/accounts/doctype/sales_invoice/sales_invoice.js index 1ef0c51cbac..68407e02210 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.js +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.js @@ -88,9 +88,12 @@ erpnext.accounts.SalesInvoiceController = class SalesInvoiceController extends e } this.show_general_ledger(); - this.show_ledger_preview(); + erpnext.accounts.ledger_preview.show_accounting_ledger_preview(this.frm); - if(doc.update_stock) this.show_stock_ledger(); + if(doc.update_stock){ + this.show_stock_ledger(); + erpnext.accounts.ledger_preview.show_stock_ledger_preview(this.frm); + } if (doc.docstatus == 1 && doc.outstanding_amount!=0 && !(cint(doc.is_return) && doc.return_against)) { diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index 0b057bc7486..903ef658e3c 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -846,20 +846,31 @@ class StockController(AccountsController): @frappe.whitelist() -def show_ledger_preview(company, doctype, docname): - frappe.db.savepoint("show_ledger_preview") +def show_accounting_ledger_preview(company, doctype, docname): + frappe.db.savepoint("show_accounting_ledger_preview") + + filters = {"company": company, "include_dimensions": 1} + doc = frappe.get_doc(doctype, docname) + + gl_columns, gl_data = get_accounting_ledger_preview(doc, filters) + + frappe.db.rollback(save_point="show_accounting_ledger_preview") + + return {"gl_columns": gl_columns, "gl_data": gl_data} + + +@frappe.whitelist() +def show_stock_ledger_preview(company, doctype, docname): + frappe.db.savepoint("show_stock_ledger_preview") filters = {"company": company} doc = frappe.get_doc(doctype, docname) sl_columns, sl_data = get_stock_ledger_preview(doc, filters) - gl_columns, gl_data = get_accounting_ledger_preview(doc, filters) - frappe.db.rollback(save_point="show_ledger_preview") + frappe.db.rollback(save_point="show_stock_ledger_preview") return { - "gl_columns": gl_columns, - "gl_data": gl_data, "sl_columns": sl_columns, "sl_data": sl_data, } @@ -877,11 +888,16 @@ def get_accounting_ledger_preview(doc, filters): "against", "party", "party_type", + "cost_center", "against_voucher_type", "against_voucher", ] doc.docstatus = 1 + + if doc.get("update_stock") or doc.doctype in ("Purchase Receipt", "Delivery Note"): + doc.update_stock_ledger() + doc.make_gl_entries() columns = get_gl_columns(filters) gl_entries = get_gl_entries_for_preview(doc.doctype, doc.name, fields) @@ -915,12 +931,12 @@ def get_stock_ledger_preview(doc, filters): "qty_after_transaction", "warehouse", "incoming_rate", - "valuation_rate", + "in_out_rate", "stock_value", "stock_value_difference", ] - if doc.update_stock or doc.doctype in ("Purchase Receipt", "Delivery Note"): + if doc.get("update_stock") or doc.doctype in ("Purchase Receipt", "Delivery Note"): doc.docstatus = 1 doc.update_stock_ledger() columns = get_sl_columns(filters) @@ -945,6 +961,8 @@ def get_sl_entries_for_preview(doctype, docname, fields): entry["out_qty"] = abs(entry.actual_qty) entry["in_qty"] = 0 + entry["in_out_rate"] = entry["valuation_rate"] + return sl_entries diff --git a/erpnext/public/js/controllers/stock_controller.js b/erpnext/public/js/controllers/stock_controller.js index ff593485be2..720423b0a49 100644 --- a/erpnext/public/js/controllers/stock_controller.js +++ b/erpnext/public/js/controllers/stock_controller.js @@ -81,69 +81,4 @@ erpnext.stock.StockController = class StockController extends frappe.ui.form.Con }, __("View")); } } - - show_ledger_preview() { - let me = this - if(this.frm.doc.docstatus == 0) { - cur_frm.add_custom_button(__('Accounting Ledger Preview'), function() { - frappe.call({ - "method": "erpnext.controllers.stock_controller.show_ledger_preview", - "args": { - "company": me.frm.doc.company, - "doctype": me.frm.doc.doctype, - "docname": me.frm.doc.name - }, - "callback": function(response) { - me.make_dialog(response.message); - } - }) - }, __("View")); - } - } - - make_dialog(data) { - let me = this; - let gl_columns = data.gl_columns; - let gl_data = data.gl_data; - let sl_columns = data.sl_columns; - let sl_data = data.sl_data; - - let dialog = new frappe.ui.Dialog({ - "size": "extra-large", - "title": __("Ledger Preview"), - "fields": [ - { - "fieldtype": "HTML", - "fieldname": "accounting_ledger_preview_html", - "label": __("Accounting Ledger"), - }, - { - "fieldtype": "HTML", - "fieldname": "stock_ledger_preview_html", - "label": __("Stock Ledger"), - } - ] - }); - - setTimeout(function() { - me.get_datatable(gl_columns, gl_data, dialog.get_field("accounting_ledger_preview_html").wrapper); - }, 200); - - dialog.show(); - } - - get_datatable(columns, data, wrapper) { - const datatable_options = { - columns: columns, - data: data, - dynamicRowHeight: true, - checkboxColumn: false, - inlineFilters: true, - }; - - new frappe.DataTable( - wrapper, - datatable_options - ); - } }; diff --git a/erpnext/public/js/erpnext.bundle.js b/erpnext/public/js/erpnext.bundle.js index cc020fc2f11..4e028e4c313 100644 --- a/erpnext/public/js/erpnext.bundle.js +++ b/erpnext/public/js/erpnext.bundle.js @@ -17,6 +17,7 @@ import "./utils/customer_quick_entry"; import "./utils/supplier_quick_entry"; import "./call_popup/call_popup"; import "./utils/dimension_tree_filter"; +import "./utils/ledger_preview.js" import "./utils/barcode_scanner"; import "./telephony"; import "./templates/call_link.html"; diff --git a/erpnext/public/js/utils/ledger_preview.js b/erpnext/public/js/utils/ledger_preview.js new file mode 100644 index 00000000000..d1ee67b9c23 --- /dev/null +++ b/erpnext/public/js/utils/ledger_preview.js @@ -0,0 +1,76 @@ +frappe.provide('erpnext.accounts'); + +erpnext.accounts.ledger_preview = { + show_accounting_ledger_preview(frm) { + let me = this; + if(!frm.is_new() && frm.doc.docstatus == 0) { + frm.add_custom_button(__('Accounting Ledger'), function() { + frappe.call({ + "method": "erpnext.controllers.stock_controller.show_accounting_ledger_preview", + "args": { + "company": frm.doc.company, + "doctype": frm.doc.doctype, + "docname": frm.doc.name + }, + "callback": function(response) { + me.make_dialog("Accounting Ledger Preview", "accounting_ledger_preview_html", response.message.gl_columns, response.message.gl_data); + } + }) + }, __("Preview")); + } + }, + + show_stock_ledger_preview(frm) { + let me = this + if(!frm.is_new() && frm.doc.docstatus == 0) { + frm.add_custom_button(__('Stock Ledger'), function() { + frappe.call({ + "method": "erpnext.controllers.stock_controller.show_stock_ledger_preview", + "args": { + "company": frm.doc.company, + "doctype": frm.doc.doctype, + "docname": frm.doc.name + }, + "callback": function(response) { + me.make_dialog("Stock Ledger Preview", "stock_ledger_preview_html", response.message.sl_columns, response.message.sl_data); + } + }) + }, __("Preview")); + } + }, + + make_dialog(label, fieldname, columns, data) { + let me = this; + let dialog = new frappe.ui.Dialog({ + "size": "extra-large", + "title": __(label), + "fields": [ + { + "fieldtype": "HTML", + "fieldname": fieldname, + }, + ] + }); + + setTimeout(function() { + me.get_datatable(columns, data, dialog.get_field(fieldname).wrapper); + }, 200); + + dialog.show(); + }, + + get_datatable(columns, data, wrapper) { + const datatable_options = { + columns: columns, + data: data, + dynamicRowHeight: true, + checkboxColumn: false, + inlineFilters: true, + }; + + new frappe.DataTable( + wrapper, + datatable_options + ); + } +} \ No newline at end of file diff --git a/erpnext/stock/doctype/delivery_note/delivery_note.js b/erpnext/stock/doctype/delivery_note/delivery_note.js index 77545e0e1ad..a648195933b 100644 --- a/erpnext/stock/doctype/delivery_note/delivery_note.js +++ b/erpnext/stock/doctype/delivery_note/delivery_note.js @@ -200,6 +200,9 @@ erpnext.stock.DeliveryNoteController = class DeliveryNoteController extends erpn } } + erpnext.accounts.ledger_preview.show_accounting_ledger_preview(this.frm); + erpnext.accounts.ledger_preview.show_stock_ledger_preview(this.frm); + if (doc.docstatus > 0) { this.show_stock_ledger(); if (erpnext.is_perpetual_inventory_enabled(doc.company)) { diff --git a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.js b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.js index 312c166f8b7..685209c02e4 100644 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.js +++ b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.js @@ -121,6 +121,10 @@ erpnext.stock.PurchaseReceiptController = class PurchaseReceiptController extend refresh() { var me = this; super.refresh(); + + erpnext.accounts.ledger_preview.show_accounting_ledger_preview(this.frm); + erpnext.accounts.ledger_preview.show_stock_ledger_preview(this.frm); + if(this.frm.doc.docstatus > 0) { this.show_stock_ledger(); //removed for temporary From 80fffbd64bce62cc61a6d5df3865cad18b46a730 Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Thu, 22 Jun 2023 15:55:18 +0530 Subject: [PATCH 39/45] fix: multiple Work Orders agaist same production plan --- .../doctype/production_plan/production_plan.js | 2 +- .../doctype/production_plan/production_plan.py | 6 ++++++ .../doctype/production_plan/test_production_plan.py | 7 +++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.js b/erpnext/manufacturing/doctype/production_plan/production_plan.js index 45a59cf7325..48986910b07 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.js +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.js @@ -99,7 +99,7 @@ frappe.ui.form.on('Production Plan', { }, __('Create')); } - if (frm.doc.mr_items && !in_list(['Material Requested', 'Closed'], frm.doc.status)) { + if (frm.doc.mr_items && frm.doc.mr_items.length && !in_list(['Material Requested', 'Closed'], frm.doc.status)) { frm.add_custom_button(__("Material Request"), ()=> { frm.trigger("make_material_request"); }, __('Create')); diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.py b/erpnext/manufacturing/doctype/production_plan/production_plan.py index 0800bdd2af9..6dc1ff6a49f 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.py @@ -515,6 +515,9 @@ class ProductionPlan(Document): self.show_list_created_message("Work Order", wo_list) self.show_list_created_message("Purchase Order", po_list) + if not wo_list: + frappe.msgprint(_("No Work Orders were created")) + def make_work_order_for_finished_goods(self, wo_list, default_warehouses): items_data = self.get_production_items() @@ -618,6 +621,9 @@ class ProductionPlan(Document): def create_work_order(self, item): from erpnext.manufacturing.doctype.work_order.work_order import OverProductionError + if item.get("qty") <= 0: + return + wo = frappe.new_doc("Work Order") wo.update(item) wo.planned_start_date = item.get("planned_start_date") or item.get("schedule_date") diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index 75b43ec1c30..fcfba7fca56 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -76,6 +76,13 @@ class TestProductionPlan(FrappeTestCase): "Work Order", fields=["name"], filters={"production_plan": pln.name}, as_list=1 ) + pln.make_work_order() + nwork_orders = frappe.get_all( + "Work Order", fields=["name"], filters={"production_plan": pln.name}, as_list=1 + ) + + self.assertTrue(len(work_orders), len(nwork_orders)) + self.assertTrue(len(work_orders), len(pln.po_items)) for name in material_requests: From 5c6e3269fba8060935181cda7a952ea565d3abad Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 22 Jun 2023 15:58:41 +0530 Subject: [PATCH 40/45] fix: Use GET request --- erpnext/public/js/utils/ledger_preview.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/erpnext/public/js/utils/ledger_preview.js b/erpnext/public/js/utils/ledger_preview.js index d1ee67b9c23..85d4a7d51e9 100644 --- a/erpnext/public/js/utils/ledger_preview.js +++ b/erpnext/public/js/utils/ledger_preview.js @@ -6,6 +6,7 @@ erpnext.accounts.ledger_preview = { if(!frm.is_new() && frm.doc.docstatus == 0) { frm.add_custom_button(__('Accounting Ledger'), function() { frappe.call({ + "type": "GET", "method": "erpnext.controllers.stock_controller.show_accounting_ledger_preview", "args": { "company": frm.doc.company, @@ -25,6 +26,7 @@ erpnext.accounts.ledger_preview = { if(!frm.is_new() && frm.doc.docstatus == 0) { frm.add_custom_button(__('Stock Ledger'), function() { frappe.call({ + "type": "GET", "method": "erpnext.controllers.stock_controller.show_stock_ledger_preview", "args": { "company": frm.doc.company, From d9e7bc545e3988b2af6e1b896286ba8673598c28 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 22 Jun 2023 16:07:32 +0530 Subject: [PATCH 41/45] fix: Do full rollback --- erpnext/controllers/stock_controller.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index 903ef658e3c..5137e030582 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -847,28 +847,24 @@ class StockController(AccountsController): @frappe.whitelist() def show_accounting_ledger_preview(company, doctype, docname): - frappe.db.savepoint("show_accounting_ledger_preview") - filters = {"company": company, "include_dimensions": 1} doc = frappe.get_doc(doctype, docname) gl_columns, gl_data = get_accounting_ledger_preview(doc, filters) - frappe.db.rollback(save_point="show_accounting_ledger_preview") + frappe.db.rollback() return {"gl_columns": gl_columns, "gl_data": gl_data} @frappe.whitelist() def show_stock_ledger_preview(company, doctype, docname): - frappe.db.savepoint("show_stock_ledger_preview") - filters = {"company": company} doc = frappe.get_doc(doctype, docname) sl_columns, sl_data = get_stock_ledger_preview(doc, filters) - frappe.db.rollback(save_point="show_stock_ledger_preview") + frappe.db.rollback() return { "sl_columns": sl_columns, From e745312a10dc56615a885dcaa97a4467a3705988 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 22 Jun 2023 19:21:52 +0530 Subject: [PATCH 42/45] fix: asset capitalization (backport #35832) (#35843) * fix: asset capitalization (#35832) * fix: misc asset capitalisation fixes * chore: add location in tests and remove unnecessary code * chore: more fixes and removals * chore: show company and fix tests * chore: make target qty read only on capitalization (cherry picked from commit fb823b53d1d9a631035016fa3b33003bf8ce297a) # Conflicts: # erpnext/assets/doctype/asset_capitalization/asset_capitalization.json # erpnext/assets/doctype/asset_capitalization/asset_capitalization.py * chore: fixing conflicts --------- Co-authored-by: Anand Baburajan --- .../asset_capitalization.js | 25 ---- .../asset_capitalization.json | 28 ++-- .../asset_capitalization.py | 137 ++++++------------ .../test_asset_capitalization.py | 25 +--- 4 files changed, 73 insertions(+), 142 deletions(-) diff --git a/erpnext/assets/doctype/asset_capitalization/asset_capitalization.js b/erpnext/assets/doctype/asset_capitalization/asset_capitalization.js index 01fcb11d817..6d55d7772b2 100644 --- a/erpnext/assets/doctype/asset_capitalization/asset_capitalization.js +++ b/erpnext/assets/doctype/asset_capitalization/asset_capitalization.js @@ -15,7 +15,6 @@ erpnext.assets.AssetCapitalization = class AssetCapitalization extends erpnext.s } refresh() { - erpnext.hide_company(); this.show_general_ledger(); if ((this.frm.doc.stock_items && this.frm.doc.stock_items.length) || !this.frm.doc.target_is_fixed_asset) { this.show_stock_ledger(); @@ -129,10 +128,6 @@ erpnext.assets.AssetCapitalization = class AssetCapitalization extends erpnext.s return this.get_target_item_details(); } - target_asset() { - return this.get_target_asset_details(); - } - item_code(doc, cdt, cdn) { var row = frappe.get_doc(cdt, cdn); if (cdt === "Asset Capitalization Stock Item") { @@ -247,26 +242,6 @@ erpnext.assets.AssetCapitalization = class AssetCapitalization extends erpnext.s } } - get_target_asset_details() { - var me = this; - - if (me.frm.doc.target_asset) { - return me.frm.call({ - method: "erpnext.assets.doctype.asset_capitalization.asset_capitalization.get_target_asset_details", - child: me.frm.doc, - args: { - asset: me.frm.doc.target_asset, - company: me.frm.doc.company, - }, - callback: function (r) { - if (!r.exc) { - me.frm.refresh_fields(); - } - } - }); - } - } - get_consumed_stock_item_details(row) { var me = this; diff --git a/erpnext/assets/doctype/asset_capitalization/asset_capitalization.json b/erpnext/assets/doctype/asset_capitalization/asset_capitalization.json index 01b35f64ab0..04b0c4e5132 100644 --- a/erpnext/assets/doctype/asset_capitalization/asset_capitalization.json +++ b/erpnext/assets/doctype/asset_capitalization/asset_capitalization.json @@ -11,13 +11,14 @@ "naming_series", "entry_type", "target_item_code", + "target_asset", "target_item_name", "target_is_fixed_asset", "target_has_batch_no", "target_has_serial_no", "column_break_9", - "target_asset", "target_asset_name", + "target_asset_location", "target_warehouse", "target_qty", "target_stock_uom", @@ -85,14 +86,13 @@ "fieldtype": "Column Break" }, { - "depends_on": "eval:doc.entry_type=='Capitalization'", "fieldname": "target_asset", "fieldtype": "Link", "in_standard_filter": 1, "label": "Target Asset", - "mandatory_depends_on": "eval:doc.entry_type=='Capitalization'", "no_copy": 1, - "options": "Asset" + "options": "Asset", + "read_only": 1 }, { "depends_on": "eval:doc.entry_type=='Capitalization'", @@ -108,11 +108,11 @@ "fieldtype": "Column Break" }, { - "fetch_from": "asset.company", "fieldname": "company", "fieldtype": "Link", "label": "Company", "options": "Company", + "remember_last_selected_value": 1, "reqd": 1 }, { @@ -158,7 +158,7 @@ "read_only": 1 }, { - "depends_on": "eval:doc.docstatus == 0 || (doc.stock_items && doc.stock_items.length)", + "depends_on": "eval:doc.entry_type=='Capitalization' && (doc.docstatus == 0 || (doc.stock_items && doc.stock_items.length))", "fieldname": "section_break_16", "fieldtype": "Section Break", "label": "Consumed Stock Items" @@ -189,7 +189,7 @@ "fieldname": "target_qty", "fieldtype": "Float", "label": "Target Qty", - "read_only_depends_on": "target_is_fixed_asset" + "read_only_depends_on": "eval:doc.entry_type=='Capitalization'" }, { "fetch_from": "target_item_code.stock_uom", @@ -227,7 +227,7 @@ "depends_on": "eval:doc.docstatus == 0 || (doc.asset_items && doc.asset_items.length)", "fieldname": "section_break_26", "fieldtype": "Section Break", - "label": "Consumed Asset Items" + "label": "Consumed Assets" }, { "fieldname": "asset_items", @@ -266,7 +266,7 @@ "options": "Finance Book" }, { - "depends_on": "eval:doc.docstatus == 0 || (doc.service_items && doc.service_items.length)", + "depends_on": "eval:doc.entry_type=='Capitalization' && (doc.docstatus == 0 || (doc.service_items && doc.service_items.length))", "fieldname": "service_expenses_section", "fieldtype": "Section Break", "label": "Service Expenses" @@ -329,12 +329,20 @@ "label": "Target Fixed Asset Account", "options": "Account", "read_only": 1 + }, + { + "depends_on": "eval:doc.entry_type=='Capitalization'", + "fieldname": "target_asset_location", + "fieldtype": "Link", + "label": "Target Asset Location", + "mandatory_depends_on": "eval:doc.entry_type=='Capitalization'", + "options": "Location" } ], "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2022-10-12 15:09:40.771332", + "modified": "2023-06-22 14:17:07.995120", "modified_by": "Administrator", "module": "Assets", "name": "Asset Capitalization", diff --git a/erpnext/assets/doctype/asset_capitalization/asset_capitalization.py b/erpnext/assets/doctype/asset_capitalization/asset_capitalization.py index 6841c56b108..a883bec71b0 100644 --- a/erpnext/assets/doctype/asset_capitalization/asset_capitalization.py +++ b/erpnext/assets/doctype/asset_capitalization/asset_capitalization.py @@ -19,9 +19,6 @@ from erpnext.assets.doctype.asset.depreciation import ( reverse_depreciation_entry_made_after_disposal, ) from erpnext.assets.doctype.asset_category.asset_category import get_asset_category_account -from erpnext.assets.doctype.asset_depreciation_schedule.asset_depreciation_schedule import ( - make_new_active_asset_depr_schedules_and_cancel_current_ones, -) from erpnext.controllers.stock_controller import StockController from erpnext.setup.doctype.brand.brand import get_brand_defaults from erpnext.setup.doctype.item_group.item_group import get_item_group_defaults @@ -45,7 +42,6 @@ force_fields = [ "target_has_batch_no", "target_stock_uom", "stock_uom", - "target_fixed_asset_account", "fixed_asset_account", "valuation_rate", ] @@ -56,7 +52,6 @@ class AssetCapitalization(StockController): self.validate_posting_time() self.set_missing_values(for_validate=True) self.validate_target_item() - self.validate_target_asset() self.validate_consumed_stock_item() self.validate_consumed_asset_item() self.validate_service_item() @@ -71,11 +66,12 @@ class AssetCapitalization(StockController): def before_submit(self): self.validate_source_mandatory() + if self.entry_type == "Capitalization": + self.create_target_asset() def on_submit(self): self.update_stock_ledger() self.make_gl_entries() - self.update_target_asset() def on_cancel(self): self.ignore_linked_doctypes = ( @@ -86,7 +82,7 @@ class AssetCapitalization(StockController): ) self.update_stock_ledger() self.make_gl_entries() - self.update_target_asset() + self.restore_consumed_asset_items() def set_title(self): self.title = self.target_asset_name or self.target_item_name or self.target_item_code @@ -97,15 +93,6 @@ class AssetCapitalization(StockController): if self.meta.has_field(k) and (not self.get(k) or k in force_fields): self.set(k, v) - # Remove asset if item not a fixed asset - if not self.target_is_fixed_asset: - self.target_asset = None - - target_asset_details = get_target_asset_details(self.target_asset, self.company) - for k, v in target_asset_details.items(): - if self.meta.has_field(k) and (not self.get(k) or k in force_fields): - self.set(k, v) - for d in self.stock_items: args = self.as_dict() args.update(d.as_dict()) @@ -157,9 +144,6 @@ class AssetCapitalization(StockController): if not target_item.is_stock_item: self.target_warehouse = None - if not target_item.is_fixed_asset: - self.target_asset = None - self.target_fixed_asset_account = None if not target_item.has_batch_no: self.target_batch_no = None if not target_item.has_serial_no: @@ -170,17 +154,6 @@ class AssetCapitalization(StockController): self.validate_item(target_item) - def validate_target_asset(self): - if self.target_asset: - target_asset = self.get_asset_for_validation(self.target_asset) - - if target_asset.item_code != self.target_item_code: - frappe.throw( - _("Asset {0} does not belong to Item {1}").format(self.target_asset, self.target_item_code) - ) - - self.validate_asset(target_asset) - def validate_consumed_stock_item(self): for d in self.stock_items: if d.item_code: @@ -386,7 +359,11 @@ class AssetCapitalization(StockController): gl_entries, target_account, target_against, precision ) + if not self.stock_items and not self.service_items and self.are_all_asset_items_non_depreciable: + return [] + self.get_gl_entries_for_target_item(gl_entries, target_against, precision) + return gl_entries def get_target_account(self): @@ -429,11 +406,14 @@ class AssetCapitalization(StockController): def get_gl_entries_for_consumed_asset_items( self, gl_entries, target_account, target_against, precision ): + self.are_all_asset_items_non_depreciable = True + # Consumed Assets for item in self.asset_items: - asset = self.get_asset(item) + asset = frappe.get_doc("Asset", item.asset) if asset.calculate_depreciation: + self.are_all_asset_items_non_depreciable = False notes = _( "This schedule was created when Asset {0} was consumed through Asset Capitalization {1}." ).format( @@ -519,40 +499,46 @@ class AssetCapitalization(StockController): ) ) - def update_target_asset(self): + def create_target_asset(self): total_target_asset_value = flt(self.total_value, self.precision("total_value")) - if self.docstatus == 1 and self.entry_type == "Capitalization": - asset_doc = frappe.get_doc("Asset", self.target_asset) - asset_doc.purchase_date = self.posting_date - asset_doc.gross_purchase_amount = total_target_asset_value - asset_doc.purchase_receipt_amount = total_target_asset_value - notes = _( - "This schedule was created when target Asset {0} was updated through Asset Capitalization {1}." - ).format( - get_link_to_form(asset_doc.doctype, asset_doc.name), get_link_to_form(self.doctype, self.name) - ) - make_new_active_asset_depr_schedules_and_cancel_current_ones(asset_doc, notes) - asset_doc.flags.ignore_validate_update_after_submit = True - asset_doc.save() - elif self.docstatus == 2: - for item in self.asset_items: - asset = self.get_asset(item) - asset.db_set("disposal_date", None) - self.set_consumed_asset_status(asset) + asset_doc = frappe.new_doc("Asset") + asset_doc.company = self.company + asset_doc.item_code = self.target_item_code + asset_doc.is_existing_asset = 1 + asset_doc.location = self.target_asset_location + asset_doc.available_for_use_date = self.posting_date + asset_doc.purchase_date = self.posting_date + asset_doc.gross_purchase_amount = total_target_asset_value + asset_doc.purchase_receipt_amount = total_target_asset_value + asset_doc.flags.ignore_validate = True + asset_doc.insert() - if asset.calculate_depreciation: - reverse_depreciation_entry_made_after_disposal(asset, self.posting_date) - notes = _( - "This schedule was created when Asset {0} was restored on Asset Capitalization {1}'s cancellation." - ).format( - get_link_to_form(asset.doctype, asset.name), get_link_to_form(self.doctype, self.name) - ) - reset_depreciation_schedule(asset, self.posting_date, notes) + self.target_asset = asset_doc.name - def get_asset(self, item): - asset = frappe.get_doc("Asset", item.asset) - self.check_finance_books(item, asset) - return asset + self.target_fixed_asset_account = get_asset_category_account( + "fixed_asset_account", item=self.target_item_code, company=asset_doc.company + ) + + frappe.msgprint( + _( + "Asset {0} has been created. Please set the depreciation details if any and submit it." + ).format(get_link_to_form("Asset", asset_doc.name)) + ) + + def restore_consumed_asset_items(self): + for item in self.asset_items: + asset = frappe.get_doc("Asset", item.asset) + asset.db_set("disposal_date", None) + self.set_consumed_asset_status(asset) + + if asset.calculate_depreciation: + reverse_depreciation_entry_made_after_disposal(asset, self.posting_date) + notes = _( + "This schedule was created when Asset {0} was restored on Asset Capitalization {1}'s cancellation." + ).format( + get_link_to_form(asset.doctype, asset.name), get_link_to_form(self.doctype, self.name) + ) + reset_depreciation_schedule(asset, self.posting_date, notes) def set_consumed_asset_status(self, asset): if self.docstatus == 1: @@ -602,33 +588,6 @@ def get_target_item_details(item_code=None, company=None): return out -@frappe.whitelist() -def get_target_asset_details(asset=None, company=None): - out = frappe._dict() - - # Get Asset Details - asset_details = frappe._dict() - if asset: - asset_details = frappe.db.get_value("Asset", asset, ["asset_name", "item_code"], as_dict=1) - if not asset_details: - frappe.throw(_("Asset {0} does not exist").format(asset)) - - # Re-set item code from Asset - out.target_item_code = asset_details.item_code - - # Set Asset Details - out.asset_name = asset_details.asset_name - - if asset_details.item_code: - out.target_fixed_asset_account = get_asset_category_account( - "fixed_asset_account", item=asset_details.item_code, company=company - ) - else: - out.target_fixed_asset_account = None - - return out - - @frappe.whitelist() def get_consumed_stock_item_details(args): if isinstance(args, str): diff --git a/erpnext/assets/doctype/asset_capitalization/test_asset_capitalization.py b/erpnext/assets/doctype/asset_capitalization/test_asset_capitalization.py index 5345d0e7f2b..6e0a6856f56 100644 --- a/erpnext/assets/doctype/asset_capitalization/test_asset_capitalization.py +++ b/erpnext/assets/doctype/asset_capitalization/test_asset_capitalization.py @@ -47,13 +47,6 @@ class TestAssetCapitalization(unittest.TestCase): total_amount = 103000 - # Create assets - target_asset = create_asset( - asset_name="Asset Capitalization Target Asset", - submit=1, - warehouse="Stores - TCP1", - company=company, - ) consumed_asset = create_asset( asset_name="Asset Capitalization Consumable Asset", asset_value=consumed_asset_value, @@ -65,7 +58,8 @@ class TestAssetCapitalization(unittest.TestCase): # Create and submit Asset Captitalization asset_capitalization = create_asset_capitalization( entry_type="Capitalization", - target_asset=target_asset.name, + target_item_code="Macbook Pro", + target_asset_location="Test Location", stock_qty=stock_qty, stock_rate=stock_rate, consumed_asset=consumed_asset.name, @@ -94,7 +88,7 @@ class TestAssetCapitalization(unittest.TestCase): self.assertEqual(asset_capitalization.target_incoming_rate, total_amount) # Test Target Asset values - target_asset.reload() + target_asset = frappe.get_doc("Asset", asset_capitalization.target_asset) self.assertEqual(target_asset.gross_purchase_amount, total_amount) self.assertEqual(target_asset.purchase_receipt_amount, total_amount) @@ -142,13 +136,6 @@ class TestAssetCapitalization(unittest.TestCase): total_amount = 103000 - # Create assets - target_asset = create_asset( - asset_name="Asset Capitalization Target Asset", - submit=1, - warehouse="Stores - _TC", - company=company, - ) consumed_asset = create_asset( asset_name="Asset Capitalization Consumable Asset", asset_value=consumed_asset_value, @@ -160,7 +147,8 @@ class TestAssetCapitalization(unittest.TestCase): # Create and submit Asset Captitalization asset_capitalization = create_asset_capitalization( entry_type="Capitalization", - target_asset=target_asset.name, + target_item_code="Macbook Pro", + target_asset_location="Test Location", stock_qty=stock_qty, stock_rate=stock_rate, consumed_asset=consumed_asset.name, @@ -189,7 +177,7 @@ class TestAssetCapitalization(unittest.TestCase): self.assertEqual(asset_capitalization.target_incoming_rate, total_amount) # Test Target Asset values - target_asset.reload() + target_asset = frappe.get_doc("Asset", asset_capitalization.target_asset) self.assertEqual(target_asset.gross_purchase_amount, total_amount) self.assertEqual(target_asset.purchase_receipt_amount, total_amount) @@ -364,6 +352,7 @@ def create_asset_capitalization(**args): "posting_time": args.posting_time or now.strftime("%H:%M:%S.%f"), "target_item_code": target_item_code, "target_asset": target_asset.name, + "target_asset_location": "Test Location", "target_warehouse": target_warehouse, "target_qty": flt(args.target_qty) or 1, "target_batch_no": args.target_batch_no, From 56e81ada56332b7dfb721610575ee1bffb2a97d3 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 22 Jun 2023 19:57:23 +0530 Subject: [PATCH 43/45] ci: use multiple python version in patch test --- .github/workflows/patch.yml | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/.github/workflows/patch.yml b/.github/workflows/patch.yml index d5f00527444..e2d89573cbf 100644 --- a/.github/workflows/patch.yml +++ b/.github/workflows/patch.yml @@ -43,9 +43,11 @@ jobs: fi - name: Setup Python - uses: "gabrielfalcao/pyenv-action@v9" + uses: "actions/setup-python@v4" with: - versions: 3.10:latest, 3.7:latest + python-version: | + 3.7 + 3.10 - name: Setup Node uses: actions/setup-node@v2 @@ -92,7 +94,6 @@ jobs: - name: Install run: | pip install frappe-bench - pyenv global $(pyenv versions | grep '3.10') bash ${GITHUB_WORKSPACE}/.github/helper/install.sh env: DB: mariadb @@ -107,7 +108,6 @@ jobs: git -C "apps/frappe" remote set-url upstream https://github.com/frappe/frappe.git git -C "apps/erpnext" remote set-url upstream https://github.com/frappe/erpnext.git - pyenv global $(pyenv versions | grep '3.7') for version in $(seq 12 13) do echo "Updating to v$version" @@ -120,7 +120,7 @@ jobs: git -C "apps/erpnext" checkout -q -f $branch_name rm -rf ~/frappe-bench/env - bench setup env + bench setup env --python python3.7 bench pip install -e ./apps/payments bench pip install -e ./apps/erpnext @@ -132,9 +132,8 @@ jobs: git -C "apps/frappe" checkout -q -f "${GITHUB_BASE_REF:-${GITHUB_REF##*/}}" git -C "apps/erpnext" checkout -q -f "$GITHUB_SHA" - pyenv global $(pyenv versions | grep '3.10') rm -rf ~/frappe-bench/env - bench -v setup env + bench -v setup env --python python3.10 bench pip install -e ./apps/payments bench pip install -e ./apps/erpnext From f37484c6fe6d95cd854befbf2f056a6d02244d5e Mon Sep 17 00:00:00 2001 From: Anand Baburajan Date: Thu, 22 Jun 2023 22:32:06 +0530 Subject: [PATCH 44/45] chore: better err msg on cancelling JE for asset scrap [dev] (#35850) chore: better err msg on cancelling JE for asset scrap --- .../doctype/journal_entry/journal_entry.js | 2 +- .../doctype/journal_entry/journal_entry.py | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.js b/erpnext/accounts/doctype/journal_entry/journal_entry.js index 6d9e3202f10..a51e38eefea 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.js +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.js @@ -8,7 +8,7 @@ frappe.provide("erpnext.journal_entry"); frappe.ui.form.on("Journal Entry", { setup: function(frm) { frm.add_fetch("bank_account", "account", "account"); - frm.ignore_doctypes_on_cancel_all = ['Sales Invoice', 'Purchase Invoice', 'Journal Entry', "Repost Payment Ledger", 'Asset Depreciation Schedule']; + frm.ignore_doctypes_on_cancel_all = ['Sales Invoice', 'Purchase Invoice', 'Journal Entry', "Repost Payment Ledger", 'Asset', 'Asset Movement', 'Asset Depreciation Schedule']; }, refresh: function(frm) { diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index 74fd5596123..83312dbd229 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -326,12 +326,10 @@ class JournalEntry(AccountsController): d.db_update() def unlink_asset_reference(self): - if self.voucher_type != "Depreciation Entry": - return - for d in self.get("accounts"): if ( - d.reference_type == "Asset" + self.voucher_type == "Depreciation Entry" + and d.reference_type == "Asset" and d.reference_name and d.account_type == "Depreciation" and d.debit @@ -370,6 +368,15 @@ class JournalEntry(AccountsController): else: asset.db_set("value_after_depreciation", asset.value_after_depreciation + d.debit) asset.set_status() + elif self.voucher_type == "Journal Entry" and d.reference_type == "Asset" and d.reference_name: + journal_entry_for_scrap = frappe.db.get_value( + "Asset", d.reference_name, "journal_entry_for_scrap" + ) + + if journal_entry_for_scrap == self.name: + frappe.throw( + _("Journal Entry for Asset scrapping cannot be cancelled. Please restore the Asset.") + ) def unlink_inter_company_jv(self): if ( From 9a993b0364fcee14a9887a5a7b91a23aa69701f1 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Fri, 23 Jun 2023 08:25:00 +0530 Subject: [PATCH 45/45] fix: show non-depreciable assets in fixed asset register (backport #35858) (#35860) fix: show non-depreciable assets in fixed asset register (#35858) fix: show non-depr assets in fixed asset register (cherry picked from commit 42d09448eed316268d9a5a2c9e25791bc48fa326) Co-authored-by: Anand Baburajan --- .../report/fixed_asset_register/fixed_asset_register.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/erpnext/assets/report/fixed_asset_register/fixed_asset_register.py b/erpnext/assets/report/fixed_asset_register/fixed_asset_register.py index f810819b4fc..6911f94bbbb 100644 --- a/erpnext/assets/report/fixed_asset_register/fixed_asset_register.py +++ b/erpnext/assets/report/fixed_asset_register/fixed_asset_register.py @@ -115,7 +115,11 @@ def get_data(filters): depreciation_amount_map = get_asset_depreciation_amount_map(filters, finance_book) for asset in assets_record: - if assets_linked_to_fb and asset.asset_id not in assets_linked_to_fb: + if ( + assets_linked_to_fb + and asset.calculate_depreciation + and asset.asset_id not in assets_linked_to_fb + ): continue asset_value = get_asset_value_after_depreciation(