From 64f3b4d68bc3872d7fa49435e8d27ad2ea2d3b4e Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 3 May 2022 13:00:00 +0530 Subject: [PATCH] fix: Wrap SLE actual_qty in `flt` to avoid NoneType operation - Since Batch cancellation SLEs do not set qtys (will fix separately), `merge_similar_entries` gets `actual_qty` as None - This causes NoneType operation error on tests that cancel batch-serial reco - Modified tests to avoid using commit and rollback explicitly (cherry picked from commit d53228b153437f9dedcb1229bf579411f3122729) --- .../doctype/stock_entry/test_stock_entry.py | 12 ++-- .../stock_reconciliation.py | 2 +- .../test_stock_reconciliation.py | 67 ++++++------------- 3 files changed, 29 insertions(+), 52 deletions(-) diff --git a/erpnext/stock/doctype/stock_entry/test_stock_entry.py b/erpnext/stock/doctype/stock_entry/test_stock_entry.py index 09f6b7bc01..7dae283ac4 100644 --- a/erpnext/stock/doctype/stock_entry/test_stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/test_stock_entry.py @@ -655,9 +655,9 @@ class TestStockEntry(FrappeTestCase): def test_serial_batch_item_stock_entry(self): """ Behaviour: 1) Submit Stock Entry (Receipt) with Serial & Batched Item - 2) Cancel same Stock Entry + 2) Cancel same Stock Entry Expected Result: 1) Batch is created with Reference in Serial No - 2) Batch is deleted and Serial No is Inactive + 2) Batch is deleted and Serial No is Inactive """ from erpnext.stock.doctype.batch.batch import get_batch_qty @@ -696,10 +696,10 @@ class TestStockEntry(FrappeTestCase): def test_serial_batch_item_qty_deduction(self): """ Behaviour: Create 2 Stock Entries, both adding Serial Nos to same batch - Expected Result: 1) Cancelling first Stock Entry (origin transaction of created batch) - should throw a LinkExistsError - 2) Cancelling second Stock Entry should make Serial Nos that are, linked to mentioned batch - and in that transaction only, Inactive. + Expected: 1) Cancelling first Stock Entry (origin transaction of created batch) + should throw a LinkExistsError + 2) Cancelling second Stock Entry should make Serial Nos that are, linked to mentioned batch + and in that transaction only, Inactive. """ from erpnext.stock.doctype.batch.batch import get_batch_qty diff --git a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py index cb9aec2563..bd60cf0a5a 100644 --- a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py +++ b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py @@ -457,7 +457,7 @@ class StockReconciliation(StockController): key = (d.item_code, d.warehouse) if key not in merge_similar_entries: - d.total_amount = d.actual_qty * d.valuation_rate + d.total_amount = flt(d.actual_qty) * d.valuation_rate merge_similar_entries[key] = d elif d.serial_no: data = merge_similar_entries[key] diff --git a/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py b/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py index 02acdb3480..6d5340ef11 100644 --- a/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py +++ b/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py @@ -289,17 +289,13 @@ class TestStockReconciliation(FrappeTestCase): stock_doc.cancel() def test_stock_reco_for_serial_and_batch_item(self): - item = frappe.db.exists("Item", {"item_name": "Batched and Serialised Item"}) - if not item: - item = create_item("Batched and Serialised Item") - item.has_batch_no = 1 - item.create_new_batch = 1 - item.has_serial_no = 1 - item.batch_number_series = "B-BATCH-.##" - item.serial_no_series = "S-.####" - item.save() - else: - item = frappe.get_doc("Item", {"item_name": "Batched and Serialised Item"}) + item = create_item("_TestBatchSerialItemReco") + item.has_batch_no = 1 + item.create_new_batch = 1 + item.has_serial_no = 1 + item.batch_number_series = "TBS-BATCH-.##" + item.serial_no_series = "TBS-.####" + item.save() warehouse = "_Test Warehouse for Stock Reco2 - _TC" @@ -316,35 +312,25 @@ class TestStockReconciliation(FrappeTestCase): self.assertEqual(frappe.db.get_value("Serial No", serial_nos[0], "status"), "Inactive") self.assertEqual(frappe.db.exists("Batch", batch_no), None) - if frappe.db.exists("Serial No", serial_nos[0]): - frappe.delete_doc("Serial No", serial_nos[0]) - def test_stock_reco_for_serial_and_batch_item_with_future_dependent_entry(self): """ Behaviour: 1) Create Stock Reconciliation, which will be the origin document - of a new batch having a serial no - 2) Create a Stock Entry that adds a serial no to the same batch following this + of a new batch having a serial no + 2) Create a Stock Entry that adds a serial no to the same batch following this Stock Reconciliation - 3) Cancel Stock Reconciliation - 4) Cancel Stock Entry - Expected Result: 3) Cancelling the Stock Reco throws a LinkExistsError since - Stock Entry is dependent on the batch involved - 4) Serial No only in the Stock Entry is Inactive and Batch qty decreases + 3) Cancel Stock Entry + Expected Result: 3) Serial No only in the Stock Entry is Inactive and Batch qty decreases """ from erpnext.stock.doctype.batch.batch import get_batch_qty from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry - item = frappe.db.exists("Item", {"item_name": "Batched and Serialised Item"}) - if not item: - item = create_item("Batched and Serialised Item") - item.has_batch_no = 1 - item.create_new_batch = 1 - item.has_serial_no = 1 - item.batch_number_series = "B-BATCH-.##" - item.serial_no_series = "S-.####" - item.save() - else: - item = frappe.get_doc("Item", {"item_name": "Batched and Serialised Item"}) + item = create_item("_TestBatchSerialItemDependentReco") + item.has_batch_no = 1 + item.create_new_batch = 1 + item.has_serial_no = 1 + item.batch_number_series = "TBSD-BATCH-.##" + item.serial_no_series = "TBSD-.####" + item.save() warehouse = "_Test Warehouse for Stock Reco2 - _TC" @@ -352,7 +338,7 @@ class TestStockReconciliation(FrappeTestCase): item_code=item.item_code, warehouse=warehouse, qty=1, rate=100 ) batch_no = stock_reco.items[0].batch_no - serial_no = get_serial_nos(stock_reco.items[0].serial_no)[0] + reco_serial_no = get_serial_nos(stock_reco.items[0].serial_no)[0] stock_entry = make_stock_entry( item_code=item.item_code, target=warehouse, qty=1, basic_rate=100, batch_no=batch_no @@ -362,12 +348,8 @@ class TestStockReconciliation(FrappeTestCase): # Check Batch qty after 2 transactions batch_qty = get_batch_qty(batch_no, warehouse, item.item_code) self.assertEqual(batch_qty, 2) - frappe.db.commit() - - # Cancelling Origin Document of Batch - self.assertRaises(frappe.LinkExistsError, stock_reco.cancel) - frappe.db.rollback() + # Cancel latest stock document stock_entry.cancel() # Check Batch qty after cancellation @@ -375,20 +357,15 @@ class TestStockReconciliation(FrappeTestCase): self.assertEqual(batch_qty, 1) # Check if Serial No from Stock Reconcilation is intact - self.assertEqual(frappe.db.get_value("Serial No", serial_no, "batch_no"), batch_no) - self.assertEqual(frappe.db.get_value("Serial No", serial_no, "status"), "Active") + self.assertEqual(frappe.db.get_value("Serial No", reco_serial_no, "batch_no"), batch_no) + self.assertEqual(frappe.db.get_value("Serial No", reco_serial_no, "status"), "Active") # Check if Serial No from Stock Entry is Unlinked and Inactive self.assertEqual(frappe.db.get_value("Serial No", serial_no_2, "batch_no"), None) self.assertEqual(frappe.db.get_value("Serial No", serial_no_2, "status"), "Inactive") - stock_reco.load_from_db() stock_reco.cancel() - for sn in (serial_no, serial_no_2): - if frappe.db.exists("Serial No", sn): - frappe.delete_doc("Serial No", sn) - def test_customer_provided_items(self): item_code = "Stock-Reco-customer-Item-100" create_item(