From 656b1bcede274002e54eb2ff034845bc1787d160 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Thu, 19 Feb 2026 20:22:25 +0530 Subject: [PATCH] =?UTF-8?q?Revert=20"feat:=20update=20item=20button=20addi?= =?UTF-8?q?tion=20for=20quotation=20(backport=20#50976)=20(#5=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit e2a1a7a36d8c057f09b029c180135d37237a42b8. --- .../supplier_quotation/supplier_quotation.js | 8 - .../supplier_quotation/supplier_quotation.py | 12 -- .../test_supplier_quotation.py | 102 +----------- erpnext/controllers/accounts_controller.py | 148 +++++++----------- .../selling/doctype/quotation/quotation.js | 7 - .../selling/doctype/quotation/quotation.py | 1 - .../doctype/quotation/test_quotation.py | 99 +----------- 7 files changed, 55 insertions(+), 322 deletions(-) diff --git a/erpnext/buying/doctype/supplier_quotation/supplier_quotation.js b/erpnext/buying/doctype/supplier_quotation/supplier_quotation.js index e81f9f9c988..fccca81f8ce 100644 --- a/erpnext/buying/doctype/supplier_quotation/supplier_quotation.js +++ b/erpnext/buying/doctype/supplier_quotation/supplier_quotation.js @@ -30,14 +30,6 @@ erpnext.buying.SupplierQuotationController = class SupplierQuotationController e cur_frm.add_custom_button(__("Purchase Order"), this.make_purchase_order, __("Create")); cur_frm.page.set_inner_btn_group_as_primary(__("Create")); cur_frm.add_custom_button(__("Quotation"), this.make_quotation, __("Create")); - - this.frm.add_custom_button(__("Update Items"), () => { - erpnext.utils.update_child_items({ - frm: this.frm, - child_docname: "items", - cannot_add_row: false, - }); - }); } else if (this.frm.doc.docstatus === 0) { erpnext.set_unit_price_items_note(this.frm); diff --git a/erpnext/buying/doctype/supplier_quotation/supplier_quotation.py b/erpnext/buying/doctype/supplier_quotation/supplier_quotation.py index 790e89f8c0e..35cb2eebf8f 100644 --- a/erpnext/buying/doctype/supplier_quotation/supplier_quotation.py +++ b/erpnext/buying/doctype/supplier_quotation/supplier_quotation.py @@ -345,15 +345,3 @@ def set_expired_status(): """, (nowdate()), ) - - -def get_purchased_items(supplier_quotation: str): - return frappe._dict( - frappe.get_all( - "Purchase Order Item", - filters={"supplier_quotation": supplier_quotation, "docstatus": 1}, - fields=["supplier_quotation_item", "sum(qty)"], - group_by="supplier_quotation_item", - as_list=1, - ) - ) diff --git a/erpnext/buying/doctype/supplier_quotation/test_supplier_quotation.py b/erpnext/buying/doctype/supplier_quotation/test_supplier_quotation.py index da4c78347f3..60c82bbc05f 100644 --- a/erpnext/buying/doctype/supplier_quotation/test_supplier_quotation.py +++ b/erpnext/buying/doctype/supplier_quotation/test_supplier_quotation.py @@ -2,115 +2,15 @@ # License: GNU General Public License v3. See license.txt -import json - import frappe from frappe.tests.utils import FrappeTestCase, change_settings from frappe.utils import add_days, today from erpnext.buying.doctype.supplier_quotation.supplier_quotation import make_purchase_order -from erpnext.controllers.accounts_controller import InvalidQtyError, update_child_qty_rate +from erpnext.controllers.accounts_controller import InvalidQtyError class TestPurchaseOrder(FrappeTestCase): - def test_update_child_supplier_quotation_add_item(self): - sq = frappe.copy_doc(test_records[0]) - sq.submit() - - trans_item = json.dumps( - [ - { - "item_code": sq.items[0].item_code, - "rate": sq.items[0].rate, - "qty": 5, - "docname": sq.items[0].name, - }, - { - "item_code": "_Test Item 2", - "rate": 300, - "qty": 3, - }, - ] - ) - update_child_qty_rate("Supplier Quotation", trans_item, sq.name) - sq.reload() - self.assertEqual(sq.get("items")[0].qty, 5) - self.assertEqual(sq.get("items")[1].rate, 300) - - def test_update_supplier_quotation_child_rate_disallow(self): - sq = frappe.copy_doc(test_records[0]) - sq.submit() - trans_item = json.dumps( - [ - { - "item_code": sq.items[0].item_code, - "rate": 300, - "qty": sq.items[0].qty, - "docname": sq.items[0].name, - }, - ] - ) - self.assertRaises( - frappe.ValidationError, update_child_qty_rate, "Supplier Quotation", trans_item, sq.name - ) - - def test_update_supplier_quotation_child_remove_item(self): - sq = frappe.copy_doc(test_records[0]) - sq.submit() - po = make_purchase_order(sq.name) - - trans_item = json.dumps( - [ - { - "item_code": sq.items[0].item_code, - "rate": sq.items[0].rate, - "qty": sq.items[0].qty, - "docname": sq.items[0].name, - }, - { - "item_code": "_Test Item 2", - "rate": 300, - "qty": 3, - }, - ] - ) - po.get("items")[0].schedule_date = add_days(today(), 1) - update_child_qty_rate("Supplier Quotation", trans_item, sq.name) - po.submit() - sq.reload() - - trans_item = json.dumps( - [ - { - "item_code": "_Test Item 2", - "rate": 300, - "qty": 3, - } - ] - ) - - frappe.db.savepoint("before_cancel") - # check if item having purchase order can be removed - self.assertRaises( - frappe.LinkExistsError, update_child_qty_rate, "Supplier Quotation", trans_item, sq.name - ) - frappe.db.rollback(save_point="before_cancel") - - trans_item = json.dumps( - [ - { - "item_code": sq.items[0].item_code, - "rate": sq.items[0].rate, - "qty": sq.items[0].qty, - "docname": sq.items[0].name, - } - ] - ) - - update_child_qty_rate("Supplier Quotation", trans_item, sq.name) - sq.reload() - self.assertEqual(len(sq.get("items")), 1) - def test_supplier_quotation_qty(self): sq = frappe.copy_doc(test_records[0]) sq.items[0].qty = 0 diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index a88f7d02fb0..66a3dd93169 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -3678,7 +3678,7 @@ def set_order_defaults(parent_doctype, parent_doctype_name, child_doctype, child conversion_factor = flt(get_conversion_factor(item.item_code, child_item.uom).get("conversion_factor")) child_item.conversion_factor = flt(trans_item.get("conversion_factor")) or conversion_factor - if child_doctype in ["Purchase Order Item", "Supplier Quotation Item"]: + if child_doctype == "Purchase Order Item": # Initialized value will update in parent validation child_item.base_rate = 1 child_item.base_amount = 1 @@ -3696,7 +3696,7 @@ def set_order_defaults(parent_doctype, parent_doctype_name, child_doctype, child return child_item -def validate_child_on_delete(row, parent, ordered_item=None): +def validate_child_on_delete(row, parent): """Check if partially transacted item (row) is being deleted.""" if parent.doctype == "Sales Order": if flt(row.delivered_qty): @@ -3724,17 +3724,13 @@ def validate_child_on_delete(row, parent, ordered_item=None): row.idx, row.item_code ) ) - if parent.doctype in ["Purchase Order", "Sales Order"]: - if flt(row.billed_amt): - frappe.throw( - _("Row #{0}: Cannot delete item {1} which has already been billed.").format( - row.idx, row.item_code - ) - ) - if parent.doctype == "Quotation": - if ordered_item.get(row.name): - frappe.throw(_("Cannot delete an item which has been ordered")) + if flt(row.billed_amt): + frappe.throw( + _("Row #{0}: Cannot delete item {1} which has already been billed.").format( + row.idx, row.item_code + ) + ) def update_bin_on_delete(row, doctype): @@ -3760,7 +3756,7 @@ def update_bin_on_delete(row, doctype): update_bin_qty(row.item_code, row.warehouse, qty_dict) -def validate_and_delete_children(parent, data, ordered_item=None) -> bool: +def validate_and_delete_children(parent, data) -> bool: deleted_children = [] updated_item_names = [d.get("docname") for d in data] for item in parent.items: @@ -3768,7 +3764,7 @@ def validate_and_delete_children(parent, data, ordered_item=None) -> bool: deleted_children.append(item) for d in deleted_children: - validate_child_on_delete(d, parent, ordered_item) + validate_child_on_delete(d, parent) d.cancel() d.delete() @@ -3777,19 +3773,16 @@ def validate_and_delete_children(parent, data, ordered_item=None) -> bool: # need to update ordered qty in Material Request first # bin uses Material Request Items to recalculate & update - if parent.doctype not in ["Quotation", "Supplier Quotation"]: - parent.update_prevdoc_status() - for d in deleted_children: - update_bin_on_delete(d, parent.doctype) + parent.update_prevdoc_status() + + for d in deleted_children: + update_bin_on_delete(d, parent.doctype) return bool(deleted_children) @frappe.whitelist() def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, child_docname="items"): - from erpnext.buying.doctype.supplier_quotation.supplier_quotation import get_purchased_items - from erpnext.selling.doctype.quotation.quotation import get_ordered_items - def check_doc_permissions(doc, perm_type="create"): try: doc.check_permission(perm_type) @@ -3828,7 +3821,7 @@ def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, chil ) def get_new_child_item(item_row): - child_doctype = parent_doctype + " Item" + child_doctype = "Sales Order Item" if parent_doctype == "Sales Order" else "Purchase Order Item" return set_order_defaults(parent_doctype, parent_doctype_name, child_doctype, child_docname, item_row) def is_allowed_zero_qty(): @@ -3853,21 +3846,6 @@ def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, chil if parent_doctype == "Purchase Order" and flt(new_data.get("qty")) < flt(child_item.received_qty): frappe.throw(_("Cannot set quantity less than received quantity")) - if parent_doctype in ["Quotation", "Supplier Quotation"]: - if (parent_doctype == "Quotation" and not ordered_items) or ( - parent_doctype == "Supplier Quotation" and not purchased_items - ): - return - - qty_to_check = ( - ordered_items.get(child_item.name) - if parent_doctype == "Quotation" - else purchased_items.get(child_item.name) - ) - if qty_to_check: - if flt(new_data.get("qty")) < qty_to_check: - frappe.throw(_("Cannot reduce quantity than ordered or purchased quantity")) - def should_update_supplied_items(doc) -> bool: """Subcontracted PO can allow following changes *after submit*: @@ -3910,6 +3888,7 @@ def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, chil frappe.throw(_("Finished Good Item {0} Qty can not be zero").format(new_data["fg_item"])) data = json.loads(trans_items) + any_qty_changed = False # updated to true if any item's qty changes items_added_or_removed = False # updated to true if any new item is added or removed any_conversion_factor_changed = False @@ -3917,16 +3896,7 @@ def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, chil parent = frappe.get_doc(parent_doctype, parent_doctype_name) check_doc_permissions(parent, "write") - - if parent_doctype == "Quotation": - ordered_items = get_ordered_items(parent.name) - _removed_items = validate_and_delete_children(parent, data, ordered_items) - elif parent_doctype == "Supplier Quotation": - purchased_items = get_purchased_items(parent.name) - _removed_items = validate_and_delete_children(parent, data, purchased_items) - else: - _removed_items = validate_and_delete_children(parent, data) - + _removed_items = validate_and_delete_children(parent, data) items_added_or_removed |= _removed_items for d in data: @@ -3966,9 +3936,7 @@ def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, chil conversion_factor_unchanged = prev_con_fac == new_con_fac any_conversion_factor_changed |= not conversion_factor_unchanged date_unchanged = ( - (prev_date == getdate(new_date) if prev_date and new_date else False) - if parent_doctype not in ["Quotation", "Supplier Quotation"] - else None + prev_date == getdate(new_date) if prev_date and new_date else False ) # in case of delivery note etc if ( rate_unchanged @@ -3981,10 +3949,6 @@ def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, chil continue validate_quantity(child_item, d) - if parent_doctype in ["Quotation", "Supplier Quotation"]: - if not rate_unchanged: - frappe.throw(_("Rates cannot be modified for quoted items")) - if flt(child_item.get("qty")) != flt(d.get("qty")): any_qty_changed = True @@ -4008,21 +3972,18 @@ def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, chil rate_unchanged = prev_rate == new_rate if not rate_unchanged and not child_item.get("qty") and is_allowed_zero_qty(): frappe.throw(_("Rate of '{}' items cannot be changed").format(frappe.bold(_("Unit Price")))) + # Amount cannot be lesser than billed amount, except for negative amounts row_rate = flt(d.get("rate"), rate_precision) - - if parent_doctype in ["Purchase Order", "Sales Order"]: - amount_below_billed_amt = flt(child_item.billed_amt, rate_precision) > flt( - row_rate * flt(d.get("qty"), qty_precision), rate_precision + amount_below_billed_amt = flt(child_item.billed_amt, rate_precision) > flt( + row_rate * flt(d.get("qty"), qty_precision), rate_precision + ) + if amount_below_billed_amt and row_rate > 0.0: + frappe.throw( + _( + "Row #{0}: Cannot set Rate if the billed amount is greater than the amount for Item {1}." + ).format(child_item.idx, child_item.item_code) ) - if amount_below_billed_amt and row_rate > 0.0: - frappe.throw( - _( - "Row #{0}: Cannot set Rate if the billed amount is greater than the amount for Item {1}." - ).format(child_item.idx, child_item.item_code) - ) - else: - child_item.rate = row_rate else: child_item.rate = row_rate @@ -4056,27 +4017,26 @@ def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, chil if d.get("bom_no") and parent_doctype == "Sales Order": child_item.bom_no = d.get("bom_no") - if parent_doctype in ["Sales Order", "Purchase Order"]: - if flt(child_item.price_list_rate): - if flt(child_item.rate) > flt(child_item.price_list_rate): - # if rate is greater than price_list_rate, set margin - # or set discount - child_item.discount_percentage = 0 - child_item.margin_type = "Amount" - child_item.margin_rate_or_amount = flt( - child_item.rate - child_item.price_list_rate, - child_item.precision("margin_rate_or_amount"), - ) - child_item.rate_with_margin = child_item.rate - else: - child_item.discount_percentage = flt( - (1 - flt(child_item.rate) / flt(child_item.price_list_rate)) * 100.0, - child_item.precision("discount_percentage"), - ) - child_item.discount_amount = flt(child_item.price_list_rate) - flt(child_item.rate) - child_item.margin_type = "" - child_item.margin_rate_or_amount = 0 - child_item.rate_with_margin = 0 + if flt(child_item.price_list_rate): + if flt(child_item.rate) > flt(child_item.price_list_rate): + # if rate is greater than price_list_rate, set margin + # or set discount + child_item.discount_percentage = 0 + child_item.margin_type = "Amount" + child_item.margin_rate_or_amount = flt( + child_item.rate - child_item.price_list_rate, + child_item.precision("margin_rate_or_amount"), + ) + child_item.rate_with_margin = child_item.rate + else: + child_item.discount_percentage = flt( + (1 - flt(child_item.rate) / flt(child_item.price_list_rate)) * 100.0, + child_item.precision("discount_percentage"), + ) + child_item.discount_amount = flt(child_item.price_list_rate) - flt(child_item.rate) + child_item.margin_type = "" + child_item.margin_rate_or_amount = 0 + child_item.rate_with_margin = 0 child_item.flags.ignore_validate_update_after_submit = True if new_child_flag: @@ -4098,15 +4058,14 @@ def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, chil parent.doctype, parent.company, parent.base_grand_total ) - if parent_doctype != "Supplier Quotation": - parent.set_payment_schedule() + parent.set_payment_schedule() if parent_doctype == "Purchase Order": parent.set_tax_withholding() parent.validate_minimum_order_qty() parent.validate_budget() if parent.is_against_so(): parent.update_status_updater() - elif parent_doctype == "Sales Order": + else: parent.check_credit_limit() # reset index of child table @@ -4139,7 +4098,7 @@ def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, chil "Items cannot be updated as Subcontracting Order is created against the Purchase Order {0}." ).format(frappe.bold(parent.name)) ) - else: + else: # Sales Order parent.validate_selling_price() parent.validate_for_duplicate_items() parent.validate_warehouse() @@ -4151,10 +4110,9 @@ def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, chil parent.reload() validate_workflow_conditions(parent) - if parent_doctype in ["Purchase Order", "Sales Order"]: - parent.update_blanket_order() - parent.update_billing_percentage() - parent.set_status() + parent.update_blanket_order() + parent.update_billing_percentage() + parent.set_status() parent.validate_uom_is_integer("uom", "qty") parent.validate_uom_is_integer("stock_uom", "stock_qty") diff --git a/erpnext/selling/doctype/quotation/quotation.js b/erpnext/selling/doctype/quotation/quotation.js index 480ca04b6a9..f0061c016bd 100644 --- a/erpnext/selling/doctype/quotation/quotation.js +++ b/erpnext/selling/doctype/quotation/quotation.js @@ -123,13 +123,6 @@ erpnext.selling.QuotationController = class QuotationController extends erpnext. frappe.datetime.get_diff(doc.valid_till, frappe.datetime.get_today()) >= 0) ) { this.frm.add_custom_button(__("Sales Order"), () => this.make_sales_order(), __("Create")); - this.frm.add_custom_button(__("Update Items"), () => { - erpnext.utils.update_child_items({ - frm: this.frm, - child_docname: "items", - cannot_add_row: false, - }); - }); } if (doc.status !== "Ordered" && this.frm.has_perm("write")) { diff --git a/erpnext/selling/doctype/quotation/quotation.py b/erpnext/selling/doctype/quotation/quotation.py index 7a31854d259..f41116203ba 100644 --- a/erpnext/selling/doctype/quotation/quotation.py +++ b/erpnext/selling/doctype/quotation/quotation.py @@ -614,7 +614,6 @@ def handle_mandatory_error(e, customer, lead_name): frappe.throw(message, title=_("Mandatory Missing")) -@frappe.whitelist() def get_ordered_items(quotation: str): return frappe._dict( frappe.get_all( diff --git a/erpnext/selling/doctype/quotation/test_quotation.py b/erpnext/selling/doctype/quotation/test_quotation.py index b11f23806cb..ae756f34288 100644 --- a/erpnext/selling/doctype/quotation/test_quotation.py +++ b/erpnext/selling/doctype/quotation/test_quotation.py @@ -1,114 +1,17 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: GNU General Public License v3. See license.txt -import json - import frappe from frappe.tests.utils import FrappeTestCase, change_settings from frappe.utils import add_days, add_months, flt, getdate, nowdate -from erpnext.controllers.accounts_controller import InvalidQtyError, update_child_qty_rate -from erpnext.selling.doctype.quotation.quotation import make_sales_order +from erpnext.controllers.accounts_controller import InvalidQtyError from erpnext.setup.utils import get_exchange_rate test_dependencies = ["Product Bundle"] class TestQuotation(FrappeTestCase): - def test_update_child_quotation_add_item(self): - from erpnext.stock.doctype.item.test_item import make_item - - item_1 = make_item("_Test Item") - item_2 = make_item("_Test Item 1") - - item_list = [ - {"item_code": item_1.item_code, "warehouse": "", "qty": 10, "rate": 300}, - {"item_code": item_2.item_code, "warehouse": "", "qty": 5, "rate": 400}, - ] - - qo = make_quotation(item_list=item_list) - first_item = qo.get("items")[0] - second_item = qo.get("items")[1] - trans_item = json.dumps( - [ - { - "item_code": first_item.item_code, - "rate": first_item.rate, - "qty": 11, - "docname": first_item.name, - }, - { - "item_code": second_item.item_code, - "rate": second_item.rate, - "qty": second_item.qty, - "docname": second_item.name, - }, - {"item_code": "_Test Item 2", "rate": 100, "qty": 7}, - ] - ) - - update_child_qty_rate("Quotation", trans_item, qo.name) - qo.reload() - self.assertEqual(qo.get("items")[0].qty, 11) - self.assertEqual(qo.get("items")[-1].rate, 100) - - def test_update_child_disallow_rate_change(self): - qo = make_quotation(qty=4) - trans_item = json.dumps( - [ - { - "item_code": qo.items[0].item_code, - "rate": 5000, - "qty": qo.items[0].qty, - "docname": qo.items[0].name, - } - ] - ) - self.assertRaises(frappe.ValidationError, update_child_qty_rate, "Quotation", trans_item, qo.name) - - def test_update_child_removing_item(self): - qo = make_quotation(qty=10) - sales_order = make_sales_order(qo.name) - sales_order.delivery_date = nowdate() - - trans_item = json.dumps( - [ - { - "item_code": qo.items[0].item_code, - "rate": qo.items[0].rate, - "qty": qo.items[0].qty, - "docname": qo.items[0].name, - }, - {"item_code": "_Test Item 2", "rate": 100, "qty": 7}, - ] - ) - - update_child_qty_rate("Quotation", trans_item, qo.name) - sales_order.submit() - qo.reload() - self.assertEqual(qo.status, "Partially Ordered") - - trans_item = json.dumps([{"item_code": "_Test Item 2", "rate": 100, "qty": 7}]) - - # check if items having a sales order can be removed - self.assertRaises(frappe.ValidationError, update_child_qty_rate, "Quotation", trans_item, qo.name) - - trans_item = json.dumps( - [ - { - "item_code": qo.items[0].item_code, - "rate": qo.items[0].rate, - "qty": qo.items[0].qty, - "docname": qo.items[0].name, - } - ] - ) - - # remove item with no sales order - update_child_qty_rate("Quotation", trans_item, qo.name) - qo.reload() - self.assertEqual(len(qo.get("items")), 1) - def test_quotation_qty(self): qo = make_quotation(qty=0, do_not_save=True) with self.assertRaises(InvalidQtyError):