From 7d351153bb4be0898ad4b8273e8562781370b9c4 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Mon, 20 Jul 2026 18:23:18 +0530 Subject: [PATCH 1/3] feat: warn when a draft linked document already exists When creating a follow-up document (SO->DN, PO->PR, PI->Payment Entry, etc.), warn the user if a draft of the target doctype already linked to the source document exists, with links to the drafts and the option to proceed anyway. The target doctype comes from the make_mapped_doc response via the new frappe.model.add_mapped_doc_guard hook, so every open_mapped_doc flow is covered without per-doctype code or method-name inference. The server lookup walks parent-level and child-table Link / Dynamic Link fields of the target doctype and queries through frappe.get_list, so role and user permissions apply and docstatus filtering happens in the query itself. Payment Entry creation bypasses open_mapped_doc, so its controller runs the same guard explicitly. --- erpnext/controllers/draft_links.py | 56 ++++++++++++++++++ erpnext/controllers/tests/test_draft_links.py | 51 +++++++++++++++++ erpnext/public/js/controllers/transaction.js | 10 +++- erpnext/public/js/erpnext.bundle.js | 1 + erpnext/public/js/utils/draft_link_guard.js | 57 +++++++++++++++++++ 5 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 erpnext/controllers/draft_links.py create mode 100644 erpnext/controllers/tests/test_draft_links.py create mode 100644 erpnext/public/js/utils/draft_link_guard.js diff --git a/erpnext/controllers/draft_links.py b/erpnext/controllers/draft_links.py new file mode 100644 index 00000000000..020045e8852 --- /dev/null +++ b/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")) + 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() diff --git a/erpnext/controllers/tests/test_draft_links.py b/erpnext/controllers/tests/test_draft_links.py new file mode 100644 index 00000000000..4c4c3ea3a22 --- /dev/null +++ b/erpnext/controllers/tests/test_draft_links.py @@ -0,0 +1,51 @@ +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_returns_empty_for_non_admin(self): + 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() + + with self.set_user("test@example.com"): + drafts = get_existing_drafts("Sales Order", so.name, "Delivery Note") + self.assertEqual(drafts, []) diff --git a/erpnext/public/js/controllers/transaction.js b/erpnext/public/js/controllers/transaction.js index 21146de9fc8..79311c9724b 100644 --- a/erpnext/public/js/controllers/transaction.js +++ b/erpnext/public/js/controllers/transaction.js @@ -2880,9 +2880,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, diff --git a/erpnext/public/js/erpnext.bundle.js b/erpnext/public/js/erpnext.bundle.js index ec579c459da..b40d4da2e9c 100644 --- a/erpnext/public/js/erpnext.bundle.js +++ b/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 "./payment/payments"; diff --git a/erpnext/public/js/utils/draft_link_guard.js b/erpnext/public/js/utils/draft_link_guard.js new file mode 100644 index 00000000000..d4984890703 --- /dev/null +++ b/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] + ); +} From c522a1fdaeba1e582fcae2a934e6e588700a63c7 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Wed, 22 Jul 2026 12:06:00 +0530 Subject: [PATCH 2/3] fix: disable pagination in draft link lookup frappe.get_list defaults to 20 rows; child-table joins can produce duplicate parent names that fill the window and hide further drafts. Also clarify the check-ordering regression test. --- erpnext/controllers/draft_links.py | 2 +- erpnext/controllers/tests/test_draft_links.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/erpnext/controllers/draft_links.py b/erpnext/controllers/draft_links.py index 020045e8852..a4af1370ddf 100644 --- a/erpnext/controllers/draft_links.py +++ b/erpnext/controllers/draft_links.py @@ -21,7 +21,7 @@ class DraftLinkFinder: names: set[str] = set() for filters in self._link_filters(): - names.update(frappe.get_list(self.target_doctype, filters=filters, pluck="name")) + names.update(frappe.get_list(self.target_doctype, filters=filters, pluck="name", limit=0)) return sorted(names) def _link_filters(self) -> Iterator[list]: diff --git a/erpnext/controllers/tests/test_draft_links.py b/erpnext/controllers/tests/test_draft_links.py index 4c4c3ea3a22..303c96e5a38 100644 --- a/erpnext/controllers/tests/test_draft_links.py +++ b/erpnext/controllers/tests/test_draft_links.py @@ -36,7 +36,10 @@ class TestDraftLinks(ERPNextTestSuite): self.assertIn(packing_slip.name, get_existing_drafts("Delivery Note", dn.name, "Packing Slip")) - def test_nonexistent_target_doctype_returns_empty_for_non_admin(self): + 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, []) From 7e0c81391b295c6109036e09e4c65cb05e87d16d Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Thu, 23 Jul 2026 10:27:02 +0530 Subject: [PATCH 3/3] test: use a role-less user for the permission check test@example.com carries System Manager in the frappe fixtures, so on a fresh CI site it can read Delivery Note and the lookup legitimately returns the draft. test1@example.com has no roles, making the no-permission assertion environment-independent. --- erpnext/controllers/tests/test_draft_links.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/erpnext/controllers/tests/test_draft_links.py b/erpnext/controllers/tests/test_draft_links.py index 303c96e5a38..a776a21f7f4 100644 --- a/erpnext/controllers/tests/test_draft_links.py +++ b/erpnext/controllers/tests/test_draft_links.py @@ -49,6 +49,7 @@ class TestDraftLinks(ERPNextTestSuite): dn = make_delivery_note(so.name) dn.insert() - with self.set_user("test@example.com"): + # 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, [])