diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js index 195b29e38c6..cb7f9d6af76 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.js @@ -237,10 +237,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(); }); } @@ -291,15 +289,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(); }); @@ -338,10 +337,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 8e9da9baa9d..058b8c8613b 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" @@ -1662,7 +1663,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 f92252df2a7..405193e1c86 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 @@ -299,6 +299,9 @@ class PurchaseInvoice(BuyingController): self.reset_default_field_value("set_from_warehouse", "items", "from_warehouse") 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 @@ -310,6 +313,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")) @@ -1855,14 +1865,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_tax_withholding(self): self.set("advance_tax", []) @@ -2083,6 +2117,7 @@ def make_stock_entry(source_name, target_doc=None): @frappe.whitelist() +<<<<<<< HEAD def change_release_date(name, release_date=None): if frappe.db.exists("Purchase Invoice", name): pi = frappe.get_doc("Purchase Invoice", name) @@ -2105,6 +2140,8 @@ def block_invoice(name, release_date, hold_comment=None): @frappe.whitelist() +======= +>>>>>>> c8125b8b5a (refactor(purchase_invoice): expose invoice hold actions as document methods) 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 5aa2faed1a1..ae9b8442c34 100644 --- a/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py @@ -287,14 +287,166 @@ class TestPurchaseInvoice(FrappeTestCase, 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",