Merge pull request #57299 from nabinhait/feat/warn-existing-draft-links

feat: warn when a draft linked document already exists

erpnext/controllers/draft_links.py

@@ -0,0 +1,56 @@
+from collections.abc import Iterator
+
+import frappe
+from frappe.model.meta import Meta
+
+
+class DraftLinkFinder:
+	"""Finds draft documents of a target DocType that link back to a source document
+	through parent-level or child-table Link / Dynamic Link fields."""
+
+	def __init__(self, source_doctype: str, source_name: str, target_doctype: str) -> None:
+		self.source_doctype = source_doctype
+		self.source_name = source_name
+		self.target_doctype = target_doctype
+
+	def find(self) -> list[str]:
+		if not frappe.db.exists("DocType", self.target_doctype):
+			return []
+		if not frappe.has_permission(self.target_doctype):
+			return []
+
+		names: set[str] = set()
+		for filters in self._link_filters():
+			names.update(frappe.get_list(self.target_doctype, filters=filters, pluck="name", limit=0))
+		return sorted(names)
+
+	def _link_filters(self) -> Iterator[list]:
+		target_meta = frappe.get_meta(self.target_doctype)
+		for meta in [target_meta, *self._child_metas(target_meta)]:
+			yield from self._link_field_filters(meta)
+			yield from self._dynamic_link_field_filters(meta)
+
+	def _child_metas(self, target_meta: Meta) -> list[Meta]:
+		return [frappe.get_meta(df.options) for df in target_meta.get_table_fields()]
+
+	def _link_field_filters(self, meta: Meta) -> Iterator[list]:
+		for field in meta.get_link_fields():
+			if field.options == self.source_doctype:
+				yield [self._draft_filter(), [meta.name, field.fieldname, "=", self.source_name]]
+
+	def _dynamic_link_field_filters(self, meta: Meta) -> Iterator[list]:
+		for field in meta.get_dynamic_link_fields():
+			yield [
+				self._draft_filter(),
+				[meta.name, field.options, "=", self.source_doctype],
+				[meta.name, field.fieldname, "=", self.source_name],
+			]
+
+	def _draft_filter(self) -> list:
+		return [self.target_doctype, "docstatus", "=", 0]
+
+
+@frappe.whitelist()
+def get_existing_drafts(source_doctype: str, source_name: str, target_doctype: str) -> list[str]:
+	"""Draft documents of *target_doctype* created from the given source document."""
+	return DraftLinkFinder(source_doctype, source_name, target_doctype).find()

erpnext/controllers/tests/test_draft_links.py

@@ -0,0 +1,55 @@
+import frappe
+
+from erpnext.accounts.doctype.payment_entry.payment_entry import get_payment_entry
+from erpnext.accounts.doctype.purchase_invoice.test_purchase_invoice import make_purchase_invoice
+from erpnext.controllers.draft_links import get_existing_drafts
+from erpnext.selling.doctype.sales_order.mapper import make_delivery_note
+from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order
+from erpnext.stock.doctype.delivery_note.mapper import make_packing_slip
+from erpnext.tests.utils import ERPNextTestSuite
+
+
+class TestDraftLinks(ERPNextTestSuite):
+	def test_finds_draft_via_child_table_link(self):
+		so = make_sales_order()
+		dn = make_delivery_note(so.name)
+		dn.insert()
+
+		self.assertIn(dn.name, get_existing_drafts("Sales Order", so.name, "Delivery Note"))
+
+		frappe.db.set_value("Delivery Note", dn.name, "docstatus", 1)
+		self.assertNotIn(dn.name, get_existing_drafts("Sales Order", so.name, "Delivery Note"))
+
+	def test_finds_draft_via_dynamic_link(self):
+		pi = make_purchase_invoice()
+		pe = get_payment_entry("Purchase Invoice", pi.name)
+		pe.insert()
+
+		self.assertIn(pe.name, get_existing_drafts("Purchase Invoice", pi.name, "Payment Entry"))
+
+	def test_finds_draft_via_parent_link(self):
+		so = make_sales_order()
+		dn = make_delivery_note(so.name)
+		dn.insert()
+		packing_slip = make_packing_slip(dn.name)
+		packing_slip.insert()
+
+		self.assertIn(packing_slip.name, get_existing_drafts("Delivery Note", dn.name, "Packing Slip"))
+
+	def test_nonexistent_target_doctype_does_not_raise_for_non_admin(self):
+		# guards check ordering: for non-Administrator users has_permission()
+		# raises DoesNotExistError on unknown doctypes, so existence must be
+		# checked first (Administrator short-circuits and would not catch this)
+		with self.set_user("test@example.com"):
+			drafts = get_existing_drafts("Sales Order", "SO-0001", "Inter Company Purchase Order")
+		self.assertEqual(drafts, [])
+
+	def test_requires_permission_on_target_doctype(self):
+		so = make_sales_order()
+		dn = make_delivery_note(so.name)
+		dn.insert()
+
+		# test1@example.com has no roles, so no read permission on Delivery Note
+		with self.set_user("test1@example.com"):
+			drafts = get_existing_drafts("Sales Order", so.name, "Delivery Note")
+		self.assertEqual(drafts, [])

