diff --git a/erpnext/accounts/doctype/pos_invoice/pos_invoice.py b/erpnext/accounts/doctype/pos_invoice/pos_invoice.py index f556275b2f5..b10f9686789 100644 --- a/erpnext/accounts/doctype/pos_invoice/pos_invoice.py +++ b/erpnext/accounts/doctype/pos_invoice/pos_invoice.py @@ -21,6 +21,7 @@ from erpnext.accounts.doctype.sales_invoice.services.loyalty import LoyaltyServi from erpnext.accounts.party import get_due_date, get_party_account from erpnext.controllers.queries import item_query as _item_query from erpnext.controllers.sales_and_purchase_return import get_sales_invoice_item_from_consolidated_invoice +from erpnext.selling.doctype.product_bundle.product_bundle import get_active_product_bundle from erpnext.stock.doctype.serial_no.serial_no import get_serial_nos from erpnext.stock.stock_ledger import is_negative_stock_allowed @@ -403,7 +404,7 @@ class POSInvoice(SalesInvoice): for d in self.get("items"): if not d.serial_and_batch_bundle: - if frappe.db.exists("Product Bundle", d.item_code): + if get_active_product_bundle(d.item_code): ( availability, is_stock_item, @@ -916,7 +917,7 @@ def get_stock_availability(item_code: str | None, warehouse: str): return bin_qty - pos_sales_qty, is_stock_item, is_negative_stock_allowed(item_code=item_code) else: is_stock_item = True - if frappe.db.exists("Product Bundle", {"name": item_code, "disabled": 0}): + if get_active_product_bundle(item_code): return get_bundle_availability(item_code, warehouse), is_stock_item, False else: is_stock_item = False @@ -926,7 +927,7 @@ def get_stock_availability(item_code: str | None, warehouse: str): def get_product_bundle_stock_availability(item_code, warehouse, item_qty): is_stock_item = True - bundle = frappe.get_doc("Product Bundle", item_code) + bundle = frappe.get_doc("Product Bundle", get_active_product_bundle(item_code)) availabilities = [] for bundle_item in bundle.items: if frappe.get_value("Item", bundle_item.item_code, "is_stock_item"): @@ -945,7 +946,7 @@ def get_product_bundle_stock_availability(item_code, warehouse, item_qty): def get_bundle_availability(bundle_item_code, warehouse): - product_bundle = frappe.get_doc("Product Bundle", bundle_item_code) + product_bundle = frappe.get_doc("Product Bundle", get_active_product_bundle(bundle_item_code)) bundle_bin_qty = 1000000 for item in product_bundle.items: diff --git a/erpnext/accounts/doctype/purchase_invoice_item/purchase_invoice_item.json b/erpnext/accounts/doctype/purchase_invoice_item/purchase_invoice_item.json index d094dfe82ff..c0bbdf37682 100644 --- a/erpnext/accounts/doctype/purchase_invoice_item/purchase_invoice_item.json +++ b/erpnext/accounts/doctype/purchase_invoice_item/purchase_invoice_item.json @@ -886,10 +886,11 @@ "read_only": 1 }, { + "description": "Parent item of the Product Bundle this row was packed from", "fieldname": "product_bundle", "fieldtype": "Link", "label": "Product Bundle", - "options": "Product Bundle", + "options": "Item", "read_only": 1 }, { diff --git a/erpnext/buying/doctype/purchase_order_item/purchase_order_item.json b/erpnext/buying/doctype/purchase_order_item/purchase_order_item.json index 8db422fee7c..87873acd6b8 100644 --- a/erpnext/buying/doctype/purchase_order_item/purchase_order_item.json +++ b/erpnext/buying/doctype/purchase_order_item/purchase_order_item.json @@ -845,10 +845,11 @@ "read_only": 1 }, { + "description": "Parent item of the Product Bundle this row was packed from", "fieldname": "product_bundle", "fieldtype": "Link", "label": "Product Bundle", - "options": "Product Bundle", + "options": "Item", "read_only": 1 }, { diff --git a/erpnext/controllers/selling_controller.py b/erpnext/controllers/selling_controller.py index 33f79f2d067..561bcb0a59e 100644 --- a/erpnext/controllers/selling_controller.py +++ b/erpnext/controllers/selling_controller.py @@ -425,7 +425,7 @@ class SellingController(StockController): row.new_item_code for row in frappe.get_all( "Product Bundle", - filters={"new_item_code": ("in", items_to_fetch), "disabled": 0}, + filters={"new_item_code": ("in", items_to_fetch), "is_active": 1, "docstatus": 1}, fields="new_item_code", ) } diff --git a/erpnext/patches.txt b/erpnext/patches.txt index 9aa01b7c630..9e56a6777e9 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -485,3 +485,4 @@ erpnext.patches.v16_0.set_default_letter_head_for_doctype_and_report erpnext.patches.v16_0.clear_procedures_from_receivable_report erpnext.patches.v16_0.migrate_address_contact_custom_fields erpnext.patches.v16_0.rename_secondary_item_type_field +erpnext.patches.v16_0.submit_existing_product_bundles diff --git a/erpnext/patches/v16_0/submit_existing_product_bundles.py b/erpnext/patches/v16_0/submit_existing_product_bundles.py new file mode 100644 index 00000000000..da7a1205d0d --- /dev/null +++ b/erpnext/patches/v16_0/submit_existing_product_bundles.py @@ -0,0 +1,85 @@ +"""Make existing Product Bundles submittable & versioned. + +Product Bundle became a submittable, versioned doctype (issue #29462). Pre-existing +bundles were drafts named after their parent item (``name == new_item_code``). This +patch migrates them to the new model: + +1. rename each legacy bundle to the versioned name ``PB--001`` +2. mark it submitted (``docstatus = 1``) +3. seed ``is_active`` from the legacy ``disabled`` flag (active = not disabled) + +No transaction stores a bundle's *name* (they snapshot components into their own +``packed_items`` tables and reference the parent item code), so renaming is +reference-safe. The patch is idempotent: already-migrated bundles (docstatus != 0 or +already prefixed) are skipped. +""" + +import frappe + +from erpnext.selling.doctype.product_bundle.product_bundle import NAME_PREFIX, build_bundle_name + + +def execute(): + legacy_bundles = frappe.get_all( + "Product Bundle", + filters={"docstatus": 0}, + fields=["name", "new_item_code", "disabled"], + order_by="creation asc", + ) + + for bundle in legacy_bundles: + # Submitted bundles are already migrated and excluded by the docstatus filter. + # A draft that still carries its legacy name needs renaming; a draft already + # named PB-* is the leftover of an interrupted run and only needs submitting. + target_name = bundle.name + + if not bundle.name.startswith(f"{NAME_PREFIX}-"): + new_name = build_bundle_name(bundle.new_item_code, _next_index(bundle.new_item_code)) + if not frappe.db.exists("Product Bundle", new_name): + frappe.rename_doc( + "Product Bundle", bundle.name, new_name, force=True, merge=False, show_alert=False + ) + target_name = new_name + + frappe.db.set_value( + "Product Bundle", + target_name, + {"docstatus": 1, "is_active": 0 if bundle.disabled else 1}, + update_modified=False, + ) + + _enforce_single_active_version() + + +def _next_index(item_code: str) -> int: + """Next free version index for a parent item among already-migrated bundles.""" + existing = frappe.get_all( + "Product Bundle", + filters={"new_item_code": item_code, "name": ("like", f"{NAME_PREFIX}-%")}, + pluck="name", + ) + from erpnext.selling.doctype.product_bundle.product_bundle import get_next_version_index + + return get_next_version_index(existing) + + +def _enforce_single_active_version(): + """Guarantee at most one active version per parent item. + + Under the old unique-name-per-item invariant duplicates can't exist, so this is a + safety net; if several are somehow active, keep the most recently created one. + """ + active = frappe.get_all( + "Product Bundle", + filters={"is_active": 1, "docstatus": 1}, + fields=["name", "new_item_code"], + order_by="new_item_code asc, creation desc", + ) + + seen = set() + for bundle in active: + if bundle.new_item_code in seen: + # a newer version for this item was already kept; deactivate the rest + frappe.db.set_value("Product Bundle", bundle.name, "is_active", 0, update_modified=False) + else: + seen.add(bundle.new_item_code) diff --git a/erpnext/selling/doctype/product_bundle/product_bundle.js b/erpnext/selling/doctype/product_bundle/product_bundle.js index 3096b692a7e..5b74f2d9f8f 100644 --- a/erpnext/selling/doctype/product_bundle/product_bundle.js +++ b/erpnext/selling/doctype/product_bundle/product_bundle.js @@ -9,5 +9,54 @@ frappe.ui.form.on("Product Bundle", { query: "erpnext.selling.doctype.product_bundle.product_bundle.get_new_item_code", }; }); + + // A submitted bundle is immutable. To change it, create a new version + // (a fresh draft copied from this one) and submit that instead. + if (frm.doc.docstatus === 1) { + frm.add_custom_button(__("Create New Version"), () => { + frappe.model.open_mapped_doc({ + method: "erpnext.selling.doctype.product_bundle.product_bundle.make_new_version", + frm: frm, + }); + }); + } + + show_supersede_hint(frm); + }, + + new_item_code: function (frm) { + show_supersede_hint(frm); }, }); + +function show_supersede_hint(frm) { + // Warn (non-blocking) when the chosen Parent Item already has an active bundle: + // submitting this draft will create a new version and deactivate that one. + frm.set_intro(""); + if (frm.doc.docstatus !== 0 || !frm.doc.new_item_code) { + return; + } + + frappe.db + .get_value( + "Product Bundle", + { + new_item_code: frm.doc.new_item_code, + is_active: 1, + docstatus: 1, + }, + "name" + ) + .then((r) => { + const active = r.message && r.message.name; + if (active && active !== frm.doc.name) { + frm.set_intro( + __( + "Item {0} already has an active Product Bundle ({1}). Submitting this will create a new version and deactivate {1}.", + [frm.doc.new_item_code, active] + ), + "orange" + ); + } + }); +} diff --git a/erpnext/selling/doctype/product_bundle/product_bundle.json b/erpnext/selling/doctype/product_bundle/product_bundle.json index 9cb95aefe05..aece3cd70ce 100644 --- a/erpnext/selling/doctype/product_bundle/product_bundle.json +++ b/erpnext/selling/doctype/product_bundle/product_bundle.json @@ -10,7 +10,9 @@ "new_item_code", "description", "column_break_eonk", + "is_active", "disabled", + "amended_from", "item_section", "items", "section_break_4", @@ -63,11 +65,33 @@ "fieldtype": "HTML", "options": "

