From 33d3696385b914a6d2a853db48091a0bf9522cfd Mon Sep 17 00:00:00 2001 From: pandiyan Date: Wed, 29 Jul 2026 06:12:17 +0530 Subject: [PATCH 1/4] refactor: reuse shared date range validation across reports --- .../tax_withholding_details.py | 5 ++--- .../tds_computation_summary.py | 4 ++-- erpnext/accounts/report/utils.py | 14 ++++++++++++++ .../batch_wise_balance_history.py | 4 ++-- .../cogs_by_item_group/cogs_by_item_group.py | 4 ++-- 5 files changed, 22 insertions(+), 9 deletions(-) diff --git a/erpnext/accounts/report/tax_withholding_details/tax_withholding_details.py b/erpnext/accounts/report/tax_withholding_details/tax_withholding_details.py index 99ac592097b7..72f80b62dd1c 100644 --- a/erpnext/accounts/report/tax_withholding_details/tax_withholding_details.py +++ b/erpnext/accounts/report/tax_withholding_details/tax_withholding_details.py @@ -7,6 +7,7 @@ from frappe.utils import flt, getdate from pypika import Tuple +from erpnext.accounts.report.utils import validate_mandatory_date_range from erpnext.accounts.utils import get_currency_precision @@ -33,9 +34,7 @@ def execute(filters=None): def validate_filters(filters): """Validate if dates are properly set""" - filters = frappe._dict(filters or {}) - if filters.from_date > filters.to_date: - frappe.throw(_("From Date must be before To Date")) + validate_mandatory_date_range(filters or {}) def get_result(filters, tds_accounts, tax_category_map, net_total_map): diff --git a/erpnext/accounts/report/tds_computation_summary/tds_computation_summary.py b/erpnext/accounts/report/tds_computation_summary/tds_computation_summary.py index a1b9c22f63f7..5e7c5d46a988 100644 --- a/erpnext/accounts/report/tds_computation_summary/tds_computation_summary.py +++ b/erpnext/accounts/report/tds_computation_summary/tds_computation_summary.py @@ -5,6 +5,7 @@ get_result, get_tds_docs, ) +from erpnext.accounts.report.utils import validate_mandatory_date_range from erpnext.accounts.utils import get_fiscal_year @@ -33,8 +34,7 @@ def execute(filters=None): def validate_filters(filters): """Validate if dates are properly set and lie in the same fiscal year""" - if filters.from_date > filters.to_date: - frappe.throw(_("From Date must be before To Date")) + validate_mandatory_date_range(filters) from_year = get_fiscal_year(filters.from_date)[0] to_year = get_fiscal_year(filters.to_date)[0] diff --git a/erpnext/accounts/report/utils.py b/erpnext/accounts/report/utils.py index 8d1730ab294e..3661e787f41f 100644 --- a/erpnext/accounts/report/utils.py +++ b/erpnext/accounts/report/utils.py @@ -1,4 +1,5 @@ import frappe +from frappe import _ from frappe.query_builder.custom import ConstantColumn from frappe.query_builder.functions import Sum from frappe.utils import flt, formatdate, get_datetime_str, get_table_name @@ -16,6 +17,19 @@ __exchange_rates = {} +def validate_mandatory_date_range(filters, from_field="from_date", to_field="to_date"): + from_date = filters.get(from_field) + to_date = filters.get(to_field) + + if not from_date or not to_date: + frappe.throw( + _("{0} and {1} are mandatory").format(frappe.bold(_("From Date")), frappe.bold(_("To Date"))) + ) + + if from_date > to_date: + frappe.throw(_("From Date must be before To Date")) + + def get_currency(filters): """ Returns a dictionary containing currency information. The keys of the dict are diff --git a/erpnext/stock/report/batch_wise_balance_history/batch_wise_balance_history.py b/erpnext/stock/report/batch_wise_balance_history/batch_wise_balance_history.py index e5cb69ff8162..a50061de1e3e 100644 --- a/erpnext/stock/report/batch_wise_balance_history/batch_wise_balance_history.py +++ b/erpnext/stock/report/batch_wise_balance_history/batch_wise_balance_history.py @@ -8,6 +8,7 @@ from frappe.utils.deprecations import deprecated from pypika import functions as fn +from erpnext.accounts.report.utils import validate_mandatory_date_range from erpnext.stock.doctype.warehouse.warehouse import apply_warehouse_filter SLE_COUNT_LIMIT = 100_000 @@ -29,8 +30,7 @@ def execute(filters=None): _("Please select either the Item or Warehouse or Warehouse Type filter to generate the report.") ) - if filters.from_date > filters.to_date: - frappe.throw(_("From Date must be before To Date")) + validate_mandatory_date_range(filters) float_precision = cint(frappe.db.get_default("float_precision")) or 3 diff --git a/erpnext/stock/report/cogs_by_item_group/cogs_by_item_group.py b/erpnext/stock/report/cogs_by_item_group/cogs_by_item_group.py index 000aca9f43e0..afae69c6ce01 100644 --- a/erpnext/stock/report/cogs_by_item_group/cogs_by_item_group.py +++ b/erpnext/stock/report/cogs_by_item_group/cogs_by_item_group.py @@ -9,6 +9,7 @@ from frappe.utils import date_diff from erpnext.accounts.report.general_ledger.general_ledger import get_gl_entries +from erpnext.accounts.report.utils import validate_mandatory_date_range Filters = frappe._dict Row = frappe._dict @@ -34,8 +35,7 @@ def update_filters_with_account(filters: Filters) -> None: def validate_filters(filters: Filters) -> None: - if filters.from_date > filters.to_date: - frappe.throw(_("From Date must be before To Date")) + validate_mandatory_date_range(filters) def get_columns() -> Columns: From 4f07e2503b4fa7895c0d7d8c78cf9436769049bc Mon Sep 17 00:00:00 2001 From: Krishna Shirsath Date: Tue, 21 Jul 2026 12:30:30 +0530 Subject: [PATCH 2/4] fix(italy): skip e-invoicing for opening invoices (cherry picked from commit f328018bfbad51ee4f23f925424c4c6bc29f9876) --- erpnext/regional/italy/utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/erpnext/regional/italy/utils.py b/erpnext/regional/italy/utils.py index d996ff9b2acb..8e2166448572 100644 --- a/erpnext/regional/italy/utils.py +++ b/erpnext/regional/italy/utils.py @@ -238,7 +238,7 @@ def get_invoice_summary(items, taxes): # Preflight for successful e-invoice export. def sales_invoice_validate(doc): # Validate company - if doc.doctype != "Sales Invoice": + if doc.doctype != "Sales Invoice" or doc.is_opening == "Yes": return if not doc.company_address: @@ -322,7 +322,7 @@ def sales_invoice_validate(doc): # Ensure payment details are valid for e-invoice. def sales_invoice_on_submit(doc, method): # Validate payment details - if get_company_country(doc.company) not in [ + if doc.is_opening == "Yes" or get_company_country(doc.company) not in [ "Italy", "Italia", "Italian Republic", @@ -388,7 +388,7 @@ def generate_single_invoice(docname): # Delete e-invoice attachment on cancel. def sales_invoice_on_cancel(doc, method): - if get_company_country(doc.company) not in [ + if doc.is_opening == "Yes" or get_company_country(doc.company) not in [ "Italy", "Italia", "Italian Republic", From 967955a926601f5224438388bd879f6db0328fbb Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 15:23:30 +0530 Subject: [PATCH 3/4] refactor(accounts): repost accounting ledger (backport #56442) (#57584) Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Diptanil Saha --- .../repost_accounting_ledger.js | 63 ++- .../repost_accounting_ledger.json | 59 ++- .../repost_accounting_ledger.py | 358 +++++++++++--- .../repost_accounting_ledger_list.js | 16 + .../test_repost_accounting_ledger.py | 467 ++++++++++++++---- .../repost_accounting_ledger_items.json | 46 +- .../repost_accounting_ledger_items.py | 6 +- erpnext/patches.txt | 1 + ...ackfill_repost_accounting_ledger_status.py | 25 + 9 files changed, 842 insertions(+), 199 deletions(-) create mode 100644 erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger_list.js create mode 100644 erpnext/patches/v16_0/backfill_repost_accounting_ledger_status.py 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 c304c7f17eb8..3ca9518a1e88 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 0d74b2150f7f..50adc5101e1a 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", @@ -80,4 +131,4 @@ "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 f11aeda4383c..ebf79077cc84 100644 --- a/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.py +++ b/erpnext/accounts/doctype/repost_accounting_ledger/repost_accounting_ledger.py @@ -7,7 +7,14 @@ 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 + +# a batch has to finish well within the timeout of the job reposting it +MAX_VOUCHERS_PER_REPOST = 50 + +HANDLED_VOUCHER_STATUSES = ("Reposted", "Skipped") from erpnext.stock import get_warehouse_account_map @@ -28,6 +35,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 +49,11 @@ def __init__(self, *args, **kwargs): 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 +90,52 @@ def validate_for_closed_fiscal_year(self): 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 +200,245 @@ def generate_preview(self): 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") + + +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.flags.in_test, + queue="long", + timeout=1500, + job_id=_repost_job_id(repost_doc_name), + deduplicate=True, + enqueue_after_commit=True, + now=frappe.flags.in_test, + ) -@frappe.whitelist() -def start_repost(account_repost_doc: str | None = None) -> None: - from erpnext.accounts.general_ledger import make_reverse_gl_entries + +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) + + _repost_vouchers(doc, repost_doc.delete_cancelled_entries) + except Exception: + frappe.db.rollback(save_point=save_point) + + x.db_set({"status": "Failed", "traceback": frappe.get_traceback()}) + else: + x.db_set({"status": "Reposted", "traceback": ""}) + finally: + if commit: + frappe.db.commit() # nosemgrep + + except Exception: + if commit: + frappe.db.rollback() + + _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 + + +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() + - 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) - - 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() - - elif doc.doctype == "Purchase Receipt": - if not repost_doc.delete_cancelled_entries: - doc.docstatus = 2 - doc.make_gl_entries_on_cancel(from_repost=True) - - doc.docstatus = 1 - doc.make_gl_entries(from_repost=True) - - 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 _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 000000000000..0ecdca3843cc --- /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 7ed999831cdc..49554914ee1f 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,20 +1,35 @@ # 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.tests.utils import FrappeTestCase, change_settings 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.test.accounts_mixin import AccountsTestMixin 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 +REPOST_MODULE = "erpnext.accounts.doctype.repost_accounting_ledger.repost_accounting_ledger" +SIMULATED_FAILURE = "Simulated repost failure" + class TestRepostAccountingLedger(AccountsTestMixin, FrappeTestCase): def setUp(self): @@ -26,8 +41,8 @@ def setUp(self): def tearDown(self): frappe.db.rollback() - def test_01_basic_functions(self): - si = create_sales_invoice( + def make_invoice(self, **kwargs): + return create_sales_invoice( item=self.item, company=self.company, customer=self.customer, @@ -35,8 +50,71 @@ def test_01_basic_functions(self): parent_cost_center=self.cost_center, cost_center=self.cost_center, 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 = self.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=self.company) + pcv = frappe.get_doc( + { + "doctype": "Period Closing Voucher", + "transaction_date": today(), + "period_start_date": fy[1], + "period_end_date": today(), + "company": self.company, + "fiscal_year": fy[0], + "cost_center": self.cost_center, + "closing_account_head": self.retained_earnings, + "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, @@ -70,51 +148,24 @@ def test_01_basic_functions(self): gle = frappe.db.get_all("GL Entry", filters={"voucher_no": si.name, "account": self.debit_to}) 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=self.item, - company=self.company, - customer=self.customer, - debit_to=self.debit_to, - parent_cost_center=self.cost_center, - cost_center=self.cost_center, - 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 = self.deferred_revenue 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 = self.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]) @change_settings("Accounts Settings", {"delete_linked_ledger_entries": 1}) def test_04_pcv_validation(self): @@ -122,86 +173,29 @@ def test_04_pcv_validation(self): gl = frappe.qb.DocType("GL Entry") qb.from_(gl).delete().where(gl.company == self.company).run() - si = create_sales_invoice( - item=self.item, - company=self.company, - customer=self.customer, - debit_to=self.debit_to, - parent_cost_center=self.cost_center, - cost_center=self.cost_center, - rate=100, - ) - fy = get_fiscal_year(today(), company=self.company) - pcv = frappe.get_doc( - { - "doctype": "Period Closing Voucher", - "transaction_date": today(), - "period_start_date": fy[1], - "period_end_date": today(), - "company": self.company, - "fiscal_year": fy[0], - "cost_center": self.cost_center, - "closing_account_head": self.retained_earnings, - "remarks": "test", - } - ) - pcv.save().submit() + si = self.make_invoice() + pcv = self.make_period_closing_voucher() - ral = frappe.new_doc("Repost Accounting Ledger") - ral.company = self.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=self.item, - company=self.company, - customer=self.customer, - debit_to=self.debit_to, - parent_cost_center=self.cost_center, - cost_center=self.cost_center, - 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 = self.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=self.item, - company=self.company, - customer=self.customer, - debit_to=self.debit_to, - parent_cost_center=self.cost_center, - cost_center=self.cost_center, - 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 = self.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})) @@ -247,11 +241,7 @@ def test_06_repost_purchase_receipt(self): another_provisional_account, ) - repost_doc = frappe.new_doc("Repost Accounting Ledger") - repost_doc.company = self.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 = [ @@ -272,6 +262,279 @@ def test_06_repost_purchase_receipt(self): 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") + + # the `discard` flow (and the `on_discard` hook it triggers) only exists on v16, + # so there is nothing to assert here on v15 + + 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}) + ) + + @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 == self.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 4a2041f88c6b..d3329dde45bc 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,28 +8,63 @@ "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": "2023-07-04 14:15:51.165584", + "modified": "2026-07-29 02:41:00.000000", "modified_by": "Administrator", "module": "Accounts", "name": "Repost Accounting Ledger Items", @@ -37,4 +73,4 @@ "sort_field": "modified", "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 6e02e3a6b98d..a895e218e4d5 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 02103c8071a4..022b33cac10e 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -445,3 +445,4 @@ erpnext.patches.v16_0.backfill_pick_list_transferred_qty erpnext.patches.v16_0.access_control_for_project_users erpnext.patches.v16_0.rename_ar_ap_ageing_filter erpnext.patches.v15_0.fix_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 000000000000..f5a240a506c0 --- /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() + ) From ade3f791a40322c68cfdcacaa79b4b4350d51764 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 30 Jul 2026 09:25:52 +0530 Subject: [PATCH 4/4] fix(stock): keep manufactured item rate at zero when inputs are free (backport #57334) (#57512) fix(stock): keep manufactured item rate at zero when inputs are free (#57334) * fix(stock): keep manufactured item rate at zero when inputs are free when a finished item is produced from raw materials consumed at zero valuation, the incoming rate fell back to the item's own valuation rate (or BOM cost), valuing free inputs as output and inflating the fg value on every production run. add has_consumption_basis() to detect when the consumed cost is known even if it is zero (consumed rows present, or a consumption entry exists for the work order). when it is, skip the get_valuation_rate and BOM-cost fallbacks so a real cost of zero is preserved. * test(stock): cover manufacture rate for zero-valued raw materials - manufacture from a free input keeps fg basic_rate and sle incoming_rate/stock_value_difference at zero even when the fg already carries a valuation in the target warehouse - material consumption on with no consumption entry does not fall back to bom/price-list rate for free inputs - zero-valued consumption entry keeps the manufacture entry's fg rate at zero (cherry picked from commit 73224d3650f5b6df7c9380dfd294ad85b8469000) Co-authored-by: Sudharsanan Ashok <135326972+Sudharsanan11@users.noreply.github.com> --- .../stock/doctype/stock_entry/stock_entry.py | 54 +++++-- .../doctype/stock_entry/test_stock_entry.py | 143 ++++++++++++++++++ 2 files changed, 185 insertions(+), 12 deletions(-) diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py index d601dc093b82..f93c41bacc5a 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/stock_entry.py @@ -1321,6 +1321,7 @@ def set_basic_rate(self, reset_outgoing_rate=True, raise_error_if_no_rate=True): # Set rate for outgoing items outgoing_items_cost = self.set_rate_for_outgoing_items(reset_outgoing_rate, raise_error_if_no_rate) finished_item_qty = sum(d.transfer_qty for d in self.items if d.is_finished_item) + has_consumption_basis = self.has_consumption_basis() items = [] # Set basic rate for incoming items @@ -1328,6 +1329,8 @@ def set_basic_rate(self, reset_outgoing_rate=True, raise_error_if_no_rate=True): if d.s_warehouse or d.set_basic_rate_manually: continue + rate_derived_from_consumption = False + if d.allow_zero_valuation_rate: d.basic_rate = 0.0 items.append(d.item_code) @@ -1335,12 +1338,17 @@ def set_basic_rate(self, reset_outgoing_rate=True, raise_error_if_no_rate=True): elif d.is_finished_item: if self.purpose == "Manufacture": d.basic_rate = self.get_basic_rate_for_manufactured_item( - finished_item_qty, outgoing_items_cost + finished_item_qty, outgoing_items_cost, has_consumption_basis ) + rate_derived_from_consumption = has_consumption_basis elif self.purpose == "Repack": d.basic_rate = self.get_basic_rate_for_repacked_items(d.transfer_qty, outgoing_items_cost) + # Repack rate comes from consumed source-warehouse rows, not consumption entries + rate_derived_from_consumption = any(item.s_warehouse for item in self.get("items")) - if not d.basic_rate and not d.allow_zero_valuation_rate: + # A rate of zero derived from the consumed items is their actual cost, not a missing + # rate. Falling back to the item's valuation here would value free inputs as output. + if not d.basic_rate and not d.allow_zero_valuation_rate and not rate_derived_from_consumption: if self.is_new(): raise_error_if_no_rate = False @@ -1375,6 +1383,31 @@ def set_basic_rate(self, reset_outgoing_rate=True, raise_error_if_no_rate=True): frappe.msgprint(message, alert=True) + def has_consumption_basis(self) -> bool: + """Whether the cost of the consumed items is known, even when that cost is zero.""" + if any(d.s_warehouse for d in self.get("items")): + return True + + settings = frappe.get_single("Manufacturing Settings") + if settings.material_consumption and settings.get_rm_cost_from_consumption_entry and self.work_order: + return bool(self.get_consumption_entries()) + + return False + + def get_consumption_entries(self) -> list[str]: + # Cached: queried in both has_consumption_basis() and get_basic_rate_for_manufactured_item() + if getattr(self, "_consumption_entries", None) is None: + self._consumption_entries = frappe.get_all( + "Stock Entry", + filters={ + "docstatus": 1, + "work_order": self.work_order, + "purpose": "Material Consumption for Manufacture", + }, + pluck="name", + ) + return self._consumption_entries + def set_rate_for_outgoing_items(self, reset_outgoing_rate=True, raise_error_if_no_rate=True): outgoing_items_cost = 0.0 for d in self.get("items"): @@ -1428,21 +1461,16 @@ def get_basic_rate_for_repacked_items(self, finished_item_qty, outgoing_items_co ) return flt(outgoing_items_cost / total_fg_qty) - def get_basic_rate_for_manufactured_item(self, finished_item_qty, outgoing_items_cost=0) -> float: + def get_basic_rate_for_manufactured_item( + self, finished_item_qty, outgoing_items_cost=0, has_consumption_basis=False + ) -> float: settings = frappe.get_single("Manufacturing Settings") scrap_items_cost = sum([flt(d.basic_amount) for d in self.get("items") if d.is_scrap_item]) if settings.material_consumption: if settings.get_rm_cost_from_consumption_entry and self.work_order: # Validate only if Material Consumption Entry exists for the Work Order. - if frappe.db.exists( - "Stock Entry", - { - "docstatus": 1, - "work_order": self.work_order, - "purpose": "Material Consumption for Manufacture", - }, - ): + if self.get_consumption_entries(): for item in self.items: if not item.is_finished_item and not item.is_scrap_item: label = frappe.get_meta(settings.doctype).get_label( @@ -1489,7 +1517,9 @@ def get_basic_rate_for_manufactured_item(self, finished_item_qty, outgoing_items ) ).run()[0][0] or 0 - elif not outgoing_items_cost: + # Estimate from the BOM only when nothing was consumed. A consumed cost of zero is a + # real cost, so substituting BOM rates would value free inputs as output. + elif not outgoing_items_cost and not has_consumption_basis: bom_items = self.get_bom_raw_materials(finished_item_qty) outgoing_items_cost = sum([flt(row.qty) * flt(row.rate) for row in bom_items.values()]) diff --git a/erpnext/stock/doctype/stock_entry/test_stock_entry.py b/erpnext/stock/doctype/stock_entry/test_stock_entry.py index b344b772cbb7..bcaa90e104f5 100644 --- a/erpnext/stock/doctype/stock_entry/test_stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/test_stock_entry.py @@ -2574,6 +2574,149 @@ def test_transferred_qty_in_material_transfer(self): material_request.reload() self.assertEqual(material_request.transfer_status, "Completed") + def test_manufacture_with_zero_valued_raw_material(self): + # A finished good produced from free inputs is worth nothing. Falling back to the item's + # own valuation would create value out of nothing and inflate it on every production run. + fg_item = make_item(properties={"is_stock_item": 1}).name + rm_item = make_item(properties={"is_stock_item": 1}).name + warehouse = "_Test Warehouse - _TC" + fg_warehouse = "Finished Goods - _TC" + + rm_receipt = make_stock_entry(item_code=rm_item, target=warehouse, qty=100, rate=0, do_not_save=True) + rm_receipt.items[0].allow_zero_valuation_rate = 1 + rm_receipt.save() + rm_receipt.submit() + + # the finished good already carries a valuation in the target warehouse + make_stock_entry(item_code=fg_item, target=fg_warehouse, qty=10, rate=100) + + se = frappe.new_doc("Stock Entry") + se.purpose = se.stock_entry_type = "Manufacture" + se.company = "_Test Company" + se.append( + "items", + {"item_code": rm_item, "s_warehouse": warehouse, "qty": 10, "conversion_factor": 1}, + ) + se.append( + "items", + { + "item_code": fg_item, + "t_warehouse": fg_warehouse, + "qty": 10, + "is_finished_item": 1, + "conversion_factor": 1, + }, + ) + se.save() + + self.assertEqual(se.items[0].basic_amount, 0) + self.assertEqual(se.items[1].basic_rate, 0) + self.assertEqual(se.items[1].basic_amount, 0) + + se.submit() + + fg_sle = frappe.db.get_value( + "Stock Ledger Entry", + {"voucher_no": se.name, "item_code": fg_item, "is_cancelled": 0}, + ["incoming_rate", "stock_value_difference"], + as_dict=True, + ) + + self.assertEqual(fg_sle.incoming_rate, 0) + self.assertEqual(fg_sle.stock_value_difference, 0) + + def _make_wo_for_free_raw_material(self, rm_item, fg_item, bom_no): + from erpnext.manufacturing.doctype.work_order.test_work_order import make_wo_order_test_record + from erpnext.manufacturing.doctype.work_order.work_order import ( + make_stock_entry as make_stock_entry_from_wo, + ) + + receipt = make_stock_entry(item_code=rm_item, target="Stores - _TC", qty=10, rate=0, do_not_save=True) + receipt.items[0].allow_zero_valuation_rate = 1 + receipt.save() + receipt.submit() + + wo = make_wo_order_test_record(production_item=fg_item, bom_no=bom_no, qty=10) + + transfer = frappe.get_doc(make_stock_entry_from_wo(wo.name, "Material Transfer for Manufacture", 10)) + transfer.items[0].s_warehouse = "Stores - _TC" + transfer.insert().submit() + + return wo + + @change_settings( + "Manufacturing Settings", {"material_consumption": 1, "get_rm_cost_from_consumption_entry": 0} + ) + def test_manufacture_does_not_fall_back_to_bom_cost_for_free_raw_material(self): + # The BOM is only an estimate for when nothing was consumed. Items that were consumed and + # cost nothing are a real cost, so a BOM rate must not stand in for them. + from erpnext.manufacturing.doctype.production_plan.test_production_plan import make_bom + from erpnext.manufacturing.doctype.work_order.work_order import ( + make_stock_entry as make_stock_entry_from_wo, + ) + + rm_item = make_item(properties={"is_stock_item": 1}).name + fg_item = make_item(properties={"is_stock_item": 1}).name + + frappe.get_doc( + { + "doctype": "Item Price", + "item_code": rm_item, + "price_list": "_Test Price List India", + "price_list_rate": 150, + "buying": 1, + } + ).insert() + + # price the BOM off the price list so that it carries a rate the free stock does not + bom = make_bom(item=fg_item, raw_materials=[rm_item], do_not_save=True) + bom.rm_cost_as_per = "Price List" + bom.buying_price_list = "_Test Price List India" + bom.currency = "INR" + bom.save() + bom.submit() + + wo = self._make_wo_for_free_raw_material(rm_item, fg_item, bom.name) + + manufacture = frappe.get_doc(make_stock_entry_from_wo(wo.name, "Manufacture", 10)) + manufacture.save() + + fg_row = next(d for d in manufacture.items if d.is_finished_item) + self.assertEqual(fg_row.basic_rate, 0) + self.assertEqual(fg_row.basic_amount, 0) + + @change_settings( + "Manufacturing Settings", {"material_consumption": 1, "get_rm_cost_from_consumption_entry": 1} + ) + def test_manufacture_with_zero_valued_consumption_entry(self): + # The raw material is consumed by a separate entry, so the Manufacture entry carries no + # consumed rows of its own. Its cost is still known, and it is zero. + from erpnext.manufacturing.doctype.production_plan.test_production_plan import make_bom + from erpnext.manufacturing.doctype.work_order.work_order import ( + make_stock_entry as make_stock_entry_from_wo, + ) + + rm_item = make_item(properties={"is_stock_item": 1}).name + fg_item = make_item(properties={"is_stock_item": 1}).name + + # the finished good already carries a valuation in the work order's target warehouse + make_stock_entry(item_code=fg_item, target="_Test Warehouse 1 - _TC", qty=10, rate=100) + + bom = make_bom(item=fg_item, raw_materials=[rm_item]).name + wo = self._make_wo_for_free_raw_material(rm_item, fg_item, bom) + + consumption = frappe.get_doc( + make_stock_entry_from_wo(wo.name, "Material Consumption for Manufacture", 10) + ) + consumption.insert().submit() + + manufacture = frappe.get_doc(make_stock_entry_from_wo(wo.name, "Manufacture", 10)) + manufacture.save() + + fg_row = next(d for d in manufacture.items if d.is_finished_item) + self.assertEqual(fg_row.basic_rate, 0) + self.assertEqual(fg_row.basic_amount, 0) + def test_disassemble_entry_without_wo(self): from erpnext.manufacturing.doctype.production_plan.test_production_plan import make_bom