From 0ded8501cf169d504edaf73e6c2b0a16e9bf2e26 Mon Sep 17 00:00:00 2001 From: Rohan Bansal Date: Thu, 2 May 2024 15:06:10 +0530 Subject: [PATCH 1/3] fix: allow nested accounts to be validated with accounting dimensions --- erpnext/accounts/general_ledger.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/general_ledger.py b/erpnext/accounts/general_ledger.py index 700d777f2515..4cfd6f1b47d6 100644 --- a/erpnext/accounts/general_ledger.py +++ b/erpnext/accounts/general_ledger.py @@ -8,6 +8,7 @@ from frappe import _ from frappe.model.meta import get_field_precision from frappe.utils import cint, flt, formatdate, getdate, now +from frappe.utils.nestedset import get_descendants_of import erpnext from erpnext.accounts.doctype.accounting_dimension.accounting_dimension import ( @@ -704,7 +705,12 @@ def validate_allowed_dimensions(gl_entry, dimension_filter_map): dimension = key[0] account = key[1] - if gl_entry.account == account: + if frappe.get_cached_value("Account", account, "is_group"): + accounts = get_descendants_of("Account", account) + else: + accounts = [account] + + if gl_entry.account in accounts: if value["is_mandatory"] and not gl_entry.get(dimension): frappe.throw( _("{0} is mandatory for account {1}").format( From 4f471b37f8e53172569c34f60072d047edfc927d Mon Sep 17 00:00:00 2001 From: Rohan Bansal Date: Thu, 30 May 2024 13:31:54 +0530 Subject: [PATCH 2/3] fix: update dimension map --- .../accounting_dimension_filter.py | 100 +++++++++++------- erpnext/accounts/general_ledger.py | 8 +- 2 files changed, 63 insertions(+), 45 deletions(-) diff --git a/erpnext/accounts/doctype/accounting_dimension_filter/accounting_dimension_filter.py b/erpnext/accounts/doctype/accounting_dimension_filter/accounting_dimension_filter.py index 2179a4d46c1a..87a22f29a7ee 100644 --- a/erpnext/accounts/doctype/accounting_dimension_filter/accounting_dimension_filter.py +++ b/erpnext/accounts/doctype/accounting_dimension_filter/accounting_dimension_filter.py @@ -1,10 +1,13 @@ # Copyright, (c) 2020, Frappe Technologies Pvt. Ltd. and contributors # For license information, please see license.txt +from typing import Any import frappe from frappe import _, scrub from frappe.model.document import Document +from frappe.query_builder import DocType +from frappe.utils.nestedset import get_descendants_of class AccountingDimensionFilter(Document): @@ -40,20 +43,21 @@ def validate(self): self.validate_applicable_accounts() def validate_applicable_accounts(self): - accounts = frappe.db.sql( - """ - SELECT a.applicable_on_account as account - FROM `tabApplicable On Account` a, `tabAccounting Dimension Filter` d - WHERE d.name = a.parent - and d.name != %s - and d.accounting_dimension = %s - """, - (self.name, self.accounting_dimension), - as_dict=1, + ApplicableOnAccount = DocType("Applicable On Account") + AccountingDimensionFilter = DocType("Accounting Dimension Filter") + + account_list = ( + frappe.qb.from_(ApplicableOnAccount) + .inner_join(AccountingDimensionFilter) + .on(AccountingDimensionFilter.name == ApplicableOnAccount.parent) + .select( + (ApplicableOnAccount.applicable_on_account).as_("account"), + ) + .where(AccountingDimensionFilter.name != self.name) + .where(AccountingDimensionFilter.accounting_dimension == self.accounting_dimension) + .run(as_list=True, pluck="account") ) - account_list = [d.account for d in accounts] - for account in self.get("accounts"): if account.applicable_on_account in account_list: frappe.throw( @@ -65,46 +69,66 @@ def validate_applicable_accounts(self): ) -def get_dimension_filter_map(): - if not frappe.flags.get("dimension_filter_map"): - filters = frappe.db.sql( - """ - SELECT - a.applicable_on_account, d.dimension_value, p.accounting_dimension, - p.allow_or_restrict, a.is_mandatory - FROM - `tabApplicable On Account` a, - `tabAccounting Dimension Filter` p - LEFT JOIN `tabAllowed Dimension` d ON d.parent = p.name - WHERE - p.name = a.parent - AND p.disabled = 0 - """, - as_dict=1, +def get_dimension_filter_map() -> dict[tuple[str, str], dict[str, Any]]: + if frappe.flags.get("dimension_filter_map"): + return frappe.flags.dimension_filter_map + + ApplicableOnAccount = DocType("Applicable On Account") + AllowedDimension = DocType("Allowed Dimension") + AccountingDimensionFilter = DocType("Accounting Dimension Filter") + + filters = ( + frappe.qb.from_(ApplicableOnAccount) + .inner_join(AccountingDimensionFilter) + .on(AccountingDimensionFilter.name == ApplicableOnAccount.parent) + .inner_join(AllowedDimension) + .on(AccountingDimensionFilter.name == AllowedDimension.parent) + .select( + ApplicableOnAccount.applicable_on_account, + ApplicableOnAccount.is_mandatory, + AllowedDimension.dimension_value, + AccountingDimensionFilter.accounting_dimension, + AccountingDimensionFilter.allow_or_restrict, ) + .run(as_dict=True) + ) - dimension_filter_map = {} - - for f in filters: - f.fieldname = scrub(f.accounting_dimension) + dimension_filter_map = {} + for f in filters: + f.fieldname = scrub(f.accounting_dimension) + if frappe.get_cached_value("Account", f.applicable_on_account, "is_group"): + accounts = get_descendants_of("Account", f.applicable_on_account) + else: + accounts = [f.applicable_on_account] + for account in accounts: build_map( dimension_filter_map, f.fieldname, - f.applicable_on_account, + account, f.dimension_value, f.allow_or_restrict, f.is_mandatory, ) - frappe.flags.dimension_filter_map = dimension_filter_map + frappe.flags.dimension_filter_map = dimension_filter_map return frappe.flags.dimension_filter_map -def build_map(map_object, dimension, account, filter_value, allow_or_restrict, is_mandatory): +def build_map( + map_object: dict[tuple[str, str], dict[str, Any]], + dimension: str, + account: str, + filter_value: str, + allow_or_restrict: str, + is_mandatory: bool, +): map_object.setdefault( (dimension, account), - {"allowed_dimensions": [], "is_mandatory": is_mandatory, "allow_or_restrict": allow_or_restrict}, + { + "allowed_dimensions": [], + "is_mandatory": is_mandatory, + "allow_or_restrict": allow_or_restrict, + }, ) - if filter_value: - map_object[(dimension, account)]["allowed_dimensions"].append(filter_value) + map_object[(dimension, account)]["allowed_dimensions"].append(filter_value) diff --git a/erpnext/accounts/general_ledger.py b/erpnext/accounts/general_ledger.py index 1b1ecc88b188..1590722e0bf9 100644 --- a/erpnext/accounts/general_ledger.py +++ b/erpnext/accounts/general_ledger.py @@ -9,7 +9,6 @@ from frappe.model.meta import get_field_precision from frappe.utils import cint, flt, formatdate, getdate, now from frappe.utils.dashboard import cache_source -from frappe.utils.nestedset import get_descendants_of import erpnext from erpnext.accounts.doctype.accounting_dimension.accounting_dimension import ( @@ -722,12 +721,7 @@ def validate_allowed_dimensions(gl_entry, dimension_filter_map): dimension = key[0] account = key[1] - if frappe.get_cached_value("Account", account, "is_group"): - accounts = get_descendants_of("Account", account) - else: - accounts = [account] - - if gl_entry.account in accounts: + if gl_entry.account == account: if value["is_mandatory"] and not gl_entry.get(dimension): frappe.throw( _("{0} is mandatory for account {1}").format( From aea10e11705d1eee2ccce16e8c210b621f2db7f4 Mon Sep 17 00:00:00 2001 From: Rohan Bansal Date: Mon, 24 Jun 2024 15:34:33 +0530 Subject: [PATCH 3/3] fix: only apply filter value if it exists --- .../accounting_dimension_filter.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/erpnext/accounts/doctype/accounting_dimension_filter/accounting_dimension_filter.py b/erpnext/accounts/doctype/accounting_dimension_filter/accounting_dimension_filter.py index 87a22f29a7ee..26b38d44dc86 100644 --- a/erpnext/accounts/doctype/accounting_dimension_filter/accounting_dimension_filter.py +++ b/erpnext/accounts/doctype/accounting_dimension_filter/accounting_dimension_filter.py @@ -1,7 +1,7 @@ # Copyright, (c) 2020, Frappe Technologies Pvt. Ltd. and contributors # For license information, please see license.txt -from typing import Any +from typing import Any, Literal import frappe from frappe import _, scrub @@ -101,6 +101,7 @@ def get_dimension_filter_map() -> dict[tuple[str, str], dict[str, Any]]: accounts = get_descendants_of("Account", f.applicable_on_account) else: accounts = [f.applicable_on_account] + for account in accounts: build_map( dimension_filter_map, @@ -120,7 +121,7 @@ def build_map( dimension: str, account: str, filter_value: str, - allow_or_restrict: str, + allow_or_restrict: Literal["Allow", "Restrict"], is_mandatory: bool, ): map_object.setdefault( @@ -131,4 +132,6 @@ def build_map( "allow_or_restrict": allow_or_restrict, }, ) - map_object[(dimension, account)]["allowed_dimensions"].append(filter_value) + + if filter_value: + map_object[(dimension, account)]["allowed_dimensions"].append(filter_value)