fix: misleading "Set Default X" fields after saving

[s-aga-r]

Source/Reference: ISS-21-22-09054

Issue(s): Purchase Receipt

Problem:

• When populating the "Accepted Warehouse" field it adds that warehouse as the default warehouse on all the item rows. This is helpful because often the same warehouse for all the items received. But if the user changes the warehouse for an item. In that case, this field becomes confusing as it is holding another warehouse.

Proposed Solution:

• If all warehouses in the child table(Items) are not matching the "default warehouse" field then set its value to None.

Images/GIF

Before:

https://user-images.githubusercontent.com/63660334/144282361-d6e8ef46-ae48-42ed-a86c-8136cf993b45.gif

After:

https://user-images.githubusercontent.com/25903035/130674996-a6d891e4-4220-4574-87fc-5b32bca6992a.png

Closes: #28644

Affected doctypes:

• Purchase Invoice
• Sales Invoice
• Sales Order
• Purchase Order
• Stock Entry
• Purchase Receipt
• Delivery Note
• Material request

[codecov]

Codecov Report

Merging #28798 (90a8f09) into develop (96a019e) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop   #28798      +/-   ##
===========================================
- Coverage    55.22%   55.22%   -0.01%     
===========================================
  Files         1122     1122              
  Lines        66782    66810      +28     
===========================================
+ Hits         36879    36893      +14     
- Misses       29903    29917      +14     

Impacted Files	Coverage Δ
...ounts/doctype/purchase_invoice/purchase_invoice.py	81.48% <100.00%> (+0.08%)	⬆️
...xt/accounts/doctype/sales_invoice/sales_invoice.py	78.11% <100.00%> (+0.01%)	⬆️
...xt/buying/doctype/purchase_order/purchase_order.py	78.37% <100.00%> (+0.06%)	⬆️
erpnext/selling/doctype/sales_order/sales_order.py	77.58% <100.00%> (+0.04%)	⬆️
...pnext/stock/doctype/delivery_note/delivery_note.py	65.64% <100.00%> (+0.09%)	⬆️
...stock/doctype/material_request/material_request.py	70.32% <100.00%> (+0.21%)	⬆️
...stock/doctype/purchase_receipt/purchase_receipt.py	90.37% <100.00%> (+0.07%)	⬆️
erpnext/stock/doctype/stock_entry/stock_entry.py	79.98% <100.00%> (+0.03%)	⬆️
erpnext/utilities/transaction_base.py	75.40% <100.00%> (+1.49%)	⬆️
...tch_item_expiry_status/batch_item_expiry_status.py	69.23% <0.00%> (-25.00%)	⬇️
... and 29 more

[rtdany10]

Issue arising from https://github.com/frappe/erpnext/blob/develop/erpnext/selling/doctype/sales_order/sales_order.py#L66

Traceback:

Traceback (most recent call last):
  File "/home/frappe/frappe-bench/apps/frappe/frappe/app.py", line 66, in application
    response = frappe.api.handle()
  File "/home/frappe/frappe-bench/apps/frappe/frappe/api.py", line 54, in handle
    return frappe.handler.handle()
  File "/home/frappe/frappe-bench/apps/frappe/frappe/handler.py", line 31, in handle
    data = execute_cmd(cmd)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/handler.py", line 67, in execute_cmd
    return frappe.call(method, **frappe.form_dict)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/__init__.py", line 1220, in call
    return fn(*args, **newargs)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/desk/form/save.py", line 20, in savedocs
    doc.save()
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 282, in save
    return self._save(*args, **kwargs)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 304, in _save
    self.insert()
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 235, in insert
    self.run_before_save_methods()
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 969, in run_before_save_methods
    self.run_method("validate")
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 866, in run_method
    out = Document.hook(fn)(self, *args, **kwargs)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 1163, in composer
    return composed(self, method, *args, **kwargs)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 1146, in runner
    add_to_return_value(self, fn(self, *args, **kwargs))
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 860, in <lambda>
    fn = lambda self, *args, **kwargs: getattr(self, method)(*args, **kwargs)
  File "/home/frappe/frappe-bench/apps/erpnext/erpnext/selling/doctype/sales_order/sales_order.py", line 66, in validate
    self.reset_default_field_value("set_warehouse", "items", "warehouse")
AttributeError: 'SalesOrder' object has no attribute 'reset_default_field_value'

@ankush @s-aga-r

[ankush]

can't reproduce locally.... all tests are also passing (validate method runs in all 😅 )

can you share version and commit id of erpnext?

ref:

erpnext/erpnext/utilities/transaction_base.py

Lines 165 to 185 in f1c0190

	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])

this code might be missing for you for some reason.

[rtdany10]

I'm on a fork, and the code seems to be present there and on my dev machine.
https://github.com/rtdany10/erpnext/blob/ledger-merger/erpnext/utilities/transaction_base.py#L165

Weird.

[rtdany10]

I'm so sorry. I updated via git pull instead of bench update.
A simple bench clear-cache and restart fixed it. Sorry again 😅

[marination]

This is kinda problematic in Manufacture entry cases where some rows will have Target Warehouse as None because only one row is a Finished Good. But users still want to have Source and Target warehouse at the top for reference.
@ankush @s-aga-r

[ankush]

@marination yeah, the simplest solution could be to ignore None/unset values?

Also, avoid applying this on target warehouse in case of manufacture entry (because of scrap)