diff --git a/erpnext/accounts/general_ledger.py b/erpnext/accounts/general_ledger.py index 9effa1a09c5..2c793765946 100644 --- a/erpnext/accounts/general_ledger.py +++ b/erpnext/accounts/general_ledger.py @@ -7,7 +7,7 @@ import copy import frappe from frappe import _ from frappe.model.meta import get_field_precision -from frappe.utils import cint, flt, formatdate, get_link_to_form, getdate, now +from frappe.utils import cint, flt, get_link_to_form, getdate, now from frappe.utils.caching import request_cache import erpnext @@ -18,11 +18,17 @@ from erpnext.accounts.doctype.accounting_dimension.accounting_dimension import ( from erpnext.accounts.doctype.accounting_dimension_filter.accounting_dimension_filter import ( get_dimension_filter_map, ) -from erpnext.accounts.doctype.accounting_period.accounting_period import ClosedAccountingPeriod from erpnext.accounts.doctype.budget.budget import validate_expense_against_budget +from erpnext.accounts.services.gl_validator import ( + check_freezing_date, + validate_accounting_period, + validate_against_pcv, + validate_allowed_dimensions, + validate_cwip_accounts, + validate_disabled_accounts, +) from erpnext.accounts.utils import create_payment_ledger_entry, is_immutable_ledger_enabled from erpnext.controllers.budget_controller import BudgetValidation -from erpnext.exceptions import InvalidAccountDimensionError, MandatoryAccountDimensionError def make_gl_entries( @@ -132,60 +138,6 @@ def get_accounting_dimensions_for_offsetting_entry(gl_map, company): return accounting_dimensions_to_offset -def validate_disabled_accounts(gl_map): - accounts = [d.account for d in gl_map if d.account] - - disabled_accounts = frappe.get_all( - "Account", - filters={"disabled": 1, "is_group": 0, "company": gl_map[0].company}, - fields=["name"], - ) - - used_disabled_accounts = set(accounts).intersection(set([d.name for d in disabled_accounts])) - if used_disabled_accounts: - account_list = "
" - account_list += ", ".join([frappe.bold(d) for d in used_disabled_accounts]) - frappe.throw( - _("Cannot create accounting entries against disabled accounts: {0}").format(account_list), - title=_("Disabled Account Selected"), - ) - - -def validate_accounting_period(gl_map): - accounting_periods = frappe.db.sql( - """ SELECT - ap.name as name, ap.exempted_role as exempted_role - FROM - `tabAccounting Period` ap, `tabClosed Document` cd - WHERE - ap.name = cd.parent - AND ap.company = %(company)s - AND ap.disabled = 0 - AND cd.closed = 1 - AND cd.document_type = %(voucher_type)s - AND %(date)s between ap.start_date and ap.end_date - """, - { - "date": gl_map[0].posting_date, - "company": gl_map[0].company, - "voucher_type": gl_map[0].voucher_type, - }, - as_dict=1, - ) - - if accounting_periods: - if accounting_periods[0].exempted_role: - exempted_roles = accounting_periods[0].exempted_role - if exempted_roles in frappe.get_roles(): - return - frappe.throw( - _( - "You cannot create or cancel any accounting entries with in the closed Accounting Period {0}" - ).format(frappe.bold(accounting_periods[0].name)), - ClosedAccountingPeriod, - ) - - def process_gl_map(gl_map, merge_entries=True, precision=None, from_repost=False): if not gl_map: return [] @@ -442,33 +394,6 @@ def make_entry(args, adv_adj, update_outstanding, from_repost=False): validate_expense_against_budget(args) -def validate_cwip_accounts(gl_map): - """Validate that CWIP account are not used in Journal Entry""" - if gl_map and gl_map[0].voucher_type != "Journal Entry": - return - - cwip_enabled = any( - cint(ac.enable_cwip_accounting) - for ac in frappe.db.get_all("Asset Category", "enable_cwip_accounting") - ) - if cwip_enabled: - cwip_accounts = [ - d[0] - for d in frappe.db.sql( - """select name from tabAccount - where account_type = 'Capital Work in Progress' and is_group=0""" - ) - ] - - for entry in gl_map: - if entry.account in cwip_accounts: - frappe.throw( - _( - "Account: {0} is capital Work in progress and can not be updated by Journal Entry" - ).format(entry.account) - ) - - def process_debit_credit_difference(gl_map): precision = get_field_precision( frappe.get_meta("GL Entry").get_field("debit"), @@ -796,48 +721,6 @@ def make_reverse_gl_entries( make_entry(new_gle, adv_adj, "Yes") -def check_freezing_date(posting_date, company, adv_adj=False): - """ - Nobody can do GL Entries where posting date is before freezing date - except authorized person - - Administrator has all the roles so this check will be bypassed if any role is allowed to post - Hence stop admin to bypass if accounts are freezed - """ - if not adv_adj: - acc_frozen_till_date = frappe.db.get_value("Company", company, "accounts_frozen_till_date") - if acc_frozen_till_date: - frozen_accounts_modifier = frappe.db.get_value( - "Company", company, "role_allowed_for_frozen_entries" - ) - if getdate(posting_date) <= getdate(acc_frozen_till_date) and ( - frozen_accounts_modifier not in frappe.get_roles() or frappe.session.user == "Administrator" - ): - frappe.throw( - _("You are not authorized to add or update entries before {0}").format( - formatdate(acc_frozen_till_date) - ) - ) - - -def validate_against_pcv(is_opening, posting_date, company): - if is_opening and frappe.db.exists("Period Closing Voucher", {"docstatus": 1, "company": company}): - frappe.throw( - _("Opening Entry can not be created after Period Closing Voucher is created."), - title=_("Invalid Opening Entry"), - ) - - last_pcv_date = frappe.db.get_value( - "Period Closing Voucher", {"docstatus": 1, "company": company}, [{"MAX": "period_end_date"}] - ) - - if last_pcv_date and getdate(posting_date) <= getdate(last_pcv_date): - message = _("Books have been closed till the period ending on {0}").format(formatdate(last_pcv_date)) - message += "
" - message += _("You cannot create/amend any accounting entries till this date.") - frappe.throw(message, title=_("Period Closed")) - - def set_as_cancel(voucher_type, voucher_no): """ Set is_cancelled=1 in all original gl entries for the voucher @@ -848,39 +731,3 @@ def set_as_cancel(voucher_type, voucher_no): where voucher_type=%s and voucher_no=%s and is_cancelled = 0""", (now(), frappe.session.user, voucher_type, voucher_no), ) - - -def validate_allowed_dimensions(gl_entry, dimension_filter_map): - for key, value in dimension_filter_map.items(): - dimension = key[0] - account = key[1] - - if gl_entry.account == account: - if value["is_mandatory"] and not gl_entry.get(dimension): - frappe.throw( - _("{0} is mandatory for account {1}").format( - frappe.bold(frappe.unscrub(dimension)), frappe.bold(gl_entry.account) - ), - MandatoryAccountDimensionError, - ) - - if value["allow_or_restrict"] == "Allow": - if gl_entry.get(dimension) and gl_entry.get(dimension) not in value["allowed_dimensions"]: - frappe.throw( - _("Invalid value {0} for {1} against account {2}").format( - frappe.bold(gl_entry.get(dimension)), - frappe.bold(frappe.unscrub(dimension)), - frappe.bold(gl_entry.account), - ), - InvalidAccountDimensionError, - ) - else: - if gl_entry.get(dimension) and gl_entry.get(dimension) in value["allowed_dimensions"]: - frappe.throw( - _("Invalid value {0} for {1} against account {2}").format( - frappe.bold(gl_entry.get(dimension)), - frappe.bold(frappe.unscrub(dimension)), - frappe.bold(gl_entry.account), - ), - InvalidAccountDimensionError, - ) diff --git a/erpnext/accounts/services/__init__.py b/erpnext/accounts/services/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/erpnext/accounts/services/gl_validator.py b/erpnext/accounts/services/gl_validator.py new file mode 100644 index 00000000000..e29b4d42103 --- /dev/null +++ b/erpnext/accounts/services/gl_validator.py @@ -0,0 +1,176 @@ +# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + +"""List-level validations for a GL map. + +These functions assert that an assembled list of GL entries is legal to post — +no disabled accounts, the period/freeze/PCV gates pass, dimensions are allowed. +They do not mutate or repair the entries; balancing and round-off live with the +posting sink in ``erpnext.accounts.general_ledger``. +""" + +import frappe +from frappe import _ +from frappe.utils import cint, formatdate, getdate + +from erpnext.accounts.doctype.accounting_period.accounting_period import ClosedAccountingPeriod +from erpnext.exceptions import InvalidAccountDimensionError, MandatoryAccountDimensionError + + +def validate_disabled_accounts(gl_map): + accounts = [d.account for d in gl_map if d.account] + + disabled_accounts = frappe.get_all( + "Account", + filters={"disabled": 1, "is_group": 0, "company": gl_map[0].company}, + fields=["name"], + ) + + used_disabled_accounts = set(accounts).intersection(set([d.name for d in disabled_accounts])) + if used_disabled_accounts: + account_list = "
" + account_list += ", ".join([frappe.bold(d) for d in used_disabled_accounts]) + frappe.throw( + _("Cannot create accounting entries against disabled accounts: {0}").format(account_list), + title=_("Disabled Account Selected"), + ) + + +def validate_accounting_period(gl_map): + accounting_periods = frappe.db.sql( + """ SELECT + ap.name as name, ap.exempted_role as exempted_role + FROM + `tabAccounting Period` ap, `tabClosed Document` cd + WHERE + ap.name = cd.parent + AND ap.company = %(company)s + AND ap.disabled = 0 + AND cd.closed = 1 + AND cd.document_type = %(voucher_type)s + AND %(date)s between ap.start_date and ap.end_date + """, + { + "date": gl_map[0].posting_date, + "company": gl_map[0].company, + "voucher_type": gl_map[0].voucher_type, + }, + as_dict=1, + ) + + if accounting_periods: + if accounting_periods[0].exempted_role: + exempted_roles = accounting_periods[0].exempted_role + if exempted_roles in frappe.get_roles(): + return + frappe.throw( + _( + "You cannot create or cancel any accounting entries with in the closed Accounting Period {0}" + ).format(frappe.bold(accounting_periods[0].name)), + ClosedAccountingPeriod, + ) + + +def validate_cwip_accounts(gl_map): + """Validate that CWIP account are not used in Journal Entry""" + if gl_map and gl_map[0].voucher_type != "Journal Entry": + return + + cwip_enabled = any( + cint(ac.enable_cwip_accounting) + for ac in frappe.db.get_all("Asset Category", "enable_cwip_accounting") + ) + if cwip_enabled: + cwip_accounts = [ + d[0] + for d in frappe.db.sql( + """select name from tabAccount + where account_type = 'Capital Work in Progress' and is_group=0""" + ) + ] + + for entry in gl_map: + if entry.account in cwip_accounts: + frappe.throw( + _( + "Account: {0} is capital Work in progress and can not be updated by Journal Entry" + ).format(entry.account) + ) + + +def check_freezing_date(posting_date, company, adv_adj=False): + """ + Nobody can do GL Entries where posting date is before freezing date + except authorized person + + Administrator has all the roles so this check will be bypassed if any role is allowed to post + Hence stop admin to bypass if accounts are freezed + """ + if not adv_adj: + acc_frozen_till_date = frappe.db.get_value("Company", company, "accounts_frozen_till_date") + if acc_frozen_till_date: + frozen_accounts_modifier = frappe.db.get_value( + "Company", company, "role_allowed_for_frozen_entries" + ) + if getdate(posting_date) <= getdate(acc_frozen_till_date) and ( + frozen_accounts_modifier not in frappe.get_roles() or frappe.session.user == "Administrator" + ): + frappe.throw( + _("You are not authorized to add or update entries before {0}").format( + formatdate(acc_frozen_till_date) + ) + ) + + +def validate_against_pcv(is_opening, posting_date, company): + if is_opening and frappe.db.exists("Period Closing Voucher", {"docstatus": 1, "company": company}): + frappe.throw( + _("Opening Entry can not be created after Period Closing Voucher is created."), + title=_("Invalid Opening Entry"), + ) + + last_pcv_date = frappe.db.get_value( + "Period Closing Voucher", {"docstatus": 1, "company": company}, [{"MAX": "period_end_date"}] + ) + + if last_pcv_date and getdate(posting_date) <= getdate(last_pcv_date): + message = _("Books have been closed till the period ending on {0}").format(formatdate(last_pcv_date)) + message += "
" + message += _("You cannot create/amend any accounting entries till this date.") + frappe.throw(message, title=_("Period Closed")) + + +def validate_allowed_dimensions(gl_entry, dimension_filter_map): + for key, value in dimension_filter_map.items(): + dimension = key[0] + account = key[1] + + if gl_entry.account == account: + if value["is_mandatory"] and not gl_entry.get(dimension): + frappe.throw( + _("{0} is mandatory for account {1}").format( + frappe.bold(frappe.unscrub(dimension)), frappe.bold(gl_entry.account) + ), + MandatoryAccountDimensionError, + ) + + if value["allow_or_restrict"] == "Allow": + if gl_entry.get(dimension) and gl_entry.get(dimension) not in value["allowed_dimensions"]: + frappe.throw( + _("Invalid value {0} for {1} against account {2}").format( + frappe.bold(gl_entry.get(dimension)), + frappe.bold(frappe.unscrub(dimension)), + frappe.bold(gl_entry.account), + ), + InvalidAccountDimensionError, + ) + else: + if gl_entry.get(dimension) and gl_entry.get(dimension) in value["allowed_dimensions"]: + frappe.throw( + _("Invalid value {0} for {1} against account {2}").format( + frappe.bold(gl_entry.get(dimension)), + frappe.bold(frappe.unscrub(dimension)), + frappe.bold(gl_entry.account), + ), + InvalidAccountDimensionError, + ) diff --git a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py index 6a8d3cc7ffa..1828528866f 100644 --- a/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py +++ b/erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py @@ -15,7 +15,7 @@ from frappe.utils.user import get_users_with_role from rq.timeouts import JobTimeoutException import erpnext -from erpnext.accounts.general_ledger import validate_accounting_period +from erpnext.accounts.services.gl_validator import validate_accounting_period from erpnext.accounts.utils import get_future_stock_vouchers, repost_gle_for_stock_vouchers from erpnext.stock.stock_ledger import ( get_affected_transactions, diff --git a/specs/accounts_refactor_spec.md b/specs/accounts_refactor_spec.md index 5215c714b01..099a4119933 100644 --- a/specs/accounts_refactor_spec.md +++ b/specs/accounts_refactor_spec.md @@ -56,7 +56,8 @@ SalesInvoiceGLComposer.compose() → gl_entries → gl_validator.validate(gl ## Bucketing `accounts_controller.py` - **Base composer (`BaseGLComposer`):** `get_gl_dict`, `get_value_in_transaction_currency`, `make_discount_gl_entries` (+ `get_amount_and_base_amount`, `get_tax_amounts`), `make_precision_loss_gl_entry`, `make_exchange_gain_loss_journal` (+ `gain_loss_journal_already_booked`), `set_transaction_currency_and_rate_in_gl_map`. Regional hooks `update_gl_dict_with_regional_fields` / `..._app_based_fields` stay free functions called inside `get_gl_dict`. - **Advances service:** `set_advances`, `get_advance_entries`, `clear_unallocated_advances`, `validate_advance_entries`, `set_advance_gain_or_loss`, `calculate_total_advance_from_ledger`, `set_total_advance_paid`, `set_advance_payment_status`, `delink_advance_entries`, `create_advance_and_reconcile`, `get_advance_payment_doctypes`, `_remove_advance_payment_ledger_entries`, module funcs `get_advance_journal_entries` / `get_advance_payment_entries`. -- **Validator (from `general_ledger.py`):** `validate_disabled_accounts`, `validate_accounting_period`, `validate_cwip_accounts`, `check_freezing_date`, `validate_against_pcv`, `validate_allowed_dimensions`, balance assertion (`get_debit_credit_difference` / `get_debit_credit_allowance` / `raise_debit_credit_not_equal_error`). +- **Validator (from `general_ledger.py`):** `validate_disabled_accounts`, `validate_accounting_period`, `validate_cwip_accounts`, `check_freezing_date`, `validate_against_pcv`, `validate_allowed_dimensions`. (Moved in Phase 1.) + - **Balance trio stays in `general_ledger.py` for now** (revised during Phase 1): `get_debit_credit_difference` / `get_debit_credit_allowance` / `raise_debit_credit_not_equal_error`. `get_debit_credit_difference` *mutates* entries (rounds debit/credit in place) and the trio is interleaved with `process_debit_credit_difference` → `make_round_off_gle` (the round-off *repair* run before and after balancing). It is not a standalone pre-post gate, so it can't move into a pure `validate(gl_entries)` without changing behavior. It travels with round-off when that moves compose-side (see below). - **Stays in compose (do NOT move to validator):** `process_debit_credit_difference` / `make_round_off_gle` — these *repair* balance by appending a round-off entry (mutation), not validation. - **Stays in composer (not validator):** row-level checks (right account for a row, dimension applicability) — validator only validates the finished list. - **Leave in controller:** `validate_company_in_accounting_dimension`, `validate_company` (dimension validation, not GL). @@ -67,8 +68,8 @@ Each phase is behavior-preserving, one draft PR, gated by the Phase-0 snapshot s ### Phase 0 — Safety net (first, mandatory) Characterization tests snapshotting `gl_entries` output for representative transactions (SI/PI with taxes, multi-currency, advances, discounts, round-off, POS). Every later phase passes iff snapshots are byte-identical. -### Phase 1 — Extract `gl_validator.py` (lowest risk) -Move list-level validations out of `general_ledger.py`; `make_gl_entries` calls `gl_validator.validate(gl_entries)`. Near-pure move; proves the safety net. +### Phase 1 — Extract `gl_validator.py` (lowest risk) — DONE +Moved the 6 pure list-level validators to `erpnext/accounts/services/gl_validator.py`; `general_ledger.py` imports and calls them at the existing call sites (no behavior change). A consolidated `gl_validator.validate(gl_entries)` facade is deferred — the current checks run at different points (make_gl_entries / save_entries per-entry / make_reverse_gl_entries), so collapsing them into one call would alter ordering. Verified: all 12 Phase-0 snapshots byte-identical. ### Phase 2 — Pilot composer on Sales Invoice only Create `BaseGLComposer` + `SalesInvoiceGLComposer`; lift bucket-A helpers from `accounts_controller`; move SI's `get_gl_entries` body into `.compose()`; old method becomes a thin shim. Do not over-generalise the base from one example.