About Product Bundle

\n\n

Aggregate group of Items into another Item. This is useful if you are bundling a certain Items into a package and you maintain stock of the packed Items and not the aggregate Item.

\n

The package Item will have Is Stock Item as No and Is Sales Item as Yes.

\n

Example:

\n

If you are selling Laptops and Backpacks separately and have a special price if the customer buys both, then the Laptop + Backpack will be a new Product Bundle Item.

" }, + { + "allow_on_submit": 1, + "default": "1", + "description": "Only one version of a Product Bundle can be active at a time for a given Parent Item. Activating a version deactivates the previously active one.", + "fieldname": "is_active", + "fieldtype": "Check", + "in_list_view": 1, + "label": "Is Active", + "no_copy": 1 + }, { "default": "0", + "depends_on": "disabled", + "description": "Deprecated: use Cancel / Is Active instead. Retained for backward compatibility.", "fieldname": "disabled", "fieldtype": "Check", - "label": "Disabled" + "label": "Disabled", + "read_only": 1 + }, + { + "fieldname": "amended_from", + "fieldtype": "Link", + "label": "Amended From", + "no_copy": 1, + "options": "Product Bundle", + "print_hide": 1, + "read_only": 1 }, { "fieldname": "column_break_eonk", @@ -76,14 +100,17 @@ ], "icon": "fa fa-sitemap", "idx": 1, + "is_submittable": 1, "links": [], - "modified": "2024-03-27 13:10:19.599302", + "modified": "2026-06-08 00:00:00.000000", "modified_by": "Administrator", "module": "Selling", "name": "Product Bundle", "owner": "Administrator", "permissions": [ { + "amend": 1, + "cancel": 1, "create": 1, "delete": 1, "email": 1, @@ -92,6 +119,7 @@ "report": 1, "role": "Stock Manager", "share": 1, + "submit": 1, "write": 1 }, { @@ -102,6 +130,8 @@ "role": "Stock User" }, { + "amend": 1, + "cancel": 1, "create": 1, "delete": 1, "email": 1, @@ -110,10 +140,11 @@ "report": 1, "role": "Sales User", "share": 1, + "submit": 1, "write": 1 } ], "sort_field": "creation", "sort_order": "ASC", "states": [] -} \ No newline at end of file +} diff --git a/erpnext/selling/doctype/product_bundle/product_bundle.py b/erpnext/selling/doctype/product_bundle/product_bundle.py index af2db4c1c54..993a9f64050 100644 --- a/erpnext/selling/doctype/product_bundle/product_bundle.py +++ b/erpnext/selling/doctype/product_bundle/product_bundle.py @@ -2,11 +2,15 @@ # License: GNU General Public License v3. See license.txt +import re + import frappe from frappe import _ from frappe.model.document import Document from frappe.query_builder import Criterion -from frappe.utils import get_link_to_form +from frappe.utils import cint, get_link_to_form + +NAME_PREFIX = "PB" class ProductBundle(Document): @@ -20,14 +24,28 @@ class ProductBundle(Document): from erpnext.selling.doctype.product_bundle_item.product_bundle_item import ProductBundleItem + amended_from: DF.Link | None description: DF.Data | None disabled: DF.Check + is_active: DF.Check items: DF.Table[ProductBundleItem] new_item_code: DF.Link # end: auto-generated types def autoname(self): - self.name = self.new_item_code + """BOM-style versioned name: ``PB--001``. + + Amended copies are excluded while computing the current index so that an + amendment naturally becomes the next version of the bundle. + """ + search_key = f"{NAME_PREFIX}-{self.new_item_code}-%" + existing = frappe.get_all( + "Product Bundle", + filters={"name": ("like", search_key), "amended_from": ["is", "not set"]}, + pluck="name", + ) + index = get_next_version_index(existing) + self.name = build_bundle_name(self.new_item_code, index) def validate(self): self.validate_main_item() @@ -37,6 +55,37 @@ class ProductBundle(Document): validate_uom_is_integer(self, "uom", "qty") + def on_submit(self): + self.make_active() + + def on_cancel(self): + self.db_set("is_active", 0) + + def on_update_after_submit(self): + # `is_active` is the only field editable after submit; keep a single active + # version per parent item in sync when the user (re)activates a version. + if self.is_active: + self.make_active() + + def make_active(self): + """Mark this version active and deactivate every other submitted version + of the same parent item.""" + if not self.is_active: + self.db_set("is_active", 1) + + others = frappe.get_all( + "Product Bundle", + filters={ + "new_item_code": self.new_item_code, + "is_active": 1, + "docstatus": 1, + "name": ("!=", self.name), + }, + pluck="name", + ) + for name in others: + frappe.db.set_value("Product Bundle", name, "is_active", 0) + def on_trash(self): linked_doctypes = [ "Delivery Note", @@ -82,7 +131,7 @@ class ProductBundle(Document): def validate_child_items(self): for item in self.items: - if frappe.db.exists("Product Bundle", {"name": item.item_code, "disabled": 0}): + if get_active_product_bundle(item.item_code): frappe.throw( _( "Row #{0}: Child Item should not be a Product Bundle. Please remove Item {1} and Save" @@ -99,11 +148,81 @@ class ProductBundle(Document): ) +def build_bundle_name(item_code: str, index: int) -> str: + """Build a ``PB--NNN`` name, truncating the item part to stay within 140 chars.""" + suffix = "%.3i" % index + name = f"{NAME_PREFIX}-{item_code}-{suffix}" + if len(name) <= 140: + return name + + truncated_length = 140 - (len(NAME_PREFIX) + len(suffix) + 2) + truncated_item = item_code[:truncated_length].rsplit(" ", 1)[0] + return f"{NAME_PREFIX}-{truncated_item}-{suffix}" + + +def get_next_version_index(existing_names: list[str]) -> int: + """Highest trailing version index across ``existing_names`` plus one (1 if none).""" + pattern = "|".join(re.escape(delim) for delim in ("/", "-")) + parts = [re.split(pattern, name) for name in existing_names] + valid = [p for p in parts if len(p) > 1 and p[-1]] + if not valid: + return 1 + return max(cint(p[-1]) for p in valid) + 1 + + +def get_active_product_bundle(item_code: str) -> str | None: + """Return the name of the active, submitted Product Bundle for ``item_code``, else None. + + This is the single resolution entry point for every consumer of bundles; it + replaces the legacy ``exists("Product Bundle", {name/new_item_code, disabled: 0})`` + lookups that assumed one mutable bundle per item. + """ + if not item_code: + return None + return frappe.db.get_value( + "Product Bundle", + {"new_item_code": item_code, "is_active": 1, "docstatus": 1}, + "name", + ) + + +@frappe.whitelist() +def make_new_version(source_name: str, target_doc: str | None = None): + """Create a fresh draft bundle copied from an existing (typically submitted) one. + + The copy keeps the same parent item and component rows but gets a new version + name on submit; it does not carry over docstatus or the active flag. + """ + from frappe.model.mapper import get_mapped_doc + + def post_process(source, target): + target.is_active = 1 + target.disabled = 0 + + return get_mapped_doc( + "Product Bundle", + source_name, + { + "Product Bundle": { + "doctype": "Product Bundle", + "field_map": {"new_item_code": "new_item_code"}, + "field_no_map": ["amended_from", "is_active", "disabled"], + }, + "Product Bundle Item": { + "doctype": "Product Bundle Item", + }, + }, + target_doc, + post_process, + ) + + @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs def get_new_item_code(doctype: str, txt: str, searchfield: str, start: int, page_len: int, filters: dict): - product_bundles = frappe.db.get_list("Product Bundle", {"disabled": 0}, pluck="name") - + # Items that already have a bundle are intentionally *not* excluded: creating a + # bundle for such an item produces a new version that supersedes the active one + # on submit (same as the "Create New Version" action). if not searchfield or searchfield == "name": searchfield = frappe.get_meta("Item").get("search_fields") @@ -122,7 +241,4 @@ def get_new_item_code(doctype: str, txt: str, searchfield: str, start: int, page if searchfield: query = query.where(Criterion.any([item[fieldname].like(f"%{txt}%") for fieldname in searchfield])) - if product_bundles: - query = query.where(item.name.notin(product_bundles)) - return query.run() diff --git a/erpnext/selling/doctype/product_bundle/test_product_bundle.py b/erpnext/selling/doctype/product_bundle/test_product_bundle.py index 4a5df94da54..97297e23f90 100644 --- a/erpnext/selling/doctype/product_bundle/test_product_bundle.py +++ b/erpnext/selling/doctype/product_bundle/test_product_bundle.py @@ -3,10 +3,22 @@ import frappe +from erpnext.selling.doctype.product_bundle.product_bundle import ( + get_active_product_bundle, + make_new_version, +) +from erpnext.stock.doctype.item.test_item import make_item +from erpnext.tests.utils import ERPNextTestSuite + def make_product_bundle(parent, items, qty=None): - if frappe.db.exists("Product Bundle", parent): - return frappe.get_doc("Product Bundle", parent) + """Create (and submit) an active Product Bundle for ``parent``. + + Product Bundle is now submittable & versioned, so the active version is resolved + by parent item code rather than by document name. + """ + if active := get_active_product_bundle(parent): + return frappe.get_doc("Product Bundle", active) product_bundle = frappe.get_doc({"doctype": "Product Bundle", "new_item_code": parent}) @@ -14,5 +26,87 @@ def make_product_bundle(parent, items, qty=None): product_bundle.append("items", {"item_code": item, "qty": qty or 1}) product_bundle.insert() + product_bundle.submit() return product_bundle + + +class TestProductBundle(ERPNextTestSuite): + def setUp(self): + self.parent = make_item("_Test PB Parent", {"is_stock_item": 0, "is_sales_item": 1}).name + make_item("_Test PB Child A", {"is_stock_item": 1}) + make_item("_Test PB Child B", {"is_stock_item": 1}) + + def test_submit_makes_bundle_active_and_versioned(self): + bundle = make_product_bundle(self.parent, ["_Test PB Child A"]) + self.assertEqual(bundle.docstatus, 1) + self.assertEqual(bundle.is_active, 1) + self.assertTrue(bundle.name.startswith("PB-")) + self.assertEqual(get_active_product_bundle(self.parent), bundle.name) + + def test_new_version_deactivates_previous(self): + v1 = make_product_bundle(self.parent, ["_Test PB Child A"]) + + v2 = make_new_version(v1.name) + v2.items[0].qty = 5 + v2.insert() + v2.submit() + + self.assertNotEqual(v1.name, v2.name) + self.assertEqual(get_active_product_bundle(self.parent), v2.name) + self.assertEqual(frappe.db.get_value("Product Bundle", v1.name, "is_active"), 0) + + def test_reactivating_old_version_deactivates_current(self): + v1 = make_product_bundle(self.parent, ["_Test PB Child A"]) + + v2 = make_new_version(v1.name) + v2.items[0].qty = 5 + v2.insert() + v2.submit() + self.assertEqual(get_active_product_bundle(self.parent), v2.name) + + # switch back to v1 by toggling is_active on the submitted doc (allow_on_submit) + v1.reload() + v1.is_active = 1 + v1.save() + + self.assertEqual(get_active_product_bundle(self.parent), v1.name) + self.assertEqual(frappe.db.get_value("Product Bundle", v2.name, "is_active"), 0) + + def test_new_bundle_from_scratch_supersedes_existing(self): + # An item that already has a bundle must remain selectable so a new version + # can be created straight from the New Product Bundle form. + from erpnext.selling.doctype.product_bundle.product_bundle import get_new_item_code + + v1 = make_product_bundle(self.parent, ["_Test PB Child A"]) + + picker = [row[0] for row in get_new_item_code("Item", self.parent, "name", 0, 20, {})] + self.assertIn(self.parent, picker) + + v2 = frappe.get_doc({"doctype": "Product Bundle", "new_item_code": self.parent}) + v2.append("items", {"item_code": "_Test PB Child B", "qty": 1}) + v2.insert() + v2.submit() + + self.assertNotEqual(v1.name, v2.name) + self.assertEqual(get_active_product_bundle(self.parent), v2.name) + self.assertEqual(frappe.db.get_value("Product Bundle", v1.name, "is_active"), 0) + + def test_cancel_clears_active(self): + bundle = make_product_bundle(self.parent, ["_Test PB Child A"]) + bundle.cancel() + self.assertEqual(frappe.db.get_value("Product Bundle", bundle.name, "is_active"), 0) + self.assertIsNone(get_active_product_bundle(self.parent)) + + def test_submitted_bundle_is_immutable(self): + bundle = make_product_bundle(self.parent, ["_Test PB Child A"]) + bundle.items[0].qty = 99 + self.assertRaises(frappe.exceptions.UpdateAfterSubmitError, bundle.save) + + def test_child_cannot_be_active_bundle(self): + make_product_bundle(self.parent, ["_Test PB Child A"]) + outer = make_item("_Test PB Outer", {"is_stock_item": 0, "is_sales_item": 1}).name + + doc = frappe.get_doc({"doctype": "Product Bundle", "new_item_code": outer}) + doc.append("items", {"item_code": self.parent, "qty": 1}) + self.assertRaises(frappe.ValidationError, doc.insert) diff --git a/erpnext/selling/doctype/sales_order/mapper.py b/erpnext/selling/doctype/sales_order/mapper.py index eb7201eb24d..a4d36e909c5 100644 --- a/erpnext/selling/doctype/sales_order/mapper.py +++ b/erpnext/selling/doctype/sales_order/mapper.py @@ -17,6 +17,7 @@ from erpnext.manufacturing.doctype.production_plan.production_plan import ( get_items_for_material_requests, get_sales_orders, ) +from erpnext.selling.doctype.product_bundle.product_bundle import get_active_product_bundle from erpnext.setup.doctype.item_group.item_group import get_item_group_defaults from erpnext.stock.doctype.item.item import get_item_defaults from erpnext.stock.doctype.packed_item.packed_item import is_product_bundle, make_packing_list @@ -71,8 +72,15 @@ def make_material_request(source_name: str, target_doc: str | Document | None = "Sales Order Item", {"name": so_item.parent_detail_docname}, ["delivered_qty"] ) - bundle_item_qty = frappe.db.get_value( - "Product Bundle Item", {"parent": so_item.parent_item, "item_code": so_item.item_code}, ["qty"] + bundle_name = get_active_product_bundle(so_item.parent_item) + bundle_item_qty = ( + frappe.db.get_value( + "Product Bundle Item", + {"parent": bundle_name, "item_code": so_item.item_code}, + ["qty"], + ) + if bundle_name + else None ) return flt( @@ -133,9 +141,7 @@ def make_material_request(source_name: str, target_doc: str | Document | None = "delivery_date": "schedule_date", "bom_no": "bom_no", }, - "condition": lambda item: not frappe.db.exists( - "Product Bundle", {"name": item.item_code, "disabled": 0} - ) + "condition": lambda item: not is_product_bundle(item.item_code) and get_remaining_qty(item) > 0, "postprocess": update_item, }, diff --git a/erpnext/selling/doctype/sales_order/sales_order.py b/erpnext/selling/doctype/sales_order/sales_order.py index f871907ce6b..437bbe1427d 100755 --- a/erpnext/selling/doctype/sales_order/sales_order.py +++ b/erpnext/selling/doctype/sales_order/sales_order.py @@ -326,9 +326,15 @@ class SalesOrder(SellingController): d.projected_qty = bin_data.get((d.item_code, d.warehouse), 0.0) def product_bundle_has_stock_item(self, product_bundle): - """Returns true if product bundle has stock item""" + """Returns true if the active bundle for `product_bundle` (a parent item code) has a stock item""" + from erpnext.selling.doctype.product_bundle.product_bundle import get_active_product_bundle + + bundle_name = get_active_product_bundle(product_bundle) + if not bundle_name: + return False + bundle_items = frappe.get_all( - "Product Bundle Item", filters={"parent": product_bundle}, pluck="item_code" + "Product Bundle Item", filters={"parent": bundle_name}, pluck="item_code" ) if not bundle_items: @@ -807,7 +813,9 @@ def get_work_order_items(sales_order: str, for_raw_material_request: int = 0): product_bundle_parents = [ pb.new_item_code for pb in frappe.get_all( - "Product Bundle", {"new_item_code": ["in", item_codes], "disabled": 0}, ["new_item_code"] + "Product Bundle", + {"new_item_code": ["in", item_codes], "is_active": 1, "docstatus": 1}, + ["new_item_code"], ) ] diff --git a/erpnext/selling/doctype/sales_order/test_sales_order.py b/erpnext/selling/doctype/sales_order/test_sales_order.py index 141eb48210c..9dc451a44e5 100644 --- a/erpnext/selling/doctype/sales_order/test_sales_order.py +++ b/erpnext/selling/doctype/sales_order/test_sales_order.py @@ -82,7 +82,7 @@ class TestSalesOrder(ERPNextTestSuite): product_bundle = make_product_bundle( "_Test Product Bundle Item", ["_Test Item", "_Test Item Home Desktop 100"] ) - so = make_sales_order(item_code=product_bundle.name, qty=2) + so = make_sales_order(item_code=product_bundle.new_item_code, qty=2) mr = make_material_request(so.name) mr.items[0].qty = 4 mr.items[1].qty = 2 diff --git a/erpnext/selling/report/available_stock_for_packing_items/available_stock_for_packing_items.py b/erpnext/selling/report/available_stock_for_packing_items/available_stock_for_packing_items.py index 5e763bb4364..b312abd4607 100644 --- a/erpnext/selling/report/available_stock_for_packing_items/available_stock_for_packing_items.py +++ b/erpnext/selling/report/available_stock_for_packing_items/available_stock_for_packing_items.py @@ -71,7 +71,8 @@ def get_item_warehouse_quantity_map(): FROM tabBin AS bi, (SELECT pb.new_item_code as parent, b.item_code, b.qty, w.name FROM `tabProduct Bundle Item` b, `tabWarehouse` w, `tabProduct Bundle` pb - where b.parent = pb.name) AS b + where b.parent = pb.name + and pb.is_active = 1 and pb.docstatus = 1) AS b WHERE bi.item_code = b.item_code AND bi.warehouse = b.name GROUP BY b.parent, b.item_code, bi.warehouse @@ -80,7 +81,8 @@ def get_item_warehouse_quantity_map(): FROM (SELECT pb.new_item_code as parent, b.item_code, b.qty, w.name FROM `tabProduct Bundle Item` b, `tabWarehouse` w, `tabProduct Bundle` pb - where b.parent = pb.name) AS b + where b.parent = pb.name + and pb.is_active = 1 and pb.docstatus = 1) AS b WHERE NOT EXISTS(SELECT * FROM `tabBin` AS bi WHERE bi.item_code = b.item_code diff --git a/erpnext/selling/report/pending_so_items_for_purchase_request/pending_so_items_for_purchase_request.py b/erpnext/selling/report/pending_so_items_for_purchase_request/pending_so_items_for_purchase_request.py index 7f9f3fb999e..82143a11eea 100644 --- a/erpnext/selling/report/pending_so_items_for_purchase_request/pending_so_items_for_purchase_request.py +++ b/erpnext/selling/report/pending_so_items_for_purchase_request/pending_so_items_for_purchase_request.py @@ -144,7 +144,9 @@ def get_data(): def get_items_with_product_bundle(item_list): bundled_items = frappe.get_all( - "Product Bundle", filters=[("new_item_code", "IN", item_list)], fields=["new_item_code"] + "Product Bundle", + filters=[("new_item_code", "IN", item_list), ("is_active", "=", 1), ("docstatus", "=", 1)], + fields=["new_item_code"], ) return [d.new_item_code for d in bundled_items] diff --git a/erpnext/stock/doctype/delivery_note/mapper.py b/erpnext/stock/doctype/delivery_note/mapper.py index 81fdf04f6fd..a77a94dcf88 100644 --- a/erpnext/stock/doctype/delivery_note/mapper.py +++ b/erpnext/stock/doctype/delivery_note/mapper.py @@ -15,6 +15,7 @@ from frappe.utils import flt from erpnext.accounts.party import CROSS_PARTY_FIELD_NO_MAP, get_due_date from erpnext.controllers.accounts_controller import get_taxes_and_charges, merge_taxes +from erpnext.stock.doctype.packed_item.packed_item import is_product_bundle def get_invoiced_qty_map(delivery_note: str) -> dict: @@ -287,8 +288,7 @@ def make_packing_slip(source_name: str, target_doc: str | Document | None = None }, "postprocess": update_item, "condition": lambda item: ( - not frappe.db.exists("Product Bundle", {"new_item_code": item.item_code, "disabled": 0}) - and flt(item.packed_qty) < flt(item.qty) + not is_product_bundle(item.item_code) and flt(item.packed_qty) < flt(item.qty) ), }, "Packed Item": { diff --git a/erpnext/stock/doctype/delivery_note/services/packing.py b/erpnext/stock/doctype/delivery_note/services/packing.py index f26ba2897e9..999174cd876 100644 --- a/erpnext/stock/doctype/delivery_note/services/packing.py +++ b/erpnext/stock/doctype/delivery_note/services/packing.py @@ -44,7 +44,7 @@ class PackingService: items_list = [item.item_code for item in self.doc.items] return frappe.db.get_all( "Product Bundle", - filters={"new_item_code": ["in", items_list], "disabled": 0}, + filters={"new_item_code": ["in", items_list], "is_active": 1, "docstatus": 1}, pluck="name", ) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index 01d8a9d1e1a..29e233a7bc8 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -712,8 +712,10 @@ class Item(Document): def validate_duplicate_product_bundles_before_merge(self, old_name, new_name): "Block merge if both old and new items have product bundles." - old_bundle = frappe.get_value("Product Bundle", filters={"new_item_code": old_name, "disabled": 0}) - new_bundle = frappe.get_value("Product Bundle", filters={"new_item_code": new_name, "disabled": 0}) + from erpnext.selling.doctype.product_bundle.product_bundle import get_active_product_bundle + + old_bundle = get_active_product_bundle(old_name) + new_bundle = get_active_product_bundle(new_name) if old_bundle and new_bundle: bundle_link = get_link_to_form("Product Bundle", old_bundle) @@ -1146,7 +1148,7 @@ class Item(Document): if doctype in ("Product Bundle", "BOM"): if doctype == "Product Bundle": - filters = {"new_item_code": self.name} + filters = {"new_item_code": self.name, "is_active": 1, "docstatus": 1} fieldname = "new_item_code as docname" else: filters = {"item": self.name, "docstatus": 1} diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index 5dd4da05768..4c05765e65d 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -542,6 +542,7 @@ class TestItem(ERPNextTestSuite): with self.assertRaises(DataValidationError): frappe.rename_doc("Item", "Test Item Bundle Item 1", "Test Item Bundle Item 2", merge=True) + bundle1.cancel() bundle1.delete() frappe.rename_doc("Item", "Test Item Bundle Item 1", "Test Item Bundle Item 2", merge=True) @@ -862,18 +863,21 @@ class TestItem(ERPNextTestSuite): item.reload() self.assertEqual(item.is_stock_item, 0) - # Step - 3: Create Product Bundle + # Step - 3: Create (and submit) an active Product Bundle for the item + component = make_item(properties={"is_stock_item": 1}).name pb = frappe.new_doc("Product Bundle") pb.new_item_code = item.name - pb.flags.ignore_mandatory = True - pb.save() + pb.append("items", {"item_code": component, "qty": 1}) + pb.insert() + pb.submit() # Step - 4: Try to enable Maintain Stock, should throw a validation error item.is_stock_item = 1 self.assertRaises(frappe.ValidationError, item.save) item.reload() - # Step - 5: Delete Product Bundle + # Step - 5: Cancel & delete Product Bundle + pb.cancel() pb.delete() # Step - 6: Again try to enable Maintain Stock diff --git a/erpnext/stock/doctype/packed_item/packed_item.py b/erpnext/stock/doctype/packed_item/packed_item.py index 162fc342df8..9e95bdbd5eb 100644 --- a/erpnext/stock/doctype/packed_item/packed_item.py +++ b/erpnext/stock/doctype/packed_item/packed_item.py @@ -111,7 +111,9 @@ def make_packing_list(doc): def is_product_bundle(item_code: str) -> bool: - return bool(frappe.db.exists("Product Bundle", {"new_item_code": item_code, "disabled": 0})) + from erpnext.selling.doctype.product_bundle.product_bundle import get_active_product_bundle + + return bool(get_active_product_bundle(item_code)) def get_indexed_packed_items_table(doc): @@ -172,7 +174,11 @@ def get_product_bundle_items(item_code): product_bundle_item.uom, product_bundle_item.description, ) - .where((product_bundle.new_item_code == item_code) & (product_bundle.disabled == 0)) + .where( + (product_bundle.new_item_code == item_code) + & (product_bundle.is_active == 1) + & (product_bundle.docstatus == 1) + ) .orderby(product_bundle_item.idx) ) return query.run(as_dict=True) diff --git a/erpnext/stock/doctype/packed_item/test_packed_item.py b/erpnext/stock/doctype/packed_item/test_packed_item.py index 8189343a820..0095c970f9c 100644 --- a/erpnext/stock/doctype/packed_item/test_packed_item.py +++ b/erpnext/stock/doctype/packed_item/test_packed_item.py @@ -36,6 +36,7 @@ def create_product_bundle( make_stock_entry(item=compoenent, to_warehouse=warehouse, qty=10 * qty, rate=100) bundle_doc.insert() + bundle_doc.submit() return bundle, components diff --git a/erpnext/stock/doctype/pick_list/pick_list.py b/erpnext/stock/doctype/pick_list/pick_list.py index 535a20ec0f3..1baab619740 100644 --- a/erpnext/stock/doctype/pick_list/pick_list.py +++ b/erpnext/stock/doctype/pick_list/pick_list.py @@ -14,6 +14,7 @@ from frappe.query_builder.functions import Coalesce, Locate, Replace, Sum from frappe.utils import cint, floor, flt, get_link_to_form from frappe.utils.nestedset import get_descendants_of +from erpnext.selling.doctype.product_bundle.product_bundle import get_active_product_bundle from erpnext.stock.doctype.serial_and_batch_bundle.serial_and_batch_bundle import ( get_auto_batch_nos, ) @@ -650,7 +651,7 @@ class PickList(TransactionBase): frappe.throw(f"Row #{item.idx}: Item Code is Mandatory") if not cint( frappe.get_cached_value("Item", item.item_code, "is_stock_item") - ) and not frappe.db.exists("Product Bundle", {"new_item_code": item.item_code, "disabled": 0}): + ) and not get_active_product_bundle(item.item_code): continue item_code = item.item_code reference = item.sales_order_item or item.material_request_item @@ -850,7 +851,7 @@ class PickList(TransactionBase): def _get_product_bundle_qty_map(self, bundles) -> dict[str, dict[str, float]]: product_bundle_qty_map = {} for data in bundles: - bundle = frappe.get_last_doc("Product Bundle", {"new_item_code": data.item_code, "disabled": 0}) + bundle = frappe.get_doc("Product Bundle", get_active_product_bundle(data.item_code)) product_bundle_qty_map[data.item_code] = {item.item_code: item.qty for item in bundle.items} return product_bundle_qty_map diff --git a/erpnext/stock/doctype/purchase_receipt_item/purchase_receipt_item.json b/erpnext/stock/doctype/purchase_receipt_item/purchase_receipt_item.json index ed43edf0046..4938654690d 100644 --- a/erpnext/stock/doctype/purchase_receipt_item/purchase_receipt_item.json +++ b/erpnext/stock/doctype/purchase_receipt_item/purchase_receipt_item.json @@ -953,10 +953,11 @@ "search_index": 1 }, { + "description": "Parent item of the Product Bundle this row was packed from", "fieldname": "product_bundle", "fieldtype": "Link", "label": "Product Bundle", - "options": "Product Bundle", + "options": "Item", "read_only": 1 }, { diff --git a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py index f63b7de01c2..75199928605 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py @@ -1416,14 +1416,17 @@ def create_repack_entry(**args): def create_product_bundle_item(new_item_code, packed_items): - if not frappe.db.exists("Product Bundle", new_item_code): + from erpnext.selling.doctype.product_bundle.product_bundle import get_active_product_bundle + + if not get_active_product_bundle(new_item_code): item = frappe.new_doc("Product Bundle") item.new_item_code = new_item_code for d in packed_items: item.append("items", {"item_code": d[0], "qty": d[1]}) - item.save() + item.insert() + item.submit() def create_items(items=None, uoms=None): diff --git a/erpnext/stock/get_item_details.py b/erpnext/stock/get_item_details.py index b6c57865d4a..e3ceea3038d 100644 --- a/erpnext/stock/get_item_details.py +++ b/erpnext/stock/get_item_details.py @@ -180,9 +180,12 @@ def remove_standard_fields(out: ItemDetails): def set_valuation_rate(out: ItemDetails | dict, ctx: ItemDetailsCtx): - if frappe.db.exists("Product Bundle", {"name": ctx.item_code, "disabled": 0}, cache=True): + from erpnext.selling.doctype.product_bundle.product_bundle import get_active_product_bundle + + active_bundle = get_active_product_bundle(ctx.item_code) + if active_bundle: valuation_rate = 0.0 - bundled_items = frappe.get_doc("Product Bundle", ctx.item_code) + bundled_items = frappe.get_doc("Product Bundle", active_bundle) for bundle_item in bundled_items.items: valuation_rate += flt( diff --git a/erpnext/stock/report/product_bundle_balance/product_bundle_balance.py b/erpnext/stock/report/product_bundle_balance/product_bundle_balance.py index 6d8aef258dc..3c1cce3216a 100644 --- a/erpnext/stock/report/product_bundle_balance/product_bundle_balance.py +++ b/erpnext/stock/report/product_bundle_balance/product_bundle_balance.py @@ -139,7 +139,7 @@ def get_items(filters): item.brand, item.stock_uom, ) - .where(IfNull(item.disabled, 0) == 0) + .where((IfNull(item.disabled, 0) == 0) & (pb.is_active == 1) & (pb.docstatus == 1)) ) if item_code := filters.get("item_code"): @@ -181,7 +181,7 @@ def get_items(filters): pbi.uom, pbi.qty, ) - .where(pb.new_item_code.isin(parent_items)) + .where(pb.new_item_code.isin(parent_items) & (pb.is_active == 1) & (pb.docstatus == 1)) ).run(as_dict=1) child_items = set() diff --git a/erpnext/tests/test_submit_existing_product_bundles.py b/erpnext/tests/test_submit_existing_product_bundles.py new file mode 100644 index 00000000000..9a22e131b7a --- /dev/null +++ b/erpnext/tests/test_submit_existing_product_bundles.py @@ -0,0 +1,77 @@ +# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + +import frappe + +from erpnext.patches.v16_0.submit_existing_product_bundles import execute +from erpnext.selling.doctype.product_bundle.product_bundle import get_active_product_bundle +from erpnext.stock.doctype.item.test_item import make_item +from erpnext.tests.utils import ERPNextTestSuite + + +class TestSubmitExistingProductBundles(ERPNextTestSuite): + def _make_legacy_bundle(self, item_code, child, disabled=0): + """Recreate the pre-migration shape: a draft bundle named after its parent item.""" + bundle = frappe.get_doc( + { + "doctype": "Product Bundle", + "new_item_code": item_code, + "items": [{"item_code": child, "qty": 1}], + } + ).insert() + # revert to the legacy state: name == item_code, draft + frappe.rename_doc("Product Bundle", bundle.name, item_code, force=True, show_alert=False) + frappe.db.set_value( + "Product Bundle", item_code, {"docstatus": 0, "disabled": disabled}, update_modified=False + ) + return item_code + + def test_patch_renames_and_submits_legacy_bundle(self): + parent = make_item("_Test Patch PB Parent", {"is_stock_item": 0, "is_sales_item": 1}).name + child = make_item("_Test Patch PB Child", {"is_stock_item": 1}).name + legacy = self._make_legacy_bundle(parent, child) + + execute() + + # legacy name is gone; an active versioned bundle now resolves for the item + self.assertFalse(frappe.db.exists("Product Bundle", legacy)) + migrated = get_active_product_bundle(parent) + self.assertTrue(migrated and migrated.startswith("PB-")) + self.assertEqual(frappe.db.get_value("Product Bundle", migrated, "docstatus"), 1) + self.assertEqual(frappe.db.get_value("Product Bundle", migrated, "is_active"), 1) + + def test_patch_seeds_is_active_from_disabled(self): + parent = make_item("_Test Patch PB Disabled Parent", {"is_stock_item": 0, "is_sales_item": 1}).name + child = make_item("_Test Patch PB Disabled Child", {"is_stock_item": 1}).name + self._make_legacy_bundle(parent, child, disabled=1) + + execute() + + # a disabled legacy bundle becomes submitted but inactive + self.assertIsNone(get_active_product_bundle(parent)) + migrated = frappe.db.get_value("Product Bundle", {"new_item_code": parent, "docstatus": 1}, "name") + self.assertTrue(migrated and migrated.startswith("PB-")) + self.assertEqual(frappe.db.get_value("Product Bundle", migrated, "is_active"), 0) + + def test_patch_submits_partially_migrated_bundle(self): + """An interrupted run can leave a bundle renamed (PB-*) but still a draft; + re-running the patch must submit it rather than skip it.""" + parent = make_item("_Test Patch PB Partial Parent", {"is_stock_item": 0, "is_sales_item": 1}).name + child = make_item("_Test Patch PB Partial Child", {"is_stock_item": 1}).name + + # a freshly inserted (unsubmitted) bundle is already PB-named: exactly the + # renamed-but-not-submitted state of an interrupted migration + bundle = frappe.get_doc( + { + "doctype": "Product Bundle", + "new_item_code": parent, + "items": [{"item_code": child, "qty": 1}], + } + ).insert() + self.assertTrue(bundle.name.startswith("PB-")) + self.assertEqual(bundle.docstatus, 0) + + execute() + + self.assertEqual(get_active_product_bundle(parent), bundle.name) + self.assertEqual(frappe.db.get_value("Product Bundle", bundle.name, "docstatus"), 1) diff --git a/erpnext/tests/utils.py b/erpnext/tests/utils.py index 99c55b6d7ba..295ab239c5f 100644 --- a/erpnext/tests/utils.py +++ b/erpnext/tests/utils.py @@ -1916,7 +1916,12 @@ class BootStrapTestData: self.make_records(["item_code", "item_name"], records) def make_product_bundle(self): - records = [ + from erpnext.selling.doctype.product_bundle.product_bundle import get_active_product_bundle + + if get_active_product_bundle("_Test Product Bundle Item"): + return + + frappe.get_doc( { "doctype": "Product Bundle", "new_item_code": "_Test Product Bundle Item", @@ -1935,8 +1940,7 @@ class BootStrapTestData: }, ], } - ] - self.make_records(["new_item_code"], records) + ).insert().submit() def make_test_account(self): records = [