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
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 e69de29..0000000
--- a/erpnext/accounts/doctype/bank_party_mapper/__init__.py
+++ /dev/null
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 b13e46a..0000000
--- 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 ddc79f2..0000000
--- 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 02e36b0..0000000
--- 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 c05b23f..0000000
--- 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 753f0c1..5d94a08 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 @@
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": <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.
+ Result (if present) is of the form: (Party Type, Party,)
"""
def __init__(self, **kwargs) -> None:
@@ -44,7 +29,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 e7de71a..b32022e 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 04c3013..f82337f 100644
--- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py
+++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py
@@ -29,9 +29,6 @@
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 @@
).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 8c6dc9d..a498545 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 @@
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 @@
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 @@
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 @@
)
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 @@
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")