From d53b1978962f7e5baa1c5069bc624dc558705725 Mon Sep 17 00:00:00 2001 From: Marica Date: Sat, 24 Jun 2023 12:30:08 +0530 Subject: [PATCH] feat: Auto set Party in Bank Transaction (#34675) * 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 * chore: Single query with or filter to search Party Mapper by name/desc * feat: Store Party bank details in party records (Customer/Supplier/Employee/Shareholder) * 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. * 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 * chore: Perform automatch on submit - misc: Clearer naming * chore: Make auto matching party configurable - Checkbox in Accounts settings "Enable Automatic Party Matching" - Check before invoking automatching methods - misc: Remove TODO comments * 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 * test: Match by Account No, IBAN, Party Name, Desc and match correction * fix: Remove bank details fields from Shareholder * 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 * fix: Tests * 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 * 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 * chore: Remove instances of `bank_party_mapper` and use `new_doc` --- .../accounts_settings/accounts_settings.json | 27 ++- .../bank_transaction/auto_match_party.py | 178 ++++++++++++++++++ .../bank_transaction/bank_transaction.json | 31 ++- .../bank_transaction/bank_transaction.py | 23 +++ .../bank_transaction/test_auto_match_party.py | 151 +++++++++++++++ .../doctype/shareholder/shareholder.json | 7 +- erpnext/buying/doctype/supplier/supplier.json | 2 +- .../selling/doctype/customer/customer.json | 2 +- erpnext/setup/doctype/employee/employee.json | 15 +- pyproject.toml | 1 + 10 files changed, 425 insertions(+), 12 deletions(-) create mode 100644 erpnext/accounts/doctype/bank_transaction/auto_match_party.py create mode 100644 erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py diff --git a/erpnext/accounts/doctype/accounts_settings/accounts_settings.json b/erpnext/accounts/doctype/accounts_settings/accounts_settings.json index 6c99d29dbfc..47c4396e102 100644 --- a/erpnext/accounts/doctype/accounts_settings/accounts_settings.json +++ b/erpnext/accounts/doctype/accounts_settings/accounts_settings.json @@ -62,7 +62,10 @@ "acc_frozen_upto", "column_break_25", "frozen_accounts_modifier", - "report_settings_sb" + "report_settings_sb", + "banking_tab", + "enable_party_matching", + "enable_fuzzy_matching" ], "fields": [ { @@ -385,6 +388,26 @@ "fieldname": "show_taxes_as_table_in_print", "fieldtype": "Check", "label": "Show Taxes as Table in Print" + }, + { + "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" + }, + { + "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", @@ -392,7 +415,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2023-06-13 18:47:46.430291", + "modified": "2023-06-15 18:47:46.430291", "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 new file mode 100644 index 00000000000..5d94a08f2f0 --- /dev/null +++ b/erpnext/accounts/doctype/bank_transaction/auto_match_party.py @@ -0,0 +1,178 @@ +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,) + """ + + def __init__(self, **kwargs) -> None: + self.__dict__.update(kwargs) + + def get(self, key): + 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() + + fuzzy_matching_enabled = frappe.db.get_single_value("Accounts Settings", "enable_fuzzy_matching") + if not result and fuzzy_matching_enabled: + result = AutoMatchbyPartyNameDescription( + 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_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 = get_parties_in_order(self.deposit) + or_filters = self.get_or_filters() + + for party in parties: + party_result = frappe.db.get_all( + "Bank Account", or_filters=or_filters, pluck="party", 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, + party_result[0], + ) + 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 + + 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) + + def get(self, key): + return self.__dict__.get(key, None) + + def match(self) -> Union[Tuple, None]: + # fuzzy search by customer/supplier & employee + if not (self.bank_party_name or self.description): + return None + + 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 = get_parties_in_order(self.deposit) + + for party in parties: + filters = {"status": "Active"} if party == "Employee" else {"disabled": 0} + names = frappe.get_all(party, filters=filters, pluck=party.lower() + "_name") + + for field in ["bank_party_name", "description"]: + 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: + # Skip 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]: + 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 + + return ( + party, + party_name, + ), 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 discontinue 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 (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 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 768d2f0fa45..bb7a4771b2d 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_account_number", + "bank_party_iban" ], "fields": [ { @@ -63,7 +67,7 @@ "fieldtype": "Select", "in_standard_filter": 1, "label": "Status", - "options": "\nPending\nSettled\nUnreconciled\nReconciled" + "options": "\nPending\nSettled\nUnreconciled\nReconciled\nCancelled" }, { "fieldname": "bank_account", @@ -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/Account Holder (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": "2022-05-29 18:36:50.475964", + "modified": "2023-06-06 13:58:12.821411", "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 b441af96600..f82337fbd77 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py @@ -15,6 +15,9 @@ class BankTransaction(StatusUpdater): self.clear_linked_payment_entries() self.set_status() + if frappe.db.get_single_value("Accounts Settings", "enable_party_matching"): + self.auto_set_party() + _saving_flag = False # nosemgrep: frappe-semgrep-rules.rules.frappe-modifying-but-not-comitting @@ -146,6 +149,26 @@ class BankTransaction(StatusUpdater): payment_entry.payment_document, payment_entry.payment_entry, clearance_date, self ) + def auto_set_party(self): + 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, + bank_party_name=self.bank_party_name, + description=self.description, + deposit=self.deposit, + ).match() + + if result: + party_type, party = result + frappe.db.set_value( + "Bank Transaction", self.name, field={"party_type": party_type, "party": party} + ) + @frappe.whitelist() def get_doctypes_for_bank_reconciliation(): 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..36ef1fca074 --- /dev/null +++ b/erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py @@ -0,0 +1,151 @@ +# 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) + 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): + 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.") + + 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.") + + 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.") + + 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") + + 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) + + +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 + + # Create Supplier and Bank Account for the same + 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"): + 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"): + 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( + description=None, + withdrawal=0, + deposit=0, + transaction_id=None, + party_name=None, + account_no=None, + iban=None, +): + doc = frappe.new_doc("Bank Transaction") + doc.update( + { + "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, + } + ) + doc.insert() + doc.submit() + doc.reload() + + return doc diff --git a/erpnext/accounts/doctype/shareholder/shareholder.json b/erpnext/accounts/doctype/shareholder/shareholder.json index e94aea94b75..e80b05720e0 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", @@ -111,11 +112,12 @@ "read_only": 1 } ], - "modified": "2019-11-17 23:24:11.395882", + "links": [], + "modified": "2023-04-10 22:02:20.406087", "modified_by": "Administrator", "module": "Accounts", "name": "Shareholder", - "name_case": "Title Case", + "naming_rule": "By \"Naming Series\" field", "owner": "Administrator", "permissions": [ { @@ -158,6 +160,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 66eafe9547a..3aca21ed6a6 100644 --- a/erpnext/buying/doctype/supplier/supplier.json +++ b/erpnext/buying/doctype/supplier/supplier.json @@ -457,7 +457,7 @@ "link_fieldname": "party" } ], - "modified": "2022-11-09 18:02:59.075203", + "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 7482a33653c..18bb4b9bb46 100644 --- a/erpnext/selling/doctype/customer/customer.json +++ b/erpnext/selling/doctype/customer/customer.json @@ -568,7 +568,7 @@ "link_fieldname": "party" } ], - "modified": "2022-11-08 15:52:34.462657", + "modified": "2023-05-09 15:38:40.255193", "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": [ diff --git a/pyproject.toml b/pyproject.toml index 5acfd392726..9b48e6b9668 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -13,6 +13,7 @@ dependencies = [ "python-stdnum~=1.16", "Unidecode~=1.2.0", "redisearch~=2.1.0", + "rapidfuzz~=2.15.0", # integration dependencies "gocardless-pro~=1.22.0",