nsinnovations/erpnext
2
0
Fork 0
mirror of https://github.com/frappe/erpnext.git synced 2026-05-27 08:54:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

chore: Remove Bank Party Mapper implementation

Browse Source 
- 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
This commit is contained in:
marination
2023-06-06 18:59:07 +05:30
parent 09872301bd
commit 752a92bd8b
9 changed files with 37 additions and 347 deletions
Download Patch File Download Diff File Expand all files Collapse all files

0
erpnext/accounts/doctype/bank_party_mapper/__init__.py
View File

10
erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.js
View File

@@ -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."));
}
},
});

80
erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.json
View File

@@ -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
}

24
erpnext/accounts/doctype/bank_party_mapper/bank_party_mapper.py
View File

@@ -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()

9
erpnext/accounts/doctype/bank_party_mapper/test_bank_party_mapper.py
View File

@@ -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

135
erpnext/accounts/doctype/bank_transaction/auto_match_party.py
View File

@@ -10,22 +10,7 @@ class AutoMatchParty:
Matches by Account/IBAN and then by Party Name/Description sequentially. Matches by Account/IBAN and then by Party Name/Description sequentially.
Returns when a result is obtained. Returns when a result is obtained.
Result (if present) is of the form: (Party Type, Party, Mapper,) Result (if present) is of the form: (Party Type, Party,)
Mapper(if present) is one of the forms:
1. {"mapper_name": <docname>}: 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": <ACC No.>, "bank_party_iban": <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": <Counter 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: def __init__(self, **kwargs) -> None:
@@ -44,7 +29,7 @@ class AutoMatchParty:
fuzzy_matching_enabled = frappe.db.get_single_value("Accounts Settings", "enable_fuzzy_matching") fuzzy_matching_enabled = frappe.db.get_single_value("Accounts Settings", "enable_fuzzy_matching")
if not result and fuzzy_matching_enabled: if not result and fuzzy_matching_enabled:
result = AutoMatchbyPartyDescription( result = AutoMatchbyPartyNameDescription(
bank_party_name=self.bank_party_name, description=self.description, deposit=self.deposit bank_party_name=self.bank_party_name, description=self.description, deposit=self.deposit
).match() ).match()
@@ -62,50 +47,16 @@ class AutoMatchbyAccountIBAN:
if not (self.bank_party_account_number or self.bank_party_iban): if not (self.bank_party_account_number or self.bank_party_iban):
return None return None
result = self.match_account_in_bank_party_mapper() result = self.match_account_in_party()
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"]})
return result return result
def match_account_in_party(self) -> Union[Tuple, None]: def match_account_in_party(self) -> Union[Tuple, None]:
"""Check if there is a IBAN/Account No. match in Customer/Supplier/Employee""" """Check if there is a IBAN/Account No. match in Customer/Supplier/Employee"""
result = None result = None
parties = get_parties_in_order(self.deposit)
parties = ["Supplier", "Employee", "Customer"] # most -> least likely to receive or_filters = self.get_or_filters()
if flt(self.deposit) > 0:
parties = ["Customer", "Supplier", "Employee"] # most -> least likely to pay
for party in parties: 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( party_result = frappe.db.get_all(
"Bank Account", or_filters=or_filters, pluck="party", limit_page_length=1 "Bank Account", or_filters=or_filters, pluck="party", limit_page_length=1
) )
@@ -123,17 +74,23 @@ class AutoMatchbyAccountIBAN:
result = ( result = (
party, party,
party_result[0], party_result[0],
{
"bank_party_account_number": self.get("bank_party_account_number"),
"bank_party_iban": self.get("bank_party_iban"),
},
) )
break break
return result 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: def __init__(self, **kwargs) -> None:
self.__dict__.update(kwargs) self.__dict__.update(kwargs)
@@ -141,52 +98,21 @@ class AutoMatchbyPartyDescription:
return self.__dict__.get(key, None) return self.__dict__.get(key, None)
def match(self) -> Union[Tuple, 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 # fuzzy search by customer/supplier & employee
if not (self.bank_party_name or self.description): if not (self.bank_party_name or self.description):
return None return None
result = self.match_party_name_in_bank_party_mapper() result = self.match_party_name_desc_in_party()
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"]},
)
return result return result
def match_party_name_desc_in_party(self) -> Union[Tuple, None]: def match_party_name_desc_in_party(self) -> Union[Tuple, None]:
"""Fuzzy search party name and/or description against parties in the system""" """Fuzzy search party name and/or description against parties in the system"""
result = None result = None
parties = ["Supplier", "Employee", "Customer"] # most-least likely to receive parties = get_parties_in_order(self.deposit)
if flt(self.deposit) > 0.0:
parties = ["Customer", "Supplier", "Employee"] # most-least likely to pay
for party in parties: for party in parties:
name_field = party.lower() + "_name"
filters = {"status": "Active"} if party == "Employee" else {"disabled": 0} 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"]: for field in ["bank_party_name", "description"]:
if not self.get(field): if not self.get(field):
@@ -197,8 +123,7 @@ class AutoMatchbyPartyDescription:
break break
if result or skip: if result or skip:
# We skip if: # Skip If: It was hard to distinguish between close matches and so match is None
# If it was hard to distinguish between close matches and so match is None
# OR if the right match was found # OR if the right match was found
break break
@@ -206,19 +131,15 @@ class AutoMatchbyPartyDescription:
def fuzzy_search_and_return_result(self, party, names, field) -> Union[Tuple, None]: def fuzzy_search_and_return_result(self, party, names, field) -> Union[Tuple, None]:
skip = False skip = False
result = process.extract(query=self.get(field), choices=names, scorer=fuzz.token_set_ratio) result = process.extract(query=self.get(field), choices=names, scorer=fuzz.token_set_ratio)
party_name, skip = self.process_fuzzy_result(result) party_name, skip = self.process_fuzzy_result(result)
if not party_name: if not party_name:
return None, skip 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 ( return (
party, party,
party_name, party_name,
mapper,
), skip ), skip
def process_fuzzy_result(self, result: Union[list, None]): 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. If there are multiple valid close matches return None as result may be faulty.
Return the result only if one accurate match stands out. 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 PARTY, SCORE, CUTOFF = 0, 1, 80
@@ -234,18 +155,24 @@ class AutoMatchbyPartyDescription:
return None, False return None, False
first_result = result[0] first_result = result[0]
if len(result) == 1: 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] second_result = result[1]
if first_result[SCORE] > CUTOFF: if first_result[SCORE] > CUTOFF:
# If multiple matches with the same score, return None but discontinue matching # 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]: if first_result[SCORE] == second_result[SCORE]:
return None, True return None, True
return first_result[PARTY], True return first_result[PARTY], True
else: else:
return None, False 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

