diff --git a/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.js b/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.js index c304c7f17eb..3ca9518a1e8 100644 --- a/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.js +++ b/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.js @@ -22,27 +22,50 @@ frappe.ui.form.on("Repost Accounting Ledger", { }, refresh: function (frm) { - frm.add_custom_button(__("Show Preview"), () => { - frm.call({ - method: "generate_preview", - doc: frm.doc, - freeze: true, - freeze_message: __("Generating Preview"), - callback: function (r) { - if (r && r.message) { - let content = r.message; - let opts = { - title: "Preview", - subtitle: "preview", - content: content, - print_settings: { orientation: "landscape" }, - columns: [], - data: [], - }; - frappe.render_grid(opts); - } - }, + // the server refuses only while the job is alive, so a dead one can be restarted here + if (frm.doc.docstatus == 1 && !["Completed", "Cancelled"].includes(frm.doc.status)) { + frm.add_custom_button(__("Start Reposting"), () => { + frm.events.start_repost(frm); }); + } + + if (frm.doc.docstatus != 2) { + frm.add_custom_button(__("Show Preview"), () => { + frm.events.generate_preview(frm); + }); + } + }, + + generate_preview: function (frm) { + frm.call({ + method: "generate_preview", + doc: frm.doc, + freeze: true, + freeze_message: __("Generating Preview"), + callback: function (r) { + if (r && r.message) { + let content = r.message; + let opts = { + title: "Preview", + subtitle: "preview", + content: content, + print_settings: { orientation: "landscape" }, + columns: [], + data: [], + }; + frappe.render_grid(opts); + } + }, + }); + }, + + start_repost: function (frm) { + frm.call({ + method: "start_repost", + doc: frm.doc, + callback: function (r) { + frm.reload_doc(); + }, }); }, }); diff --git a/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.json b/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.json index 818b0e38fe1..90044abf40d 100644 --- a/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.json +++ b/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.json @@ -1,5 +1,6 @@ { "actions": [], + "allow_bulk_edit": 1, "creation": "2023-07-04 13:07:32.923675", "default_view": "List", "doctype": "DocType", @@ -7,16 +8,24 @@ "engine": "InnoDB", "field_order": [ "company", - "column_break_vpup", "delete_cancelled_entries", + "column_break_vpup", + "status", "section_break_metl", "vouchers", - "amended_from" + "error_section", + "error_log", + "miscellaneous_section", + "amended_from", + "column_break_hrah", + "scheduled_job" ], "fields": [ { "fieldname": "company", "fieldtype": "Link", + "in_list_view": 1, + "in_standard_filter": 1, "label": "Company", "options": "Company" }, @@ -48,12 +57,54 @@ "fieldname": "delete_cancelled_entries", "fieldtype": "Check", "label": "Delete Cancelled Ledger Entries" + }, + { + "fieldname": "error_section", + "fieldtype": "Section Break", + "label": "Error" + }, + { + "fieldname": "error_log", + "fieldtype": "Code", + "label": "Error Log", + "no_copy": 1, + "print_hide": 1, + "read_only": 1 + }, + { + "fieldname": "miscellaneous_section", + "fieldtype": "Section Break", + "label": "Miscellaneous" + }, + { + "fieldname": "column_break_hrah", + "fieldtype": "Column Break" + }, + { + "depends_on": "eval:doc.docstatus >= 1;", + "fieldname": "status", + "fieldtype": "Select", + "in_list_view": 1, + "in_standard_filter": 1, + "label": "Status", + "no_copy": 1, + "options": "\nQueued\nIn Progress\nPartially Reposted\nCompleted\nFailed\nCancelled", + "read_only": 1 + }, + { + "fieldname": "scheduled_job", + "fieldtype": "Link", + "hidden": 1, + "label": "Scheduled Job", + "no_copy": 1, + "options": "RQ Job", + "read_only": 1 } ], "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2024-06-03 17:30:37.012593", + "modified": "2026-07-28 00:56:50.290314", "modified_by": "Administrator", "module": "Accounts", "name": "Repost Accounting Ledger", @@ -76,8 +127,9 @@ "write": 1 } ], + "row_format": "Dynamic", "sort_field": "creation", "sort_order": "DESC", "states": [], "track_changes": 1 -} \ No newline at end of file +} diff --git a/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.py b/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.py index 8727aba48b0..7ef67554ae7 100644 --- a/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.py +++ b/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.py @@ -7,9 +7,14 @@ import frappe from frappe import _, qb from frappe.desk.form.linked_with import get_child_tables_of_doctypes from frappe.model.document import Document +from frappe.utils.background_jobs import create_job_id, is_job_enqueued from frappe.utils.data import comma_and +from frappe.utils.scheduler import is_scheduler_inactive -from erpnext.stock import get_warehouse_account_map +# a batch has to finish well within the timeout of the job reposting it +MAX_VOUCHERS_PER_REPOST = 50 + +HANDLED_VOUCHER_STATUSES = ("Reposted", "Skipped") class RepostAccountingLedger(Document): @@ -28,6 +33,11 @@ class RepostAccountingLedger(Document): amended_from: DF.Link | None company: DF.Link | None delete_cancelled_entries: DF.Check + error_log: DF.Code | None + scheduled_job: DF.Link | None + status: DF.Literal[ + "", "Queued", "In Progress", "Partially Reposted", "Completed", "Failed", "Cancelled" + ] vouchers: DF.Table[RepostAccountingLedgerItems] # end: auto-generated types @@ -37,6 +47,11 @@ class RepostAccountingLedger(Document): def validate(self): self.validate_vouchers() + self.validate_repost_preconditions() + + def validate_repost_preconditions(self): + """The checks a repost queued days ago could have outlived, re-run before it touches + the ledger. Vouchers cancelled since are skipped one by one while reposting.""" self.validate_for_closed_fiscal_year() self.validate_for_deferred_accounting() @@ -73,8 +88,52 @@ class RepostAccountingLedger(Document): frappe.throw(_("Cannot Resubmit Ledger entries for vouchers in Closed fiscal year.")) def validate_vouchers(self): - if self.vouchers: - validate_docs_for_voucher_types([x.voucher_type for x in self.vouchers]) + if not self.vouchers: + frappe.throw(_("Add atleast one voucher to repost.")) + + if len(self.vouchers) > MAX_VOUCHERS_PER_REPOST: + frappe.throw( + _("Cannot repost more than {0} vouchers at once. Split them into multiple documents.").format( + MAX_VOUCHERS_PER_REPOST + ) + ) + + validate_docs_for_voucher_types([x.voucher_type for x in self.vouchers]) + + self.validate_no_duplicate_vouchers() + self.validate_vouchers_are_submitted() + + def validate_no_duplicate_vouchers(self): + vouchers = [(x.voucher_type, x.voucher_no) for x in self.vouchers] + + if len(vouchers) != len(set(vouchers)): + frappe.throw(_("Duplicate vouchers found. Remove the duplicate vouchers to continue to repost.")) + + def validate_vouchers_are_submitted(self): + voucher_type_wise_map = {} + for d in self.vouchers: + voucher_type_wise_map.setdefault(d.voucher_type, []) + voucher_type_wise_map[d.voucher_type].append(d.voucher_no) + + non_submitted_vouchers = [] + for key in voucher_type_wise_map.keys(): + non_submitted_vouchers.extend( + frappe.get_all( + key, + filters={"name": ["in", voucher_type_wise_map[key]], "docstatus": ["!=", 1]}, + pluck="name", + ) + ) + + if non_submitted_vouchers: + frappe.throw( + _("The following vouchers are not submitted: {0}").format( + comma_and(non_submitted_vouchers, add_quotes=True) + ) + ) + + def on_discard(self): + self.db_set("status", "Cancelled") def get_existing_ledger_entries(self): vouchers = [x.voucher_no for x in self.vouchers] @@ -139,80 +198,245 @@ class RepostAccountingLedger(Document): return rendered_page def on_submit(self): - if len(self.vouchers) > 5: - job_name = "repost_accounting_ledger_" + self.name - frappe.enqueue( - method="erpnext.accounts.doctype.repost_accounting_ledger.repost_accounting_ledger.start_repost", - account_repost_doc=self.name, - is_async=True, - job_name=job_name, - enqueue_after_commit=True, + self.start_repost() + + def before_cancel(self): + self._raise_error_if_reposting_in_progress() + + def on_cancel(self): + self.db_set("status", "Cancelled") + + def _raise_error_if_reposting_in_progress(self): + if self.scheduled_job and is_job_enqueued(_repost_job_id(self.name)): + frappe.throw(_("Reposting is still in progress in background.")) + + @frappe.whitelist() + def start_repost(self): + if self.docstatus != 1: + frappe.throw(_("Reposting can be started only for submitted document.")) + + # under a row lock, so two concurrent starts cannot both get past here + status = frappe.db.get_value(self.doctype, self.name, "status", for_update=True) + if status in ("Completed", "Cancelled"): + frappe.throw(_("Reposting cannot be started when status is {0}.").format(status)) + + # `Queued` and `In Progress` are held back by the job, not by the status: a worker that + # died leaves the status behind and the document has to stay restartable + self._raise_error_if_reposting_in_progress() + + self.check_permission("write") + + # workers pick up enqueued jobs whether or not the scheduler runs, so this is a warning + if is_scheduler_inactive(): + frappe.msgprint( + _("Scheduler is inactive. Reposting will only run once background jobs are processed."), + alert=True, + indicator="orange", ) - frappe.msgprint(_("Repost has started in the background")) - else: - start_repost(self.name) + + self.db_set({"status": "Queued", "scheduled_job": create_job_id(_repost_job_id(self.name))}) + _enqueue_repost(self.name) + frappe.msgprint(_("Repost has started in the background"), alert=True, indicator="blue") -@frappe.whitelist() -def start_repost(account_repost_doc: str | None = None) -> None: - from erpnext.accounts.general_ledger import make_reverse_gl_entries +def _repost_job_id(repost_doc_name: str) -> str: + """Derived from the document, so a repost can only ever have one job.""" + return f"repost_accounting_ledger::{repost_doc_name}" + + +def _enqueue_repost(repost_doc_name: str) -> None: + """Hand the repost to a background worker. + + Tests run it in the foreground, inside their own transaction: documents edited after submit + repost themselves through `repost_accounting_entries`, and tests across apps assert on the + ledger right after doing so. + """ + frappe.enqueue( + method="erpnext.accounts.doctype.repost_accounting_ledger.repost_accounting_ledger.repost", + repost_doc_name=repost_doc_name, + commit=not frappe.in_test, + queue="long", + timeout=1500, + job_id=_repost_job_id(repost_doc_name), + deduplicate=True, + enqueue_after_commit=True, + now=frappe.in_test, + ) + + +def _lock_vouchers(vouchers) -> dict: + """Lock every voucher up front so a concurrent repost cannot touch the same GL entries. + + Returns them keyed by voucher, so reposting does not load them again. These are file locks + under the site directory: they serialise nothing across hosts that do not share it, and a + worker killed outright leaves them behind until they expire. + """ + locked_docs = {} + try: + for x in vouchers: + doc = frappe.get_doc(x.voucher_type, x.voucher_no) + doc.lock() + locked_docs[(x.voucher_type, x.voucher_no)] = doc + except Exception: + for doc in locked_docs.values(): + doc.unlock() + raise + return locked_docs + + +def repost(repost_doc_name: str, commit: bool = True): + """Repost every voucher of the document, one transaction at a time. + + `commit` says whether this call owns the transaction. The background job does, and commits + after every voucher so progress survives a crash; a caller inside its own passes `False`. + """ + from erpnext.accounts.utils import _delete_accounting_ledger_entries, _delete_adv_pl_entries frappe.flags.through_repost_accounting_ledger = True - if account_repost_doc: - repost_doc = frappe.get_doc("Repost Accounting Ledger", account_repost_doc) - repost_doc.check_permission("write") - if repost_doc.docstatus == 1: - # Prevent repost on invoices with deferred accounting - repost_doc.validate_for_deferred_accounting() + repost_doc = frappe.get_doc("Repost Accounting Ledger", repost_doc_name) + locked_docs = {} - for x in repost_doc.vouchers: - doc = frappe.get_doc(x.voucher_type, x.voucher_no) + try: + repost_doc.validate_repost_preconditions() + + # a retry leaves the vouchers it is done with alone: they are not locked, not loaded + # and not reposted again + pending = [x for x in repost_doc.vouchers if x.status not in HANDLED_VOUCHER_STATUSES] + locked_docs = _lock_vouchers(pending) + + repost_doc.db_set("status", "In Progress", commit=commit) + + for position, x in enumerate(pending, start=1): + frappe.publish_progress( + position * 100 / len(pending), + doctype=repost_doc.doctype, + docname=repost_doc.name, + description=_("Reposting {0} {1}").format(x.voucher_type, x.voucher_no), + ) + + save_point = "reposting" + frappe.db.savepoint(save_point=save_point) + try: + doc = locked_docs[(x.voucher_type, x.voucher_no)] + + if doc.docstatus == 2: + x.db_set({"status": "Skipped", "traceback": ""}) + continue if repost_doc.delete_cancelled_entries: - frappe.db.delete( - "GL Entry", filters={"voucher_type": doc.doctype, "voucher_no": doc.name} - ) - frappe.db.delete( - "Payment Ledger Entry", filters={"voucher_type": doc.doctype, "voucher_no": doc.name} - ) - frappe.db.delete( - "Advance Payment Ledger Entry", - filters={"voucher_type": doc.doctype, "voucher_no": doc.name}, - ) + _delete_accounting_ledger_entries(doc.doctype, doc.name) + _delete_adv_pl_entries(doc.doctype, doc.name) - if doc.doctype in ["Sales Invoice", "Purchase Invoice"]: - if not repost_doc.delete_cancelled_entries: - doc.docstatus = 2 - doc.make_gl_entries_on_cancel(from_repost=True) + _repost_vouchers(doc, repost_doc.delete_cancelled_entries) + except Exception: + frappe.db.rollback(save_point=save_point) - doc.docstatus = 1 - if doc.doctype == "Sales Invoice": - doc.force_set_against_income_account() - else: - doc.force_set_against_expense_account() - doc.make_gl_entries() + x.db_set({"status": "Failed", "traceback": frappe.get_traceback()}) + else: + x.db_set({"status": "Reposted", "traceback": ""}) + finally: + if commit: + frappe.db.commit() # nosemgrep - elif doc.doctype == "Purchase Receipt": - if not repost_doc.delete_cancelled_entries: - doc.docstatus = 2 - doc.make_gl_entries_on_cancel(from_repost=True) + except Exception: + if commit: + frappe.db.rollback() - doc.docstatus = 1 - doc.make_gl_entries(from_repost=True) + _record_repost_failure(repost_doc, commit=commit) + raise + else: + repost_doc.db_set({"status": _derive_status(repost_doc), "error_log": ""}, notify=True) + finally: + for doc in locked_docs.values(): + doc.unlock() + if commit: + frappe.db.commit() # nosemgrep - elif doc.doctype in ["Payment Entry", "Journal Entry", "Expense Claim"]: - if not repost_doc.delete_cancelled_entries: - doc.make_gl_entries(1) - doc.make_gl_entries() - elif doc.doctype in frappe.get_hooks("repost_allowed_doctypes"): - if hasattr(doc, "make_gl_entries") and callable(doc.make_gl_entries): - if not repost_doc.delete_cancelled_entries: - if "cancel" in inspect.getfullargspec(doc.make_gl_entries): - doc.make_gl_entries(cancel=1) - else: - make_reverse_gl_entries(voucher_type=doc.doctype, voucher_no=doc.name) - doc.make_gl_entries() + +def _derive_status(repost_doc) -> str: + """Vouchers are committed one by one, so the status follows what was actually handled.""" + handled = sum(1 for voucher in repost_doc.vouchers if voucher.status in HANDLED_VOUCHER_STATUSES) + + if handled == len(repost_doc.vouchers): + return "Completed" + elif handled == 0: + return "Failed" + + return "Partially Reposted" + + +def _record_repost_failure(repost_doc, commit=False) -> None: + """Persist the traceback of a run that could not finish, without discarding its progress.""" + # the traceback with frame locals goes to the Error Log, which is permissioned separately + traceback = frappe.get_traceback() + + frappe.log_error( + title=_("Unable to Repost Accounting Ledger"), + reference_doctype=repost_doc.doctype, + reference_name=repost_doc.name, + ) + + frappe.db.set_value( + repost_doc.doctype, repost_doc.name, {"error_log": traceback, "status": _derive_status(repost_doc)} + ) + + if commit: + frappe.db.commit() + + +def _repost_vouchers(doc, delete_cancelled_entries: bool | int | None): + if doc.doctype in ["Sales Invoice", "Purchase Invoice"]: + _repost_invoices(doc, delete_cancelled_entries) + + elif doc.doctype == "Purchase Receipt": + _repost_purchase_receipt(doc, delete_cancelled_entries) + + elif doc.doctype in ["Payment Entry", "Journal Entry"]: + _repost_pe_je(doc, delete_cancelled_entries) + + elif doc.doctype in frappe.get_hooks("repost_allowed_doctypes"): + _repost_allowed_hook_doctypes(doc, delete_cancelled_entries) + + +def _repost_invoices(invoice_doc, delete_cancelled_entries): + if not delete_cancelled_entries: + invoice_doc.docstatus = 2 + invoice_doc.make_gl_entries_on_cancel(from_repost=True) + + invoice_doc.docstatus = 1 + if invoice_doc.doctype == "Sales Invoice": + invoice_doc.force_set_against_income_account() + else: + invoice_doc.force_set_against_expense_account() + invoice_doc.make_gl_entries() + + +def _repost_purchase_receipt(receipt_doc, delete_cancelled_entries): + if not delete_cancelled_entries: + receipt_doc.docstatus = 2 + receipt_doc.make_gl_entries_on_cancel(from_repost=True) + + receipt_doc.docstatus = 1 + receipt_doc.make_gl_entries(from_repost=True) + + +def _repost_pe_je(entry_doc, delete_cancelled_entries): + if not delete_cancelled_entries: + entry_doc.make_gl_entries(cancel=1) + entry_doc.make_gl_entries() + + +def _repost_allowed_hook_doctypes(repost_doc, delete_cancelled_entries: bool | int | None): + from erpnext.accounts.general_ledger import make_reverse_gl_entries + + if hasattr(repost_doc, "make_gl_entries") and callable(repost_doc.make_gl_entries): + if not delete_cancelled_entries: + if "cancel" in inspect.getfullargspec(repost_doc.make_gl_entries).args: + repost_doc.make_gl_entries(cancel=1) + else: + make_reverse_gl_entries(voucher_type=repost_doc.doctype, voucher_no=repost_doc.name) + repost_doc.make_gl_entries() def get_allowed_types_from_settings(child_doc: bool = False): diff --git a/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger_list.js b/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger_list.js new file mode 100644 index 00000000000..0ecdca3843c --- /dev/null +++ b/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger_list.js @@ -0,0 +1,16 @@ +frappe.listview_settings["Repost Accounting Ledger"] = { + add_fields: ["status"], + // drafts and cancelled documents are coloured by the framework before it gets here + get_indicator: function (doc) { + if (!doc.status) return; + + const status_color = { + Queued: "yellow", + "In Progress": "blue", + "Partially Reposted": "orange", + Completed: "green", + Failed: "red", + }; + return [__(doc.status), status_color[doc.status] || "gray", "status,=," + doc.status]; + }, +}; diff --git a/erpnext/accounts/doctype/repost_accounting_ledger/test_repost_accounting_ledger.py b/erpnext/accounts/doctype/repost_accounting_ledger/test_repost_accounting_ledger.py index 935047e2e35..5c436dc360f 100644 --- a/erpnext/accounts/doctype/repost_accounting_ledger/test_repost_accounting_ledger.py +++ b/erpnext/accounts/doctype/repost_accounting_ledger/test_repost_accounting_ledger.py @@ -1,27 +1,42 @@ # Copyright (c) 2023, Frappe Technologies Pvt. Ltd. and Contributors # See license.txt +from contextlib import contextmanager +from unittest.mock import patch + import frappe from frappe import qb from frappe.query_builder.functions import Sum from frappe.utils import add_days, nowdate, today +from erpnext.accounts.doctype.journal_entry.test_journal_entry import make_journal_entry from erpnext.accounts.doctype.payment_entry.payment_entry import get_payment_entry from erpnext.accounts.doctype.payment_request.payment_request import make_payment_request +from erpnext.accounts.doctype.repost_accounting_ledger.repost_accounting_ledger import ( + _lock_vouchers, + _record_repost_failure, + _repost_allowed_hook_doctypes, + _repost_job_id, + _repost_vouchers, + repost, +) from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice from erpnext.accounts.utils import get_fiscal_year from erpnext.stock.doctype.item.test_item import make_item from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import get_gl_entries, make_purchase_receipt from erpnext.tests.utils import ERPNextTestSuite +REPOST_MODULE = "erpnext.accounts.doctype.repost_accounting_ledger.repost_accounting_ledger" +SIMULATED_FAILURE = "Simulated repost failure" + class TestRepostAccountingLedger(ERPNextTestSuite): def setUp(self): frappe.db.set_single_value("Selling Settings", "validate_selling_price", 0) update_repost_settings() - def test_01_basic_functions(self): - si = create_sales_invoice( + def make_invoice(self, **kwargs): + return create_sales_invoice( item="_Test Item", company="_Test Company", customer="_Test Customer", @@ -29,8 +44,71 @@ class TestRepostAccountingLedger(ERPNextTestSuite): parent_cost_center="Main - _TC", cost_center="Main - _TC", rate=100, + **kwargs, ) + def make_invoice_and_payment(self): + si = self.make_invoice() + pe = get_payment_entry(si.doctype, si.name) + pe.save().submit() + return si, pe + + def create_repost_doc(self, vouchers, delete_cancelled_entries=False, submit=False): + ral = frappe.new_doc("Repost Accounting Ledger") + ral.company = "_Test Company" + ral.delete_cancelled_entries = delete_cancelled_entries + for voucher in vouchers: + ral.append("vouchers", {"voucher_type": voucher.doctype, "voucher_no": voucher.name}) + + ral.save() + if submit: + ral.submit() + ral.reload() + return ral + + @contextmanager + def patched_repost(self, fail_for=()): + """Yield the vouchers handed over to `_repost_vouchers`, failing the given types.""" + reposted = [] + + def repost_voucher(doc, delete_cancelled_entries): + reposted.append(doc.name) + if doc.doctype in fail_for: + frappe.throw(SIMULATED_FAILURE) + _repost_vouchers(doc, delete_cancelled_entries) + + with patch(f"{REPOST_MODULE}._repost_vouchers", new=repost_voucher): + yield reposted + + def make_period_closing_voucher(self): + fy = get_fiscal_year(today(), company="_Test Company") + pcv = frappe.get_doc( + { + "doctype": "Period Closing Voucher", + "transaction_date": today(), + "period_start_date": fy[1], + "period_end_date": today(), + "company": "_Test Company", + "fiscal_year": fy[0], + "cost_center": "Main - _TC", + "closing_account_head": "Retained Earnings - _TC", + "remarks": "test", + } + ) + return pcv.save().submit() + + def get_gl_totals(self, voucher_no, is_cancelled=0): + gl = qb.DocType("GL Entry") + return ( + qb.from_(gl) + .select(Sum(gl.debit).as_("debit"), Sum(gl.credit).as_("credit")) + .where((gl.voucher_no == voucher_no) & (gl.is_cancelled == is_cancelled)) + .run() + )[0] + + def test_01_basic_functions(self): + si = self.make_invoice() + preq = frappe.get_doc( make_payment_request( dt=si.doctype, @@ -64,51 +142,24 @@ class TestRepostAccountingLedger(ERPNextTestSuite): gle = frappe.db.get_all("GL Entry", filters={"voucher_no": si.name, "account": "Debtors - _TC"}) frappe.db.set_value("GL Entry", gle[0], "debit", 90) - gl = qb.DocType("GL Entry") - res = ( - qb.from_(gl) - .select(gl.voucher_no, Sum(gl.debit).as_("debit"), Sum(gl.credit).as_("credit")) - .where((gl.voucher_no == si.name) & (gl.is_cancelled == 0)) - .run() - ) - # Assert incorrect ledger balance - self.assertNotEqual(res[0], (si.name, 100, 100)) + self.assertNotEqual(self.get_gl_totals(si.name), (100, 100)) # Submit repost document ral.save().submit() - res = ( - qb.from_(gl) - .select(gl.voucher_no, Sum(gl.debit).as_("debit"), Sum(gl.credit).as_("credit")) - .where((gl.voucher_no == si.name) & (gl.is_cancelled == 0)) - .run() - ) - # Ledger should reflect correct amount post repost - self.assertEqual(res[0], (si.name, 100, 100)) + self.assertEqual(self.get_gl_totals(si.name), (100, 100)) def test_02_deferred_accounting_valiations(self): - si = create_sales_invoice( - item="_Test Item", - company="_Test Company", - customer="_Test Customer", - debit_to="Debtors - _TC", - parent_cost_center="Main - _TC", - cost_center="Main - _TC", - rate=100, - do_not_submit=True, - ) + si = self.make_invoice(do_not_submit=True) si.items[0].enable_deferred_revenue = True si.items[0].deferred_revenue_account = "Deferred Revenue - _TC" si.items[0].service_start_date = nowdate() si.items[0].service_end_date = add_days(nowdate(), 90) si.save().submit() - ral = frappe.new_doc("Repost Accounting Ledger") - ral.company = "_Test Company" - ral.append("vouchers", {"voucher_type": si.doctype, "voucher_no": si.name}) - self.assertRaises(frappe.ValidationError, ral.save) + self.assertRaises(frappe.ValidationError, self.create_repost_doc, [si]) @ERPNextTestSuite.change_settings("Accounts Settings", {"delete_linked_ledger_entries": 1}) def test_04_pcv_validation(self): @@ -116,86 +167,29 @@ class TestRepostAccountingLedger(ERPNextTestSuite): gl = frappe.qb.DocType("GL Entry") qb.from_(gl).delete().where(gl.company == "_Test Company").run() - si = create_sales_invoice( - item="_Test Item", - company="_Test Company", - customer="_Test Customer", - debit_to="Debtors - _TC", - parent_cost_center="Main - _TC", - cost_center="Main - _TC", - rate=100, - ) - fy = get_fiscal_year(today(), company="_Test Company") - pcv = frappe.get_doc( - { - "doctype": "Period Closing Voucher", - "transaction_date": today(), - "period_start_date": fy[1], - "period_end_date": today(), - "company": "_Test Company", - "fiscal_year": fy[0], - "cost_center": "Main - _TC", - "closing_account_head": "Retained Earnings - _TC", - "remarks": "test", - } - ) - pcv.save().submit() + si = self.make_invoice() + pcv = self.make_period_closing_voucher() - ral = frappe.new_doc("Repost Accounting Ledger") - ral.company = "_Test Company" - ral.append("vouchers", {"voucher_type": si.doctype, "voucher_no": si.name}) - self.assertRaises(frappe.ValidationError, ral.save) + self.assertRaises(frappe.ValidationError, self.create_repost_doc, [si]) pcv.reload() pcv.cancel() pcv.delete() def test_03_deletion_flag_and_preview_function(self): - si = create_sales_invoice( - item="_Test Item", - company="_Test Company", - customer="_Test Customer", - debit_to="Debtors - _TC", - parent_cost_center="Main - _TC", - cost_center="Main - _TC", - rate=100, - ) - - pe = get_payment_entry(si.doctype, si.name) - pe.save().submit() + si, pe = self.make_invoice_and_payment() # with deletion flag set - ral = frappe.new_doc("Repost Accounting Ledger") - ral.company = "_Test Company" - ral.delete_cancelled_entries = True - ral.append("vouchers", {"voucher_type": si.doctype, "voucher_no": si.name}) - ral.append("vouchers", {"voucher_type": pe.doctype, "voucher_no": pe.name}) - ral.save().submit() + self.create_repost_doc([si, pe], delete_cancelled_entries=True, submit=True) self.assertIsNone(frappe.db.exists("GL Entry", {"voucher_no": si.name, "is_cancelled": 1})) self.assertIsNone(frappe.db.exists("GL Entry", {"voucher_no": pe.name, "is_cancelled": 1})) def test_05_without_deletion_flag(self): - si = create_sales_invoice( - item="_Test Item", - company="_Test Company", - customer="_Test Customer", - debit_to="Debtors - _TC", - parent_cost_center="Main - _TC", - cost_center="Main - _TC", - rate=100, - ) - - pe = get_payment_entry(si.doctype, si.name) - pe.save().submit() + si, pe = self.make_invoice_and_payment() # without deletion flag set - ral = frappe.new_doc("Repost Accounting Ledger") - ral.company = "_Test Company" - ral.delete_cancelled_entries = False - ral.append("vouchers", {"voucher_type": si.doctype, "voucher_no": si.name}) - ral.append("vouchers", {"voucher_type": pe.doctype, "voucher_no": pe.name}) - ral.save().submit() + self.create_repost_doc([si, pe], submit=True) self.assertIsNotNone(frappe.db.exists("GL Entry", {"voucher_no": si.name, "is_cancelled": 1})) self.assertIsNotNone(frappe.db.exists("GL Entry", {"voucher_no": pe.name, "is_cancelled": 1})) @@ -246,11 +240,7 @@ class TestRepostAccountingLedger(ERPNextTestSuite): another_provisional_account, ) - repost_doc = frappe.new_doc("Repost Accounting Ledger") - repost_doc.company = "_Test Company" - repost_doc.delete_cancelled_entries = True - repost_doc.append("vouchers", {"voucher_type": pr.doctype, "voucher_no": pr.name}) - repost_doc.save().submit() + repost_doc = self.create_repost_doc([pr], delete_cancelled_entries=True, submit=True) pr_gles_after_repost = get_gl_entries(pr.doctype, pr.name, skip_cancelled=True) expected_pr_gles_after_repost = [ @@ -271,6 +261,281 @@ class TestRepostAccountingLedger(ERPNextTestSuite): company.default_provisional_account = None company.save() + def test_07_voucher_validations(self): + submitted_si = self.make_invoice() + draft_si = self.make_invoice(do_not_submit=True) + cancelled_si = self.make_invoice() + cancelled_si.cancel() + + for vouchers, exception, message in ( + ([], frappe.ValidationError, "Add atleast one voucher"), + ([submitted_si, submitted_si], frappe.ValidationError, "Duplicate vouchers found"), + ([draft_si], frappe.ValidationError, f"not submitted.*{draft_si.name}"), + # cancelled vouchers don't make it past link validation + ([cancelled_si], frappe.CancelledLinkError, "Cannot link cancelled document"), + ): + with self.subTest(vouchers=[x.name for x in vouchers]): + self.assertRaisesRegex(exception, message, self.create_repost_doc, vouchers) + + self.create_repost_doc([submitted_si]) + + def test_08_voucher_count_limit(self): + si, pe = self.make_invoice_and_payment() + another_si = self.make_invoice() + + with patch(f"{REPOST_MODULE}.MAX_VOUCHERS_PER_REPOST", 2): + self.create_repost_doc([si, pe]) + self.assertRaisesRegex( + frappe.ValidationError, + "Cannot repost more than 2 vouchers", + self.create_repost_doc, + [si, pe, another_si], + ) + + def test_09_status_lifecycle(self): + si, pe = self.make_invoice_and_payment() + + ral = self.create_repost_doc([si, pe]) + self.assertEqual(ral.status, "") + + ral.submit() + ral.reload() + + self.assertEqual(ral.status, "Completed") + self.assertFalse(ral.error_log) + for voucher in ral.vouchers: + self.assertEqual(voucher.status, "Reposted") + self.assertFalse(voucher.traceback) + + ral.cancel() + ral.reload() + self.assertEqual(ral.status, "Cancelled") + + discarded = self.create_repost_doc([si]) + discarded.discard() + discarded.reload() + self.assertEqual(discarded.status, "Cancelled") + + def test_10_start_repost_guards(self): + si = self.make_invoice() + ral = self.create_repost_doc([si]) + + self.assertRaisesRegex(frappe.ValidationError, "only for submitted document", ral.start_repost) + + ral.submit() + ral.reload() + self.assertRaisesRegex( + frappe.ValidationError, "cannot be started when status is Completed", ral.start_repost + ) + + # a document left behind by a worker that died mid-repost + ral.db_set("status", "In Progress") + + with patch(f"{REPOST_MODULE}.is_job_enqueued", return_value=True): + self.assertRaisesRegex( + frappe.ValidationError, "still in progress in background", ral.start_repost + ) + self.assertRaisesRegex(frappe.ValidationError, "still in progress in background", ral.cancel) + + # `cancel` flips docstatus in memory before running `before_cancel` + ral.reload() + + with patch(f"{REPOST_MODULE}.is_job_enqueued", return_value=False): + # the job is gone, so `In Progress` must not keep the document stuck + ral.start_repost() + + ral.reload() + self.assertEqual(ral.status, "Completed") + + def test_11_repost_job_is_tied_to_the_document(self): + si = self.make_invoice() + ral = self.create_repost_doc([si], submit=True) + ral.db_set("status", "Failed") + + with patch(f"{REPOST_MODULE}.frappe.enqueue") as enqueue: + ral.start_repost() + + kwargs = enqueue.call_args.kwargs + self.assertEqual(kwargs["repost_doc_name"], ral.name) + self.assertEqual(kwargs["job_id"], _repost_job_id(ral.name)) + # a second start cannot queue a second job for the same document + self.assertTrue(kwargs["deduplicate"]) + + def test_12_voucher_failures_are_isolated_and_retried(self): + si, pe = self.make_invoice_and_payment() + pe_gl_entries = frappe.db.count("GL Entry", {"voucher_no": pe.name}) + + # the deletion flag drops the existing entries before reposting them + ral = self.create_repost_doc([si, pe], delete_cancelled_entries=True) + with self.patched_repost(fail_for=["Payment Entry"]): + ral.submit() + + ral.reload() + self.assertEqual(ral.status, "Partially Reposted") + + si_row, pe_row = ral.vouchers + self.assertEqual((si_row.status, pe_row.status), ("Reposted", "Failed")) + self.assertFalse(si_row.traceback) + self.assertIn(SIMULATED_FAILURE, pe_row.traceback) + + # the failed voucher is rolled back to its savepoint, so its entries are back + self.assertEqual(frappe.db.count("GL Entry", {"voucher_no": pe.name}), pe_gl_entries) + + # a retry only picks up the vouchers that are not reposted yet, and leaves the rest + # alone entirely: they are not locked or loaded either + with ( + patch(f"{REPOST_MODULE}._lock_vouchers", side_effect=_lock_vouchers) as lock_vouchers, + self.patched_repost() as retried, + ): + ral.start_repost() + + self.assertEqual(retried, [pe.name]) + self.assertEqual([x.voucher_no for x in lock_vouchers.call_args.args[0]], [pe.name]) + + ral.reload() + self.assertEqual(ral.status, "Completed") + for voucher in ral.vouchers: + self.assertEqual(voucher.status, "Reposted") + self.assertFalse(voucher.traceback) + + def test_13_status_of_a_run_that_could_not_finish(self): + si, pe = self.make_invoice_and_payment() + + ral = self.create_repost_doc([si, pe]) + with self.patched_repost(fail_for=["Payment Entry"]): + ral.submit() + + ral.reload() + + # the job dies after the loop committed the invoice, e.g. killed or timed out + try: + frappe.throw(SIMULATED_FAILURE) + except frappe.ValidationError: + _record_repost_failure(ral) + + ral.reload() + + # progress already committed must not be reported as a total failure + self.assertEqual(ral.status, "Partially Reposted") + self.assertIn(SIMULATED_FAILURE, ral.error_log) + self.assertTrue( + frappe.db.exists("Error Log", {"reference_doctype": ral.doctype, "reference_name": ral.name}) + ) + + @ERPNextTestSuite.change_settings("Accounts Settings", {"delete_linked_ledger_entries": 1}) + def test_14_period_closed_after_the_repost_was_started(self): + gl = qb.DocType("GL Entry") + qb.from_(gl).delete().where(gl.company == "_Test Company").run() + + si = self.make_invoice() + ral = self.create_repost_doc([si], submit=True) + ral.db_set("status", "Failed") + ral.vouchers[0].db_set("status", "Pending") + + # the period is closed between the repost being started and the job running + self.make_period_closing_voucher() + + gl_entries = frappe.db.count("GL Entry", {"voucher_no": si.name}) + self.assertRaisesRegex(frappe.ValidationError, "Closed fiscal year", repost, ral.name, commit=False) + + ral.reload() + self.assertEqual(ral.status, "Failed") + self.assertIn("Closed fiscal year", ral.error_log) + + # the ledger is left exactly as it was + self.assertEqual(frappe.db.count("GL Entry", {"voucher_no": si.name}), gl_entries) + self.assertEqual(ral.vouchers[0].status, "Pending") + + def test_15_failed_repost_skips_cancelled_voucher(self): + si = self.make_invoice() + + ral = self.create_repost_doc([si]) + with self.patched_repost(fail_for=["Sales Invoice"]): + ral.submit() + + ral.reload() + self.assertEqual(ral.status, "Failed") + + si.reload() + si.cancel() + + ral.start_repost() + ral.reload() + + # nothing was reposted, but there is nothing left to repost either + self.assertEqual(ral.status, "Completed") + self.assertEqual(ral.vouchers[0].status, "Skipped") + self.assertFalse(ral.vouchers[0].traceback) + + def test_16_concurrent_repost_is_blocked_by_voucher_lock(self): + si, pe = self.make_invoice_and_payment() + ral = self.create_repost_doc([si, pe]) + + # a concurrent repost holding the lock on the second voucher + locked_pe = frappe.get_doc(pe.doctype, pe.name) + locked_pe.lock() + try: + self.assertRaises(frappe.DocumentLockedError, ral.submit) + + # vouchers locked before the failure are released again + self.assertFalse(frappe.get_doc(si.doctype, si.name).is_locked) + finally: + locked_pe.unlock() + + def test_17_journal_entry_repost(self): + je = make_journal_entry("_Test Bank - _TC", "_Test Cash - _TC", 500, submit=True) + je = frappe.get_doc("Journal Entry", je.name) + + self.assertEqual(self.get_gl_totals(je.name), (500.0, 500.0)) + + # without the deletion flag the 2 original entries are marked as cancelled, + # along with the 2 reverse entries booked against them + for delete_cancelled_entries, cancelled_entries in ((False, 4), (True, 0)): + with self.subTest(delete_cancelled_entries=delete_cancelled_entries): + ral = self.create_repost_doc( + [je], delete_cancelled_entries=delete_cancelled_entries, submit=True + ) + + self.assertEqual(ral.status, "Completed") + self.assertEqual(self.get_gl_totals(je.name), (500.0, 500.0)) + self.assertEqual( + frappe.db.count("GL Entry", {"voucher_no": je.name, "is_cancelled": 1}), + cancelled_entries, + ) + + def test_18_hook_allowed_doctype_repost(self): + class VoucherWithCancelArg: + doctype = "Test Repost Voucher" + name = "TRV-00001" + + def __init__(self): + self.calls = [] + + def make_gl_entries(self, cancel=0): + self.calls.append(cancel) + + class VoucherWithoutCancelArg(VoucherWithCancelArg): + def make_gl_entries(self): + self.calls.append("repost") + + # vouchers that can reverse their own entries are asked to do so first + doc = VoucherWithCancelArg() + _repost_allowed_hook_doctypes(doc, delete_cancelled_entries=False) + self.assertEqual(doc.calls, [1, 0]) + + # nothing to reverse when the old entries are deleted + doc = VoucherWithCancelArg() + _repost_allowed_hook_doctypes(doc, delete_cancelled_entries=True) + self.assertEqual(doc.calls, [0]) + + # the rest fall back to the generic reversal + doc = VoucherWithoutCancelArg() + with patch("erpnext.accounts.general_ledger.make_reverse_gl_entries") as make_reverse_gl_entries: + _repost_allowed_hook_doctypes(doc, delete_cancelled_entries=False) + + make_reverse_gl_entries.assert_called_once_with(voucher_type=doc.doctype, voucher_no=doc.name) + self.assertEqual(doc.calls, ["repost"]) + def update_repost_settings(): allowed_types = [ diff --git a/erpnext/accounts/doctype/repost_accounting_ledger_items/repost_accounting_ledger_items.json b/erpnext/accounts/doctype/repost_accounting_ledger_items/repost_accounting_ledger_items.json index fd5bb92959d..c6d9468e36f 100644 --- a/erpnext/accounts/doctype/repost_accounting_ledger_items/repost_accounting_ledger_items.json +++ b/erpnext/accounts/doctype/repost_accounting_ledger_items/repost_accounting_ledger_items.json @@ -1,5 +1,6 @@ { "actions": [], + "allow_bulk_edit": 1, "allow_rename": 1, "creation": "2023-07-04 14:14:01.243848", "doctype": "DocType", @@ -7,34 +8,70 @@ "engine": "InnoDB", "field_order": [ "voucher_type", - "voucher_no" + "column_break_ndex", + "voucher_no", + "reposting_status_section", + "status", + "traceback" ], "fields": [ { + "columns": 5, "fieldname": "voucher_type", "fieldtype": "Link", "in_list_view": 1, "label": "Voucher Type", - "options": "DocType" + "options": "DocType", + "reqd": 1 }, { + "fieldname": "column_break_ndex", + "fieldtype": "Column Break" + }, + { + "columns": 5, "fieldname": "voucher_no", "fieldtype": "Dynamic Link", "in_list_view": 1, "label": "Voucher No", - "options": "voucher_type" + "options": "voucher_type", + "reqd": 1 + }, + { + "fieldname": "reposting_status_section", + "fieldtype": "Section Break", + "label": "Reposting Status" + }, + { + "columns": 2, + "default": "Pending", + "fieldname": "status", + "fieldtype": "Select", + "in_list_view": 1, + "label": "Status", + "no_copy": 1, + "options": "Pending\nReposted\nSkipped\nFailed", + "read_only": 1 + }, + { + "fieldname": "traceback", + "fieldtype": "Code", + "label": "Traceback", + "no_copy": 1, + "read_only": 1 } ], "index_web_pages_for_search": 1, "istable": 1, "links": [], - "modified": "2024-03-27 13:10:32.170897", + "modified": "2026-07-29 02:41:00.000000", "modified_by": "Administrator", "module": "Accounts", "name": "Repost Accounting Ledger Items", "owner": "Administrator", "permissions": [], + "row_format": "Dynamic", "sort_field": "creation", "sort_order": "DESC", "states": [] -} \ No newline at end of file +} diff --git a/erpnext/accounts/doctype/repost_accounting_ledger_items/repost_accounting_ledger_items.py b/erpnext/accounts/doctype/repost_accounting_ledger_items/repost_accounting_ledger_items.py index 6e02e3a6b98..a895e218e4d 100644 --- a/erpnext/accounts/doctype/repost_accounting_ledger_items/repost_accounting_ledger_items.py +++ b/erpnext/accounts/doctype/repost_accounting_ledger_items/repost_accounting_ledger_items.py @@ -17,8 +17,10 @@ class RepostAccountingLedgerItems(Document): parent: DF.Data parentfield: DF.Data parenttype: DF.Data - voucher_no: DF.DynamicLink | None - voucher_type: DF.Link | None + status: DF.Literal["Pending", "Reposted", "Skipped", "Failed"] + traceback: DF.Code | None + voucher_no: DF.DynamicLink + voucher_type: DF.Link # end: auto-generated types pass diff --git a/erpnext/patches.txt b/erpnext/patches.txt index 7be752b8694..e5258222013 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -494,3 +494,4 @@ erpnext.patches.v16_0.access_control_for_project_users erpnext.patches.v16_0.enable_book_stock_expense_gl_entries erpnext.patches.v16_0.rename_ar_ap_ageing_filter erpnext.patches.v16_0.fix_subcontracting_titles +erpnext.patches.v16_0.backfill_repost_accounting_ledger_status diff --git a/erpnext/patches/v16_0/backfill_repost_accounting_ledger_status.py b/erpnext/patches/v16_0/backfill_repost_accounting_ledger_status.py new file mode 100644 index 00000000000..f5a240a506c --- /dev/null +++ b/erpnext/patches/v16_0/backfill_repost_accounting_ledger_status.py @@ -0,0 +1,25 @@ +import frappe +from frappe.query_builder.functions import Coalesce + + +def execute(): + """Backfill the statuses of documents reposted before those fields existed. + + Without it they show up as drafts and are offered a `Start Reposting` button that would + repost vouchers which are already reposted. + """ + ral = frappe.qb.DocType("Repost Accounting Ledger") + items = frappe.qb.DocType("Repost Accounting Ledger Items") + + reposted = ( + frappe.qb.from_(ral).select(ral.name).where((ral.docstatus == 1) & (Coalesce(ral.status, "") == "")) + ) + frappe.qb.update(items).set(items.status, "Reposted").where(items.parent.isin(reposted)).run() + + for docstatus, status in ((1, "Completed"), (2, "Cancelled")): + ( + frappe.qb.update(ral) + .set(ral.status, status) + .where((ral.docstatus == docstatus) & (Coalesce(ral.status, "") == "")) + .run() + )