erpnext/public/js/controllers/transaction.js

@@ -2864,9 +2864,17 @@ erpnext.TransactionController = class TransactionController extends erpnext.taxe
 		}
 	}
 
-	make_mapped_payment_entry(args) {
+	async make_mapped_payment_entry(args) {
 		var me = this;
 		args = args || { dt: this.frm.doc.doctype, dn: this.frm.doc.name };
+		// get_method_for_payment bypasses open_mapped_doc, so run the draft guard explicitly
+		let via_journal_entry = this.frm.doc.__onload && this.frm.doc.__onload.make_payment_via_journal_entry;
+		if (
+			!via_journal_entry &&
+			!(await erpnext.utils.confirm_if_drafts_exist(this.frm.doc, "Payment Entry"))
+		) {
+			return;
+		}
 		return frappe.call({
 			method: me.get_method_for_payment(),
 			args: args,

erpnext/public/js/erpnext.bundle.js

@@ -4,6 +4,7 @@ import "./stock_reservation";
 import "./queries";
 import "./sms_manager";
 import "./utils/party";
+import "./utils/draft_link_guard";
 import "./controllers/stock_controller";
 import "./utils/serial_no_batch_selector";
 import "./utils/serial_batch_inline_editor";

erpnext/public/js/utils/draft_link_guard.js

@@ -0,0 +1,57 @@
+frappe.provide("erpnext.utils");
+
+// Warns before creating a follow-up document (e.g. Delivery Note from Sales Order)
+// when a draft of the target DocType already exists for the same source document.
+
+erpnext.utils.confirm_if_drafts_exist = async function (source_doc, target_doctype) {
+	// resolves true to proceed; fails open so a broken check never blocks creation
+	if (!source_doc || !source_doc.name || source_doc.__islocal) {
+		return true;
+	}
+
+	let drafts;
+	try {
+		drafts = await frappe.xcall("erpnext.controllers.draft_links.get_existing_drafts", {
+			source_doctype: source_doc.doctype,
+			source_name: source_doc.name,
+			target_doctype: target_doctype,
+		});
+	} catch (e) {
+		console.error(e);
+		return true;
+	}
+
+	if (!drafts.length) {
+		return true;
+	}
+
+	return new Promise((resolve) => {
+		frappe.confirm(
+			get_draft_warning(source_doc, target_doctype, drafts),
+			() => resolve(true),
+			() => resolve(false)
+		);
+	});
+};
+
+if (frappe.model.add_mapped_doc_guard) {
+	frappe.model.add_mapped_doc_guard((mapped_doc, opts) =>
+		erpnext.utils.confirm_if_drafts_exist(opts.frm && opts.frm.doc, mapped_doc.doctype)
+	);
+}
+
+function get_draft_warning(source_doc, target_doctype, drafts) {
+	const links = drafts.map((name) => frappe.utils.get_form_link(target_doctype, name, true)).join(", ");
+
+	if (drafts.length === 1) {
+		return __("A draft {0} already exists for this {1}: {2}. Do you still want to create a new one?", [
+			__(target_doctype),
+			__(source_doc.doctype),
+			links,
+		]);
+	}
+	return __(
+		"{0} draft {1} documents already exist for this {2}: {3}. Do you still want to create a new one?",
+		[drafts.length, __(target_doctype), __(source_doc.doctype), links]
+	);
+}