From d7bc1928045d4e89e9acc61b6a71da25537b19a5 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 5 Apr 2023 15:28:47 +0530 Subject: [PATCH] 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 9707f79a2a..2a2aff1eef 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 25385a03ca..cf3a6e648a 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."""