diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js index 9fd911a2762..9986dc89053 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 f4766ef7413..2d1a4bc596b 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.json +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.json @@ -360,6 +360,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" @@ -1694,7 +1695,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 fb4836026d6..62df2632262 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py @@ -5,7 +5,7 @@ import frappe from frappe import _, throw from frappe.model.document import Document -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 @@ -306,6 +306,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 @@ -317,6 +320,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")) @@ -820,14 +830,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(): @@ -925,24 +959,3 @@ def get_list_context(context=None): @erpnext.allow_regional def make_regional_gl_entries(gl_entries, doc): return gl_entries - - -@frappe.whitelist() -def change_release_date(name: str, release_date: str | None = None): - pi = frappe.get_lazy_doc("Purchase Invoice", name) - pi.check_permission() - pi.db_set("release_date", release_date) - - -@frappe.whitelist() -def unblock_invoice(name: str): - if frappe.db.exists("Purchase Invoice", name): - pi = frappe.get_lazy_doc("Purchase Invoice", name) - pi.unblock_invoice() - - -@frappe.whitelist() -def block_invoice(name: str, release_date: str, hold_comment: str | None = None): - if frappe.db.exists("Purchase Invoice", name): - pi = frappe.get_lazy_doc("Purchase Invoice", name) - pi.block_invoice(hold_comment, release_date) diff --git a/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py index e60d3f4614c..dc80f2d5ef8 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",