From 6485ac4e596dced1a0c5bfc2fc7f9fb920d3b6d8 Mon Sep 17 00:00:00 2001 From: Sagar Sharma Date: Thu, 9 Dec 2021 17:04:00 +0530 Subject: [PATCH] fix: misleading "Set Default X" fields after saving (#28798) * fix: misleading "Set Default X" fields after saving * refactor: remove unncessary code and minor formatting * fix: extend to more doctypes and correct fieldnames Co-authored-by: Ankush Menat --- .../purchase_invoice/purchase_invoice.py | 3 +++ .../doctype/sales_invoice/sales_invoice.py | 2 ++ .../doctype/purchase_order/purchase_order.py | 1 + .../tests/test_transaction_base.py | 22 +++++++++++++++++++ .../doctype/sales_order/sales_order.py | 2 ++ .../doctype/delivery_note/delivery_note.py | 1 + .../material_request/material_request.py | 3 +++ .../purchase_receipt/purchase_receipt.py | 4 ++++ .../stock/doctype/stock_entry/stock_entry.py | 2 ++ erpnext/utilities/transaction_base.py | 22 +++++++++++++++++++ 10 files changed, 62 insertions(+) create mode 100644 erpnext/controllers/tests/test_transaction_base.py diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py index 48b5cb9459..f4fd1bf16e 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py @@ -114,6 +114,9 @@ class PurchaseInvoice(BuyingController): self.set_status() self.validate_purchase_receipt_if_update_stock() validate_inter_company_party(self.doctype, self.supplier, self.company, self.inter_company_invoice_reference) + self.reset_default_field_value("set_warehouse", "items", "warehouse") + self.reset_default_field_value("rejected_warehouse", "items", "rejected_warehouse") + self.reset_default_field_value("set_from_warehouse", "items", "from_warehouse") def validate_release_date(self): if self.release_date and getdate(nowdate()) >= getdate(self.release_date): diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py index c4d59f1ef7..64712b550f 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -155,6 +155,8 @@ class SalesInvoice(SellingController): if self.redeem_loyalty_points and self.loyalty_program and self.loyalty_points and not self.is_consolidated: validate_loyalty_points(self, self.loyalty_points) + self.reset_default_field_value("set_warehouse", "items", "warehouse") + def validate_fixed_asset(self): for d in self.get("items"): if d.is_fixed_asset and d.meta.get_field("asset") and d.asset: diff --git a/erpnext/buying/doctype/purchase_order/purchase_order.py b/erpnext/buying/doctype/purchase_order/purchase_order.py index 5eab21bd9d..1b5f35efbb 100644 --- a/erpnext/buying/doctype/purchase_order/purchase_order.py +++ b/erpnext/buying/doctype/purchase_order/purchase_order.py @@ -72,6 +72,7 @@ class PurchaseOrder(BuyingController): self.create_raw_materials_supplied("supplied_items") self.set_received_qty_for_drop_ship_items() validate_inter_company_party(self.doctype, self.supplier, self.company, self.inter_company_order_reference) + self.reset_default_field_value("set_warehouse", "items", "warehouse") def validate_with_previous_doc(self): super(PurchaseOrder, self).validate_with_previous_doc({ diff --git a/erpnext/controllers/tests/test_transaction_base.py b/erpnext/controllers/tests/test_transaction_base.py new file mode 100644 index 0000000000..13aa697610 --- /dev/null +++ b/erpnext/controllers/tests/test_transaction_base.py @@ -0,0 +1,22 @@ +import unittest + +import frappe + + +class TestUtils(unittest.TestCase): + def test_reset_default_field_value(self): + doc = frappe.get_doc({ + "doctype": "Purchase Receipt", + "set_warehouse": "Warehouse 1", + }) + + # Same values + doc.items = [{"warehouse": "Warehouse 1"}, {"warehouse": "Warehouse 1"}, {"warehouse": "Warehouse 1"}] + doc.reset_default_field_value("set_warehouse", "items", "warehouse") + self.assertEqual(doc.set_warehouse, "Warehouse 1") + + # Mixed values + doc.items = [{"warehouse": "Warehouse 1"}, {"warehouse": "Warehouse 2"}, {"warehouse": "Warehouse 1"}] + doc.reset_default_field_value("set_warehouse", "items", "warehouse") + self.assertEqual(doc.set_warehouse, None) + diff --git a/erpnext/selling/doctype/sales_order/sales_order.py b/erpnext/selling/doctype/sales_order/sales_order.py index e69e28da92..cc951850a4 100755 --- a/erpnext/selling/doctype/sales_order/sales_order.py +++ b/erpnext/selling/doctype/sales_order/sales_order.py @@ -63,6 +63,8 @@ class SalesOrder(SellingController): if not self.billing_status: self.billing_status = 'Not Billed' if not self.delivery_status: self.delivery_status = 'Not Delivered' + self.reset_default_field_value("set_warehouse", "items", "warehouse") + def validate_po(self): # validate p.o date v/s delivery date if self.po_date and not self.skip_delivery_note: diff --git a/erpnext/stock/doctype/delivery_note/delivery_note.py b/erpnext/stock/doctype/delivery_note/delivery_note.py index 52684607b4..70d48a42d7 100644 --- a/erpnext/stock/doctype/delivery_note/delivery_note.py +++ b/erpnext/stock/doctype/delivery_note/delivery_note.py @@ -138,6 +138,7 @@ class DeliveryNote(SellingController): self.update_current_stock() if not self.installation_status: self.installation_status = 'Not Installed' + self.reset_default_field_value("set_warehouse", "items", "warehouse") def validate_with_previous_doc(self): super(DeliveryNote, self).validate_with_previous_doc({ diff --git a/erpnext/stock/doctype/material_request/material_request.py b/erpnext/stock/doctype/material_request/material_request.py index d717c50919..103e8d6a88 100644 --- a/erpnext/stock/doctype/material_request/material_request.py +++ b/erpnext/stock/doctype/material_request/material_request.py @@ -80,6 +80,9 @@ class MaterialRequest(BuyingController): # NOTE: Since Item BOM and FG quantities are combined, using current data, it cannot be validated # Though the creation of Material Request from a Production Plan can be rethought to fix this + self.reset_default_field_value("set_warehouse", "items", "warehouse") + self.reset_default_field_value("set_from_warehouse", "items", "from_warehouse") + def set_title(self): '''Set title as comma separated list of items''' if not self.title: diff --git a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py index 762f45f75f..c97b306c4e 100644 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py @@ -118,6 +118,10 @@ class PurchaseReceipt(BuyingController): if getdate(self.posting_date) > getdate(nowdate()): throw(_("Posting Date cannot be future date")) + self.reset_default_field_value("set_warehouse", "items", "warehouse") + self.reset_default_field_value("rejected_warehouse", "items", "rejected_warehouse") + self.reset_default_field_value("set_from_warehouse", "items", "from_warehouse") + def validate_cwip_accounts(self): for item in self.get('items'): diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py index a38dfa5062..a00d63e6f2 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/stock_entry.py @@ -103,6 +103,8 @@ class StockEntry(StockController): self.set_actual_qty() self.calculate_rate_and_amount() self.validate_putaway_capacity() + self.reset_default_field_value("from_warehouse", "items", "s_warehouse") + self.reset_default_field_value("to_warehouse", "items", "t_warehouse") def on_submit(self): self.update_stock_ledger() diff --git a/erpnext/utilities/transaction_base.py b/erpnext/utilities/transaction_base.py index 14b3afa5a7..1d8b3a8db6 100644 --- a/erpnext/utilities/transaction_base.py +++ b/erpnext/utilities/transaction_base.py @@ -162,6 +162,28 @@ class TransactionBase(StatusUpdater): return ret + def reset_default_field_value(self, default_field: str, child_table: str, child_table_field: str): + """ Reset "Set default X" fields on forms to avoid confusion. + + example: + doc = { + "set_from_warehouse": "Warehouse A", + "items": [{"from_warehouse": "warehouse B"}, {"from_warehouse": "warehouse A"}], + } + Since this has dissimilar values in child table, the default field will be erased. + + doc.reset_default_field_value("set_from_warehouse", "items", "from_warehouse") + """ + child_table_values = set() + + for row in self.get(child_table): + child_table_values.add(row.get(child_table_field)) + + if len(child_table_values) > 1: + self.set(default_field, None) + else: + self.set(default_field, list(child_table_values)[0]) + def delete_events(ref_type, ref_name): events = frappe.db.sql_list(""" SELECT distinct `tabEvent`.name