13
erpnext/accounts/doctype/bank_transaction/bank_transaction.json
View File

@@ -37,8 +37,7 @@
"column_break_3czf", "column_break_3czf",
"bank_party_name", "bank_party_name",
"bank_party_account_number", "bank_party_account_number",
"bank_party_iban", "bank_party_iban"
"bank_party_mapper"
], ],
"fields": [ "fields": [
{ {
@@ -226,19 +225,11 @@
"fieldname": "bank_party_account_number", "fieldname": "bank_party_account_number",
"fieldtype": "Data", "fieldtype": "Data",
"label": "Party Account No. (Bank Statement)" "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, "is_submittable": 1,
"links": [], "links": [],
"modified": "2023-05-17 14:56:10.547480", "modified": "2023-06-06 13:58:12.821411",
"modified_by": "Administrator", "modified_by": "Administrator",
"module": "Accounts", "module": "Accounts",
"name": "Bank Transaction", "name": "Bank Transaction",

34
erpnext/accounts/doctype/bank_transaction/bank_transaction.py
View File

@@ -29,9 +29,6 @@ class BankTransaction(StatusUpdater):
self.update_allocations() self.update_allocations()
self._saving_flag = False 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): def on_cancel(self):
self.clear_linked_payment_entries(for_cancel=True) self.clear_linked_payment_entries(for_cancel=True)
self.set_status(update=True) self.set_status(update=True)
@@ -167,33 +164,10 @@ class BankTransaction(StatusUpdater):
).match() ).match()
if result: if result:
party_type, party, mapper = result party_type, party = result
to_update = {"party_type": party_type, "party": party} frappe.db.set_value(
"Bank Transaction", self.name, field={"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()
@frappe.whitelist() @frappe.whitelist()

79
erpnext/accounts/doctype/bank_transaction/test_auto_match_party.py
View File

@@ -22,7 +22,6 @@ class TestAutoMatchParty(FrappeTestCase):
frappe.db.set_single_value("Accounts Settings", "enable_fuzzy_matching", 0) frappe.db.set_single_value("Accounts Settings", "enable_fuzzy_matching", 0)
def test_match_by_account_number(self): 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") create_supplier_for_match(account_no="000000003716541159")
doc = create_bank_transaction( doc = create_bank_transaction(
withdrawal=1200, withdrawal=1200,
@@ -33,22 +32,6 @@ class TestAutoMatchParty(FrappeTestCase):
self.assertEqual(doc.party_type, "Supplier") self.assertEqual(doc.party_type, "Supplier")
self.assertEqual(doc.party, "John Doe & Co.") 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): def test_match_by_iban(self):
create_supplier_for_match(iban="DE02000000003716541159") create_supplier_for_match(iban="DE02000000003716541159")
@@ -61,12 +44,6 @@ class TestAutoMatchParty(FrappeTestCase):
self.assertEqual(doc.party_type, "Supplier") self.assertEqual(doc.party_type, "Supplier")
self.assertEqual(doc.party, "John Doe & Co.") 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): def test_match_by_party_name(self):
create_supplier_for_match(supplier_name="Jackson Ella W.") 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_type, "Supplier")
self.assertEqual(doc.party, "Jackson Ella W.") 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): def test_match_by_description(self):
create_supplier_for_match(supplier_name="Microsoft") create_supplier_for_match(supplier_name="Microsoft")
@@ -107,46 +68,6 @@ class TestAutoMatchParty(FrappeTestCase):
self.assertEqual(doc.party, "Microsoft") self.assertEqual(doc.party, "Microsoft")
self.assertFalse(doc.bank_party_mapper) 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): 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 & General Stores")
create_supplier_for_match(supplier_name="Adithya Medical And General Stores") create_supplier_for_match(supplier_name="Adithya Medical And General Stores")