From c8125b8b5a305aad22e49f7849204dd3759d1962 Mon Sep 17 00:00:00 2001 From: diptanilsaha Date: Thu, 6 Aug 2026 07:41:09 +0530 Subject: [PATCH 1/3] refactor(purchase_invoice): expose invoice hold actions as document methods --- .../purchase_invoice/purchase_invoice.js | 26 ++- .../purchase_invoice/purchase_invoice.json | 3 +- .../purchase_invoice/purchase_invoice.py | 70 ++++---- .../purchase_invoice/test_purchase_invoice.py | 154 +++++++++++++++++- 4 files changed, 208 insertions(+), 45 deletions(-) diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js index 68dc8d26d1b..7936cee69b1 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js @@ -240,10 +240,8 @@ erpnext.accounts.PurchaseInvoice = class PurchaseInvoice extends erpnext.buying. unblock_invoice() { const me = this; - frappe.call({ - method: "erpnext.accounts.doctype.purchase_invoice.purchase_invoice.unblock_invoice", - args: { name: me.frm.doc.name }, - callback: (r) => me.frm.reload_doc(), + me.frm.call("unblock_invoice", null, () => { + me.frm.reload_doc(); }); } @@ -294,15 +292,16 @@ erpnext.accounts.PurchaseInvoice = class PurchaseInvoice extends erpnext.buying. this.dialog.set_primary_action(__("Save"), function () { const dialog_data = me.dialog.get_values(); - frappe.call({ - method: "erpnext.accounts.doctype.purchase_invoice.purchase_invoice.block_invoice", - args: { - name: me.frm.doc.name, + me.frm.call( + "block_invoice", + { hold_comment: dialog_data.hold_comment, release_date: dialog_data.release_date, }, - callback: (r) => me.frm.reload_doc(), - }); + () => { + me.frm.reload_doc(); + } + ); me.dialog.hide(); }); @@ -341,10 +340,9 @@ erpnext.accounts.PurchaseInvoice = class PurchaseInvoice extends erpnext.buying. } set_release_date(data) { - return frappe.call({ - method: "erpnext.accounts.doctype.purchase_invoice.purchase_invoice.change_release_date", - args: data, - callback: (r) => this.frm.reload_doc(), + const me = this; + return me.frm.call("change_release_date", { release_date: data.release_date }, () => { + me.frm.reload_doc(); }); } diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.json b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.json index 9a2bcd715ab..d2c45a8681f 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.json +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.json @@ -352,6 +352,7 @@ { "collapsible": 1, "collapsible_depends_on": "eval:doc.on_hold", + "depends_on": "eval:doc.on_hold", "fieldname": "sb_14", "fieldtype": "Section Break", "label": "Hold Invoice" @@ -1702,7 +1703,7 @@ "idx": 204, "is_submittable": 1, "links": [], - "modified": "2026-07-12 23:54:21.263951", + "modified": "2026-08-05 15:40:16.519774", "modified_by": "Administrator", "module": "Accounts", "name": "Purchase Invoice", diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py index 8d91408a002..82fed84eafb 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py @@ -8,7 +8,7 @@ import frappe from frappe import _, qb, throw from frappe.model.mapper import get_mapped_doc from frappe.query_builder.functions import Sum -from frappe.utils import cint, cstr, flt, formatdate, get_link_to_form, getdate, nowdate +from frappe.utils import DateTimeLikeObject, cint, cstr, flt, formatdate, get_link_to_form, getdate, nowdate import erpnext from erpnext.accounts.deferred_revenue import validate_service_stop_date @@ -309,6 +309,9 @@ class PurchaseInvoice(BuyingController): PurchaseTaxWithholding(self).on_validate() self.set_percentage_received() + if self.on_hold: + self.validate_invoice_hold() + def set_percentage_received(self): total_billed_qty = 0.0 total_received_qty = 0.0 @@ -320,6 +323,13 @@ class PurchaseInvoice(BuyingController): if total_billed_qty and total_received_qty: self.per_received = total_received_qty / total_billed_qty * 100 + def validate_invoice_hold(self): + if self.is_return: + frappe.throw(_("Return Purchase Invoice cannot be held.")) + + if self.docstatus < 1: + frappe.throw(_("Purchase Invoice can be held after submitting.")) + def validate_release_date(self): if self.release_date and getdate(nowdate()) >= getdate(self.release_date): frappe.throw(_("Release date must be in the future")) @@ -1901,14 +1911,38 @@ class PurchaseInvoice(BuyingController): def on_recurring(self, reference_doc, auto_repeat_doc): self.due_date = None - def block_invoice(self, hold_comment=None, release_date=None): - self.db_set("on_hold", 1) - self.db_set("hold_comment", cstr(hold_comment)) + @frappe.whitelist(methods=["POST"]) + def block_invoice(self, hold_comment: str | None = None, release_date: DateTimeLikeObject | None = None): + self.check_permission("write") + self.on_hold = 1 + self.release_date = release_date + self.validate_block_invoice() + + self.db_set({"on_hold": 1, "hold_comment": cstr(hold_comment), "release_date": release_date}) + + @frappe.whitelist(methods=["POST"]) + def unblock_invoice(self): + self.check_permission("write") + self.db_set({"on_hold": 0, "release_date": None}) + + @frappe.whitelist(methods=["POST"]) + def change_release_date(self, release_date: DateTimeLikeObject | None = None): + self.check_permission("write") + + if not self.on_hold: + frappe.throw(_("Invoice is not blocked. Block the invoice to change the release date.")) + + self.release_date = release_date + self.validate_block_invoice() + self.db_set("release_date", release_date) - def unblock_invoice(self): - self.db_set("on_hold", 0) - self.db_set("release_date", None) + def validate_block_invoice(self): + self.validate_invoice_hold() + if self.outstanding_amount <= 0: + frappe.throw(_("Purchase Invoice without any outstanding amount cannot be held.")) + + self.validate_release_date() def set_status(self, update=False, status=None, update_modified=True): if self.is_new(): @@ -2033,28 +2067,6 @@ def make_stock_entry(source_name, target_doc=None): return doc -@frappe.whitelist() -def change_release_date(name, release_date=None): - if frappe.db.exists("Purchase Invoice", name): - pi = frappe.get_lazy_doc("Purchase Invoice", name) - pi.check_permission() - pi.db_set("release_date", release_date) - - -@frappe.whitelist() -def unblock_invoice(name): - if frappe.db.exists("Purchase Invoice", name): - pi = frappe.get_lazy_doc("Purchase Invoice", name) - pi.unblock_invoice() - - -@frappe.whitelist() -def block_invoice(name, release_date, hold_comment=None): - if frappe.db.exists("Purchase Invoice", name): - pi = frappe.get_lazy_doc("Purchase Invoice", name) - pi.block_invoice(hold_comment, release_date) - - @frappe.whitelist() def make_inter_company_sales_invoice(source_name, target_doc=None): from erpnext.accounts.doctype.sales_invoice.sales_invoice import make_inter_company_transaction diff --git a/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py index f0952bf3307..8d9e7309366 100644 --- a/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py @@ -278,14 +278,166 @@ class TestPurchaseInvoice(ERPNextTestSuite, StockTestMixin): def test_purchase_invoice_explicit_block(self): pi = make_purchase_invoice() - pi.block_invoice() + release_date = add_days(nowdate(), 10) + + pi.block_invoice(hold_comment="Waiting for the goods", release_date=release_date) self.assertEqual(pi.on_hold, 1) + on_hold, hold_comment, saved_release_date = frappe.db.get_value( + "Purchase Invoice", pi.name, ["on_hold", "hold_comment", "release_date"] + ) + self.assertEqual(on_hold, 1) + self.assertEqual(hold_comment, "Waiting for the goods") + self.assertEqual(getdate(saved_release_date), getdate(release_date)) + pi.unblock_invoice() self.assertEqual(pi.on_hold, 0) + on_hold, saved_release_date = frappe.db.get_value( + "Purchase Invoice", pi.name, ["on_hold", "release_date"] + ) + self.assertEqual(on_hold, 0) + self.assertIsNone(saved_release_date) + + def test_purchase_invoice_cannot_be_held_before_submission(self): + pi = make_purchase_invoice(do_not_save=True) + pi.on_hold = 1 + + self.assertRaises(frappe.ValidationError, pi.save) + + pi.on_hold = 0 + pi.save() + pi.submit() + + pi.block_invoice() + self.assertEqual(frappe.db.get_value("Purchase Invoice", pi.name, "on_hold"), 1) + + def test_return_purchase_invoice_cannot_be_held(self): + from erpnext.controllers.sales_and_purchase_return import make_return_doc + + pi = make_purchase_invoice() + + return_pi = make_return_doc(pi.doctype, pi.name) + return_pi.on_hold = 1 + self.assertRaisesRegex(frappe.ValidationError, "cannot be held", return_pi.save) + + return_pi.on_hold = 0 + return_pi.save() + return_pi.submit() + + self.assertRaisesRegex(frappe.ValidationError, "cannot be held", return_pi.block_invoice) + + def test_return_purchase_invoice_is_not_affected_by_hold_validations(self): + from erpnext.controllers.sales_and_purchase_return import make_return_doc + + pi = make_purchase_invoice() + + # a return has a negative outstanding amount, which must not be mistaken + # for an invalid hold on a document that was never held + return_pi = make_return_doc(pi.doctype, pi.name) + return_pi.save() + return_pi.submit() + + self.assertEqual(return_pi.docstatus, 1) + self.assertEqual(return_pi.on_hold, 0) + self.assertLess(return_pi.outstanding_amount, 0) + + def test_settled_purchase_invoice_cannot_be_held(self): + pi = make_purchase_invoice() + + pe = get_payment_entry("Purchase Invoice", dn=pi.name, bank_account="_Test Bank - _TC") + pe.reference_no = "1" + pe.reference_date = nowdate() + pe.save() + pe.submit() + + pi.reload() + self.assertEqual(pi.outstanding_amount, 0) + + self.assertRaises(frappe.ValidationError, pi.block_invoice) + self.assertEqual(frappe.db.get_value("Purchase Invoice", pi.name, "on_hold"), 0) + + def test_release_date_of_held_invoice_must_be_in_future(self): + pi = make_purchase_invoice() + + self.assertRaises(frappe.ValidationError, pi.block_invoice, "Hold", add_days(nowdate(), -1)) + self.assertRaises(frappe.ValidationError, pi.block_invoice, "Hold", nowdate()) + + def test_rejected_hold_does_not_partially_update_invoice(self): + pi = make_purchase_invoice() + + self.assertRaises(frappe.ValidationError, pi.block_invoice, "Hold", add_days(nowdate(), -1)) + + pi.reload() + self.assertEqual(pi.on_hold, 0) + self.assertIsNone(pi.release_date) + + def test_change_release_date_of_held_invoice(self): + pi = make_purchase_invoice() + pi.block_invoice(hold_comment="Hold", release_date=add_days(nowdate(), 10)) + + new_release_date = add_days(nowdate(), 20) + pi.change_release_date(new_release_date) + + self.assertEqual( + getdate(frappe.db.get_value("Purchase Invoice", pi.name, "release_date")), + getdate(new_release_date), + ) + + self.assertRaises(frappe.ValidationError, pi.change_release_date, add_days(nowdate(), -1)) + + def test_release_date_cannot_be_changed_on_an_invoice_that_is_not_held(self): + pi = make_purchase_invoice() + + self.assertRaisesRegex( + frappe.ValidationError, + "Invoice is not blocked", + pi.change_release_date, + add_days(nowdate(), 10), + ) + + self.assertIsNone(frappe.db.get_value("Purchase Invoice", pi.name, "release_date")) + + def test_hold_methods_are_whitelisted_document_methods(self): + import erpnext.accounts.doctype.purchase_invoice.purchase_invoice as purchase_invoice_module + + pi = frappe.new_doc("Purchase Invoice") + + for method in ("block_invoice", "unblock_invoice", "change_release_date"): + # raises if the method is not whitelisted for client side calls + pi.is_whitelisted(method) + + self.assertFalse( + hasattr(purchase_invoice_module, method), + f"{method} should only be exposed as a document method", + ) + + def test_hold_methods_require_write_permission(self): + pi = make_purchase_invoice() + user = "test_pi_hold_permission@example.com" + + if not frappe.db.exists("User", user): + frappe.get_doc( + { + "doctype": "User", + "email": user, + "first_name": "Test PI Hold", + "roles": [{"role": "Employee"}], + } + ).insert(ignore_permissions=True) + + frappe.set_user(user) + try: + self.assertRaises(frappe.PermissionError, pi.block_invoice) + self.assertRaises(frappe.PermissionError, pi.unblock_invoice) + self.assertRaises(frappe.PermissionError, pi.change_release_date, add_days(nowdate(), 10)) + finally: + frappe.set_user("Administrator") + + self.assertEqual(frappe.db.get_value("Purchase Invoice", pi.name, "on_hold"), 0) + def test_gl_entries_with_perpetual_inventory_against_pr(self): pr = make_purchase_receipt( company="_Test Company with perpetual inventory", From 04718e15c954a082197cfbc7f294ac95fff9e65f Mon Sep 17 00:00:00 2001 From: diptanilsaha Date: Thu, 6 Aug 2026 16:37:57 +0530 Subject: [PATCH 2/3] fix(journal_entry): validate blocked purchase invoices --- .../accounts/doctype/journal_entry/journal_entry.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/erpnext/accounts/doctype/journal_entry/journal_entry.py b/erpnext/accounts/doctype/journal_entry/journal_entry.py index facb15f15f7..65db883c430 100644 --- a/erpnext/accounts/doctype/journal_entry/journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/journal_entry.py @@ -906,6 +906,18 @@ class JournalEntry(AccountsController): ) ) + if reference_type == "Purchase Invoice" and invoice.invoice_is_blocked(): + msg = ( + _("{0} {1} is blocked and on hold until {2}.").format( + invoice.doctype, invoice.name, invoice.release_date + ) + if invoice.release_date + else _("{0} {1} is blocked.").format( + invoice.doctype, invoice.name, invoice.release_date + ) + ) + frappe.throw(msg) + def set_against_account(self): accounts_debited, accounts_credited = [], [] if self.voucher_type in ("Deferred Revenue", "Deferred Expense"): From 0bb0f6d68994dabc88c245845ab628aa308d00cc Mon Sep 17 00:00:00 2001 From: diptanilsaha Date: Thu, 6 Aug 2026 12:31:00 +0530 Subject: [PATCH 3/3] test(journal_entry): added test cases for blocked purchase invoices --- .../journal_entry/test_journal_entry.py | 65 ++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/journal_entry/test_journal_entry.py b/erpnext/accounts/doctype/journal_entry/test_journal_entry.py index 581a0866721..51922757d05 100644 --- a/erpnext/accounts/doctype/journal_entry/test_journal_entry.py +++ b/erpnext/accounts/doctype/journal_entry/test_journal_entry.py @@ -2,7 +2,7 @@ # License: GNU General Public License v3. See license.txt import frappe -from frappe.utils import flt, nowdate +from frappe.utils import add_days, flt, nowdate from erpnext.accounts.doctype.account.test_account import get_inventory_account from erpnext.accounts.doctype.journal_entry.journal_entry import StockAccountInvalidTransaction @@ -609,6 +609,69 @@ class TestJournalEntry(ERPNextTestSuite): jv.save() self.assertRaises(frappe.ValidationError, jv.submit) + def make_jv_against_purchase_invoice(self, invoice, amount=100): + jv = make_journal_entry("Creditors - _TC", "_Test Cash - _TC", amount, save=False) + jv.accounts[0].party_type = "Supplier" + jv.accounts[0].party = invoice.supplier + jv.accounts[0].reference_type = "Purchase Invoice" + jv.accounts[0].reference_name = invoice.name + return jv + + def test_jv_against_purchase_invoice_respects_hold_state(self): + """Payment can be booked against a Purchase Invoice only while it is not on hold.""" + from erpnext.accounts.doctype.purchase_invoice.test_purchase_invoice import make_purchase_invoice + + release_date = add_days(nowdate(), 10) + + def never_held(): + return make_purchase_invoice() + + def held_until_a_future_date(): + invoice = make_purchase_invoice() + invoice.block_invoice(hold_comment="Waiting for the goods", release_date=release_date) + return invoice + + def held_without_a_release_date(): + invoice = make_purchase_invoice() + invoice.block_invoice(hold_comment="Under dispute") + return invoice + + def held_until_a_date_that_has_passed(): + invoice = held_until_a_future_date() + frappe.db.set_value("Purchase Invoice", invoice.name, "release_date", add_days(nowdate(), -1)) + return invoice + + def unblocked_again(): + invoice = held_until_a_future_date() + invoice.unblock_invoice() + return invoice + + for build_invoice in (held_until_a_future_date, held_without_a_release_date): + with self.subTest(build_invoice.__name__): + jv = self.make_jv_against_purchase_invoice(build_invoice()) + self.assertRaisesRegex(frappe.ValidationError, "is blocked", jv.insert) + + for build_invoice in (never_held, held_until_a_date_that_has_passed, unblocked_again): + with self.subTest(build_invoice.__name__): + invoice = build_invoice() + jv = self.make_jv_against_purchase_invoice(invoice) + jv.insert() + self.assertEqual(jv.reference_types[invoice.name], "Purchase Invoice") + + def test_jv_against_blocked_sales_invoice_reference_is_not_checked(self): + """A Sales Invoice has no hold state, so the check must skip it rather than fail.""" + from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice + + invoice = create_sales_invoice(rate=500) + jv = make_journal_entry("_Test Cash - _TC", "Debtors - _TC", 100, save=False) + jv.accounts[1].party_type = "Customer" + jv.accounts[1].party = "_Test Customer" + jv.accounts[1].reference_type = "Sales Invoice" + jv.accounts[1].reference_name = invoice.name + jv.insert() + + self.assertEqual(jv.reference_types[invoice.name], "Sales Invoice") + def make_journal_entry( account1,