mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-14 23:18:40 +00:00
refactor: extract list-level GL validations into gl_validator service
Phase 1 of the accounts/controller refactor. Moves the six pure list-level validators (validate_disabled_accounts, validate_accounting_period, validate_cwip_accounts, check_freezing_date, validate_against_pcv, validate_allowed_dimensions) out of general_ledger.py into the new erpnext/accounts/services/gl_validator.py. general_ledger.py imports and calls them at the existing sites; no behavior change (Phase 0 GL snapshots remain byte-identical). The debit/credit balance trio stays in general_ledger.py for now since get_debit_credit_difference mutates entries and is interleaved with the round-off repair.
This commit is contained in:
Nabin Hait
@@ -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 = "<br>"
|
||||
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: <b>{0}</b> 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 += "</br >"
|
||||
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,
|
||||
)
|
||||
|
||||
0
erpnext/accounts/services/__init__.py
Normal file
0
erpnext/accounts/services/__init__.py
Normal file
176
erpnext/accounts/services/gl_validator.py
Normal file
176
erpnext/accounts/services/gl_validator.py
Normal file
@@ -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 = "<br>"
|
||||
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: <b>{0}</b> 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 += "</br >"
|
||||
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,
|
||||
)
|
||||
@@ -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,
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user