From 6361b652e02494bec7d48be2cff928d427fa70f9 Mon Sep 17 00:00:00 2001 From: FawazAlhafiz Date: Tue, 3 Nov 2026 10:57:41 +0300 Subject: [PATCH 01/14] feat(overdue-limit): add the Time Based Overdue Limit doctype A rule names one Customer or Customer Group, an optional company, a day threshold and an action: Warn on Save, Block on Save or Block on Submit. Only Accounts Manager and System Manager can edit rules, so sales users cannot add their own role to the exempt list. Co-Authored-By: Claude Opus 5.5 --- .../time_based_overdue_limit/__init__.py | 0 .../time_based_overdue_limit.json | 137 ++++++++++++++++++ .../time_based_overdue_limit.py | 57 ++++++++ .../__init__.py | 0 .../time_based_overdue_limit_exempt_role.json | 35 +++++ .../time_based_overdue_limit_exempt_role.py | 21 +++ 6 files changed, 250 insertions(+) create mode 100644 optima_payment/optima_payment/doctype/time_based_overdue_limit/__init__.py create mode 100644 optima_payment/optima_payment/doctype/time_based_overdue_limit/time_based_overdue_limit.json create mode 100644 optima_payment/optima_payment/doctype/time_based_overdue_limit/time_based_overdue_limit.py create mode 100644 optima_payment/optima_payment/doctype/time_based_overdue_limit_exempt_role/__init__.py create mode 100644 optima_payment/optima_payment/doctype/time_based_overdue_limit_exempt_role/time_based_overdue_limit_exempt_role.json create mode 100644 optima_payment/optima_payment/doctype/time_based_overdue_limit_exempt_role/time_based_overdue_limit_exempt_role.py diff --git a/optima_payment/optima_payment/doctype/time_based_overdue_limit/__init__.py b/optima_payment/optima_payment/doctype/time_based_overdue_limit/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/optima_payment/optima_payment/doctype/time_based_overdue_limit/time_based_overdue_limit.json b/optima_payment/optima_payment/doctype/time_based_overdue_limit/time_based_overdue_limit.json new file mode 100644 index 0000000..de6a0b3 --- /dev/null +++ b/optima_payment/optima_payment/doctype/time_based_overdue_limit/time_based_overdue_limit.json @@ -0,0 +1,137 @@ +{ + "actions": [], + "autoname": "TBOL-.#####", + "creation": "2026-11-02 10:00:00", + "doctype": "DocType", + "engine": "InnoDB", + "field_order": [ + "section_break_rule", + "enabled", + "applies_to", + "party", + "company", + "column_break_action", + "action", + "threshold_days", + "exempt_roles" + ], + "fields": [ + { + "fieldname": "section_break_rule", + "fieldtype": "Section Break" + }, + { + "default": "1", + "fieldname": "enabled", + "fieldtype": "Check", + "in_list_view": 1, + "label": "Enabled" + }, + { + "default": "Customer", + "fieldname": "applies_to", + "fieldtype": "Select", + "in_list_view": 1, + "in_standard_filter": 1, + "label": "Applies To", + "options": "Customer\nCustomer Group", + "reqd": 1 + }, + { + "fieldname": "party", + "fieldtype": "Dynamic Link", + "in_list_view": 1, + "in_standard_filter": 1, + "label": "Party", + "options": "applies_to", + "reqd": 1 + }, + { + "description": "Leave blank to apply the rule in every company.", + "fieldname": "company", + "fieldtype": "Link", + "in_standard_filter": 1, + "label": "Company", + "options": "Company" + }, + { + "fieldname": "column_break_action", + "fieldtype": "Column Break" + }, + { + "fieldname": "action", + "fieldtype": "Select", + "in_list_view": 1, + "in_standard_filter": 1, + "label": "Action", + "options": "Warn on Save\nBlock on Save\nBlock on Submit", + "reqd": 1 + }, + { + "description": "The rule applies once the customer's oldest overdue Sales Invoice is at least this many days past its due date.", + "fieldname": "threshold_days", + "fieldtype": "Int", + "in_list_view": 1, + "label": "Threshold Days", + "non_negative": 1, + "reqd": 1 + }, + { + "depends_on": "eval:doc.action != \"Warn on Save\"", + "description": "Users with any of these roles get a warning instead of being blocked.", + "fieldname": "exempt_roles", + "fieldtype": "Table MultiSelect", + "label": "Exempt Roles", + "options": "Time Based Overdue Limit Exempt Role" + } + ], + "grid_page_length": 50, + "index_web_pages_for_search": 1, + "links": [], + "modified": "2026-11-02 10:00:00", + "modified_by": "Administrator", + "module": "Optima Payment", + "name": "Time Based Overdue Limit", + "naming_rule": "Expression", + "owner": "Administrator", + "permissions": [ + { + "create": 1, + "delete": 1, + "email": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "Accounts Manager", + "share": 1, + "write": 1 + }, + { + "create": 1, + "delete": 1, + "email": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "System Manager", + "share": 1, + "write": 1 + }, + { + "read": 1, + "role": "Accounts User" + }, + { + "read": 1, + "role": "Sales User" + } + ], + "row_format": "Dynamic", + "search_fields": "party", + "sort_field": "modified", + "sort_order": "DESC", + "states": [], + "title_field": "party" +} diff --git a/optima_payment/optima_payment/doctype/time_based_overdue_limit/time_based_overdue_limit.py b/optima_payment/optima_payment/doctype/time_based_overdue_limit/time_based_overdue_limit.py new file mode 100644 index 0000000..e377f09 --- /dev/null +++ b/optima_payment/optima_payment/doctype/time_based_overdue_limit/time_based_overdue_limit.py @@ -0,0 +1,57 @@ +# Copyright (c) 2026, IT Systematic and contributors +# For license information, please see license.txt + +import frappe +from frappe import _ +from frappe.model.document import Document +from frappe.utils import cint + + +class TimeBasedOverdueLimit(Document): + # begin: auto-generated types + # This code is auto-generated. Do not modify anything in this block. + + from typing import TYPE_CHECKING + + if TYPE_CHECKING: + from frappe.types import DF + from optima_payment.optima_payment.doctype.time_based_overdue_limit_exempt_role.time_based_overdue_limit_exempt_role import TimeBasedOverdueLimitExemptRole + + action: DF.Literal["Warn on Save", "Block on Save", "Block on Submit"] + applies_to: DF.Literal["Customer", "Customer Group"] + company: DF.Link | None + enabled: DF.Check + exempt_roles: DF.TableMultiSelect[TimeBasedOverdueLimitExemptRole] + party: DF.DynamicLink + threshold_days: DF.Int + # end: auto-generated types + + def validate(self): + self.validate_threshold_days() + self.validate_unique_rule() + + def validate_threshold_days(self): + if cint(self.threshold_days) <= 0: + frappe.throw(_("Threshold Days must be greater than 0.")) + + def validate_unique_rule(self): + if not self.enabled: + return + + duplicate = frappe.db.exists( + self.doctype, + { + "name": ("!=", self.name), + "enabled": 1, + "applies_to": self.applies_to, + "party": self.party, + "company": self.company or ("is", "not set"), + "action": self.action, + }, + ) + if duplicate: + frappe.throw( + _("Enabled rule {0} already has the same Applies To, Party, Company and Action.").format( + frappe.bold(duplicate) + ) + ) diff --git a/optima_payment/optima_payment/doctype/time_based_overdue_limit_exempt_role/__init__.py b/optima_payment/optima_payment/doctype/time_based_overdue_limit_exempt_role/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/optima_payment/optima_payment/doctype/time_based_overdue_limit_exempt_role/time_based_overdue_limit_exempt_role.json b/optima_payment/optima_payment/doctype/time_based_overdue_limit_exempt_role/time_based_overdue_limit_exempt_role.json new file mode 100644 index 0000000..838c9d1 --- /dev/null +++ b/optima_payment/optima_payment/doctype/time_based_overdue_limit_exempt_role/time_based_overdue_limit_exempt_role.json @@ -0,0 +1,35 @@ +{ + "actions": [], + "allow_rename": 1, + "creation": "2026-11-02 10:00:00", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "role" + ], + "fields": [ + { + "fieldname": "role", + "fieldtype": "Link", + "in_list_view": 1, + "label": "Role", + "options": "Role", + "reqd": 1 + } + ], + "grid_page_length": 50, + "index_web_pages_for_search": 1, + "istable": 1, + "links": [], + "modified": "2026-11-02 10:00:00", + "modified_by": "Administrator", + "module": "Optima Payment", + "name": "Time Based Overdue Limit Exempt Role", + "owner": "Administrator", + "permissions": [], + "row_format": "Dynamic", + "sort_field": "creation", + "sort_order": "DESC", + "states": [] +} diff --git a/optima_payment/optima_payment/doctype/time_based_overdue_limit_exempt_role/time_based_overdue_limit_exempt_role.py b/optima_payment/optima_payment/doctype/time_based_overdue_limit_exempt_role/time_based_overdue_limit_exempt_role.py new file mode 100644 index 0000000..fb762fc --- /dev/null +++ b/optima_payment/optima_payment/doctype/time_based_overdue_limit_exempt_role/time_based_overdue_limit_exempt_role.py @@ -0,0 +1,21 @@ +# Copyright (c) 2026, IT Systematic and contributors +# For license information, please see license.txt + +from frappe.model.document import Document + + +class TimeBasedOverdueLimitExemptRole(Document): + # begin: auto-generated types + # This code is auto-generated. Do not modify anything in this block. + + from typing import TYPE_CHECKING + + if TYPE_CHECKING: + from frappe.types import DF + + parent: DF.Data + parentfield: DF.Data + parenttype: DF.Data + role: DF.Link + # end: auto-generated types + pass From f510d45f84f1b2f7761f68c8dc7d870329d88a3c Mon Sep 17 00:00:00 2001 From: FawazAlhafiz Date: Tue, 3 Nov 2026 10:57:41 +0300 Subject: [PATCH 02/14] feat(overdue-limit): decide when an overdue customer's sales documents warn or block The customer's oldest overdue Sales Invoice in the document's company sets the day count. Only the most specific tier of rules applies: the customer's own, else its group's, else the nearest ancestor group's. A customer whose GL balance is zero or less is never checked, because unallocated payments leave invoices outstanding. Not wired into hooks.py yet. Co-Authored-By: Claude Opus 5.5 --- optima_payment/doc_events/overdue_limit.py | 11 + optima_payment/overdue_limit/__init__.py | 0 optima_payment/overdue_limit/limit.py | 255 +++++++++++++++++++++ 3 files changed, 266 insertions(+) create mode 100644 optima_payment/doc_events/overdue_limit.py create mode 100644 optima_payment/overdue_limit/__init__.py create mode 100644 optima_payment/overdue_limit/limit.py diff --git a/optima_payment/doc_events/overdue_limit.py b/optima_payment/doc_events/overdue_limit.py new file mode 100644 index 0000000..da4a794 --- /dev/null +++ b/optima_payment/doc_events/overdue_limit.py @@ -0,0 +1,11 @@ +"""Sales Invoice and Sales Order hooks for the Time Based Overdue Limit.""" + +from optima_payment.overdue_limit import limit + + +def before_save(doc, method=None): + limit.check(doc, limit.SAVE) + + +def before_submit(doc, method=None): + limit.check(doc, limit.SUBMIT) diff --git a/optima_payment/overdue_limit/__init__.py b/optima_payment/overdue_limit/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/optima_payment/overdue_limit/limit.py b/optima_payment/overdue_limit/limit.py new file mode 100644 index 0000000..e772302 --- /dev/null +++ b/optima_payment/overdue_limit/limit.py @@ -0,0 +1,255 @@ +# Copyright (c) 2026, IT Systematic and contributors +# For license information, please see license.txt + +"""Time Based Overdue Limit: warn about or block a Sales Invoice or Sales Order whose customer has +a Sales Invoice unpaid too long past its due date. + +The age that counts is that of the customer's oldest overdue invoice in the document's company. Only +the most specific tier of rules applies: the customer's own, else its group's, else the nearest +ancestor group's. Every rule in that tier counts. + +Branching lives in the DECISIONS functions, which take plain rows and return values. +""" + +from collections import namedtuple + +import frappe +from frappe import _ +from frappe.model.document import Document +from frappe.query_builder.functions import Sum +from frappe.utils import flt, formatdate, getdate + +RULE = "Time Based Overdue Limit" +EXEMPT_ROLE = "Time Based Overdue Limit Exempt Role" +CUSTOMER = "Customer" +CUSTOMER_GROUP = "Customer Group" + +WARN_ON_SAVE = "Warn on Save" +BLOCK_ON_SAVE = "Block on Save" +BLOCK_ON_SUBMIT = "Block on Submit" + +SAVE = "save" +SUBMIT = "submit" + +WARN = "warn" +BLOCK = "block" + +SKIP_FLAGS = ("in_migrate", "in_patch", "in_install", "in_import") + +Hit = namedtuple("Hit", ["rule", "exempt"]) +Verdict = namedtuple("Verdict", ["outcome", "hits"]) + + +class OverdueLimitError(frappe.ValidationError): + pass + + +# ==================================================================================================== +# DECISIONS +# Pure: no database. + + +def is_exempt_document(is_return, is_consolidated, flags) -> bool: + """Credit notes only lower what the customer owes, and blocking a POS consolidated invoice would + break POS Closing. Nothing written by migrate, a patch, an install or a data import is checked.""" + return bool(is_return or is_consolidated or any(flags.get(flag) for flag in SKIP_FLAGS)) + + +def select_rules(rules: list, customer: str, group_chain: list[str], company: str) -> list: + """The rules governing ``customer`` in ``company``: those of the first tier that has any. + + ``group_chain`` is the customer's group followed by its ancestors, nearest first. A rule with no + company applies in every company, and a rule for another company does not shadow a wider tier. + """ + in_company = [rule for rule in rules if not rule.company or rule.company == company] + tiers = [(CUSTOMER, customer)] + [(CUSTOMER_GROUP, group) for group in group_chain] + for applies_to, party in tiers: + tier = [rule for rule in in_company if rule.applies_to == applies_to and rule.party == party] + if tier: + return tier + return [] + + +def days_overdue(due_date, today) -> int: + return (getdate(today) - getdate(due_date)).days + + +def is_exempt_user(rule, user_roles: set) -> bool: + """Exempt roles only turn a block into a warning, so they mean nothing on a Warn on Save rule.""" + return rule.action != WARN_ON_SAVE and bool(user_roles & set(rule.exempt_roles)) + + +def rule_outcome(action: str, stage: str, exempt: bool) -> str | None: + """What one triggered rule does at ``stage``. + + Block on Save blocks at submit too, because submitting skips before_save: a draft saved before + the customer went overdue would otherwise get through. Block on Submit warns at save, so the user + hears of the block before reaching it. + """ + if action == WARN_ON_SAVE: + return WARN if stage == SAVE else None + blocks_now = action == BLOCK_ON_SAVE or stage == SUBMIT + return BLOCK if blocks_now and not exempt else WARN + + +def decide(rules: list, days: int, user_roles, stage: str) -> Verdict: + """The document's outcome when its customer is ``days`` overdue. A rule triggers once ``days`` + reaches its ``threshold_days``. A block outranks warnings, which its message then leaves out.""" + roles = set(user_roles) + hits = {BLOCK: [], WARN: []} + for rule in rules: + if days < rule.threshold_days: + continue + exempt = is_exempt_user(rule, roles) + outcome = rule_outcome(rule.action, stage, exempt) + if outcome: + hits[outcome].append(Hit(rule, exempt)) + + for outcome in (BLOCK, WARN): + if hits[outcome]: + return Verdict(outcome, hits[outcome]) + return Verdict(None, []) + + +# ==================================================================================================== +# MESSAGES + + +def message(verdict: Verdict, customer: str, invoice, days: int) -> str: + header = _("Customer {0} has Sales Invoice {1} overdue by {2} days; it was due on {3}.").format( + frappe.bold(customer), frappe.bold(invoice.name), frappe.bold(days), formatdate(invoice.due_date) + ) + lines = "".join(f"
  • {_hit_line(hit)}
    • " for hit in verdict.hits) + return f"{header}
      {lines}
    " + + +def _hit_line(hit: Hit) -> str: + rule = hit.rule + line = _rule_text(rule.action).format(frappe.bold(rule.name), rule.threshold_days) + if hit.exempt: + return f"{line} {_('Your role lets you continue.')}" + if rule.action != WARN_ON_SAVE and rule.exempt_roles: + roles = ", ".join(_(role) for role in rule.exempt_roles) + return f"{line} {_('Roles that can continue: {0}.').format(roles)}" + return line + + +def _rule_text(action: str) -> str: + if action == WARN_ON_SAVE: + return _("Rule {0} warns from {1} days overdue.") + if action == BLOCK_ON_SAVE: + return _("Rule {0} blocks saving and submitting from {1} days overdue.") + return _("Rule {0} blocks submitting from {1} days overdue.") + + +# ==================================================================================================== +# CHECK + + +def check(doc: Document, stage: str) -> None: + """Warns about or blocks a Sales Invoice or Sales Order at ``stage``, as its customer's rules say.""" + if not doc.customer or is_exempt_document(doc.get("is_return"), doc.get("is_consolidated"), frappe.flags): + return + + rules = get_rules(doc.customer, doc.company) + if not rules: + return + + today = getdate() + invoice = get_oldest_overdue_invoice(doc.customer, doc.company, today) + if not invoice: + return + + days = days_overdue(invoice.due_date, today) + verdict = decide(rules, days, frappe.get_roles(), stage) + # A payment or credit note not yet allocated to the invoice leaves it outstanding, though the + # customer owes nothing. + if not verdict.outcome or get_customer_gl_balance(doc.customer, doc.company) <= 0: + return + + text = message(verdict, doc.customer, invoice, days) + if verdict.outcome == BLOCK: + frappe.throw(text, OverdueLimitError, title=_("Overdue Limit")) + frappe.msgprint(text, title=_("Overdue Limit"), indicator="orange") + + +# ==================================================================================================== +# QUERIES + + +def get_rules(customer: str, company: str) -> list: + group_chain = get_group_chain(frappe.db.get_value(CUSTOMER, customer, "customer_group")) + return select_rules(get_enabled_rules([customer, *group_chain]), customer, group_chain, company) + + +def get_group_chain(customer_group: str | None) -> list[str]: + """``customer_group`` and its ancestors, nearest first. + + Read lft and rgt uncached: adding a group renumbers the rest of the tree in SQL, which leaves + their cached documents stale. + """ + if not customer_group: + return [] + + lft, rgt = frappe.db.get_value(CUSTOMER_GROUP, customer_group, ["lft", "rgt"]) + return frappe.get_all( + CUSTOMER_GROUP, + filters={"lft": ("<=", lft), "rgt": (">=", rgt)}, + order_by="lft desc", + pluck="name", + ) + + +def get_enabled_rules(parties: list[str]) -> list: + """The enabled rules naming any of ``parties``, each with its ``exempt_roles`` as a list.""" + rules = frappe.get_all( + RULE, + filters={"enabled": 1, "party": ("in", parties)}, + fields=["name", "applies_to", "party", "company", "action", "threshold_days"], + order_by="threshold_days asc, name asc", + ) + if not rules: + return [] + + roles = frappe.get_all( + EXEMPT_ROLE, + filters={"parenttype": RULE, "parent": ("in", [rule.name for rule in rules])}, + fields=["parent", "role"], + ) + for rule in rules: + rule.exempt_roles = [row.role for row in roles if row.parent == rule.name] + return rules + + +def get_oldest_overdue_invoice(customer: str, company: str, today): + """The submitted, unpaid Sales Invoice whose due date is furthest past, or None.""" + invoices = frappe.get_all( + "Sales Invoice", + filters={ + "customer": customer, + "company": company, + "docstatus": 1, + "is_return": 0, + "outstanding_amount": (">", 0), + "due_date": ("<", today), + }, + fields=["name", "due_date"], + order_by="due_date asc, name asc", + limit=1, + ) + return invoices[0] if invoices else None + + +def get_customer_gl_balance(customer: str, company: str) -> float: + gle = frappe.qb.DocType("GL Entry") + balance = ( + frappe.qb.from_(gle) + .select(Sum(gle.debit) - Sum(gle.credit)) + .where( + (gle.party_type == CUSTOMER) + & (gle.party == customer) + & (gle.company == company) + & (gle.is_cancelled == 0) + ) + ).run() + return flt(balance[0][0]) From 991ff11adf476a4eb41961f0e0675d0b46c95b0d Mon Sep 17 00:00:00 2001 From: FawazAlhafiz Date: Tue, 3 Nov 2026 12:57:18 +0300 Subject: [PATCH 03/14] feat(overdue-limit): run the overdue check on Sales Invoice and Sales Order Co-Authored-By: Claude Opus 5.5 --- optima_payment/hooks.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/optima_payment/hooks.py b/optima_payment/hooks.py index c6d6532..c50771b 100644 --- a/optima_payment/hooks.py +++ b/optima_payment/hooks.py @@ -189,6 +189,14 @@ "Purchase Receipt": { "after_delete": "optima_payment.purchases_bulk.vouchers.unlink_tool_rows", }, + "Sales Invoice": { + "before_save": "optima_payment.doc_events.overdue_limit.before_save", + "before_submit": "optima_payment.doc_events.overdue_limit.before_submit", + }, + "Sales Order": { + "before_save": "optima_payment.doc_events.overdue_limit.before_save", + "before_submit": "optima_payment.doc_events.overdue_limit.before_submit", + }, # "Expense Claim" : { # "on_submit" : "optima_payment.doc_events.expense_claim.on_submit", # "on_cancel" : "optima_payment.doc_events.expense_claim.on_cancel", From 80b07097fee84d7c46e8f524ab9e7b56d67fb5de Mon Sep 17 00:00:00 2001 From: FawazAlhafiz Date: Tue, 3 Nov 2026 12:57:18 +0300 Subject: [PATCH 04/14] fix(overdue-limit): keep a blank company blank when a rule is inserted Frappe fills an empty Link field of a new document from the default keyed by its fieldname, and every ERPNext site has a global company default, so an every-company rule was silently saved as a rule for the default company. The form still pre-fills the company; clearing it now sticks. Co-Authored-By: Claude Opus 5.5 --- .../time_based_overdue_limit/time_based_overdue_limit.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/optima_payment/optima_payment/doctype/time_based_overdue_limit/time_based_overdue_limit.py b/optima_payment/optima_payment/doctype/time_based_overdue_limit/time_based_overdue_limit.py index e377f09..70b1060 100644 --- a/optima_payment/optima_payment/doctype/time_based_overdue_limit/time_based_overdue_limit.py +++ b/optima_payment/optima_payment/doctype/time_based_overdue_limit/time_based_overdue_limit.py @@ -26,6 +26,10 @@ class TimeBasedOverdueLimit(Document): threshold_days: DF.Int # end: auto-generated types + def __setup__(self): + # A blank company means every company, so insert must not fill in the default company. + self.dont_update_if_missing = ["company"] + def validate(self): self.validate_threshold_days() self.validate_unique_rule() From d4456d1d0bcf64bbc15ff6a777d73da2ad976eaa Mon Sep 17 00:00:00 2001 From: FawazAlhafiz Date: Tue, 3 Nov 2026 12:57:29 +0300 Subject: [PATCH 05/14] fix(test-factories): give the test customer a tax category Sites with the KSA customizations make tax_category mandatory on Customer, so get_or_create_customer failed there before any assertion ran. On ims.local this takes the Letter of Credit suite from 17 errors to 1 and the party statement integration suite from 7 to 5; what remains in both is the site's cost-center rule on balance sheet accounts. Co-Authored-By: Claude Opus 5.5 --- optima_payment/tests/utils.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/optima_payment/tests/utils.py b/optima_payment/tests/utils.py index c769f15..7a4a146 100644 --- a/optima_payment/tests/utils.py +++ b/optima_payment/tests/utils.py @@ -158,6 +158,7 @@ def get_or_create_customer(customer_name: str = "Optima Test Customer") -> str: # customer_type=Individual dodges KSA site customizations that make # registration_type / registration_value / tax_id mandatory for Companies. "customer_type": "Individual", + "tax_category": get_test_tax_category(), # Site-specific mandatory custom field; the key is ignored where it doesn't exist. "customer_name_in_arabic": customer_name, } @@ -166,9 +167,9 @@ def get_or_create_customer(customer_name: str = "Optima Test Customer") -> str: def get_test_tax_category() -> str | None: - """The Tax Category test suppliers carry, and so the category of every voucher made for them. + """The Tax Category test parties carry, and so the category of every voucher made for them. - KSA/ZATCA customizations on this site make tax_category mandatory on Supplier. + KSA/ZATCA customizations on some sites make tax_category mandatory on Customer and Supplier. """ return frappe.db.get_value("Tax Category", {}, "name") From 15e48561dcd57fdf7372a47dd2f5abe95e42e82a Mon Sep 17 00:00:00 2001 From: FawazAlhafiz Date: Tue, 3 Nov 2026 12:57:35 +0300 Subject: [PATCH 06/14] test(overdue-limit): cover the overdue check from plain rules to real saves The overdue invoice is posted today and its due date moved 25 days back, rather than backdated, so the suite still runs in January on CI, whose fiscal year is the calendar year. The test Sales Invoice carries no header cost center, as real invoices don't, because a payment copies it onto balance sheet lines that some sites refuse one on. make_reference_sales_order now wraps the new make_sales_order. Co-Authored-By: Claude Opus 5.5 --- .../test_time_based_overdue_limit.py | 47 ++++ .../overdue_limit/tests/__init__.py | 0 .../overdue_limit/tests/test_limit.py | 253 ++++++++++++++++++ .../tests/test_limit_integration.py | 160 +++++++++++ optima_payment/tests/utils.py | 159 +++++++++-- 5 files changed, 601 insertions(+), 18 deletions(-) create mode 100644 optima_payment/optima_payment/doctype/time_based_overdue_limit/test_time_based_overdue_limit.py create mode 100644 optima_payment/overdue_limit/tests/__init__.py create mode 100644 optima_payment/overdue_limit/tests/test_limit.py create mode 100644 optima_payment/overdue_limit/tests/test_limit_integration.py diff --git a/optima_payment/optima_payment/doctype/time_based_overdue_limit/test_time_based_overdue_limit.py b/optima_payment/optima_payment/doctype/time_based_overdue_limit/test_time_based_overdue_limit.py new file mode 100644 index 0000000..512f8f5 --- /dev/null +++ b/optima_payment/optima_payment/doctype/time_based_overdue_limit/test_time_based_overdue_limit.py @@ -0,0 +1,47 @@ +# Copyright (c) 2026, IT Systematic and Contributors +# See license.txt + +"""The rule's own validation: a positive threshold, and one enabled rule per party, company and +action.""" + +import erpnext +import frappe +from frappe.tests.utils import FrappeTestCase + +from optima_payment.overdue_limit.limit import BLOCK_ON_SAVE, WARN_ON_SAVE +from optima_payment.tests.utils import get_or_create_customer, make_overdue_limit + +CUSTOMER = "Optima Overdue Test Customer" + + +class TestTimeBasedOverdueLimit(FrappeTestCase): + def setUp(self): + self.customer = get_or_create_customer(CUSTOMER) + + def tearDown(self): + frappe.db.rollback() + + def test_the_threshold_must_be_above_zero(self): + for days in (0, -5): + with self.subTest(days=days), self.assertRaises(frappe.ValidationError): + make_overdue_limit(party=self.customer, action=WARN_ON_SAVE, threshold_days=days) + + def test_a_second_enabled_rule_for_the_same_party_company_and_action_is_refused(self): + make_overdue_limit(party=self.customer, action=WARN_ON_SAVE, threshold_days=20) + with self.assertRaises(frappe.ValidationError): + make_overdue_limit(party=self.customer, action=WARN_ON_SAVE, threshold_days=30) + + def test_rules_differing_in_action_company_or_enabled_are_not_duplicates(self): + make_overdue_limit(party=self.customer, action=WARN_ON_SAVE) + make_overdue_limit(party=self.customer, action=BLOCK_ON_SAVE) + make_overdue_limit(party=self.customer, action=WARN_ON_SAVE, company=erpnext.get_default_company()) + make_overdue_limit(party=self.customer, action=WARN_ON_SAVE, enabled=0) + + def test_a_blank_company_is_not_filled_with_the_default_company(self): + rule = make_overdue_limit(party=self.customer, action=WARN_ON_SAVE) + self.assertFalse(frappe.db.get_value(rule.doctype, rule.name, "company")) + + def test_a_rule_can_be_saved_again(self): + rule = make_overdue_limit(party=self.customer, action=WARN_ON_SAVE) + rule.threshold_days = 25 + rule.save(ignore_permissions=True) diff --git a/optima_payment/overdue_limit/tests/__init__.py b/optima_payment/overdue_limit/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/optima_payment/overdue_limit/tests/test_limit.py b/optima_payment/overdue_limit/tests/test_limit.py new file mode 100644 index 0000000..cebbf56 --- /dev/null +++ b/optima_payment/overdue_limit/tests/test_limit.py @@ -0,0 +1,253 @@ +# Copyright (c) 2026, IT Systematic and Contributors +# See license.txt + +"""The branch matrix for the Time Based Overdue Limit. Nothing here writes to the database. + +Rules are ``frappe._dict`` rows shaped like get_enabled_rules returns them, so no customer, group or +rule has to exist on the site. The wiring through real saves is in test_limit_integration.py. +""" + +import frappe +from frappe.tests.utils import FrappeTestCase +from frappe.utils import formatdate, getdate + +from optima_payment.overdue_limit.limit import ( + BLOCK, + BLOCK_ON_SAVE, + BLOCK_ON_SUBMIT, + CUSTOMER, + CUSTOMER_GROUP, + SAVE, + SKIP_FLAGS, + SUBMIT, + WARN, + WARN_ON_SAVE, + Hit, + Verdict, + days_overdue, + decide, + is_exempt_document, + is_exempt_user, + message, + select_rules, +) + +CUSTOMER_NAME = "Al Noor Trading" +GROUP = "Retail" +PARENT_GROUP = "Commercial" +ROOT_GROUP = "All Customer Groups" +GROUP_CHAIN = [GROUP, PARENT_GROUP, ROOT_GROUP] +COMPANY = "Optima Test Company" +OTHER_COMPANY = "Optima Other Company" +EXEMPT_ROLE = "Accounts Manager" +SALES_ROLES = ["Sales User"] +INVOICE = frappe._dict(name="SINV-2026-00001", due_date="2026-10-08") + + +def rule( + name="TBOL-00001", + *, + applies_to=CUSTOMER, + party=CUSTOMER_NAME, + company=None, + action=WARN_ON_SAVE, + threshold_days=20, + exempt_roles=(), +): + """A rule the way get_enabled_rules returns it.""" + return frappe._dict( + name=name, + applies_to=applies_to, + party=party, + company=company, + action=action, + threshold_days=threshold_days, + exempt_roles=list(exempt_roles), + ) + + +def group_rule(name, group, **fields): + return rule(name, applies_to=CUSTOMER_GROUP, party=group, **fields) + + +def names(rules): + return [rule.name for rule in rules] + + +def hit_names(verdict): + return [hit.rule.name for hit in verdict.hits] + + +class TestIsExemptDocument(FrappeTestCase): + def test_only_credit_notes_pos_consolidations_and_system_writes_are_skipped(self): + cases = [ + ("an ordinary invoice", 0, 0, {}, False), + ("a Sales Order, which has neither field", None, None, {}, False), + ("a credit note", 1, 0, {}, True), + ("a POS consolidated invoice", 0, 1, {}, True), + ("a skip flag set to false", 0, 0, {"in_import": False}, False), + ("an unrelated flag", 0, 0, {"in_test": True}, False), + *[(flag, 0, 0, {flag: True}, True) for flag in SKIP_FLAGS], + ] + for label, is_return, is_consolidated, flags, expected in cases: + with self.subTest(label): + self.assertEqual(is_exempt_document(is_return, is_consolidated, frappe._dict(flags)), expected) + + +class TestSelectRules(FrappeTestCase): + def select(self, rules, company=COMPANY): + return names(select_rules(rules, CUSTOMER_NAME, GROUP_CHAIN, company)) + + def test_customer_rules_beat_group_rules(self): + rules = [group_rule("G", GROUP), rule("C")] + self.assertEqual(self.select(rules), ["C"]) + + def test_the_nearest_group_beats_its_ancestors(self): + rules = [group_rule("ROOT", ROOT_GROUP), group_rule("PARENT", PARENT_GROUP), group_rule("G", GROUP)] + self.assertEqual(self.select(rules), ["G"]) + + def test_an_ancestor_applies_when_nearer_tiers_have_none(self): + rules = [group_rule("ROOT", ROOT_GROUP), group_rule("PARENT", PARENT_GROUP)] + self.assertEqual(self.select(rules), ["PARENT"]) + + def test_every_rule_of_the_tier_applies(self): + rules = [ + rule("WARN", action=WARN_ON_SAVE, threshold_days=20), + rule("BLOCK", action=BLOCK_ON_SUBMIT, threshold_days=30), + group_rule("G", GROUP), + ] + self.assertEqual(self.select(rules), ["WARN", "BLOCK"]) + + def test_a_blank_company_matches_every_company(self): + rules = [rule("ALL"), rule("MINE", company=COMPANY)] + self.assertEqual(self.select(rules), ["ALL", "MINE"]) + self.assertEqual(self.select(rules, company=OTHER_COMPANY), ["ALL"]) + + def test_a_rule_for_another_company_neither_applies_nor_shadows_a_wider_tier(self): + rules = [rule("C", company=OTHER_COMPANY), group_rule("G", GROUP)] + self.assertEqual(self.select(rules), ["G"]) + + def test_a_group_named_like_the_customer_is_matched_as_a_group(self): + rules = [group_rule("G", CUSTOMER_NAME)] + self.assertEqual(self.select(rules), []) + self.assertEqual(names(select_rules(rules, CUSTOMER_NAME, [CUSTOMER_NAME], COMPANY)), ["G"]) + + def test_a_customer_without_rules_or_group_selects_nothing(self): + self.assertEqual(self.select([]), []) + self.assertEqual(names(select_rules([group_rule("G", GROUP)], CUSTOMER_NAME, [], COMPANY)), []) + + +class TestDaysOverdue(FrappeTestCase): + def test_days_are_counted_from_the_due_date(self): + self.assertEqual(days_overdue("2026-10-08", "2026-11-02"), 25) + self.assertEqual(days_overdue(getdate("2026-11-01"), getdate("2026-11-02")), 1) + + +class TestIsExemptUser(FrappeTestCase): + def test_a_shared_role_exempts_only_from_a_block_rule(self): + cases = [ + ("a shared role on a block rule", BLOCK_ON_SAVE, [EXEMPT_ROLE], {EXEMPT_ROLE}, True), + ("a shared role on a block-on-submit rule", BLOCK_ON_SUBMIT, [EXEMPT_ROLE], {EXEMPT_ROLE}, True), + ("no shared role", BLOCK_ON_SAVE, [EXEMPT_ROLE], set(SALES_ROLES), False), + ("no exempt roles on the rule", BLOCK_ON_SAVE, [], {EXEMPT_ROLE}, False), + ("a shared role on a warn rule", WARN_ON_SAVE, [EXEMPT_ROLE], {EXEMPT_ROLE}, False), + ] + for label, action, exempt_roles, user_roles, expected in cases: + with self.subTest(label): + self.assertEqual(is_exempt_user(rule(action=action, exempt_roles=exempt_roles), user_roles), expected) + + +class TestDecide(FrappeTestCase): + def test_the_action_table(self): + """Every action at both stages, for a user with and without the rule's exempt role.""" + table = [ + (WARN_ON_SAVE, SAVE, False, WARN), + (WARN_ON_SAVE, SAVE, True, WARN), + (WARN_ON_SAVE, SUBMIT, False, None), + (WARN_ON_SAVE, SUBMIT, True, None), + (BLOCK_ON_SAVE, SAVE, False, BLOCK), + (BLOCK_ON_SAVE, SAVE, True, WARN), + (BLOCK_ON_SAVE, SUBMIT, False, BLOCK), + (BLOCK_ON_SAVE, SUBMIT, True, WARN), + (BLOCK_ON_SUBMIT, SAVE, False, WARN), + (BLOCK_ON_SUBMIT, SAVE, True, WARN), + (BLOCK_ON_SUBMIT, SUBMIT, False, BLOCK), + (BLOCK_ON_SUBMIT, SUBMIT, True, WARN), + ] + for action, stage, exempt, expected in table: + with self.subTest(action=action, stage=stage, exempt=exempt): + user_roles = [EXEMPT_ROLE] if exempt else SALES_ROLES + verdict = decide([rule(action=action, exempt_roles=[EXEMPT_ROLE])], 30, user_roles, stage) + self.assertEqual(verdict.outcome, expected) + + def test_a_rule_triggers_from_its_threshold(self): + rules = [rule(action=BLOCK_ON_SAVE, threshold_days=20)] + for days, expected in [(19, None), (20, BLOCK), (21, BLOCK)]: + with self.subTest(days=days): + self.assertEqual(decide(rules, days, SALES_ROLES, SAVE).outcome, expected) + + def test_a_block_outranks_warnings_and_leaves_them_out(self): + rules = [rule("WARN", threshold_days=20), rule("BLOCK", action=BLOCK_ON_SAVE, threshold_days=30)] + verdict = decide(rules, 30, SALES_ROLES, SAVE) + self.assertEqual(verdict.outcome, BLOCK) + self.assertEqual(hit_names(verdict), ["BLOCK"]) + + def test_every_triggered_warning_is_listed(self): + rules = [rule("WARN", threshold_days=20), rule("NOTICE", action=BLOCK_ON_SUBMIT, threshold_days=25)] + verdict = decide(rules, 30, SALES_ROLES, SAVE) + self.assertEqual(verdict.outcome, WARN) + self.assertEqual(hit_names(verdict), ["WARN", "NOTICE"]) + + def test_a_rule_below_its_threshold_is_left_out(self): + rules = [rule("WARN", threshold_days=20), rule("BLOCK", action=BLOCK_ON_SAVE, threshold_days=40)] + verdict = decide(rules, 30, SALES_ROLES, SAVE) + self.assertEqual(verdict.outcome, WARN) + self.assertEqual(hit_names(verdict), ["WARN"]) + + def test_an_exempt_user_is_warned_and_the_hit_says_so(self): + verdict = decide([rule(action=BLOCK_ON_SAVE, exempt_roles=[EXEMPT_ROLE])], 30, [EXEMPT_ROLE], SAVE) + self.assertEqual(verdict.outcome, WARN) + self.assertTrue(verdict.hits[0].exempt) + + def test_no_rules_decide_nothing(self): + self.assertEqual(decide([], 30, SALES_ROLES, SAVE), Verdict(None, [])) + + +class TestMessage(FrappeTestCase): + def render(self, *hits, outcome=BLOCK): + return message(Verdict(outcome, list(hits)), CUSTOMER_NAME, INVOICE, 25) + + def test_the_header_names_the_customer_invoice_days_and_due_date(self): + text = self.render(Hit(rule(action=BLOCK_ON_SAVE), False)) + self.assertIn( + f"Customer {CUSTOMER_NAME} has Sales Invoice {INVOICE.name} " + f"overdue by 25 days; it was due on {formatdate(INVOICE.due_date)}.", + text, + ) + + def test_each_action_reads_as_what_it_does(self): + cases = [ + (WARN_ON_SAVE, "Rule TBOL-00001 warns from 20 days overdue."), + (BLOCK_ON_SAVE, "Rule TBOL-00001 blocks saving and submitting from 20 days overdue."), + (BLOCK_ON_SUBMIT, "Rule TBOL-00001 blocks submitting from 20 days overdue."), + ] + for action, expected in cases: + with self.subTest(action): + self.assertIn(f"
  • {expected}
    • ", self.render(Hit(rule(action=action), False))) + + def test_a_blocked_user_is_told_which_roles_can_continue(self): + text = self.render(Hit(rule(action=BLOCK_ON_SAVE, exempt_roles=[EXEMPT_ROLE, "System Manager"]), False)) + self.assertIn("Roles that can continue: Accounts Manager, System Manager.", text) + + def test_an_exempt_user_is_told_their_role_lets_them_continue(self): + text = self.render(Hit(rule(action=BLOCK_ON_SAVE, exempt_roles=[EXEMPT_ROLE]), True), outcome=WARN) + self.assertIn("Your role lets you continue.", text) + self.assertNotIn("Roles that can continue", text) + + def test_a_warn_rule_lists_no_roles(self): + text = self.render(Hit(rule(action=WARN_ON_SAVE, exempt_roles=[EXEMPT_ROLE]), False), outcome=WARN) + self.assertNotIn("Roles that can continue", text) + + def test_every_hit_gets_its_own_line(self): + text = self.render(Hit(rule("FIRST"), False), Hit(rule("SECOND"), False), outcome=WARN) + self.assertEqual(text.count("
  • "), 2) diff --git a/optima_payment/overdue_limit/tests/test_limit_integration.py b/optima_payment/overdue_limit/tests/test_limit_integration.py new file mode 100644 index 0000000..060ce30 --- /dev/null +++ b/optima_payment/overdue_limit/tests/test_limit_integration.py @@ -0,0 +1,160 @@ +# Copyright (c) 2026, IT Systematic and Contributors +# See license.txt + +"""The Time Based Overdue Limit through real saves and submits of Sales Invoices and Sales Orders. + +The branch matrix and the message text are in test_limit.py. These prove what only a real document +shows: the hooks fire on both doctypes at both stages, an earlier draft cannot be submitted past a +Block on Save rule, the acting user's roles decide the exemption, credit notes pass, the group tree is +read from the database, and a payment lifts the block whether or not it is allocated to the invoice. + +Each test starts from a customer whose Sales Invoice fell due 25 days ago and is unpaid. +""" + +import erpnext +import frappe +from frappe.tests.utils import FrappeTestCase +from frappe.utils import add_days, nowdate + +from optima_payment.overdue_limit.limit import BLOCK_ON_SAVE, BLOCK_ON_SUBMIT, WARN_ON_SAVE, OverdueLimitError +from optima_payment.tests.utils import ( + get_or_create_customer, + get_or_create_user, + make_credit_note, + make_invoice_payment, + make_overdue_limit, + make_sales_invoice, + make_sales_order, +) + +CUSTOMER = "Optima Overdue Test Customer" +SALES_ROLES = ["Sales User", "Sales Manager", "Accounts User"] +EXEMPT_ROLE = "Accounts Manager" +SALES_DOCUMENTS = {"Sales Invoice": make_sales_invoice, "Sales Order": make_sales_order} + + +class TestOverdueLimitIntegration(FrappeTestCase): + def setUp(self): + self.company = erpnext.get_default_company() + self.customer = get_or_create_customer(CUSTOMER) + self.overdue_invoice = make_sales_invoice(customer=self.customer) + # Posted today rather than backdated, so it stays in the fiscal year even in January (CI's is + # the calendar year). The check reads only the due date. + self.overdue_invoice.db_set("due_date", add_days(nowdate(), -25)) + frappe.clear_messages() + + def tearDown(self): + frappe.set_user("Administrator") + frappe.db.rollback() + + def make_draft(self, doctype="Sales Invoice"): + return SALES_DOCUMENTS[doctype](customer=self.customer, do_not_submit=True) + + def overdue_messages(self) -> list: + return [msg for msg in frappe.get_message_log() if self.overdue_invoice.name in msg.message] + + def act_as(self, email, roles): + frappe.set_user(get_or_create_user(email, roles)) + + def submit(self, doc): + doc.flags.ignore_permissions = True + doc.submit() + + # ============================================================================================ + # ACTIONS + + def test_warn_on_save_saves_the_draft_with_a_warning(self): + make_overdue_limit(party=self.customer, action=WARN_ON_SAVE) + for doctype in SALES_DOCUMENTS: + with self.subTest(doctype): + frappe.clear_messages() + draft = self.make_draft(doctype) + self.assertFalse(draft.is_new()) + self.assertTrue(self.overdue_messages()) + + def test_warn_on_save_lets_the_submit_through_silently(self): + make_overdue_limit(party=self.customer, action=WARN_ON_SAVE) + draft = self.make_draft() + frappe.clear_messages() + self.submit(draft) + self.assertEqual(draft.docstatus, 1) + self.assertFalse(self.overdue_messages()) + + def test_block_on_save_refuses_the_draft(self): + make_overdue_limit(party=self.customer, action=BLOCK_ON_SAVE) + for doctype in SALES_DOCUMENTS: + with self.subTest(doctype), self.assertRaises(OverdueLimitError): + self.make_draft(doctype) + + def test_block_on_save_refuses_to_submit_a_draft_saved_before_the_rule(self): + draft = self.make_draft() + make_overdue_limit(party=self.customer, action=BLOCK_ON_SAVE) + with self.assertRaises(OverdueLimitError): + self.submit(draft) + + def test_block_on_submit_saves_the_draft_with_a_warning_and_refuses_the_submit(self): + make_overdue_limit(party=self.customer, action=BLOCK_ON_SUBMIT) + for doctype in SALES_DOCUMENTS: + with self.subTest(doctype): + frappe.clear_messages() + draft = self.make_draft(doctype) + self.assertTrue(self.overdue_messages()) + with self.assertRaises(OverdueLimitError): + self.submit(draft) + + # ============================================================================================ + # WHO AND WHAT IS LET THROUGH + + def test_the_acting_users_roles_decide_the_exemption(self): + make_overdue_limit(party=self.customer, action=BLOCK_ON_SAVE, exempt_roles=[EXEMPT_ROLE]) + + self.act_as("optima.overdue.sales@example.com", SALES_ROLES) + with self.assertRaises(OverdueLimitError): + self.make_draft() + + self.act_as("optima.overdue.exempt@example.com", [*SALES_ROLES, EXEMPT_ROLE]) + draft = self.make_draft() + self.submit(draft) + self.assertEqual(draft.docstatus, 1) + + def test_a_credit_note_submits_under_a_block(self): + make_overdue_limit(party=self.customer, action=BLOCK_ON_SAVE) + credit_note = make_credit_note(self.overdue_invoice.name) + credit_note.insert(ignore_permissions=True) + self.submit(credit_note) + self.assertEqual(credit_note.docstatus, 1) + + def test_a_rule_on_an_ancestor_group_applies(self): + group = frappe.db.get_value("Customer", self.customer, "customer_group") + parent_group = frappe.db.get_value("Customer Group", group, "parent_customer_group") + make_overdue_limit(applies_to="Customer Group", party=parent_group, action=BLOCK_ON_SAVE) + with self.assertRaises(OverdueLimitError): + self.make_draft() + + def test_a_rule_for_another_company_does_not_apply(self): + other_company = frappe.db.get_value("Company", {"name": ("!=", self.company)}, "name") + if not other_company: + self.skipTest("the site has a single company") + + make_overdue_limit(party=self.customer, action=BLOCK_ON_SAVE, company=other_company) + self.assertFalse(self.make_draft().is_new()) + + # ============================================================================================ + # LIFTING THE BLOCK + + def test_paying_the_overdue_invoice_lifts_the_block(self): + make_overdue_limit(party=self.customer, action=BLOCK_ON_SAVE) + make_invoice_payment(self.overdue_invoice.name) + self.assertFalse(self.make_draft().is_new()) + + def test_an_unallocated_payment_covering_the_debt_lifts_the_block(self): + make_overdue_limit(party=self.customer, action=BLOCK_ON_SAVE) + make_invoice_payment(self.overdue_invoice.name, allocate=False) + outstanding = frappe.db.get_value("Sales Invoice", self.overdue_invoice.name, "outstanding_amount") + self.assertEqual(outstanding, self.overdue_invoice.grand_total) + self.assertFalse(self.make_draft().is_new()) + + def test_a_disabled_rule_does_nothing(self): + make_overdue_limit(party=self.customer, action=BLOCK_ON_SAVE, enabled=0) + self.assertFalse(self.make_draft().is_new()) + self.assertFalse(self.overdue_messages()) diff --git a/optima_payment/tests/utils.py b/optima_payment/tests/utils.py index 7a4a146..9d77cf0 100644 --- a/optima_payment/tests/utils.py +++ b/optima_payment/tests/utils.py @@ -6,13 +6,16 @@ Setting, Bank, Account, Cost Center, Project, Customer/Supplier, and a reference Sales/Purchase Order with its own taxes table pre-populated to dodge this site's KSA/ZATCA mandatory-field customizations), plus Purchases Bulk Tool -documents with their purchase taxes template, and the party Journal Entries -and users the statement of account needs. Tests import these factories +documents with their purchase taxes template, the party Journal Entries +and users the statement of account needs, and the Sales Invoices, payments +and rules the Time Based Overdue Limit needs. Tests import these factories instead of constructing documents inline. """ import frappe import erpnext +from erpnext.accounts.doctype.payment_entry.payment_entry import get_payment_entry +from erpnext.accounts.doctype.sales_invoice.sales_invoice import make_sales_return from erpnext.accounts.party import get_party_account from frappe.utils import add_days, nowdate @@ -272,33 +275,56 @@ def get_item_tax_charge() -> tuple[str, float]: # REFERENCE SALES / PURCHASE ORDERS -def make_reference_sales_order(company: str, customer: str) -> str: - """Create and submit a minimal Sales Order to use as a Bank Guarantee-BG reference.""" - item = get_or_create_item() +def get_sales_lines(company: str) -> dict: + """One item line and the pre-seeded taxes row of a Sales Order or Invoice, on the test cost center.""" cost_center = get_or_create_cost_center(company) account_head, tax_rate = get_item_tax_charge() + return { + "items": [{"item_code": get_or_create_item(), "qty": 1, "rate": 100, "cost_center": cost_center}], + "taxes": [ + { + "charge_type": "On Net Total", + "account_head": account_head, + "rate": tax_rate, + "cost_center": cost_center, + "description": account_head, + } + ], + } + + +def make_sales_order( + *, + customer: str, + company: str | None = None, + do_not_insert: bool = False, + do_not_submit: bool = False, + **overrides, +) -> frappe.model.document.Document: + company = company or erpnext.get_default_company() so = frappe.get_doc( { "doctype": "Sales Order", "company": company, "customer": customer, - "cost_center": cost_center, + "cost_center": get_or_create_cost_center(company), "delivery_date": nowdate(), - "items": [{"item_code": item, "qty": 1, "rate": 100, "cost_center": cost_center}], - "taxes": [ - { - "charge_type": "On Net Total", - "account_head": account_head, - "rate": tax_rate, - "cost_center": cost_center, - "description": account_head, - } - ], + **get_sales_lines(company), + **overrides, } ) + if do_not_insert: + return so + so.insert(ignore_permissions=True) - so.submit() - return so.name + if not do_not_submit: + so.submit() + return so + + +def make_reference_sales_order(company: str, customer: str) -> str: + """Create and submit a minimal Sales Order to use as a Bank Guarantee-BG reference.""" + return make_sales_order(customer=customer, company=company).name def make_reference_purchase_order(company: str, supplier: str) -> str: @@ -799,3 +825,100 @@ def get_or_create_user(email: str, roles: list[str]) -> str: user.set("roles", [{"role": role} for role in roles]) user.save(ignore_permissions=True) return email + + +# ==================================================================================================== +# TIME BASED OVERDUE LIMIT FACTORIES + + +def make_sales_invoice( + *, + customer: str, + company: str | None = None, + posting_date: str | None = None, + due_date: str | None = None, + do_not_insert: bool = False, + do_not_submit: bool = False, + **overrides, +) -> frappe.model.document.Document: + """A one-line Sales Invoice, due on ``posting_date`` unless ``due_date`` says otherwise. + + set_posting_time keeps a past posting date, which validate would otherwise reset to today. The + header has no cost center: a payment copies it onto balance sheet lines, which some sites refuse. + """ + company = company or erpnext.get_default_company() + posting_date = posting_date or nowdate() + si = frappe.get_doc( + { + "doctype": "Sales Invoice", + "company": company, + "customer": customer, + "set_posting_time": 1, + "posting_date": posting_date, + "due_date": due_date or posting_date, + **get_sales_lines(company), + **overrides, + } + ) + if do_not_insert: + return si + + si.insert(ignore_permissions=True) + if not do_not_submit: + si.submit() + return si + + +def make_credit_note(invoice: str) -> frappe.model.document.Document: + """An unsaved full return of ``invoice``.""" + credit_note = make_sales_return(invoice) + # Site-specific field, mandatory on returns; ignored where it doesn't exist. + credit_note.reason_for_issuance = "Optima test return" + return credit_note + + +def make_invoice_payment(invoice: str, *, allocate: bool = True) -> frappe.model.document.Document: + """A submitted Payment Entry receiving the whole outstanding amount of ``invoice``. + + Without ``allocate`` the payment names no invoice, so ``invoice`` stays outstanding while the + customer owes nothing. + """ + company = frappe.db.get_value("Sales Invoice", invoice, "company") + pe = get_payment_entry("Sales Invoice", invoice, bank_account=get_payment_entry_account(company)) + if not allocate: + pe.set("references", []) + pe.reference_no = "OPTIMA-TEST" + pe.reference_date = nowdate() + # Some sites make remarks mandatory on Payment Entry. + pe.remarks = "Optima test payment" + pe.insert(ignore_permissions=True) + pe.submit() + return pe + + +def make_overdue_limit( + *, + party: str, + action: str, + threshold_days: int = 20, + applies_to: str = "Customer", + company: str | None = None, + exempt_roles: list[str] | None = None, + enabled: int = 1, + do_not_insert: bool = False, +) -> frappe.model.document.Document: + rule = frappe.get_doc( + { + "doctype": "Time Based Overdue Limit", + "enabled": enabled, + "applies_to": applies_to, + "party": party, + "company": company, + "action": action, + "threshold_days": threshold_days, + "exempt_roles": [{"role": role} for role in exempt_roles or []], + } + ) + if not do_not_insert: + rule.insert(ignore_permissions=True) + return rule From dbd228f26e21d089ca740c0beba5dbaa0c862840 Mon Sep 17 00:00:00 2001 From: FawazAlhafiz Date: Tue, 3 Nov 2026 12:57:35 +0300 Subject: [PATCH 07/14] docs(overdue-limit): document how overdue rules warn and block Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 1 + docs/README.md | 5 +- docs/development/testing.md | 11 ++ docs/overdue-limit/README.md | 16 ++ docs/overdue-limit/reference.md | 242 +++++++++++++++++++++++++++++++ docs/overdue-limit/user-guide.md | 154 ++++++++++++++++++++ 6 files changed, 427 insertions(+), 2 deletions(-) create mode 100644 docs/overdue-limit/README.md create mode 100644 docs/overdue-limit/reference.md create mode 100644 docs/overdue-limit/user-guide.md diff --git a/CLAUDE.md b/CLAUDE.md index 81644b1..b88eb20 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -22,6 +22,7 @@ optima_payment/ optima_payment/doctype/ # this app's doctypes (bank_guarantee_bg, letter_of_credit, cheque_*) cheque/ # cheque GL + payment entry override logic purchases_bulk/ # Purchases Bulk Tool: pure decisions + voucher create/rollback (vouchers.py), form door (api.py) + overdue_limit/ # Time Based Overdue Limit: pure decisions + queries + check (limit.py), hooked from doc_events/ override/ # Python class / whitelist overrides tests/utils.py # SHARED test factories (import these, don't build docs inline) docs/ # developer + user documentation (start at docs/README.md) diff --git a/docs/README.md b/docs/README.md index 93b1c4b..ec02f29 100644 --- a/docs/README.md +++ b/docs/README.md @@ -11,6 +11,7 @@ Documentation is organized **by domain**. Each folder has its own index. | [Letter of Credit](letter-of-credit/) | Accountants, implementers & developers | [letter-of-credit/](letter-of-credit/README.md) | | [Purchases Bulk Tool](purchases-bulk-tool/) | Accountants, implementers & developers | [purchases-bulk-tool/](purchases-bulk-tool/README.md) | | [Statement of Account](party-statement/) | Accountants, implementers & developers | [party-statement/](party-statement/README.md) | +| [Time Based Overdue Limit](overdue-limit/) | Accountants, implementers & developers | [overdue-limit/](overdue-limit/README.md) | | [Setup framework](setup/) | Developers | [setup/](setup/README.md) | ## Cross-cutting @@ -23,5 +24,5 @@ Documentation is organized **by domain**. Each folder has its own index. ### Quick links by audience -- **Accountants / implementers:** [Cheque](cheque/user-guide.md) · [Bank Guarantee](bank-guarantee/user-guide.md) · [Letter of Credit](letter-of-credit/user-guide.md) · [Purchases Bulk Tool](purchases-bulk-tool/user-guide.md) · [Statement of Account](party-statement/user-guide.md) -- **Developers:** [Cheque](cheque/reference.md) · [Bank Guarantee](bank-guarantee/reference.md) · [Letter of Credit](letter-of-credit/reference.md) · [Purchases Bulk Tool](purchases-bulk-tool/reference.md) · [Statement of Account](party-statement/reference.md) · [Setup architecture](setup/architecture.md) · [Setup Q&A](setup/questions-and-answers.md) · [Testing](development/testing.md) +- **Accountants / implementers:** [Cheque](cheque/user-guide.md) · [Bank Guarantee](bank-guarantee/user-guide.md) · [Letter of Credit](letter-of-credit/user-guide.md) · [Purchases Bulk Tool](purchases-bulk-tool/user-guide.md) · [Statement of Account](party-statement/user-guide.md) · [Overdue Limit](overdue-limit/user-guide.md) +- **Developers:** [Cheque](cheque/reference.md) · [Bank Guarantee](bank-guarantee/reference.md) · [Letter of Credit](letter-of-credit/reference.md) · [Purchases Bulk Tool](purchases-bulk-tool/reference.md) · [Statement of Account](party-statement/reference.md) · [Overdue Limit](overdue-limit/reference.md) · [Setup architecture](setup/architecture.md) · [Setup Q&A](setup/questions-and-answers.md) · [Testing](development/testing.md) diff --git a/docs/development/testing.md b/docs/development/testing.md index c88b64a..a09e085 100644 --- a/docs/development/testing.md +++ b/docs/development/testing.md @@ -21,6 +21,10 @@ creates survives the test. milliseconds and needs no data on the site. `test_vouchers_integration.py` asserts on the real vouchers (lines, header dates, docstatus) and the row status / reference stamps. See [purchases-bulk-tool/reference.md](../purchases-bulk-tool/reference.md). +- **Time Based Overdue Limit** pairs the same way. `test_limit.py` runs the action table and the + rule precedence on plain rules; `test_limit_integration.py` saves and submits real Sales + Invoices and Sales Orders for a customer with an invoice 25 days overdue. See + [overdue-limit/reference.md](../overdue-limit/reference.md). --- @@ -40,6 +44,11 @@ factories instead of building documents inline: | `get_or_create_purchase_item(company, item_code, is_stock_item=…)` | A purchase Item taxed by the suite's own Item Tax Template, its tax row carrying the test suppliers' tax category so that template applies; pass `is_stock_item=1` for Purchase Receipt tests | | `get_or_create_purchase_item_tax_template(company)` | The Item Tax Template behind those items, on the same account and rate | | `set_test_cost_center_default(company)` | Makes a leaf cost center the session user's `cost_center` default, so the tool's vouchers carry one on sites that make it mandatory on Purchase Invoice. It lives in the defaults cache: pair it with `frappe.clear_cache(user=frappe.session.user)` after the rollback | +| `make_sales_order(customer=…, do_not_insert=…, do_not_submit=…, **overrides)` | A one-line Sales Order on the test item, cost center and pre-seeded tax row; `make_reference_sales_order` wraps it | +| `make_sales_invoice(customer=…, posting_date=…, due_date=…, do_not_insert=…, do_not_submit=…, **overrides)` | A one-line Sales Invoice; a past `posting_date` is kept, and the due date defaults to it. The header has no cost center, as on real invoices, because a payment copies it onto balance sheet lines that some sites refuse one on | +| `make_credit_note(invoice)` | An unsaved full return of the invoice, with the return reason some sites require | +| `make_invoice_payment(invoice, allocate=…)` | A submitted Payment Entry for the invoice's outstanding amount; `allocate=False` leaves it unallocated, so the invoice stays outstanding while the customer owes nothing | +| `make_overdue_limit(party=…, action=…, threshold_days=…, applies_to=…, company=…, exempt_roles=…, enabled=…)` | A Time Based Overdue Limit rule | The factories are defensive (`get_or_create_*`) and work around this bench's KSA/ZATCA customizations (mandatory Arabic name fields, tax category, item tax templates, order-level @@ -57,6 +66,8 @@ bench --site run-tests --module optima_payment.optima_payment.doctype.let bench --site run-tests --module optima_payment.optima_payment.doctype.bank_guarantee_bg.test_bank_guarantee_bg bench --site run-tests --module optima_payment.purchases_bulk.tests.test_vouchers bench --site run-tests --module optima_payment.purchases_bulk.tests.test_vouchers_integration +bench --site run-tests --module optima_payment.overdue_limit.tests.test_limit +bench --site run-tests --module optima_payment.overdue_limit.tests.test_limit_integration ``` The site needs `optima_payment` installed and `allow_tests` enabled diff --git a/docs/overdue-limit/README.md b/docs/overdue-limit/README.md new file mode 100644 index 0000000..18bc044 --- /dev/null +++ b/docs/overdue-limit/README.md @@ -0,0 +1,16 @@ +# Time Based Overdue Limit — Docs + +Everything about warning about or blocking sales to a customer whose invoices have been unpaid too +long past their due date. + +| Document | For | What it covers | +|----------|-----|----------------| +| [user-guide.md](user-guide.md) | Accountants & implementers | Setting up a rule, the three actions, which rule wins, how the days are counted, exempt roles, what is never checked, troubleshooting | +| [reference.md](reference.md) | Developers | The hooks and when they fire, rule selection by tier, the day-count query, the action table, the GL balance guard, the blank company, known limits, tests | + +The feature is layered on **Time Based Overdue Limit**, with the child table **Time Based Overdue +Limit Exempt Role**. It checks standard **Sales Invoice** and **Sales Order** documents when they +are saved and submitted. It is ERPNext's Credit Limit keyed on how long the debt has been overdue +instead of how large it is. + +See also: [Testing](../development/testing.md) for how the suite is written and run. diff --git a/docs/overdue-limit/reference.md b/docs/overdue-limit/reference.md new file mode 100644 index 0000000..2570017 --- /dev/null +++ b/docs/overdue-limit/reference.md @@ -0,0 +1,242 @@ +# Time Based Overdue Limit + +Developer + power-user reference for the **Time Based Overdue Limit** doctype: when its check runs +on Sales Invoices and Sales Orders, which rules apply, how the overdue days are counted, and what +each action does at each stage. For a click-by-click guide aimed at accountants, see +[user-guide.md](user-guide.md). + +--- + +## What it is + +A credit-control rule keyed on the **age** of a customer's debt rather than its amount. It is +ERPNext's Credit Limit (`erpnext/selling/doctype/customer/customer.py`, `check_credit_limit`) with +the same shape: rules on Customer and Customer Group, a customer rule overriding its group's, and +a role that is let through. A rule names one party, an optional company, a day threshold and one +of three actions. The check runs from `doc_events` on **Sales Invoice** and **Sales Order** at +`before_save` and `before_submit`, warns with `frappe.msgprint` or blocks with `frappe.throw`, and +needs no client script. + +The doctype is wholly owned by this app and adds no field to another app's doctype, so the +[setup framework](../setup/architecture.md) and its `sync()` patches are not involved. + +--- + +## Where the code lives + +The work is split into deciding and doing. The decisions are plain functions that take rows and +return values, with no database access. Everything that reads calls them. + +``` +optima_payment/overdue_limit/ +└── limit.py + ├── DECISIONS # pure: skipped documents, rule selection by tier, day count, exemption, + │ # one rule's outcome, the document's verdict + ├── MESSAGES # the warning / refusal text, one line per rule + ├── CHECK # check(doc, stage): loads, decides, then msgprint or throw + └── QUERIES # group chain, enabled rules + exempt roles, oldest overdue invoice, GL balance + +optima_payment/doc_events/overdue_limit.py # before_save / before_submit → limit.check +optima_payment/hooks.py # doc_events for Sales Invoice and Sales Order + +optima_payment/optima_payment/doctype/ +├── time_based_overdue_limit/ # the rule; controller validates threshold and duplicates +└── time_based_overdue_limit_exempt_role/ # child table, one Role per row +``` + +--- + +## Data model + +**Time Based Overdue Limit** (not submittable, named `TBOL-.#####`, title `party`): + +| Field | Type | Notes | +|-------|------|-------| +| `enabled` | Check | Default 1. Only enabled rules are loaded or count as duplicates | +| `applies_to` | Select | Customer / Customer Group. Default Customer | +| `party` | Dynamic Link → `applies_to` | Required | +| `company` | Link Company | Optional. Blank means every company (see [Blank company](#blank-company)) | +| `action` | Select | Warn on Save / Block on Save / Block on Submit | +| `threshold_days` | Int | Required, must be above 0 | +| `exempt_roles` | Table MultiSelect → Time Based Overdue Limit Exempt Role | Shown only for the two block actions. Ignored on Warn on Save even when rows remain from an earlier action | + +**Time Based Overdue Limit Exempt Role** (child): `role`, Link Role, required. + +Controller `validate`: + +- `threshold_days` must be above 0. +- No other **enabled** rule may share `applies_to`, `party`, `company` and `action`. A blank company + only clashes with another blank company, so an every-company rule and a one-company rule with the + same action can coexist, and both apply. + +Permissions: **Accounts Manager** and **System Manager** have full access; **Accounts User** and +**Sales User** read only. If sales users could edit rules or their exempt roles, the control would +be worthless. + +--- + +## When the check runs + +Frappe runs `validate → before_save` on a save and `validate → before_submit` on a submit +(`frappe/model/document.py`, `run_before_save_methods`). Submitting from the form does **not** run +`before_save`, which is why a Block on Save rule is checked at submit too. + +```mermaid +flowchart LR + A[before_save / before_submit] --> B{credit note, POS consolidated,
    migrate, patch, install, import?} + B -- yes --> Z[nothing] + B -- no --> C{rules in the most
    specific tier?} + C -- none --> Z + C -- some --> D{oldest overdue
    Sales Invoice?} + D -- none --> Z + D -- found --> E{decide: warn,
    block or nothing} + E -- nothing --> Z + E -- warn / block --> F{customer GL
    balance > 0?} + F -- no --> Z + F -- yes --> G[msgprint warning
    or throw OverdueLimitError] +``` + +The cheap checks run first, and the GL balance query only once a warning or block is already +decided. + +Skipped documents, `is_exempt_document`: + +| Skipped | Why | +|---------|-----| +| `is_return` | A credit note only lowers what the customer owes | +| `is_consolidated` | POS Closing creates these; blocking one would stop the closing | +| `frappe.flags.in_migrate` / `in_patch` / `in_install` / `in_import` | System writes and Data Import are not sales decisions | + +Sales Order has neither `is_return` nor `is_consolidated`; `doc.get` returns `None` for them. + +--- + +## Rule selection + +`get_group_chain` reads the customer's group (`Customer.customer_group`, not the document's +fetched copy) and its ancestors, nearest first, with one `lft <= … and rgt >= …` query. The +`lft` / `rgt` of the group are read uncached: adding a group renumbers the tree in SQL, which +leaves cached documents stale. `get_enabled_rules` loads every enabled rule naming the customer or +any group in the chain, with their exempt roles. + +`select_rules` then keeps the rules whose company is blank or equals the document's, and returns +the first tier that has any, in this order: + +``` +(Customer, Al Noor Trading) → (Customer Group, Retail) → (Customer Group, Commercial) → (Customer Group, All Customer Groups) +``` + +Every rule of that tier applies. The company filter runs **before** the tier search, so a +customer rule for another company does not hide the group's rules. `applies_to` is matched with +`party`, so a group named like the customer is still a group. + +--- + +## Day count + +`get_oldest_overdue_invoice` returns the Sales Invoice with the earliest `due_date` among: + +``` +customer = doc.customer, company = doc.company, docstatus = 1, is_return = 0, +outstanding_amount > 0, due_date < today +``` + +ordered `due_date, name` and limited to one, so the message can name it. `days_overdue` is +`today − due_date`, and a rule triggers when `days >= threshold_days`. + +`Sales Invoice.due_date` is the **latest** due date of the payment schedule +(`erpnext/controllers/accounts_controller.py`, `self.due_date = max(due_dates)`). An installment +invoice therefore counts as overdue only after its last installment falls due: the check +under-counts, but never blocks wrongly. Per-installment ageing from `Payment Schedule` is a possible +follow-up. + +--- + +## The action table + +`rule_outcome(action, stage, exempt)` for one triggered rule. "Exempt" means the acting user +(`frappe.get_roles()`, the session user, not the document owner) holds one of the rule's exempt +roles, which only counts on the two block actions (`is_exempt_user`). + +| Action | Save | Save, exempt | Submit | Submit, exempt | +|--------|------|--------------|--------|----------------| +| **Warn on Save** | warn | warn | — | — | +| **Block on Save** | **block** | warn | **block** | warn | +| **Block on Submit** | warn | warn | **block** | warn | + +`decide` combines the triggered rules of the tier into a `Verdict(outcome, hits)`: + +- Any block wins. Its hits are only the blocking rules, so the refusal lists only what refuses. +- Otherwise every triggered rule that warns is a hit, and the warning lists them all. +- No triggered rule: `Verdict(None, [])`. + +Each `Hit` carries the rule and whether the user was exempt from it; the message turns each into +one line, adding *Your role lets you continue* or the list of exempt roles. A block is raised as +`OverdueLimitError`, a subclass of `frappe.ValidationError`. A warning is `msgprint` with an orange +indicator, and the document is saved. + +`frappe.get_roles("Administrator")` returns every role, so Administrator is exempt from any rule +that lists at least one exempt role. + +--- + +## GL balance guard + +A Payment Entry or credit note that is not allocated to the invoice leaves its `outstanding_amount` +untouched, so the invoice looks unpaid though the customer owes nothing. Before warning or +blocking, `check` reads the customer's net GL balance for the company: + +``` +sum(debit) − sum(credit) from `tabGL Entry` +where party_type = 'Customer' and party = … and company = … and is_cancelled = 0 +``` + +and does nothing when it is zero or less. This is the GL half of ERPNext's +`get_customer_outstanding`. It uses the `(party_type, party)` index on GL Entry; on a site with +about 256,000 GL rows it took under half a millisecond for the busiest customer. On that site, 39 +of the 90 customer-and-company pairs with an overdue invoice owed nothing overall. + +It catches only a customer who owes nothing at all. A customer who paid the old invoice without +allocating it, and now owes only on a newer invoice that isn't due yet, is still blocked by the old +one until the payment is reconciled. + +--- + +## Blank company + +Frappe fills an empty Link field of a new document from `frappe.defaults` keyed by the +**fieldname** (`frappe/model/create_new.py`, `get_user_default_value`), and every ERPNext site has +a global `company` default. Left alone, insert would turn every blank-company rule into a +default-company rule. The controller's `__setup__` sets `dont_update_if_missing = ["company"]`, so +the server keeps a blank company blank. + +The form still pre-fills the user's default company on a new rule (the client-side +`frappe.model.get_default_value`); clearing it is what makes a rule apply in every company. + +--- + +## Known limits + +| Limit | Effect | Possible follow-up | +|-------|--------|--------------------| +| Installment invoices age from their last due date | Overdue installments count late | Age from `Payment Schedule` rows | +| Residual balances count | 0.01 outstanding can block | An "ignore outstanding below" amount | +| Unallocated payments, when other debt remains | The old invoice still counts | None; reconcile the payment | +| A submitted Sales Order does not pass its Sales Invoice | The invoice is checked on its own | Intended | +| Cancel + amend, Auto Repeat and other server inserts | Checked like any save | Intended | + +--- + +## Tests + +See [testing.md](../development/testing.md). The check's tests come as a pair in +`optima_payment/overdue_limit/tests/`, and the rule's own validation sits beside the doctype: + +| File | Layer | What it proves | +|------|-------|----------------| +| `test_limit.py` | Matrix: plain rules, nothing written | Skipped documents and flags, tier precedence, blank and other company, a group named like the customer, the day count, exemption on block rules only, the whole action table, the threshold boundary (19 / 20 / 21 against 20), block outranking warnings, the message lines | +| `test_limit_integration.py` | Integration: real saves | Each action on Sales Invoice and Sales Order at save and submit, Block on Save refusing an earlier draft's submit, the acting user's roles deciding the exemption, a credit note submitting under a block, an ancestor group's rule applying, another company's rule not applying, payment and an unallocated payment lifting the block, a disabled rule | +| `doctype/time_based_overdue_limit/test_time_based_overdue_limit.py` | Integration | Threshold above 0, the duplicate rule, what is not a duplicate, a blank company staying blank, re-saving a rule | + +Every integration test starts from a customer whose Sales Invoice fell due 25 days ago and is +unpaid. The fixtures come from `optima_payment/tests/utils.py`. diff --git a/docs/overdue-limit/user-guide.md b/docs/overdue-limit/user-guide.md new file mode 100644 index 0000000..3427f8c --- /dev/null +++ b/docs/overdue-limit/user-guide.md @@ -0,0 +1,154 @@ +# Time Based Overdue Limit — User Guide + +A plain-language guide for accountants and implementers to warn about, or stop, new sales to +customers who have left an invoice unpaid too long. For the technical model, see +[reference.md](reference.md). + +--- + +## What it does + +You set a rule for a customer, or for a whole customer group: *"once this customer has an invoice +**20 days** past its due date, **warn** the person making a new sale"*, or **block** the sale +instead. The rule is checked on every new **Sales Invoice** and **Sales Order** for that customer. + +### Worked example + +Today is 02-11-2026. Customer **Al Noor Trading** has one unpaid invoice: + +| Invoice | Due date | Outstanding | +|---------|----------|-------------| +| SINV-2026-00041 | 08-10-2026 | 1,150.00 | + +It is **25 days** past its due date. Two rules are set for the customer: + +| Rule | Action | Threshold Days | +|------|--------|----------------| +| TBOL-00001 | Warn on Save | 20 | +| TBOL-00002 | Block on Save | 30 | + +A sales user saves a new Sales Order for Al Noor Trading. 25 days is past TBOL-00001's 20, but not +yet TBOL-00002's 30, so the order **saves** and shows: + +> Customer **Al Noor Trading** has Sales Invoice **SINV-2026-00041** overdue by **25** days; it was +> due on 08-10-2026. +> - Rule **TBOL-00001** warns from 20 days overdue. + +Five days later the invoice is 30 days overdue, and TBOL-00002 **refuses** the save until the +invoice is paid. + +--- + +## Who can set rules + +Only **Accounts Manager** and **System Manager** can create, change or delete rules. Sales Users +and Accounts Users can open them but not change them, so nobody can lift a block on their own +customer or add their own role to the exempt list. + +--- + +## Create a rule (step by step) + +1. New **Time Based Overdue Limit**. +2. **Applies To**: *Customer* for one customer, or *Customer Group* for every customer in that + group and in all the groups under it. +3. **Party**: pick the customer or the group. +4. **Company**: the rule only counts invoices of this company and only checks its documents. + A new rule starts with your default company. **Clear it** to apply the rule in every company. +5. **Action**: see the table below. +6. **Threshold Days**: how many days past its due date the oldest unpaid invoice must be. The + rule applies **from** that day: with 20, an invoice 20 days overdue already counts. +7. *(Block actions only)* **Exempt Roles**: users with any of these roles get the warning but are + not blocked. +8. **Save**. Untick **Enabled** to switch a rule off without deleting it. + +A customer can have several rules, for example a 20-day warning and a 30-day block. Two +enabled rules with the same customer or group, company and action are not allowed. + +--- + +## The three actions + +| Action | Saving a draft | Submitting | +|--------|----------------|------------| +| **Warn on Save** | Saves, with a warning | Goes through, no message | +| **Block on Save** | **Refused** | **Refused** | +| **Block on Submit** | Saves, with a warning that submit will be refused | **Refused** | + +- **Block on Save** also refuses the submit. Otherwise a draft saved before the customer became + overdue could still be submitted afterwards. +- **Block on Submit** warns at save, so you find out about the block before trying to submit. +- A user with an **exempt role** sees the warning instead of the refusal, at save and at submit. + Exempt roles have no effect on **Warn on Save**: everybody sees that warning. +- The **Administrator** user holds every role, so is exempt from any rule that lists at least one + exempt role. + +--- + +## Which rule applies + +The most specific rules win, and only those: + +1. The customer's **own** rules. +2. If it has none, the rules of **its customer group**. +3. If that group has none, the rules of the **parent group**, then its parent, and so on up the + tree. + +All the rules at that first level apply together. A rule for a different company is ignored and +does not hide the rules above it. + +``` +Al Noor Trading (no rule) → Retail (no rule) → Commercial: Block on Save, 30 days ← applies + ↓ + All Customer Groups: Warn on Save, 10 days ← ignored +``` + +--- + +## How the days are counted + +- Only **submitted Sales Invoices** of the customer, in the document's **company**, with an + **outstanding amount** and a **due date before today** are counted. Credit notes are not. +- The **oldest** due date among them decides. Newer overdue invoices don't change the count. +- The day count is today minus that due date. +- On an invoice paid in installments, the due date is that of the **last** installment, so an + earlier installment that is already late is not counted yet. + +--- + +## What is never checked + +- **Credit notes** (return invoices), so you can always reduce what the customer owes. +- **POS consolidated invoices**, which POS Closing creates. Blocking them would stop the closing. +- Documents created by **Data Import**, a migration or an app install. +- A customer who, on the company's books, **owes nothing at all**, even if an invoice still shows + as unpaid. This happens when a payment or credit note was recorded but not allocated to the + invoice. To clear the invoice itself, match the payment to it with **Payment Reconciliation**. + +--- + +## Things to know + +- **A submitted Sales Order does not let its Sales Invoice through.** The invoice is checked on its + own when it is made. +- **Cancel and amend.** The amended document is a new save, so it is blocked like any other + unless you hold an exempt role. +- **With Block on Save, a sales user can't even prepare a draft** for an overdue customer. Choose + **Block on Submit** if drafts should still be allowed. +- **A tiny leftover balance counts.** An invoice with 0.01 outstanding is still unpaid and can + block. Write such balances off. +- **Background jobs are checked too.** A Sales Invoice made automatically (for example by Auto + Repeat) for a blocked customer is refused the same way. + +--- + +## Troubleshooting + +| Message or symptom | Cause | Fix | +|--------------------|-------|-----| +| *Customer X has Sales Invoice Y overdue by N days … blocks saving and submitting* | A Block on Save rule applies | Collect and record the payment against invoice Y, or ask a user with an exempt role | +| *… blocks submitting* when you submit | A Block on Submit rule applies | As above. The draft stays saved | +| A customer is blocked though they have paid | The payment is recorded but not allocated, and they still owe on another invoice | Allocate the payment with **Payment Reconciliation** | +| *Threshold Days must be greater than 0* | Threshold Days is 0 or empty | Enter a number of days above 0 | +| *Enabled rule … already has the same Applies To, Party, Company and Action* | A second rule repeats an enabled one | Change the existing rule instead, or disable it first | +| A rule never fires in one company | It names a different company | Clear **Company**, or set the right one | From 96b8737b2a272587dbc01b97daa62709487f0706 Mon Sep 17 00:00:00 2001 From: FawazAlhafiz Date: Tue, 3 Nov 2026 13:42:52 +0300 Subject: [PATCH 08/14] test(overdue-limit): keep only the matrix cases that catch a plausible bug The matrix goes from 24 cases to 12. Checked against 23 bugs injected into limit.py, both versions catch 21. The old one missed an exemption from one block rule lifting another, and a misspelt skip flag, which its loop over SKIP_FLAGS could never catch. The new one misses only two wording changes, which it no longer pins. Dropped: cases restating one-liners (the day count, the role intersection), empty-in-empty-out, and the message sentences. The action table is now the reference doc's three-by-four table, tested against rule_outcome. Co-Authored-By: Claude Opus 5.5 --- docs/overdue-limit/reference.md | 2 +- .../overdue_limit/tests/test_limit.py | 244 ++++++------------ 2 files changed, 87 insertions(+), 159 deletions(-) diff --git a/docs/overdue-limit/reference.md b/docs/overdue-limit/reference.md index 2570017..48780d3 100644 --- a/docs/overdue-limit/reference.md +++ b/docs/overdue-limit/reference.md @@ -234,7 +234,7 @@ See [testing.md](../development/testing.md). The check's tests come as a pair in | File | Layer | What it proves | |------|-------|----------------| -| `test_limit.py` | Matrix: plain rules, nothing written | Skipped documents and flags, tier precedence, blank and other company, a group named like the customer, the day count, exemption on block rules only, the whole action table, the threshold boundary (19 / 20 / 21 against 20), block outranking warnings, the message lines | +| `test_limit.py` | Matrix: plain rules, nothing written | The documents never checked, tier precedence, other-company rules, a group named like the customer, the action table, the threshold day, how several triggered rules combine (including an exemption from only one), what the message says about exemption. The message wording is not pinned | | `test_limit_integration.py` | Integration: real saves | Each action on Sales Invoice and Sales Order at save and submit, Block on Save refusing an earlier draft's submit, the acting user's roles deciding the exemption, a credit note submitting under a block, an ancestor group's rule applying, another company's rule not applying, payment and an unallocated payment lifting the block, a disabled rule | | `doctype/time_based_overdue_limit/test_time_based_overdue_limit.py` | Integration | Threshold above 0, the duplicate rule, what is not a duplicate, a blank company staying blank, re-saving a rule | diff --git a/optima_payment/overdue_limit/tests/test_limit.py b/optima_payment/overdue_limit/tests/test_limit.py index cebbf56..30f3646 100644 --- a/optima_payment/overdue_limit/tests/test_limit.py +++ b/optima_payment/overdue_limit/tests/test_limit.py @@ -1,15 +1,16 @@ # Copyright (c) 2026, IT Systematic and Contributors # See license.txt -"""The branch matrix for the Time Based Overdue Limit. Nothing here writes to the database. +"""The Time Based Overdue Limit's decisions, on plain rows. Nothing here writes to the database. -Rules are ``frappe._dict`` rows shaped like get_enabled_rules returns them, so no customer, group or -rule has to exist on the site. The wiring through real saves is in test_limit_integration.py. +Only what a plausible edit could break is here: which tier of rules applies, the action table, how +several triggered rules combine, the day a rule starts, what the message tells the user about +exemption, and the documents never checked, which no integration test reaches for POS. The wiring is +in test_limit_integration.py. """ import frappe from frappe.tests.utils import FrappeTestCase -from frappe.utils import formatdate, getdate from optima_payment.overdue_limit.limit import ( BLOCK, @@ -18,17 +19,14 @@ CUSTOMER, CUSTOMER_GROUP, SAVE, - SKIP_FLAGS, SUBMIT, WARN, WARN_ON_SAVE, - Hit, - Verdict, days_overdue, decide, is_exempt_document, - is_exempt_user, message, + rule_outcome, select_rules, ) @@ -70,184 +68,114 @@ def group_rule(name, group, **fields): return rule(name, applies_to=CUSTOMER_GROUP, party=group, **fields) -def names(rules): - return [rule.name for rule in rules] - - def hit_names(verdict): return [hit.rule.name for hit in verdict.hits] class TestIsExemptDocument(FrappeTestCase): - def test_only_credit_notes_pos_consolidations_and_system_writes_are_skipped(self): - cases = [ - ("an ordinary invoice", 0, 0, {}, False), - ("a Sales Order, which has neither field", None, None, {}, False), - ("a credit note", 1, 0, {}, True), - ("a POS consolidated invoice", 0, 1, {}, True), - ("a skip flag set to false", 0, 0, {"in_import": False}, False), - ("an unrelated flag", 0, 0, {"in_test": True}, False), - *[(flag, 0, 0, {flag: True}, True) for flag in SKIP_FLAGS], - ] - for label, is_return, is_consolidated, flags, expected in cases: - with self.subTest(label): - self.assertEqual(is_exempt_document(is_return, is_consolidated, frappe._dict(flags)), expected) + def test_credit_notes_pos_consolidations_and_system_writes_are_never_checked(self): + """Flags by their literal names: looping over SKIP_FLAGS cannot catch a misspelt one.""" + self.assertFalse(is_exempt_document(None, None, frappe._dict())) + self.assertTrue(is_exempt_document(1, 0, frappe._dict())) + self.assertTrue(is_exempt_document(0, 1, frappe._dict())) + for flag in ("in_migrate", "in_patch", "in_install", "in_import"): + with self.subTest(flag): + self.assertTrue(is_exempt_document(0, 0, frappe._dict({flag: True}))) class TestSelectRules(FrappeTestCase): def select(self, rules, company=COMPANY): - return names(select_rules(rules, CUSTOMER_NAME, GROUP_CHAIN, company)) - - def test_customer_rules_beat_group_rules(self): - rules = [group_rule("G", GROUP), rule("C")] - self.assertEqual(self.select(rules), ["C"]) - - def test_the_nearest_group_beats_its_ancestors(self): - rules = [group_rule("ROOT", ROOT_GROUP), group_rule("PARENT", PARENT_GROUP), group_rule("G", GROUP)] - self.assertEqual(self.select(rules), ["G"]) - - def test_an_ancestor_applies_when_nearer_tiers_have_none(self): - rules = [group_rule("ROOT", ROOT_GROUP), group_rule("PARENT", PARENT_GROUP)] - self.assertEqual(self.select(rules), ["PARENT"]) - - def test_every_rule_of_the_tier_applies(self): - rules = [ - rule("WARN", action=WARN_ON_SAVE, threshold_days=20), - rule("BLOCK", action=BLOCK_ON_SUBMIT, threshold_days=30), - group_rule("G", GROUP), + return [rule.name for rule in select_rules(rules, CUSTOMER_NAME, GROUP_CHAIN, company)] + + def test_the_nearest_tier_with_a_rule_wins(self): + customer, group, parent, root = ( + rule("CUSTOMER"), + group_rule("GROUP", GROUP), + group_rule("PARENT", PARENT_GROUP), + group_rule("ROOT", ROOT_GROUP), + ) + # Furthest first, so the order of the rows cannot be what picks the winner. + cases = [ + ([root, parent, group, customer], "CUSTOMER"), + ([root, parent, group], "GROUP"), + ([root, parent], "PARENT"), ] - self.assertEqual(self.select(rules), ["WARN", "BLOCK"]) - - def test_a_blank_company_matches_every_company(self): - rules = [rule("ALL"), rule("MINE", company=COMPANY)] - self.assertEqual(self.select(rules), ["ALL", "MINE"]) - self.assertEqual(self.select(rules, company=OTHER_COMPANY), ["ALL"]) - - def test_a_rule_for_another_company_neither_applies_nor_shadows_a_wider_tier(self): - rules = [rule("C", company=OTHER_COMPANY), group_rule("G", GROUP)] - self.assertEqual(self.select(rules), ["G"]) + for rules, winner in cases: + with self.subTest(winner): + self.assertEqual(self.select(rules), [winner]) - def test_a_group_named_like_the_customer_is_matched_as_a_group(self): - rules = [group_rule("G", CUSTOMER_NAME)] - self.assertEqual(self.select(rules), []) - self.assertEqual(names(select_rules(rules, CUSTOMER_NAME, [CUSTOMER_NAME], COMPANY)), ["G"]) - - def test_a_customer_without_rules_or_group_selects_nothing(self): - self.assertEqual(self.select([]), []) - self.assertEqual(names(select_rules([group_rule("G", GROUP)], CUSTOMER_NAME, [], COMPANY)), []) + def test_every_rule_of_the_winning_tier_applies(self): + rules = [rule("WARN"), rule("BLOCK", action=BLOCK_ON_SAVE), group_rule("GROUP", GROUP)] + self.assertEqual(self.select(rules), ["WARN", "BLOCK"]) + def test_a_company_rule_neither_applies_nor_shadows_in_another_company(self): + rules = [rule("ELSEWHERE", company=OTHER_COMPANY), group_rule("EVERYWHERE", GROUP)] + self.assertEqual(self.select(rules, company=COMPANY), ["EVERYWHERE"]) + self.assertEqual(self.select(rules, company=OTHER_COMPANY), ["ELSEWHERE"]) -class TestDaysOverdue(FrappeTestCase): - def test_days_are_counted_from_the_due_date(self): - self.assertEqual(days_overdue("2026-10-08", "2026-11-02"), 25) - self.assertEqual(days_overdue(getdate("2026-11-01"), getdate("2026-11-02")), 1) + def test_a_group_named_like_the_customer_is_not_a_customer_rule(self): + self.assertEqual(self.select([group_rule("GROUP", CUSTOMER_NAME)]), []) -class TestIsExemptUser(FrappeTestCase): - def test_a_shared_role_exempts_only_from_a_block_rule(self): - cases = [ - ("a shared role on a block rule", BLOCK_ON_SAVE, [EXEMPT_ROLE], {EXEMPT_ROLE}, True), - ("a shared role on a block-on-submit rule", BLOCK_ON_SUBMIT, [EXEMPT_ROLE], {EXEMPT_ROLE}, True), - ("no shared role", BLOCK_ON_SAVE, [EXEMPT_ROLE], set(SALES_ROLES), False), - ("no exempt roles on the rule", BLOCK_ON_SAVE, [], {EXEMPT_ROLE}, False), - ("a shared role on a warn rule", WARN_ON_SAVE, [EXEMPT_ROLE], {EXEMPT_ROLE}, False), - ] - for label, action, exempt_roles, user_roles, expected in cases: - with self.subTest(label): - self.assertEqual(is_exempt_user(rule(action=action, exempt_roles=exempt_roles), user_roles), expected) +class TestRuleOutcome(FrappeTestCase): + def test_the_action_table(self): + """The table in docs/overdue-limit/reference.md.""" + columns = [(SAVE, False), (SAVE, True), (SUBMIT, False), (SUBMIT, True)] + table = { + WARN_ON_SAVE: [WARN, WARN, None, None], + BLOCK_ON_SAVE: [BLOCK, WARN, BLOCK, WARN], + BLOCK_ON_SUBMIT: [WARN, WARN, BLOCK, WARN], + } + for action, row in table.items(): + for (stage, exempt), expected in zip(columns, row): + with self.subTest(action=action, stage=stage, exempt=exempt): + self.assertEqual(rule_outcome(action, stage, exempt), expected) class TestDecide(FrappeTestCase): - def test_the_action_table(self): - """Every action at both stages, for a user with and without the rule's exempt role.""" - table = [ - (WARN_ON_SAVE, SAVE, False, WARN), - (WARN_ON_SAVE, SAVE, True, WARN), - (WARN_ON_SAVE, SUBMIT, False, None), - (WARN_ON_SAVE, SUBMIT, True, None), - (BLOCK_ON_SAVE, SAVE, False, BLOCK), - (BLOCK_ON_SAVE, SAVE, True, WARN), - (BLOCK_ON_SAVE, SUBMIT, False, BLOCK), - (BLOCK_ON_SAVE, SUBMIT, True, WARN), - (BLOCK_ON_SUBMIT, SAVE, False, WARN), - (BLOCK_ON_SUBMIT, SAVE, True, WARN), - (BLOCK_ON_SUBMIT, SUBMIT, False, BLOCK), - (BLOCK_ON_SUBMIT, SUBMIT, True, WARN), - ] - for action, stage, exempt, expected in table: - with self.subTest(action=action, stage=stage, exempt=exempt): - user_roles = [EXEMPT_ROLE] if exempt else SALES_ROLES - verdict = decide([rule(action=action, exempt_roles=[EXEMPT_ROLE])], 30, user_roles, stage) - self.assertEqual(verdict.outcome, expected) + def outcome_on(self, today, rules): + return decide(rules, days_overdue(INVOICE.due_date, today), SALES_ROLES, SAVE).outcome - def test_a_rule_triggers_from_its_threshold(self): + def test_a_rule_starts_on_its_threshold_day(self): rules = [rule(action=BLOCK_ON_SAVE, threshold_days=20)] - for days, expected in [(19, None), (20, BLOCK), (21, BLOCK)]: - with self.subTest(days=days): - self.assertEqual(decide(rules, days, SALES_ROLES, SAVE).outcome, expected) - - def test_a_block_outranks_warnings_and_leaves_them_out(self): - rules = [rule("WARN", threshold_days=20), rule("BLOCK", action=BLOCK_ON_SAVE, threshold_days=30)] - verdict = decide(rules, 30, SALES_ROLES, SAVE) - self.assertEqual(verdict.outcome, BLOCK) - self.assertEqual(hit_names(verdict), ["BLOCK"]) + self.assertIsNone(self.outcome_on("2026-10-27", rules)) + self.assertEqual(self.outcome_on("2026-10-28", rules), BLOCK) - def test_every_triggered_warning_is_listed(self): - rules = [rule("WARN", threshold_days=20), rule("NOTICE", action=BLOCK_ON_SUBMIT, threshold_days=25)] - verdict = decide(rules, 30, SALES_ROLES, SAVE) - self.assertEqual(verdict.outcome, WARN) - self.assertEqual(hit_names(verdict), ["WARN", "NOTICE"]) + def test_the_refusal_lists_only_the_rules_that_block_this_user(self): + rules = [ + rule("WARN"), + rule("EXEMPTED", action=BLOCK_ON_SAVE, exempt_roles=[EXEMPT_ROLE]), + rule("BLOCK", action=BLOCK_ON_SAVE), + ] + verdict = decide(rules, 30, [EXEMPT_ROLE], SAVE) + self.assertEqual((verdict.outcome, hit_names(verdict)), (BLOCK, ["BLOCK"])) - def test_a_rule_below_its_threshold_is_left_out(self): - rules = [rule("WARN", threshold_days=20), rule("BLOCK", action=BLOCK_ON_SAVE, threshold_days=40)] + def test_the_warning_lists_every_triggered_rule_and_no_other(self): + rules = [ + rule("WARN", threshold_days=20), + rule("NOTICE", action=BLOCK_ON_SUBMIT, threshold_days=25), + rule("NOT YET", action=BLOCK_ON_SAVE, threshold_days=40), + ] verdict = decide(rules, 30, SALES_ROLES, SAVE) - self.assertEqual(verdict.outcome, WARN) - self.assertEqual(hit_names(verdict), ["WARN"]) - - def test_an_exempt_user_is_warned_and_the_hit_says_so(self): - verdict = decide([rule(action=BLOCK_ON_SAVE, exempt_roles=[EXEMPT_ROLE])], 30, [EXEMPT_ROLE], SAVE) - self.assertEqual(verdict.outcome, WARN) - self.assertTrue(verdict.hits[0].exempt) - - def test_no_rules_decide_nothing(self): - self.assertEqual(decide([], 30, SALES_ROLES, SAVE), Verdict(None, [])) + self.assertEqual((verdict.outcome, hit_names(verdict)), (WARN, ["WARN", "NOTICE"])) class TestMessage(FrappeTestCase): - def render(self, *hits, outcome=BLOCK): - return message(Verdict(outcome, list(hits)), CUSTOMER_NAME, INVOICE, 25) - - def test_the_header_names_the_customer_invoice_days_and_due_date(self): - text = self.render(Hit(rule(action=BLOCK_ON_SAVE), False)) - self.assertIn( - f"Customer {CUSTOMER_NAME} has Sales Invoice {INVOICE.name} " - f"overdue by 25 days; it was due on {formatdate(INVOICE.due_date)}.", - text, - ) - - def test_each_action_reads_as_what_it_does(self): - cases = [ - (WARN_ON_SAVE, "Rule TBOL-00001 warns from 20 days overdue."), - (BLOCK_ON_SAVE, "Rule TBOL-00001 blocks saving and submitting from 20 days overdue."), - (BLOCK_ON_SUBMIT, "Rule TBOL-00001 blocks submitting from 20 days overdue."), - ] - for action, expected in cases: - with self.subTest(action): - self.assertIn(f"
  • {expected}
    • ", self.render(Hit(rule(action=action), False))) + def render(self, action, user_roles): + verdict = decide([rule(action=action, exempt_roles=[EXEMPT_ROLE])], 30, user_roles, SAVE) + return message(verdict, CUSTOMER_NAME, INVOICE, 30) def test_a_blocked_user_is_told_which_roles_can_continue(self): - text = self.render(Hit(rule(action=BLOCK_ON_SAVE, exempt_roles=[EXEMPT_ROLE, "System Manager"]), False)) - self.assertIn("Roles that can continue: Accounts Manager, System Manager.", text) + self.assertIn(EXEMPT_ROLE, self.render(BLOCK_ON_SAVE, SALES_ROLES)) def test_an_exempt_user_is_told_their_role_lets_them_continue(self): - text = self.render(Hit(rule(action=BLOCK_ON_SAVE, exempt_roles=[EXEMPT_ROLE]), True), outcome=WARN) + text = self.render(BLOCK_ON_SAVE, [EXEMPT_ROLE]) self.assertIn("Your role lets you continue.", text) - self.assertNotIn("Roles that can continue", text) - - def test_a_warn_rule_lists_no_roles(self): - text = self.render(Hit(rule(action=WARN_ON_SAVE, exempt_roles=[EXEMPT_ROLE]), False), outcome=WARN) - self.assertNotIn("Roles that can continue", text) - - def test_every_hit_gets_its_own_line(self): - text = self.render(Hit(rule("FIRST"), False), Hit(rule("SECOND"), False), outcome=WARN) - self.assertEqual(text.count("
  • "), 2) + self.assertNotIn(EXEMPT_ROLE, text) + + def test_a_warn_rule_ignores_the_exempt_roles_the_form_hides(self): + for user_roles in (SALES_ROLES, [EXEMPT_ROLE]): + with self.subTest(user_roles=user_roles): + text = self.render(WARN_ON_SAVE, user_roles) + self.assertNotIn(EXEMPT_ROLE, text) + self.assertNotIn("Your role lets you continue.", text) From 0d1800f2e74964cbdd173f8c624c6845efff9419 Mon Sep 17 00:00:00 2001 From: FawazAlhafiz Date: Tue, 3 Nov 2026 13:42:52 +0300 Subject: [PATCH 09/14] docs(testing): require each matrix case to name the bug it catches Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 7 ++++--- docs/development/testing.md | 7 ++++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index b88eb20..6bbef0d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -62,9 +62,10 @@ mandatory before submit. Issue **commission is never reversed** on return/cancel - `FrappeTestCase` (unittest), isolated by `frappe.db.rollback()` in `tearDown`. No mocking. - Most suites are live-DB integration tests. Code that keeps its decisions in plain functions - pairs two files instead: `test_.py` checks every branch with plain rows and writes nothing, - and `test__integration.py` covers only what a real save shows. Example: - `purchases_bulk/tests/`. + pairs two files instead: `test_.py` checks those decisions with plain rows and writes + nothing, and `test__integration.py` covers only what a real save shows. A matrix case must + name the plausible bug it catches; one-liners, the code's own constants and message wording get + none. Example: `overdue_limit/tests/`. - Build fixtures with the factories in `optima_payment/tests/utils.py` (`make_optima_payment_setting`, `make_bank_guarantee_bg`, `make_letter_of_credit`). They are defensive and work around this bench's KSA/ZATCA mandatory-field customizations. diff --git a/docs/development/testing.md b/docs/development/testing.md index a09e085..d0680fc 100644 --- a/docs/development/testing.md +++ b/docs/development/testing.md @@ -17,7 +17,7 @@ creates survives the test. assert on those Payment Entries (`get_lc_payment_entries`, filtered by the `is_lc_*` flags). See [letter-of-credit/reference.md](../letter-of-credit/reference.md). - **Purchases Bulk Tool** keeps its decisions in plain functions, so its tests come as a pair. - `test_vouchers.py` checks every branch with plain rows and writes nothing; it runs in + `test_vouchers.py` checks those decisions with plain rows and writes nothing; it runs in milliseconds and needs no data on the site. `test_vouchers_integration.py` asserts on the real vouchers (lines, header dates, docstatus) and the row status / reference stamps. See [purchases-bulk-tool/reference.md](../purchases-bulk-tool/reference.md). @@ -26,6 +26,11 @@ creates survives the test. Invoices and Sales Orders for a customer with an invoice 25 days overdue. See [overdue-limit/reference.md](../overdue-limit/reference.md). +A matrix case earns its place by naming the plausible bug it catches: how rules combine, a +boundary, a reachable edge case, or a spec table from the docs. A one-liner, a loop over the code's +own constant, and a message's wording get no case; assert which line a branch produces, not the +sentence. `overdue_limit/tests/test_limit.py` is the worked example. + --- ## Shared fixtures From 3b3fb5cfdbde1afd771ae290f3adfa9b9e3e7864 Mon Sep 17 00:00:00 2001 From: FawazAlhafiz Date: Tue, 3 Nov 2026 17:55:04 +0300 Subject: [PATCH 10/14] feat(overdue-limit): block on an unpaid overdue invoice regardless of customer balance A payment or credit note that isn't allocated to the overdue invoice no longer lifts the block, even when the customer owes nothing overall: it has to be matched to the invoice with Payment Reconciliation. This also drops the GL Entry query the check ran before every warning or block. --- docs/overdue-limit/README.md | 2 +- docs/overdue-limit/reference.md | 35 +++---------------- docs/overdue-limit/user-guide.md | 8 ++--- optima_payment/overdue_limit/limit.py | 22 ++---------- .../tests/test_limit_integration.py | 9 +++-- 5 files changed, 15 insertions(+), 61 deletions(-) diff --git a/docs/overdue-limit/README.md b/docs/overdue-limit/README.md index 18bc044..e11bc34 100644 --- a/docs/overdue-limit/README.md +++ b/docs/overdue-limit/README.md @@ -6,7 +6,7 @@ long past their due date. | Document | For | What it covers | |----------|-----|----------------| | [user-guide.md](user-guide.md) | Accountants & implementers | Setting up a rule, the three actions, which rule wins, how the days are counted, exempt roles, what is never checked, troubleshooting | -| [reference.md](reference.md) | Developers | The hooks and when they fire, rule selection by tier, the day-count query, the action table, the GL balance guard, the blank company, known limits, tests | +| [reference.md](reference.md) | Developers | The hooks and when they fire, rule selection by tier, the day-count query, the action table, the blank company, known limits, tests | The feature is layered on **Time Based Overdue Limit**, with the child table **Time Based Overdue Limit Exempt Role**. It checks standard **Sales Invoice** and **Sales Order** documents when they diff --git a/docs/overdue-limit/reference.md b/docs/overdue-limit/reference.md index 48780d3..2374202 100644 --- a/docs/overdue-limit/reference.md +++ b/docs/overdue-limit/reference.md @@ -34,7 +34,7 @@ optima_payment/overdue_limit/ │ # one rule's outcome, the document's verdict ├── MESSAGES # the warning / refusal text, one line per rule ├── CHECK # check(doc, stage): loads, decides, then msgprint or throw - └── QUERIES # group chain, enabled rules + exempt roles, oldest overdue invoice, GL balance + └── QUERIES # group chain, enabled rules + exempt roles, oldest overdue invoice optima_payment/doc_events/overdue_limit.py # before_save / before_submit → limit.check optima_payment/hooks.py # doc_events for Sales Invoice and Sales Order @@ -91,14 +91,9 @@ flowchart LR D -- none --> Z D -- found --> E{decide: warn,
    block or nothing} E -- nothing --> Z - E -- warn / block --> F{customer GL
    balance > 0?} - F -- no --> Z - F -- yes --> G[msgprint warning
    or throw OverdueLimitError] + E -- warn / block --> G[msgprint warning
    or throw OverdueLimitError] ``` -The cheap checks run first, and the GL balance query only once a warning or block is already -decided. - Skipped documents, `is_exempt_document`: | Skipped | Why | @@ -180,28 +175,6 @@ that lists at least one exempt role. --- -## GL balance guard - -A Payment Entry or credit note that is not allocated to the invoice leaves its `outstanding_amount` -untouched, so the invoice looks unpaid though the customer owes nothing. Before warning or -blocking, `check` reads the customer's net GL balance for the company: - -``` -sum(debit) − sum(credit) from `tabGL Entry` -where party_type = 'Customer' and party = … and company = … and is_cancelled = 0 -``` - -and does nothing when it is zero or less. This is the GL half of ERPNext's -`get_customer_outstanding`. It uses the `(party_type, party)` index on GL Entry; on a site with -about 256,000 GL rows it took under half a millisecond for the busiest customer. On that site, 39 -of the 90 customer-and-company pairs with an overdue invoice owed nothing overall. - -It catches only a customer who owes nothing at all. A customer who paid the old invoice without -allocating it, and now owes only on a newer invoice that isn't due yet, is still blocked by the old -one until the payment is reconciled. - ---- - ## Blank company Frappe fills an empty Link field of a new document from `frappe.defaults` keyed by the @@ -221,7 +194,7 @@ The form still pre-fills the user's default company on a new rule (the client-si |-------|--------|--------------------| | Installment invoices age from their last due date | Overdue installments count late | Age from `Payment Schedule` rows | | Residual balances count | 0.01 outstanding can block | An "ignore outstanding below" amount | -| Unallocated payments, when other debt remains | The old invoice still counts | None; reconcile the payment | +| Unallocated payments and credit notes | The invoice stays outstanding and still counts, even when the customer owes nothing overall | None; reconcile them with Payment Reconciliation | | A submitted Sales Order does not pass its Sales Invoice | The invoice is checked on its own | Intended | | Cancel + amend, Auto Repeat and other server inserts | Checked like any save | Intended | @@ -235,7 +208,7 @@ See [testing.md](../development/testing.md). The check's tests come as a pair in | File | Layer | What it proves | |------|-------|----------------| | `test_limit.py` | Matrix: plain rules, nothing written | The documents never checked, tier precedence, other-company rules, a group named like the customer, the action table, the threshold day, how several triggered rules combine (including an exemption from only one), what the message says about exemption. The message wording is not pinned | -| `test_limit_integration.py` | Integration: real saves | Each action on Sales Invoice and Sales Order at save and submit, Block on Save refusing an earlier draft's submit, the acting user's roles deciding the exemption, a credit note submitting under a block, an ancestor group's rule applying, another company's rule not applying, payment and an unallocated payment lifting the block, a disabled rule | +| `test_limit_integration.py` | Integration: real saves | Each action on Sales Invoice and Sales Order at save and submit, Block on Save refusing an earlier draft's submit, the acting user's roles deciding the exemption, a credit note submitting under a block, an ancestor group's rule applying, another company's rule not applying, an allocated payment lifting the block and an unallocated one not, a disabled rule | | `doctype/time_based_overdue_limit/test_time_based_overdue_limit.py` | Integration | Threshold above 0, the duplicate rule, what is not a duplicate, a blank company staying blank, re-saving a rule | Every integration test starts from a customer whose Sales Invoice fell due 25 days ago and is diff --git a/docs/overdue-limit/user-guide.md b/docs/overdue-limit/user-guide.md index 3427f8c..53e67e7 100644 --- a/docs/overdue-limit/user-guide.md +++ b/docs/overdue-limit/user-guide.md @@ -121,9 +121,6 @@ Al Noor Trading (no rule) → Retail (no rule) → Commercial: Block on Save, 30 - **Credit notes** (return invoices), so you can always reduce what the customer owes. - **POS consolidated invoices**, which POS Closing creates. Blocking them would stop the closing. - Documents created by **Data Import**, a migration or an app install. -- A customer who, on the company's books, **owes nothing at all**, even if an invoice still shows - as unpaid. This happens when a payment or credit note was recorded but not allocated to the - invoice. To clear the invoice itself, match the payment to it with **Payment Reconciliation**. --- @@ -135,6 +132,9 @@ Al Noor Trading (no rule) → Retail (no rule) → Commercial: Block on Save, 30 unless you hold an exempt role. - **With Block on Save, a sales user can't even prepare a draft** for an overdue customer. Choose **Block on Submit** if drafts should still be allowed. +- **Only allocated payments count.** A payment or credit note recorded for the customer but not + allocated to the overdue invoice leaves that invoice unpaid, so the rule still applies, even + when the customer owes nothing overall. Match it to the invoice with **Payment Reconciliation**. - **A tiny leftover balance counts.** An invoice with 0.01 outstanding is still unpaid and can block. Write such balances off. - **Background jobs are checked too.** A Sales Invoice made automatically (for example by Auto @@ -148,7 +148,7 @@ Al Noor Trading (no rule) → Retail (no rule) → Commercial: Block on Save, 30 |--------------------|-------|-----| | *Customer X has Sales Invoice Y overdue by N days … blocks saving and submitting* | A Block on Save rule applies | Collect and record the payment against invoice Y, or ask a user with an exempt role | | *… blocks submitting* when you submit | A Block on Submit rule applies | As above. The draft stays saved | -| A customer is blocked though they have paid | The payment is recorded but not allocated, and they still owe on another invoice | Allocate the payment with **Payment Reconciliation** | +| A customer is blocked though they have paid | The payment or credit note is recorded but not allocated to the overdue invoice | Allocate it with **Payment Reconciliation** | | *Threshold Days must be greater than 0* | Threshold Days is 0 or empty | Enter a number of days above 0 | | *Enabled rule … already has the same Applies To, Party, Company and Action* | A second rule repeats an enabled one | Change the existing rule instead, or disable it first | | A rule never fires in one company | It names a different company | Clear **Company**, or set the right one | diff --git a/optima_payment/overdue_limit/limit.py b/optima_payment/overdue_limit/limit.py index e772302..01d34da 100644 --- a/optima_payment/overdue_limit/limit.py +++ b/optima_payment/overdue_limit/limit.py @@ -16,8 +16,7 @@ import frappe from frappe import _ from frappe.model.document import Document -from frappe.query_builder.functions import Sum -from frappe.utils import flt, formatdate, getdate +from frappe.utils import formatdate, getdate RULE = "Time Based Overdue Limit" EXEMPT_ROLE = "Time Based Overdue Limit Exempt Role" @@ -162,9 +161,7 @@ def check(doc: Document, stage: str) -> None: days = days_overdue(invoice.due_date, today) verdict = decide(rules, days, frappe.get_roles(), stage) - # A payment or credit note not yet allocated to the invoice leaves it outstanding, though the - # customer owes nothing. - if not verdict.outcome or get_customer_gl_balance(doc.customer, doc.company) <= 0: + if not verdict.outcome: return text = message(verdict, doc.customer, invoice, days) @@ -238,18 +235,3 @@ def get_oldest_overdue_invoice(customer: str, company: str, today): limit=1, ) return invoices[0] if invoices else None - - -def get_customer_gl_balance(customer: str, company: str) -> float: - gle = frappe.qb.DocType("GL Entry") - balance = ( - frappe.qb.from_(gle) - .select(Sum(gle.debit) - Sum(gle.credit)) - .where( - (gle.party_type == CUSTOMER) - & (gle.party == customer) - & (gle.company == company) - & (gle.is_cancelled == 0) - ) - ).run() - return flt(balance[0][0]) diff --git a/optima_payment/overdue_limit/tests/test_limit_integration.py b/optima_payment/overdue_limit/tests/test_limit_integration.py index 060ce30..785dadf 100644 --- a/optima_payment/overdue_limit/tests/test_limit_integration.py +++ b/optima_payment/overdue_limit/tests/test_limit_integration.py @@ -6,7 +6,7 @@ The branch matrix and the message text are in test_limit.py. These prove what only a real document shows: the hooks fire on both doctypes at both stages, an earlier draft cannot be submitted past a Block on Save rule, the acting user's roles decide the exemption, credit notes pass, the group tree is -read from the database, and a payment lifts the block whether or not it is allocated to the invoice. +read from the database, and only a payment allocated to the invoice lifts the block. Each test starts from a customer whose Sales Invoice fell due 25 days ago and is unpaid. """ @@ -147,12 +147,11 @@ def test_paying_the_overdue_invoice_lifts_the_block(self): make_invoice_payment(self.overdue_invoice.name) self.assertFalse(self.make_draft().is_new()) - def test_an_unallocated_payment_covering_the_debt_lifts_the_block(self): + def test_an_unallocated_payment_covering_the_debt_keeps_the_block(self): make_overdue_limit(party=self.customer, action=BLOCK_ON_SAVE) make_invoice_payment(self.overdue_invoice.name, allocate=False) - outstanding = frappe.db.get_value("Sales Invoice", self.overdue_invoice.name, "outstanding_amount") - self.assertEqual(outstanding, self.overdue_invoice.grand_total) - self.assertFalse(self.make_draft().is_new()) + with self.assertRaises(OverdueLimitError): + self.make_draft() def test_a_disabled_rule_does_nothing(self): make_overdue_limit(party=self.customer, action=BLOCK_ON_SAVE, enabled=0) From 829e76a5f88fd1819eba735a8b9bb7669bbb3627 Mon Sep 17 00:00:00 2001 From: FawazAlhafiz Date: Tue, 3 Nov 2026 17:55:10 +0300 Subject: [PATCH 11/14] feat(overdue-limit): count invoices that ERPNext marks Overdue The oldest overdue invoice is now found by status, Overdue or Overdue and Discounted, instead of outstanding amount and due date, so what counts as overdue is ERPNext's decision. ERPNext's daily update_invoice_status job sets that status, so the rules depend on the scheduler: on a site where it is disabled, invoices stay Unpaid and no rule fires. The integration fixture now turns its invoice Overdue by running that same job instead of only backdating the due date. --- docs/development/testing.md | 1 + docs/overdue-limit/reference.md | 29 ++++++++++++++----- docs/overdue-limit/user-guide.md | 13 ++++++--- optima_payment/overdue_limit/limit.py | 23 +++++++-------- .../tests/test_limit_integration.py | 9 +++--- optima_payment/tests/utils.py | 15 ++++++++++ 6 files changed, 61 insertions(+), 29 deletions(-) diff --git a/docs/development/testing.md b/docs/development/testing.md index d0680fc..df0af1e 100644 --- a/docs/development/testing.md +++ b/docs/development/testing.md @@ -51,6 +51,7 @@ factories instead of building documents inline: | `set_test_cost_center_default(company)` | Makes a leaf cost center the session user's `cost_center` default, so the tool's vouchers carry one on sites that make it mandatory on Purchase Invoice. It lives in the defaults cache: pair it with `frappe.clear_cache(user=frappe.session.user)` after the rollback | | `make_sales_order(customer=…, do_not_insert=…, do_not_submit=…, **overrides)` | A one-line Sales Order on the test item, cost center and pre-seeded tax row; `make_reference_sales_order` wraps it | | `make_sales_invoice(customer=…, posting_date=…, due_date=…, do_not_insert=…, do_not_submit=…, **overrides)` | A one-line Sales Invoice; a past `posting_date` is kept, and the due date defaults to it. The header has no cost center, as on real invoices, because a payment copies it onto balance sheet lines that some sites refuse one on | +| `make_invoice_overdue(invoice, days)` | Moves the invoice's due date and payment schedule `days` into the past, then runs ERPNext's `update_invoice_status` so the invoice turns Overdue as it would on a live site | | `make_credit_note(invoice)` | An unsaved full return of the invoice, with the return reason some sites require | | `make_invoice_payment(invoice, allocate=…)` | A submitted Payment Entry for the invoice's outstanding amount; `allocate=False` leaves it unallocated, so the invoice stays outstanding while the customer owes nothing | | `make_overdue_limit(party=…, action=…, threshold_days=…, applies_to=…, company=…, exempt_roles=…, enabled=…)` | A Time Based Overdue Limit rule | diff --git a/docs/overdue-limit/reference.md b/docs/overdue-limit/reference.md index 2374202..5e3aebd 100644 --- a/docs/overdue-limit/reference.md +++ b/docs/overdue-limit/reference.md @@ -132,16 +132,30 @@ customer rule for another company does not hide the group's rules. `applies_to` `get_oldest_overdue_invoice` returns the Sales Invoice with the earliest `due_date` among: ``` -customer = doc.customer, company = doc.company, docstatus = 1, is_return = 0, -outstanding_amount > 0, due_date < today +customer = doc.customer, company = doc.company, status in ('Overdue', 'Overdue and Discounted') ``` ordered `due_date, name` and limited to one, so the message can name it. `days_overdue` is `today − due_date`, and a rule triggers when `days >= threshold_days`. -`Sales Invoice.due_date` is the **latest** due date of the payment schedule -(`erpnext/controllers/accounts_controller.py`, `self.due_date = max(due_dates)`). An installment -invoice therefore counts as overdue only after its last installment falls due: the check +What counts as overdue is ERPNext's decision, read from the invoice's `status`. ERPNext gives +`Overdue` only to a submitted invoice with an outstanding amount, so drafts, cancelled and paid +invoices and credit notes never count. An Unpaid invoice becomes Overdue in two ways: + +- `update_invoice_status` (`erpnext/controllers/accounts_controller.py`), in ERPNext's + `daily_maintenance` scheduler events. It is the only thing that flips an invoice nobody touches. +- `set_status`, whenever something else updates the invoice, such as a payment against it. + +So the status trails the calendar by up to a day: only a 1-day threshold can fire late. On a site +whose **scheduler is disabled**, untouched invoices stay Unpaid and the rules stop firing. + +`Overdue and Discounted` is the same status for an invoice under Invoice Discounting: the bank has +advanced the money, but the customer still owes it. + +`Sales Invoice.due_date` is the **latest** due date of the payment schedule (`self.due_date = +max(due_dates)`). ERPNext marks an installment invoice Overdue as soon as any installment is late, +but the day count runs from that last due date, which may still be ahead: the days then come out +negative and no rule triggers until the last installment is `threshold_days` late. The check under-counts, but never blocks wrongly. Per-installment ageing from `Payment Schedule` is a possible follow-up. @@ -193,6 +207,7 @@ The form still pre-fills the user's default company on a new rule (the client-si | Limit | Effect | Possible follow-up | |-------|--------|--------------------| | Installment invoices age from their last due date | Overdue installments count late | Age from `Payment Schedule` rows | +| Overdue comes from ERPNext's daily job | A site with the scheduler disabled never blocks; elsewhere a 1-day threshold can fire up to a day late | Keep the scheduler enabled | | Residual balances count | 0.01 outstanding can block | An "ignore outstanding below" amount | | Unallocated payments and credit notes | The invoice stays outstanding and still counts, even when the customer owes nothing overall | None; reconcile them with Payment Reconciliation | | A submitted Sales Order does not pass its Sales Invoice | The invoice is checked on its own | Intended | @@ -211,5 +226,5 @@ See [testing.md](../development/testing.md). The check's tests come as a pair in | `test_limit_integration.py` | Integration: real saves | Each action on Sales Invoice and Sales Order at save and submit, Block on Save refusing an earlier draft's submit, the acting user's roles deciding the exemption, a credit note submitting under a block, an ancestor group's rule applying, another company's rule not applying, an allocated payment lifting the block and an unallocated one not, a disabled rule | | `doctype/time_based_overdue_limit/test_time_based_overdue_limit.py` | Integration | Threshold above 0, the duplicate rule, what is not a duplicate, a blank company staying blank, re-saving a rule | -Every integration test starts from a customer whose Sales Invoice fell due 25 days ago and is -unpaid. The fixtures come from `optima_payment/tests/utils.py`. +Every integration test starts from a customer whose Sales Invoice fell due 25 days ago and that +ERPNext's own `update_invoice_status` has marked Overdue (`make_invoice_overdue`). The fixtures come from `optima_payment/tests/utils.py`. diff --git a/docs/overdue-limit/user-guide.md b/docs/overdue-limit/user-guide.md index 53e67e7..af57f24 100644 --- a/docs/overdue-limit/user-guide.md +++ b/docs/overdue-limit/user-guide.md @@ -107,12 +107,16 @@ Al Noor Trading (no rule) → Retail (no rule) → Commercial: Block on Save, 30 ## How the days are counted -- Only **submitted Sales Invoices** of the customer, in the document's **company**, with an - **outstanding amount** and a **due date before today** are counted. Credit notes are not. +- Only the customer's **Sales Invoices** in the document's **company** that ERPNext shows as + **Overdue** (or **Overdue and Discounted**, under Invoice Discounting) are counted. Paid + invoices, drafts and credit notes never are. - The **oldest** due date among them decides. Newer overdue invoices don't change the count. - The day count is today minus that due date. -- On an invoice paid in installments, the due date is that of the **last** installment, so an - earlier installment that is already late is not counted yet. +- ERPNext marks invoices Overdue once a day, so an invoice that fell due yesterday can still show + **Unpaid** for a few hours. If the site's **scheduler** is switched off, invoices stay Unpaid and + no rule fires. +- On an invoice paid in installments, ERPNext shows **Overdue** as soon as one installment is late, + but the days are counted from the **last** installment's due date, so the rule waits for that one. --- @@ -151,4 +155,5 @@ Al Noor Trading (no rule) → Retail (no rule) → Commercial: Block on Save, 30 | A customer is blocked though they have paid | The payment or credit note is recorded but not allocated to the overdue invoice | Allocate it with **Payment Reconciliation** | | *Threshold Days must be greater than 0* | Threshold Days is 0 or empty | Enter a number of days above 0 | | *Enabled rule … already has the same Applies To, Party, Company and Action* | A second rule repeats an enabled one | Change the existing rule instead, or disable it first | +| A rule never fires though the invoice is long past due | The invoice still shows **Unpaid**: the site's scheduler is switched off | Ask the administrator to enable the scheduler | | A rule never fires in one company | It names a different company | Clear **Company**, or set the right one | diff --git a/optima_payment/overdue_limit/limit.py b/optima_payment/overdue_limit/limit.py index 01d34da..eba6ee9 100644 --- a/optima_payment/overdue_limit/limit.py +++ b/optima_payment/overdue_limit/limit.py @@ -34,6 +34,7 @@ BLOCK = "block" SKIP_FLAGS = ("in_migrate", "in_patch", "in_install", "in_import") +OVERDUE_STATUSES = ("Overdue", "Overdue and Discounted") Hit = namedtuple("Hit", ["rule", "exempt"]) Verdict = namedtuple("Verdict", ["outcome", "hits"]) @@ -154,12 +155,11 @@ def check(doc: Document, stage: str) -> None: if not rules: return - today = getdate() - invoice = get_oldest_overdue_invoice(doc.customer, doc.company, today) + invoice = get_oldest_overdue_invoice(doc.customer, doc.company) if not invoice: return - days = days_overdue(invoice.due_date, today) + days = days_overdue(invoice.due_date, getdate()) verdict = decide(rules, days, frappe.get_roles(), stage) if not verdict.outcome: return @@ -218,18 +218,15 @@ def get_enabled_rules(parties: list[str]) -> list: return rules -def get_oldest_overdue_invoice(customer: str, company: str, today): - """The submitted, unpaid Sales Invoice whose due date is furthest past, or None.""" +def get_oldest_overdue_invoice(customer: str, company: str): + """The Sales Invoice ERPNext marks Overdue whose due date is furthest past, or None. + + ERPNext's daily ``update_invoice_status`` job is what turns an Unpaid invoice Overdue, so on a + site whose scheduler is off, no invoice counts. + """ invoices = frappe.get_all( "Sales Invoice", - filters={ - "customer": customer, - "company": company, - "docstatus": 1, - "is_return": 0, - "outstanding_amount": (">", 0), - "due_date": ("<", today), - }, + filters={"customer": customer, "company": company, "status": ("in", OVERDUE_STATUSES)}, fields=["name", "due_date"], order_by="due_date asc, name asc", limit=1, diff --git a/optima_payment/overdue_limit/tests/test_limit_integration.py b/optima_payment/overdue_limit/tests/test_limit_integration.py index 785dadf..f389fcf 100644 --- a/optima_payment/overdue_limit/tests/test_limit_integration.py +++ b/optima_payment/overdue_limit/tests/test_limit_integration.py @@ -8,19 +8,20 @@ Block on Save rule, the acting user's roles decide the exemption, credit notes pass, the group tree is read from the database, and only a payment allocated to the invoice lifts the block. -Each test starts from a customer whose Sales Invoice fell due 25 days ago and is unpaid. +Each test starts from a customer whose Sales Invoice fell due 25 days ago and that ERPNext's daily +status job has marked Overdue. """ import erpnext import frappe from frappe.tests.utils import FrappeTestCase -from frappe.utils import add_days, nowdate from optima_payment.overdue_limit.limit import BLOCK_ON_SAVE, BLOCK_ON_SUBMIT, WARN_ON_SAVE, OverdueLimitError from optima_payment.tests.utils import ( get_or_create_customer, get_or_create_user, make_credit_note, + make_invoice_overdue, make_invoice_payment, make_overdue_limit, make_sales_invoice, @@ -38,9 +39,7 @@ def setUp(self): self.company = erpnext.get_default_company() self.customer = get_or_create_customer(CUSTOMER) self.overdue_invoice = make_sales_invoice(customer=self.customer) - # Posted today rather than backdated, so it stays in the fiscal year even in January (CI's is - # the calendar year). The check reads only the due date. - self.overdue_invoice.db_set("due_date", add_days(nowdate(), -25)) + make_invoice_overdue(self.overdue_invoice.name, days=25) frappe.clear_messages() def tearDown(self): diff --git a/optima_payment/tests/utils.py b/optima_payment/tests/utils.py index 9d77cf0..93ad5d6 100644 --- a/optima_payment/tests/utils.py +++ b/optima_payment/tests/utils.py @@ -17,6 +17,7 @@ from erpnext.accounts.doctype.payment_entry.payment_entry import get_payment_entry from erpnext.accounts.doctype.sales_invoice.sales_invoice import make_sales_return from erpnext.accounts.party import get_party_account +from erpnext.controllers.accounts_controller import update_invoice_status from frappe.utils import add_days, nowdate from optima_payment.override.doctype_class.payment_entry import CustomPaymentEntry @@ -869,6 +870,20 @@ def make_sales_invoice( return si +def make_invoice_overdue(invoice: str, days: int) -> None: + """Move the due date of ``invoice`` and of its payment schedule ``days`` into the past, then run + ERPNext's daily status job, which marks it Overdue as it would on a live site. + + Only the due dates move, not the posting date, so the invoice stays in the fiscal year even in + January (CI's is the calendar year). Validate refuses a due date before the posting date, hence + the direct writes. + """ + due_date = add_days(nowdate(), -days) + frappe.db.set_value("Sales Invoice", invoice, "due_date", due_date) + frappe.db.set_value("Payment Schedule", {"parenttype": "Sales Invoice", "parent": invoice}, "due_date", due_date) + update_invoice_status() + + def make_credit_note(invoice: str) -> frappe.model.document.Document: """An unsaved full return of ``invoice``.""" credit_note = make_sales_return(invoice) From 03ffbc76864ddbd2a9e09503307695d6268999e8 Mon Sep 17 00:00:00 2001 From: FawazAlhafiz Date: Tue, 3 Nov 2026 18:45:26 +0300 Subject: [PATCH 12/14] feat(overdue-limit): check each action at its own hook Warn on Save runs at validate, Block on Save at before_save and Block on Submit at before_submit, and each hook looks only at its own action's rules, so a rule does what its name says and nothing more: - Warn on Save also warns at submit, because Frappe runs validate there too. - Block on Save no longer refuses the submit of a draft saved before the customer went overdue; a Block on Submit rule is what stops a submit. - Block on Submit no longer warns at save. The tier is still chosen before the action filter, so a customer's warning rule keeps shadowing its group's block rule. Each message line now names the rule's action. --- docs/development/testing.md | 4 +- docs/overdue-limit/README.md | 2 +- docs/overdue-limit/reference.md | 82 ++++++++++++------- docs/overdue-limit/user-guide.md | 32 ++++---- optima_payment/doc_events/overdue_limit.py | 10 ++- optima_payment/hooks.py | 2 + optima_payment/overdue_limit/limit.py | 66 ++++++--------- .../overdue_limit/tests/test_limit.py | 69 +++++++--------- .../tests/test_limit_integration.py | 25 +++--- 9 files changed, 148 insertions(+), 144 deletions(-) diff --git a/docs/development/testing.md b/docs/development/testing.md index df0af1e..cbcdb14 100644 --- a/docs/development/testing.md +++ b/docs/development/testing.md @@ -21,8 +21,8 @@ creates survives the test. milliseconds and needs no data on the site. `test_vouchers_integration.py` asserts on the real vouchers (lines, header dates, docstatus) and the row status / reference stamps. See [purchases-bulk-tool/reference.md](../purchases-bulk-tool/reference.md). -- **Time Based Overdue Limit** pairs the same way. `test_limit.py` runs the action table and the - rule precedence on plain rules; `test_limit_integration.py` saves and submits real Sales +- **Time Based Overdue Limit** pairs the same way. `test_limit.py` runs the rule precedence and how + triggered rules combine on plain rules; `test_limit_integration.py` saves and submits real Sales Invoices and Sales Orders for a customer with an invoice 25 days overdue. See [overdue-limit/reference.md](../overdue-limit/reference.md). diff --git a/docs/overdue-limit/README.md b/docs/overdue-limit/README.md index e11bc34..1990da6 100644 --- a/docs/overdue-limit/README.md +++ b/docs/overdue-limit/README.md @@ -6,7 +6,7 @@ long past their due date. | Document | For | What it covers | |----------|-----|----------------| | [user-guide.md](user-guide.md) | Accountants & implementers | Setting up a rule, the three actions, which rule wins, how the days are counted, exempt roles, what is never checked, troubleshooting | -| [reference.md](reference.md) | Developers | The hooks and when they fire, rule selection by tier, the day-count query, the action table, the blank company, known limits, tests | +| [reference.md](reference.md) | Developers | Which hook checks each action, rule selection by tier, the day-count query, the outcome table, the blank company, known limits, tests | The feature is layered on **Time Based Overdue Limit**, with the child table **Time Based Overdue Limit Exempt Role**. It checks standard **Sales Invoice** and **Sales Order** documents when they diff --git a/docs/overdue-limit/reference.md b/docs/overdue-limit/reference.md index 5e3aebd..a455f81 100644 --- a/docs/overdue-limit/reference.md +++ b/docs/overdue-limit/reference.md @@ -1,8 +1,8 @@ # Time Based Overdue Limit Developer + power-user reference for the **Time Based Overdue Limit** doctype: when its check runs -on Sales Invoices and Sales Orders, which rules apply, how the overdue days are counted, and what -each action does at each stage. For a click-by-click guide aimed at accountants, see +on Sales Invoices and Sales Orders, which rules apply, how the overdue days are counted, and which +hook checks each action. For a click-by-click guide aimed at accountants, see [user-guide.md](user-guide.md). --- @@ -13,9 +13,9 @@ A credit-control rule keyed on the **age** of a customer's debt rather than its ERPNext's Credit Limit (`erpnext/selling/doctype/customer/customer.py`, `check_credit_limit`) with the same shape: rules on Customer and Customer Group, a customer rule overriding its group's, and a role that is let through. A rule names one party, an optional company, a day threshold and one -of three actions. The check runs from `doc_events` on **Sales Invoice** and **Sales Order** at -`before_save` and `before_submit`, warns with `frappe.msgprint` or blocks with `frappe.throw`, and -needs no client script. +of three actions, and each action is checked at one hook of **Sales Invoice** and **Sales Order**: +Warn on Save at `validate`, Block on Save at `before_save`, Block on Submit at `before_submit`. It +warns with `frappe.msgprint` or blocks with `frappe.throw`, and needs no client script. The doctype is wholly owned by this app and adds no field to another app's doctype, so the [setup framework](../setup/architecture.md) and its `sync()` patches are not involved. @@ -30,13 +30,13 @@ return values, with no database access. Everything that reads calls them. ``` optima_payment/overdue_limit/ └── limit.py - ├── DECISIONS # pure: skipped documents, rule selection by tier, day count, exemption, - │ # one rule's outcome, the document's verdict + ├── DECISIONS # pure: skipped documents, rule selection by tier and action, day count, + │ # exemption, the document's verdict ├── MESSAGES # the warning / refusal text, one line per rule - ├── CHECK # check(doc, stage): loads, decides, then msgprint or throw + ├── CHECK # check(doc, action): loads, decides, then msgprint or throw └── QUERIES # group chain, enabled rules + exempt roles, oldest overdue invoice -optima_payment/doc_events/overdue_limit.py # before_save / before_submit → limit.check +optima_payment/doc_events/overdue_limit.py # validate / before_save / before_submit → limit.check, one action each optima_payment/hooks.py # doc_events for Sales Invoice and Sales Order optima_payment/optima_payment/doctype/ @@ -77,15 +77,29 @@ be worthless. ## When the check runs +Each hook checks the rules of one action only, so a rule does what its name says and nothing more: + +| Hook | Action checked | Runs on | +|------|----------------|---------| +| `validate` | **Warn on Save** | save and submit | +| `before_save` | **Block on Save** | save | +| `before_submit` | **Block on Submit** | submit | + Frappe runs `validate → before_save` on a save and `validate → before_submit` on a submit -(`frappe/model/document.py`, `run_before_save_methods`). Submitting from the form does **not** run -`before_save`, which is why a Block on Save rule is checked at submit too. +(`frappe/model/document.py`, `run_before_save_methods`). Three consequences: + +- A Warn on Save rule warns again at submit, because `validate` runs there too. +- Submitting does **not** run `before_save`, so a draft saved before the customer went overdue (or + before the rule existed) submits past a Block on Save rule. A Block on Submit rule is what stops a + submit. +- When a Warn on Save and a Block on Save rule both trigger, `validate` shows the warning and + `before_save` then refuses; the user sees both messages. ```mermaid flowchart LR - A[before_save / before_submit] --> B{credit note, POS consolidated,
    migrate, patch, install, import?} + A["validate / before_save / before_submit
    (one action each)"] --> B{credit note, POS consolidated,
    migrate, patch, install, import?} B -- yes --> Z[nothing] - B -- no --> C{rules in the most
    specific tier?} + B -- no --> C{rules of that action in
    the most specific tier?} C -- none --> Z C -- some --> D{oldest overdue
    Sales Invoice?} D -- none --> Z @@ -114,14 +128,15 @@ fetched copy) and its ancestors, nearest first, with one `lft <= … and rgt >= leaves cached documents stale. `get_enabled_rules` loads every enabled rule naming the customer or any group in the chain, with their exempt roles. -`select_rules` then keeps the rules whose company is blank or equals the document's, and returns -the first tier that has any, in this order: +`select_rules` then keeps the rules whose company is blank or equals the document's, finds the +first tier that has any, in this order, and returns that tier's rules of the hook's action: ``` (Customer, Al Noor Trading) → (Customer Group, Retail) → (Customer Group, Commercial) → (Customer Group, All Customer Groups) ``` -Every rule of that tier applies. The company filter runs **before** the tier search, so a +The tier is chosen **before** the action filter: a customer with only a Warn on Save rule is never +blocked by its group's Block on Save rule. The company filter runs before the tier search, so a customer rule for another company does not hide the group's rules. `applies_to` is matched with `party`, so a group named like the customer is still a group. @@ -161,26 +176,30 @@ follow-up. --- -## The action table +## Outcome + +`decide(rules, days, user_roles)` takes the rules of one action. A rule triggers when +`days >= threshold_days`, and then: -`rule_outcome(action, stage, exempt)` for one triggered rule. "Exempt" means the acting user -(`frappe.get_roles()`, the session user, not the document owner) holds one of the rule's exempt -roles, which only counts on the two block actions (`is_exempt_user`). +| Action | User not exempt | User exempt | +|--------|-----------------|-------------| +| **Warn on Save** | warn | warn | +| **Block on Save** | **block** | warn | +| **Block on Submit** | **block** | warn | -| Action | Save | Save, exempt | Submit | Submit, exempt | -|--------|------|--------------|--------|----------------| -| **Warn on Save** | warn | warn | — | — | -| **Block on Save** | **block** | warn | **block** | warn | -| **Block on Submit** | warn | warn | **block** | warn | +"Exempt" means the acting user (`frappe.get_roles()`, the session user, not the document owner) +holds one of the rule's exempt roles, which only counts on the two block actions (`is_exempt_user`). -`decide` combines the triggered rules of the tier into a `Verdict(outcome, hits)`: +One hook can meet two rules of its action: an every-company rule and a one-company rule. `decide` +combines the triggered ones into a `Verdict(outcome, hits)`: - Any block wins. Its hits are only the blocking rules, so the refusal lists only what refuses. -- Otherwise every triggered rule that warns is a hit, and the warning lists them all. +- Otherwise every triggered rule is a hit, and the warning lists them all. - No triggered rule: `Verdict(None, [])`. -Each `Hit` carries the rule and whether the user was exempt from it; the message turns each into -one line, adding *Your role lets you continue* or the list of exempt roles. A block is raised as +Each `Hit` carries the rule and whether the user was exempt from it. The message turns each into one +line naming the rule, its action and its threshold (*Rule TBOL-00002 (Block on Save) applies from 30 +days overdue.*), adding *Your role lets you continue* or the list of exempt roles. A block is raised as `OverdueLimitError`, a subclass of `frappe.ValidationError`. A warning is `msgprint` with an orange indicator, and the document is saved. @@ -206,6 +225,7 @@ The form still pre-fills the user's default company on a new rule (the client-si | Limit | Effect | Possible follow-up | |-------|--------|--------------------| +| Block on Save is not checked at submit | A draft saved before the customer went overdue can be submitted | Add a Block on Submit rule for the same party | | Installment invoices age from their last due date | Overdue installments count late | Age from `Payment Schedule` rows | | Overdue comes from ERPNext's daily job | A site with the scheduler disabled never blocks; elsewhere a 1-day threshold can fire up to a day late | Keep the scheduler enabled | | Residual balances count | 0.01 outstanding can block | An "ignore outstanding below" amount | @@ -222,8 +242,8 @@ See [testing.md](../development/testing.md). The check's tests come as a pair in | File | Layer | What it proves | |------|-------|----------------| -| `test_limit.py` | Matrix: plain rules, nothing written | The documents never checked, tier precedence, other-company rules, a group named like the customer, the action table, the threshold day, how several triggered rules combine (including an exemption from only one), what the message says about exemption. The message wording is not pinned | -| `test_limit_integration.py` | Integration: real saves | Each action on Sales Invoice and Sales Order at save and submit, Block on Save refusing an earlier draft's submit, the acting user's roles deciding the exemption, a credit note submitting under a block, an ancestor group's rule applying, another company's rule not applying, an allocated payment lifting the block and an unallocated one not, a disabled rule | +| `test_limit.py` | Matrix: plain rules, nothing written | The documents never checked, tier precedence, the tier chosen before the action, other-company rules, a group named like the customer, the threshold day, how several triggered rules combine (including an exemption from only one), what the message says about exemption. The message wording is not pinned | +| `test_limit_integration.py` | Integration: real saves | Each action at its own hook on Sales Invoice and Sales Order: Warn on Save warning at save and again at submit, Block on Save refusing the save but not an earlier draft's submit, Block on Submit saving silently and refusing the submit; the acting user's roles deciding the exemption, a credit note saving and submitting under both blocks, an ancestor group's rule applying, another company's rule not applying, an allocated payment lifting the block and an unallocated one not, a disabled rule | | `doctype/time_based_overdue_limit/test_time_based_overdue_limit.py` | Integration | Threshold above 0, the duplicate rule, what is not a duplicate, a blank company staying blank, re-saving a rule | Every integration test starts from a customer whose Sales Invoice fell due 25 days ago and that diff --git a/docs/overdue-limit/user-guide.md b/docs/overdue-limit/user-guide.md index af57f24..ef3f7e1 100644 --- a/docs/overdue-limit/user-guide.md +++ b/docs/overdue-limit/user-guide.md @@ -32,10 +32,10 @@ yet TBOL-00002's 30, so the order **saves** and shows: > Customer **Al Noor Trading** has Sales Invoice **SINV-2026-00041** overdue by **25** days; it was > due on 08-10-2026. -> - Rule **TBOL-00001** warns from 20 days overdue. +> - Rule **TBOL-00001** (Warn on Save) applies from 20 days overdue. -Five days later the invoice is 30 days overdue, and TBOL-00002 **refuses** the save until the -invoice is paid. +Five days later the invoice is 30 days overdue. Saving shows the TBOL-00001 warning, then TBOL-00002 +**refuses** the save, and it keeps refusing until the invoice is paid. --- @@ -69,17 +69,20 @@ enabled rules with the same customer or group, company and action are not allowe ## The three actions +Each action does exactly what its name says, and nothing at the other step: + | Action | Saving a draft | Submitting | |--------|----------------|------------| -| **Warn on Save** | Saves, with a warning | Goes through, no message | -| **Block on Save** | **Refused** | **Refused** | -| **Block on Submit** | Saves, with a warning that submit will be refused | **Refused** | - -- **Block on Save** also refuses the submit. Otherwise a draft saved before the customer became - overdue could still be submitted afterwards. -- **Block on Submit** warns at save, so you find out about the block before trying to submit. -- A user with an **exempt role** sees the warning instead of the refusal, at save and at submit. - Exempt roles have no effect on **Warn on Save**: everybody sees that warning. +| **Warn on Save** | Saves, with a warning | Goes through, with the same warning | +| **Block on Save** | **Refused** | Not checked | +| **Block on Submit** | Not checked | **Refused** | + +- **Warn on Save** shows again when you submit, because submitting saves the document once more. +- **Block on Save** does not stop a submit: a draft saved before the customer became overdue can + still be submitted. To stop submits too, add a **Block on Submit** rule for the same customer. +- **Block on Submit** says nothing at save; the draft saves normally and only the submit is refused. +- A user with an **exempt role** sees the warning instead of the refusal. Exempt roles have no + effect on **Warn on Save**: everybody sees that warning. - The **Administrator** user holds every role, so is exempt from any rule that lists at least one exempt role. @@ -150,8 +153,9 @@ Al Noor Trading (no rule) → Retail (no rule) → Commercial: Block on Save, 30 | Message or symptom | Cause | Fix | |--------------------|-------|-----| -| *Customer X has Sales Invoice Y overdue by N days … blocks saving and submitting* | A Block on Save rule applies | Collect and record the payment against invoice Y, or ask a user with an exempt role | -| *… blocks submitting* when you submit | A Block on Submit rule applies | As above. The draft stays saved | +| *Customer X has Sales Invoice Y overdue by N days … (Block on Save) applies …* when you save | A Block on Save rule applies | Collect and record the payment against invoice Y, or ask a user with an exempt role | +| *… (Block on Submit) applies …* when you submit | A Block on Submit rule applies | As above. The draft stays saved | +| A draft was submitted for an overdue customer | The customer only has a Block on Save rule, and the draft was saved before the rule applied | Add a Block on Submit rule | | A customer is blocked though they have paid | The payment or credit note is recorded but not allocated to the overdue invoice | Allocate it with **Payment Reconciliation** | | *Threshold Days must be greater than 0* | Threshold Days is 0 or empty | Enter a number of days above 0 | | *Enabled rule … already has the same Applies To, Party, Company and Action* | A second rule repeats an enabled one | Change the existing rule instead, or disable it first | diff --git a/optima_payment/doc_events/overdue_limit.py b/optima_payment/doc_events/overdue_limit.py index da4a794..0e7f675 100644 --- a/optima_payment/doc_events/overdue_limit.py +++ b/optima_payment/doc_events/overdue_limit.py @@ -1,11 +1,15 @@ -"""Sales Invoice and Sales Order hooks for the Time Based Overdue Limit.""" +"""Sales Invoice and Sales Order hooks for the Time Based Overdue Limit, one action per hook.""" from optima_payment.overdue_limit import limit +def validate(doc, method=None): + limit.check(doc, limit.WARN_ON_SAVE) + + def before_save(doc, method=None): - limit.check(doc, limit.SAVE) + limit.check(doc, limit.BLOCK_ON_SAVE) def before_submit(doc, method=None): - limit.check(doc, limit.SUBMIT) + limit.check(doc, limit.BLOCK_ON_SUBMIT) diff --git a/optima_payment/hooks.py b/optima_payment/hooks.py index c50771b..4ae0288 100644 --- a/optima_payment/hooks.py +++ b/optima_payment/hooks.py @@ -190,10 +190,12 @@ "after_delete": "optima_payment.purchases_bulk.vouchers.unlink_tool_rows", }, "Sales Invoice": { + "validate": "optima_payment.doc_events.overdue_limit.validate", "before_save": "optima_payment.doc_events.overdue_limit.before_save", "before_submit": "optima_payment.doc_events.overdue_limit.before_submit", }, "Sales Order": { + "validate": "optima_payment.doc_events.overdue_limit.validate", "before_save": "optima_payment.doc_events.overdue_limit.before_save", "before_submit": "optima_payment.doc_events.overdue_limit.before_submit", }, diff --git a/optima_payment/overdue_limit/limit.py b/optima_payment/overdue_limit/limit.py index eba6ee9..697e88e 100644 --- a/optima_payment/overdue_limit/limit.py +++ b/optima_payment/overdue_limit/limit.py @@ -6,7 +6,8 @@ The age that counts is that of the customer's oldest overdue invoice in the document's company. Only the most specific tier of rules applies: the customer's own, else its group's, else the nearest -ancestor group's. Every rule in that tier counts. +ancestor group's. Each action is checked at its own hook: Warn on Save at validate, Block on Save at +before_save, Block on Submit at before_submit. Branching lives in the DECISIONS functions, which take plain rows and return values. """ @@ -27,9 +28,6 @@ BLOCK_ON_SAVE = "Block on Save" BLOCK_ON_SUBMIT = "Block on Submit" -SAVE = "save" -SUBMIT = "submit" - WARN = "warn" BLOCK = "block" @@ -55,18 +53,20 @@ def is_exempt_document(is_return, is_consolidated, flags) -> bool: return bool(is_return or is_consolidated or any(flags.get(flag) for flag in SKIP_FLAGS)) -def select_rules(rules: list, customer: str, group_chain: list[str], company: str) -> list: - """The rules governing ``customer`` in ``company``: those of the first tier that has any. +def select_rules(rules: list, customer: str, group_chain: list[str], company: str, action: str) -> list: + """The ``action`` rules governing ``customer`` in ``company``. - ``group_chain`` is the customer's group followed by its ancestors, nearest first. A rule with no - company applies in every company, and a rule for another company does not shadow a wider tier. + The tier is the first that has any rule, whatever its action, so a customer's Warn on Save rule + still shadows its group's Block on Save rule. ``group_chain`` is the customer's group followed by + its ancestors, nearest first. A rule with no company applies in every company, and a rule for + another company does not shadow a wider tier. """ in_company = [rule for rule in rules if not rule.company or rule.company == company] tiers = [(CUSTOMER, customer)] + [(CUSTOMER_GROUP, group) for group in group_chain] for applies_to, party in tiers: tier = [rule for rule in in_company if rule.applies_to == applies_to and rule.party == party] if tier: - return tier + return [rule for rule in tier if rule.action == action] return [] @@ -79,31 +79,21 @@ def is_exempt_user(rule, user_roles: set) -> bool: return rule.action != WARN_ON_SAVE and bool(user_roles & set(rule.exempt_roles)) -def rule_outcome(action: str, stage: str, exempt: bool) -> str | None: - """What one triggered rule does at ``stage``. +def decide(rules: list, days: int, user_roles) -> Verdict: + """What ``rules``, all of one action, do once the customer is ``days`` overdue. - Block on Save blocks at submit too, because submitting skips before_save: a draft saved before - the customer went overdue would otherwise get through. Block on Submit warns at save, so the user - hears of the block before reaching it. + A rule triggers once ``days`` reaches its ``threshold_days``. Warn on Save warns. A block rule + blocks, but only warns a user holding one of its exempt roles. A block outranks warnings, which its + message then leaves out. """ - if action == WARN_ON_SAVE: - return WARN if stage == SAVE else None - blocks_now = action == BLOCK_ON_SAVE or stage == SUBMIT - return BLOCK if blocks_now and not exempt else WARN - - -def decide(rules: list, days: int, user_roles, stage: str) -> Verdict: - """The document's outcome when its customer is ``days`` overdue. A rule triggers once ``days`` - reaches its ``threshold_days``. A block outranks warnings, which its message then leaves out.""" roles = set(user_roles) hits = {BLOCK: [], WARN: []} for rule in rules: if days < rule.threshold_days: continue exempt = is_exempt_user(rule, roles) - outcome = rule_outcome(rule.action, stage, exempt) - if outcome: - hits[outcome].append(Hit(rule, exempt)) + outcome = WARN if rule.action == WARN_ON_SAVE or exempt else BLOCK + hits[outcome].append(Hit(rule, exempt)) for outcome in (BLOCK, WARN): if hits[outcome]: @@ -125,7 +115,9 @@ def message(verdict: Verdict, customer: str, invoice, days: int) -> str: def _hit_line(hit: Hit) -> str: rule = hit.rule - line = _rule_text(rule.action).format(frappe.bold(rule.name), rule.threshold_days) + line = _("Rule {0} ({1}) applies from {2} days overdue.").format( + frappe.bold(rule.name), _(rule.action), rule.threshold_days + ) if hit.exempt: return f"{line} {_('Your role lets you continue.')}" if rule.action != WARN_ON_SAVE and rule.exempt_roles: @@ -134,24 +126,16 @@ def _hit_line(hit: Hit) -> str: return line -def _rule_text(action: str) -> str: - if action == WARN_ON_SAVE: - return _("Rule {0} warns from {1} days overdue.") - if action == BLOCK_ON_SAVE: - return _("Rule {0} blocks saving and submitting from {1} days overdue.") - return _("Rule {0} blocks submitting from {1} days overdue.") - - # ==================================================================================================== # CHECK -def check(doc: Document, stage: str) -> None: - """Warns about or blocks a Sales Invoice or Sales Order at ``stage``, as its customer's rules say.""" +def check(doc: Document, action: str) -> None: + """Warns about or blocks a Sales Invoice or Sales Order as its customer's ``action`` rules say.""" if not doc.customer or is_exempt_document(doc.get("is_return"), doc.get("is_consolidated"), frappe.flags): return - rules = get_rules(doc.customer, doc.company) + rules = get_rules(doc.customer, doc.company, action) if not rules: return @@ -160,7 +144,7 @@ def check(doc: Document, stage: str) -> None: return days = days_overdue(invoice.due_date, getdate()) - verdict = decide(rules, days, frappe.get_roles(), stage) + verdict = decide(rules, days, frappe.get_roles()) if not verdict.outcome: return @@ -174,9 +158,9 @@ def check(doc: Document, stage: str) -> None: # QUERIES -def get_rules(customer: str, company: str) -> list: +def get_rules(customer: str, company: str, action: str) -> list: group_chain = get_group_chain(frappe.db.get_value(CUSTOMER, customer, "customer_group")) - return select_rules(get_enabled_rules([customer, *group_chain]), customer, group_chain, company) + return select_rules(get_enabled_rules([customer, *group_chain]), customer, group_chain, company, action) def get_group_chain(customer_group: str | None) -> list[str]: diff --git a/optima_payment/overdue_limit/tests/test_limit.py b/optima_payment/overdue_limit/tests/test_limit.py index 30f3646..12d1dcd 100644 --- a/optima_payment/overdue_limit/tests/test_limit.py +++ b/optima_payment/overdue_limit/tests/test_limit.py @@ -3,10 +3,10 @@ """The Time Based Overdue Limit's decisions, on plain rows. Nothing here writes to the database. -Only what a plausible edit could break is here: which tier of rules applies, the action table, how -several triggered rules combine, the day a rule starts, what the message tells the user about -exemption, and the documents never checked, which no integration test reaches for POS. The wiring is -in test_limit_integration.py. +Only what a plausible edit could break is here: which tier of rules applies and which of its rules +each action picks, how several triggered rules combine, the day a rule starts, what the message tells +the user about exemption, and the documents never checked, which no integration test reaches for POS. +Which hook checks which action is wiring, in test_limit_integration.py. """ import frappe @@ -15,18 +15,14 @@ from optima_payment.overdue_limit.limit import ( BLOCK, BLOCK_ON_SAVE, - BLOCK_ON_SUBMIT, CUSTOMER, CUSTOMER_GROUP, - SAVE, - SUBMIT, WARN, WARN_ON_SAVE, days_overdue, decide, is_exempt_document, message, - rule_outcome, select_rules, ) @@ -84,8 +80,8 @@ def test_credit_notes_pos_consolidations_and_system_writes_are_never_checked(sel class TestSelectRules(FrappeTestCase): - def select(self, rules, company=COMPANY): - return [rule.name for rule in select_rules(rules, CUSTOMER_NAME, GROUP_CHAIN, company)] + def select(self, rules, company=COMPANY, action=WARN_ON_SAVE): + return [rule.name for rule in select_rules(rules, CUSTOMER_NAME, GROUP_CHAIN, company, action)] def test_the_nearest_tier_with_a_rule_wins(self): customer, group, parent, root = ( @@ -104,9 +100,20 @@ def test_the_nearest_tier_with_a_rule_wins(self): with self.subTest(winner): self.assertEqual(self.select(rules), [winner]) - def test_every_rule_of_the_winning_tier_applies(self): - rules = [rule("WARN"), rule("BLOCK", action=BLOCK_ON_SAVE), group_rule("GROUP", GROUP)] - self.assertEqual(self.select(rules), ["WARN", "BLOCK"]) + def test_every_rule_of_the_winning_tier_with_the_action_applies(self): + rules = [ + rule("EVERYWHERE", action=BLOCK_ON_SAVE), + rule("HERE", company=COMPANY, action=BLOCK_ON_SAVE), + rule("WARN"), + group_rule("GROUP", GROUP, action=BLOCK_ON_SAVE), + ] + self.assertEqual(self.select(rules, action=BLOCK_ON_SAVE), ["EVERYWHERE", "HERE"]) + + def test_the_tier_is_chosen_before_the_action(self): + """Filtering by action first would let the group's block through a customer that has only a + warning.""" + rules = [rule("WARN"), group_rule("GROUP", GROUP, action=BLOCK_ON_SAVE)] + self.assertEqual(self.select(rules, action=BLOCK_ON_SAVE), []) def test_a_company_rule_neither_applies_nor_shadows_in_another_company(self): rules = [rule("ELSEWHERE", company=OTHER_COMPANY), group_rule("EVERYWHERE", GROUP)] @@ -117,24 +124,9 @@ def test_a_group_named_like_the_customer_is_not_a_customer_rule(self): self.assertEqual(self.select([group_rule("GROUP", CUSTOMER_NAME)]), []) -class TestRuleOutcome(FrappeTestCase): - def test_the_action_table(self): - """The table in docs/overdue-limit/reference.md.""" - columns = [(SAVE, False), (SAVE, True), (SUBMIT, False), (SUBMIT, True)] - table = { - WARN_ON_SAVE: [WARN, WARN, None, None], - BLOCK_ON_SAVE: [BLOCK, WARN, BLOCK, WARN], - BLOCK_ON_SUBMIT: [WARN, WARN, BLOCK, WARN], - } - for action, row in table.items(): - for (stage, exempt), expected in zip(columns, row): - with self.subTest(action=action, stage=stage, exempt=exempt): - self.assertEqual(rule_outcome(action, stage, exempt), expected) - - class TestDecide(FrappeTestCase): def outcome_on(self, today, rules): - return decide(rules, days_overdue(INVOICE.due_date, today), SALES_ROLES, SAVE).outcome + return decide(rules, days_overdue(INVOICE.due_date, today), SALES_ROLES).outcome def test_a_rule_starts_on_its_threshold_day(self): rules = [rule(action=BLOCK_ON_SAVE, threshold_days=20)] @@ -143,26 +135,21 @@ def test_a_rule_starts_on_its_threshold_day(self): def test_the_refusal_lists_only_the_rules_that_block_this_user(self): rules = [ - rule("WARN"), rule("EXEMPTED", action=BLOCK_ON_SAVE, exempt_roles=[EXEMPT_ROLE]), - rule("BLOCK", action=BLOCK_ON_SAVE), + rule("BLOCK", company=COMPANY, action=BLOCK_ON_SAVE), ] - verdict = decide(rules, 30, [EXEMPT_ROLE], SAVE) + verdict = decide(rules, 30, [EXEMPT_ROLE]) self.assertEqual((verdict.outcome, hit_names(verdict)), (BLOCK, ["BLOCK"])) - def test_the_warning_lists_every_triggered_rule_and_no_other(self): - rules = [ - rule("WARN", threshold_days=20), - rule("NOTICE", action=BLOCK_ON_SUBMIT, threshold_days=25), - rule("NOT YET", action=BLOCK_ON_SAVE, threshold_days=40), - ] - verdict = decide(rules, 30, SALES_ROLES, SAVE) - self.assertEqual((verdict.outcome, hit_names(verdict)), (WARN, ["WARN", "NOTICE"])) + def test_the_warning_lists_only_the_triggered_rules(self): + rules = [rule("WARN", threshold_days=20), rule("NOT YET", company=COMPANY, threshold_days=40)] + verdict = decide(rules, 30, SALES_ROLES) + self.assertEqual((verdict.outcome, hit_names(verdict)), (WARN, ["WARN"])) class TestMessage(FrappeTestCase): def render(self, action, user_roles): - verdict = decide([rule(action=action, exempt_roles=[EXEMPT_ROLE])], 30, user_roles, SAVE) + verdict = decide([rule(action=action, exempt_roles=[EXEMPT_ROLE])], 30, user_roles) return message(verdict, CUSTOMER_NAME, INVOICE, 30) def test_a_blocked_user_is_told_which_roles_can_continue(self): diff --git a/optima_payment/overdue_limit/tests/test_limit_integration.py b/optima_payment/overdue_limit/tests/test_limit_integration.py index f389fcf..36820bb 100644 --- a/optima_payment/overdue_limit/tests/test_limit_integration.py +++ b/optima_payment/overdue_limit/tests/test_limit_integration.py @@ -4,9 +4,10 @@ """The Time Based Overdue Limit through real saves and submits of Sales Invoices and Sales Orders. The branch matrix and the message text are in test_limit.py. These prove what only a real document -shows: the hooks fire on both doctypes at both stages, an earlier draft cannot be submitted past a -Block on Save rule, the acting user's roles decide the exemption, credit notes pass, the group tree is -read from the database, and only a payment allocated to the invoice lifts the block. +shows: each action is checked at its own hook on both doctypes (Warn on Save at validate, so at +submit too; Block on Save at before_save only; Block on Submit at before_submit only), the acting +user's roles decide the exemption, credit notes pass, the group tree is read from the database, and +only a payment allocated to the invoice lifts the block. Each test starts from a customer whose Sales Invoice fell due 25 days ago and that ERPNext's daily status job has marked Overdue. @@ -71,13 +72,14 @@ def test_warn_on_save_saves_the_draft_with_a_warning(self): self.assertFalse(draft.is_new()) self.assertTrue(self.overdue_messages()) - def test_warn_on_save_lets_the_submit_through_silently(self): + def test_warn_on_save_warns_again_at_submit(self): + """Frappe runs validate at submit too.""" make_overdue_limit(party=self.customer, action=WARN_ON_SAVE) draft = self.make_draft() frappe.clear_messages() self.submit(draft) self.assertEqual(draft.docstatus, 1) - self.assertFalse(self.overdue_messages()) + self.assertTrue(self.overdue_messages()) def test_block_on_save_refuses_the_draft(self): make_overdue_limit(party=self.customer, action=BLOCK_ON_SAVE) @@ -85,19 +87,19 @@ def test_block_on_save_refuses_the_draft(self): with self.subTest(doctype), self.assertRaises(OverdueLimitError): self.make_draft(doctype) - def test_block_on_save_refuses_to_submit_a_draft_saved_before_the_rule(self): + def test_block_on_save_lets_a_draft_saved_before_the_rule_submit(self): draft = self.make_draft() make_overdue_limit(party=self.customer, action=BLOCK_ON_SAVE) - with self.assertRaises(OverdueLimitError): - self.submit(draft) + self.submit(draft) + self.assertEqual(draft.docstatus, 1) - def test_block_on_submit_saves_the_draft_with_a_warning_and_refuses_the_submit(self): + def test_block_on_submit_saves_the_draft_silently_and_refuses_the_submit(self): make_overdue_limit(party=self.customer, action=BLOCK_ON_SUBMIT) for doctype in SALES_DOCUMENTS: with self.subTest(doctype): frappe.clear_messages() draft = self.make_draft(doctype) - self.assertTrue(self.overdue_messages()) + self.assertFalse(self.overdue_messages()) with self.assertRaises(OverdueLimitError): self.submit(draft) @@ -116,8 +118,9 @@ def test_the_acting_users_roles_decide_the_exemption(self): self.submit(draft) self.assertEqual(draft.docstatus, 1) - def test_a_credit_note_submits_under_a_block(self): + def test_a_credit_note_saves_and_submits_under_both_blocks(self): make_overdue_limit(party=self.customer, action=BLOCK_ON_SAVE) + make_overdue_limit(party=self.customer, action=BLOCK_ON_SUBMIT) credit_note = make_credit_note(self.overdue_invoice.name) credit_note.insert(ignore_permissions=True) self.submit(credit_note) From 4ca049e90d329a5be0b25eec4577a06b969c2804 Mon Sep 17 00:00:00 2001 From: FawazAlhafiz Date: Wed, 4 Nov 2026 14:34:38 +0300 Subject: [PATCH 13/14] refactor(overdue-limit): name decide's inputs overdue_days and user_roles --- optima_payment/overdue_limit/limit.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/optima_payment/overdue_limit/limit.py b/optima_payment/overdue_limit/limit.py index 697e88e..5faad46 100644 --- a/optima_payment/overdue_limit/limit.py +++ b/optima_payment/overdue_limit/limit.py @@ -79,19 +79,19 @@ def is_exempt_user(rule, user_roles: set) -> bool: return rule.action != WARN_ON_SAVE and bool(user_roles & set(rule.exempt_roles)) -def decide(rules: list, days: int, user_roles) -> Verdict: - """What ``rules``, all of one action, do once the customer is ``days`` overdue. +def decide(rules: list, overdue_days: int, user_roles) -> Verdict: + """What ``rules``, all of one action, do once the customer is ``overdue_days`` overdue. - A rule triggers once ``days`` reaches its ``threshold_days``. Warn on Save warns. A block rule - blocks, but only warns a user holding one of its exempt roles. A block outranks warnings, which its - message then leaves out. + A rule triggers once ``overdue_days`` reaches its ``threshold_days``. Warn on Save warns. A block + rule blocks, but only warns a user holding one of its exempt roles. A block outranks warnings, which + its message then leaves out. """ - roles = set(user_roles) + user_roles = set(user_roles) hits = {BLOCK: [], WARN: []} for rule in rules: - if days < rule.threshold_days: + if overdue_days < rule.threshold_days: continue - exempt = is_exempt_user(rule, roles) + exempt = is_exempt_user(rule, user_roles) outcome = WARN if rule.action == WARN_ON_SAVE or exempt else BLOCK hits[outcome].append(Hit(rule, exempt)) From f3476e375719724be889f0706f1db2967647eeef Mon Sep 17 00:00:00 2001 From: FawazAlhafiz Date: Wed, 4 Nov 2026 14:34:38 +0300 Subject: [PATCH 14/14] docs(overdue-limit): describe what the desk shows when several rules apply When a save is refused, the desk drops the warnings sent with it, so only the refusal shows; the docs had said both appear. An exempt user sees each hook's message stacked in one dialog. Also spell out that any rule makes a level win whatever its action, that exemption is per rule, and that a block rule with no exempt roles blocks Administrator too. --- docs/overdue-limit/reference.md | 20 ++++++++++++-------- docs/overdue-limit/user-guide.md | 17 +++++++++++------ 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/docs/overdue-limit/reference.md b/docs/overdue-limit/reference.md index a455f81..90d80ce 100644 --- a/docs/overdue-limit/reference.md +++ b/docs/overdue-limit/reference.md @@ -92,8 +92,12 @@ Frappe runs `validate → before_save` on a save and `validate → before_submit - Submitting does **not** run `before_save`, so a draft saved before the customer went overdue (or before the rule existed) submits past a Block on Save rule. A Block on Submit rule is what stops a submit. -- When a Warn on Save and a Block on Save rule both trigger, `validate` shows the warning and - `before_save` then refuses; the user sees both messages. +- Each hook sends its own message. When a Warn on Save and a Block on Save rule both trigger, + `validate` queues the warning and `before_save` then refuses. Both reach the browser in + `_server_messages`, but the desk shows only the refusal: given a list in which any message + raised, `frappe.msgprint` drops the others (`frappe/public/js/frappe/ui/messages.js`). A client + reading the API response gets both. When the user is exempt from the block, nothing raises, and + the desk stacks the two warnings in one dialog, each with its own header line. ```mermaid flowchart LR @@ -151,7 +155,7 @@ customer = doc.customer, company = doc.company, status in ('Overdue', 'Overdue a ``` ordered `due_date, name` and limited to one, so the message can name it. `days_overdue` is -`today − due_date`, and a rule triggers when `days >= threshold_days`. +`today − due_date`, and a rule triggers when `overdue_days >= threshold_days`. What counts as overdue is ERPNext's decision, read from the invoice's `status`. ERPNext gives `Overdue` only to a submitted invoice with an outstanding amount, so drafts, cancelled and paid @@ -178,8 +182,8 @@ follow-up. ## Outcome -`decide(rules, days, user_roles)` takes the rules of one action. A rule triggers when -`days >= threshold_days`, and then: +`decide(rules, overdue_days, user_roles)` takes the rules of one action. A rule triggers when +`overdue_days >= threshold_days`, and then: | Action | User not exempt | User exempt | |--------|-----------------|-------------| @@ -201,10 +205,10 @@ Each `Hit` carries the rule and whether the user was exempt from it. The message line naming the rule, its action and its threshold (*Rule TBOL-00002 (Block on Save) applies from 30 days overdue.*), adding *Your role lets you continue* or the list of exempt roles. A block is raised as `OverdueLimitError`, a subclass of `frappe.ValidationError`. A warning is `msgprint` with an orange -indicator, and the document is saved. +indicator, and the save or submit goes on. -`frappe.get_roles("Administrator")` returns every role, so Administrator is exempt from any rule -that lists at least one exempt role. +`frappe.get_roles("Administrator")` returns every role, so Administrator is exempt from any block +rule that lists at least one exempt role. A block rule with no exempt roles blocks everyone. --- diff --git a/docs/overdue-limit/user-guide.md b/docs/overdue-limit/user-guide.md index ef3f7e1..bae7aaa 100644 --- a/docs/overdue-limit/user-guide.md +++ b/docs/overdue-limit/user-guide.md @@ -34,8 +34,10 @@ yet TBOL-00002's 30, so the order **saves** and shows: > due on 08-10-2026. > - Rule **TBOL-00001** (Warn on Save) applies from 20 days overdue. -Five days later the invoice is 30 days overdue. Saving shows the TBOL-00001 warning, then TBOL-00002 -**refuses** the save, and it keeps refusing until the invoice is paid. +Five days later the invoice is 30 days overdue, and TBOL-00002 **refuses** the save until the +invoice is paid. Only the refusal shows: when a save is refused, the TBOL-00001 warning is not +displayed with it. A user with an exempt role on TBOL-00002 would instead see two messages, one +under the other: the TBOL-00001 warning, then TBOL-00002 with *Your role lets you continue*. --- @@ -83,8 +85,10 @@ Each action does exactly what its name says, and nothing at the other step: - **Block on Submit** says nothing at save; the draft saves normally and only the submit is refused. - A user with an **exempt role** sees the warning instead of the refusal. Exempt roles have no effect on **Warn on Save**: everybody sees that warning. -- The **Administrator** user holds every role, so is exempt from any rule that lists at least one - exempt role. +- Exemption is **per rule**. Being exempt from one block rule does not help against another block + rule that lists different roles, or none. +- The **Administrator** user holds every role, so is exempt from any block rule that lists at least + one exempt role. A block rule with no exempt roles blocks everyone, Administrator included. --- @@ -97,8 +101,9 @@ The most specific rules win, and only those: 3. If that group has none, the rules of the **parent group**, then its parent, and so on up the tree. -All the rules at that first level apply together. A rule for a different company is ignored and -does not hide the rules above it. +All the rules at that first level apply together. A level counts as soon as it has any rule, +whatever its action: a customer with only its own Warn on Save rule is never blocked by its group's +Block on Save rule. A rule for a different company is ignored and does not hide the rules above it. ``` Al Noor Trading (no rule) → Retail (no rule) → Commercial: Block on Save, 30 days ← applies