From 2993747636b77672fb9398fbdcbad72754cfd3da Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Mon, 3 Aug 2026 15:57:35 +0530 Subject: [PATCH 1/3] fix(stock): validate only the variant attributes that changed Disabling an Item Attribute writes `disabled = 1` into every Item Variant Attribute row, including the rows on the template. `validate_variant` runs on every save and walks the whole attribute table, so any later save of an existing variant re-checked its untouched rows against the now-disabled template row and threw. `update_variants` hit the same wall, which made a single template save fail once an attribute was disabled. The flag exists to keep an attribute out of new variants, not to freeze the variants that already use it. item.js only reads it to drop the attribute from the variant creation dialog. Skip rows that are unchanged since the last save. New and edited rows are still checked, so a disabled attribute cannot be added to an existing variant, and the same guard covers the sibling checks for attributes and values that the template no longer offers. (cherry picked from commit 25cd7936176cd946ad4d5899294524125d181f40) --- erpnext/stock/doctype/item/item.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index 7f077cfd4dd..fb2a7ee95f8 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -837,7 +837,17 @@ class Item(Document): frappe.throw(_("Item {0} is not a template item.").format(frappe.bold(self.variant_of))) if based_on == "Item Attribute": + previous_doc = self.get_doc_before_save() + saved_attributes = ( + {(row.attribute, row.attribute_value) for row in previous_doc.attributes} + if previous_doc + else set() + ) + for d in self.attributes: + if (d.attribute, d.attribute_value) in saved_attributes: + continue + if not frappe.db.exists( "Item Variant Attribute", {"attribute": d.attribute, "parent": self.variant_of} ): From 005b6264820466f96000aefb28b9f2994e24326a Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Mon, 3 Aug 2026 15:57:42 +0530 Subject: [PATCH 2/3] test(stock): cover editing a variant whose attribute is disabled Assert that a variant saves after its attribute is disabled when the edit leaves the attribute rows alone, and that changing an attribute value still throws. (cherry picked from commit 8d5326196e40f2b6e2aa08d7ea02be8127ac823a) --- erpnext/stock/doctype/item/test_item.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index 073c8c8be93..05846afbc58 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -360,6 +360,24 @@ class TestItem(FrappeTestCase): self.assertRaises(InvalidItemAttributeValueError, attribute.save) frappe.db.rollback() + def test_disabled_attribute_blocks_only_attribute_changes(self): + frappe.delete_doc_if_exists("Item", "_Test Variant Item-L", force=1) + + variant = create_variant("_Test Variant Item", {"Test Size": "Large"}) + variant.save() + + attribute = frappe.get_doc("Item Attribute", "Test Size") + attribute.disabled = 1 + attribute.save() + + variant.reload() + variant.description = "Edited after the attribute was disabled" + variant.save() + + variant.reload() + variant.attributes[0].attribute_value = "Small" + self.assertRaises(frappe.ValidationError, variant.save) + def test_rename_attribute_value_updates_variants(self): frappe.delete_doc_if_exists("Item", "_Test Variant Item-L", force=1) From c488de8f1219ea0a0914fa8ce0207923b4438c92 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Mon, 3 Aug 2026 16:43:07 +0530 Subject: [PATCH 3/3] test(stock): isolate the disabled attribute fixtures The test disabled the shared `Test Size` Item Attribute. On version-15 `FrappeTestCase` rolls back once per class instead of once per test, so the flag stayed visible for the rest of `TestItem` and broke the seven tests that build a variant from that attribute. Build a dedicated attribute and template instead. Nothing the test writes is reachable from another test, on either branch, so no cleanup is needed. --- erpnext/stock/doctype/item/test_item.py | 27 ++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index 05846afbc58..deda65c911c 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -361,12 +361,33 @@ class TestItem(FrappeTestCase): frappe.db.rollback() def test_disabled_attribute_blocks_only_attribute_changes(self): - frappe.delete_doc_if_exists("Item", "_Test Variant Item-L", force=1) + frappe.delete_doc_if_exists("Item", "_Test Disabled Attribute Template-L", force=1) + frappe.delete_doc_if_exists("Item", "_Test Disabled Attribute Template", force=1) + frappe.delete_doc_if_exists("Item Attribute", "_Test Disabled Size", force=1) - variant = create_variant("_Test Variant Item", {"Test Size": "Large"}) + attribute = frappe.get_doc( + { + "doctype": "Item Attribute", + "attribute_name": "_Test Disabled Size", + "item_attribute_values": [ + {"attribute_value": "Large", "abbr": "L"}, + {"attribute_value": "Small", "abbr": "S"}, + ], + } + ).insert() + + template = make_item( + "_Test Disabled Attribute Template", + { + "has_variants": 1, + "variant_based_on": "Item Attribute", + "attributes": [{"attribute": attribute.name}], + }, + ) + + variant = create_variant(template.name, {attribute.name: "Large"}) variant.save() - attribute = frappe.get_doc("Item Attribute", "Test Size") attribute.disabled = 1 attribute.save()