From 0db3013c9b078120f21e3a8c568b5395fc4ea4fd Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 6 May 2022 12:09:08 +0530 Subject: [PATCH 1/4] fix: double future qty updates update_qty_in_future_sle is reprocessing rows which are already processed by process_sle_against_current_voucher (cherry picked from commit 7c839c45032e9f39fee76996745a172b44b97c9b) # Conflicts: # erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py --- .../test_stock_ledger_entry.py | 96 +++++++++++++++++++ erpnext/stock/stock_ledger.py | 10 +- 2 files changed, 100 insertions(+), 6 deletions(-) diff --git a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py index 74775b98e392..ba9062611599 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py @@ -683,6 +683,102 @@ def _day(days): # original amount self.assertEqual(50, _get_stock_credit(final_consumption)) +<<<<<<< HEAD +======= + def test_tie_breaking(self): + frappe.flags.dont_execute_stock_reposts = True + self.addCleanup(frappe.flags.pop, "dont_execute_stock_reposts") + + item = make_item().name + warehouse = "_Test Warehouse - _TC" + + posting_date = "2022-01-01" + posting_time = "00:00:01" + sle = frappe.qb.DocType("Stock Ledger Entry") + + def ordered_qty_after_transaction(): + return ( + frappe.qb.from_(sle) + .select("qty_after_transaction") + .where((sle.item_code == item) & (sle.warehouse == warehouse) & (sle.is_cancelled == 0)) + .orderby(CombineDatetime(sle.posting_date, sle.posting_time)) + .orderby(sle.creation) + ).run(pluck=True) + + first = make_stock_entry( + item_code=item, + to_warehouse=warehouse, + qty=10, + posting_time=posting_time, + posting_date=posting_date, + do_not_submit=True, + ) + second = make_stock_entry( + item_code=item, + to_warehouse=warehouse, + qty=1, + posting_date=posting_date, + posting_time=posting_time, + do_not_submit=True, + ) + + first.submit() + second.submit() + + self.assertEqual([10, 11], ordered_qty_after_transaction()) + + first.cancel() + self.assertEqual([1], ordered_qty_after_transaction()) + + backdated = make_stock_entry( + item_code=item, + to_warehouse=warehouse, + qty=1, + posting_date="2021-01-01", + posting_time=posting_time, + ) + self.assertEqual([1, 2], ordered_qty_after_transaction()) + + backdated.cancel() + self.assertEqual([1], ordered_qty_after_transaction()) + + def test_timestamp_clash(self): + + item = make_item().name + warehouse = "_Test Warehouse - _TC" + + reciept = make_stock_entry( + item_code=item, + to_warehouse=warehouse, + qty=100, + rate=10, + posting_date="2021-01-01", + posting_time="01:00:00", + ) + + consumption = make_stock_entry( + item_code=item, + from_warehouse=warehouse, + qty=50, + posting_date="2021-01-01", + posting_time="02:00:00.1234", # ms are possible when submitted without editing posting time + ) + + backdated_receipt = make_stock_entry( + item_code=item, + to_warehouse=warehouse, + qty=100, + posting_date="2021-01-01", + rate=10, + posting_time="02:00:00", # same posting time as consumption but ms part stripped + ) + + try: + backdated_receipt.cancel() + except Exception as e: + self.fail("Double processing of qty for clashing timestamp.") + +>>>>>>> 7c839c4503 (fix: double future qty updates) def create_repack_entry(**args): args = frappe._dict(args) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index d2c10018ba54..c3f9561a1cc4 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -1241,6 +1241,8 @@ def update_qty_in_future_sle(args, allow_negative_stock=False): datetime_limit_condition = "" qty_shift = args.actual_qty + args["time_format"] = "%H:%i:%s" + # find difference/shift in qty caused by stock reconciliation if args.voucher_type == "Stock Reconciliation": qty_shift = get_stock_reco_qty_shift(args) @@ -1261,12 +1263,8 @@ def update_qty_in_future_sle(args, allow_negative_stock=False): and warehouse = %(warehouse)s and voucher_no != %(voucher_no)s and is_cancelled = 0 - and (timestamp(posting_date, posting_time) > timestamp(%(posting_date)s, %(posting_time)s) - or ( - timestamp(posting_date, posting_time) = timestamp(%(posting_date)s, %(posting_time)s) - and creation > %(creation)s - ) - ) + and timestamp(posting_date, time_format(posting_time, %(time_format)s)) + > timestamp(%(posting_date)s, time_format(%(posting_time)s, %(time_format)s)) {datetime_limit_condition} """.format( qty_shift=qty_shift, datetime_limit_condition=datetime_limit_condition From e27fb58130f3300054c3d77c2c943d3c1304d19d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 9 May 2022 11:13:31 +0530 Subject: [PATCH 2/4] fix: sort before picking next stock reco (cherry picked from commit 7e2fbc050a7e83bbb1a1f5f6a3f0df77155dca50) --- .../test_stock_reconciliation.py | 70 +++++++------------ erpnext/stock/stock_ledger.py | 7 +- 2 files changed, 30 insertions(+), 47 deletions(-) diff --git a/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py b/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py index 1596c96bba7e..b8347809fcaa 100644 --- a/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py +++ b/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py @@ -31,6 +31,7 @@ def setUpClass(cls): def tearDown(self): frappe.local.future_sle = {} + frappe.flags.pop("dont_execute_stock_reposts", None) def test_reco_for_fifo(self): self._test_reco_sle_gle("FIFO") @@ -383,6 +384,7 @@ def test_backdated_stock_reco_qty_reposting(self): ------------------------------------------- Var | Doc | Qty | Balance ------------------------------------------- + PR5 | PR | 10 | 10 (posting date: today-4) [backdated] SR5 | Reco | 0 | 8 (posting date: today-4) [backdated] PR1 | PR | 10 | 18 (posting date: today-3) PR2 | PR | 1 | 19 (posting date: today-2) @@ -392,6 +394,14 @@ def test_backdated_stock_reco_qty_reposting(self): item_code = make_item().name warehouse = "_Test Warehouse - _TC" + frappe.flags.dont_execute_stock_reposts = True + + def assertBalance(doc, qty_after_transaction): + sle_balance = frappe.db.get_value( + "Stock Ledger Entry", {"voucher_no": doc.name, "is_cancelled": 0}, "qty_after_transaction" + ) + self.assertEqual(sle_balance, qty_after_transaction) + pr1 = make_purchase_receipt( item_code=item_code, warehouse=warehouse, qty=10, rate=100, posting_date=add_days(nowdate(), -3) ) @@ -401,62 +411,37 @@ def test_backdated_stock_reco_qty_reposting(self): pr3 = make_purchase_receipt( item_code=item_code, warehouse=warehouse, qty=1, rate=100, posting_date=nowdate() ) - - pr1_balance = frappe.db.get_value( - "Stock Ledger Entry", {"voucher_no": pr1.name, "is_cancelled": 0}, "qty_after_transaction" - ) - pr3_balance = frappe.db.get_value( - "Stock Ledger Entry", {"voucher_no": pr3.name, "is_cancelled": 0}, "qty_after_transaction" - ) - self.assertEqual(pr1_balance, 10) - self.assertEqual(pr3_balance, 12) + assertBalance(pr1, 10) + assertBalance(pr3, 12) # post backdated stock reco in between sr4 = create_stock_reconciliation( item_code=item_code, warehouse=warehouse, qty=6, rate=100, posting_date=add_days(nowdate(), -1) ) - pr3_balance = frappe.db.get_value( - "Stock Ledger Entry", {"voucher_no": pr3.name, "is_cancelled": 0}, "qty_after_transaction" - ) - self.assertEqual(pr3_balance, 7) + assertBalance(pr3, 7) # post backdated stock reco at the start sr5 = create_stock_reconciliation( item_code=item_code, warehouse=warehouse, qty=8, rate=100, posting_date=add_days(nowdate(), -4) ) - pr1_balance = frappe.db.get_value( - "Stock Ledger Entry", {"voucher_no": pr1.name, "is_cancelled": 0}, "qty_after_transaction" - ) - pr2_balance = frappe.db.get_value( - "Stock Ledger Entry", {"voucher_no": pr2.name, "is_cancelled": 0}, "qty_after_transaction" - ) - sr4_balance = frappe.db.get_value( - "Stock Ledger Entry", {"voucher_no": sr4.name, "is_cancelled": 0}, "qty_after_transaction" + assertBalance(pr1, 18) + assertBalance(pr2, 19) + assertBalance(sr4, 6) # check if future stock reco is unaffected + + # Make a backdated receipt and check only entries till first SR are affected + pr5 = make_purchase_receipt( + item_code=item_code, warehouse=warehouse, qty=10, rate=100, posting_date=add_days(nowdate(), -5) ) - self.assertEqual(pr1_balance, 18) - self.assertEqual(pr2_balance, 19) - self.assertEqual(sr4_balance, 6) # check if future stock reco is unaffected + assertBalance(pr5, 10) + # check if future stock reco is unaffected + assertBalance(sr4, 6) + assertBalance(sr5, 8) # cancel backdated stock reco and check future impact sr5.cancel() - pr1_balance = frappe.db.get_value( - "Stock Ledger Entry", {"voucher_no": pr1.name, "is_cancelled": 0}, "qty_after_transaction" - ) - pr2_balance = frappe.db.get_value( - "Stock Ledger Entry", {"voucher_no": pr2.name, "is_cancelled": 0}, "qty_after_transaction" - ) - sr4_balance = frappe.db.get_value( - "Stock Ledger Entry", {"voucher_no": sr4.name, "is_cancelled": 0}, "qty_after_transaction" - ) - self.assertEqual(pr1_balance, 10) - self.assertEqual(pr2_balance, 11) - self.assertEqual(sr4_balance, 6) # check if future stock reco is unaffected - - # teardown - sr4.cancel() - pr3.cancel() - pr2.cancel() - pr1.cancel() + assertBalance(pr1, 10) + assertBalance(pr2, 11) + assertBalance(sr4, 6) # check if future stock reco is unaffected @change_settings("Stock Settings", {"allow_negative_stock": 0}) def test_backdated_stock_reco_future_negative_stock(self): @@ -562,7 +547,6 @@ def test_intermediate_sr_bin_update(self): # repost will make this test useless, qty should update in realtime without reposts frappe.flags.dont_execute_stock_reposts = True - self.addCleanup(frappe.flags.pop, "dont_execute_stock_reposts") item_code = make_item().name warehouse = "_Test Warehouse - _TC" diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index c3f9561a1cc4..7bdf2078c4a8 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -1255,7 +1255,7 @@ def update_qty_in_future_sle(args, allow_negative_stock=False): datetime_limit_condition = get_datetime_limit_condition(detail) frappe.db.sql( - """ + f""" update `tabStock Ledger Entry` set qty_after_transaction = qty_after_transaction + {qty_shift} where @@ -1266,9 +1266,7 @@ def update_qty_in_future_sle(args, allow_negative_stock=False): and timestamp(posting_date, time_format(posting_time, %(time_format)s)) > timestamp(%(posting_date)s, time_format(%(posting_time)s, %(time_format)s)) {datetime_limit_condition} - """.format( - qty_shift=qty_shift, datetime_limit_condition=datetime_limit_condition - ), + """, args, ) @@ -1319,6 +1317,7 @@ def get_next_stock_reco(args): and creation > %(creation)s ) ) + order by timestamp(posting_date, posting_time) asc, creation asc limit 1 """, args, From 6312938fed3b1c436456e2b893f18f651693c6df Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 10 May 2022 14:50:14 +0530 Subject: [PATCH 3/4] chore: conflicts --- .../test_stock_ledger_entry.py | 60 ------------------- 1 file changed, 60 deletions(-) diff --git a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py index ba9062611599..4d8ee777e58d 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py @@ -683,65 +683,6 @@ def _day(days): # original amount self.assertEqual(50, _get_stock_credit(final_consumption)) -<<<<<<< HEAD -======= - def test_tie_breaking(self): - frappe.flags.dont_execute_stock_reposts = True - self.addCleanup(frappe.flags.pop, "dont_execute_stock_reposts") - - item = make_item().name - warehouse = "_Test Warehouse - _TC" - - posting_date = "2022-01-01" - posting_time = "00:00:01" - sle = frappe.qb.DocType("Stock Ledger Entry") - - def ordered_qty_after_transaction(): - return ( - frappe.qb.from_(sle) - .select("qty_after_transaction") - .where((sle.item_code == item) & (sle.warehouse == warehouse) & (sle.is_cancelled == 0)) - .orderby(CombineDatetime(sle.posting_date, sle.posting_time)) - .orderby(sle.creation) - ).run(pluck=True) - - first = make_stock_entry( - item_code=item, - to_warehouse=warehouse, - qty=10, - posting_time=posting_time, - posting_date=posting_date, - do_not_submit=True, - ) - second = make_stock_entry( - item_code=item, - to_warehouse=warehouse, - qty=1, - posting_date=posting_date, - posting_time=posting_time, - do_not_submit=True, - ) - - first.submit() - second.submit() - - self.assertEqual([10, 11], ordered_qty_after_transaction()) - - first.cancel() - self.assertEqual([1], ordered_qty_after_transaction()) - - backdated = make_stock_entry( - item_code=item, - to_warehouse=warehouse, - qty=1, - posting_date="2021-01-01", - posting_time=posting_time, - ) - self.assertEqual([1, 2], ordered_qty_after_transaction()) - - backdated.cancel() - self.assertEqual([1], ordered_qty_after_transaction()) - def test_timestamp_clash(self): item = make_item().name @@ -778,7 +719,6 @@ def test_timestamp_clash(self): except Exception as e: self.fail("Double processing of qty for clashing timestamp.") ->>>>>>> 7c839c4503 (fix: double future qty updates) def create_repack_entry(**args): args = frappe._dict(args) From 4d682face28dbd9a372454a2efbc0293e6922d6a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 6 May 2022 13:42:09 +0530 Subject: [PATCH 4/4] chore: remove datettime formatting from debug report This hides some information that would otherwise help during debugging (cherry picked from commit ae842d81455ccd27045a9a9f88cc9eb819e3f5e4) --- .../stock_ledger_invariant_check.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py b/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py index 98f0387a0fce..208b265afb83 100644 --- a/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py +++ b/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py @@ -105,17 +105,17 @@ def get_columns(): }, { "fieldname": "posting_date", - "fieldtype": "Date", + "fieldtype": "Data", "label": "Posting Date", }, { "fieldname": "posting_time", - "fieldtype": "Time", + "fieldtype": "Data", "label": "Posting Time", }, { "fieldname": "creation", - "fieldtype": "Datetime", + "fieldtype": "Data", "label": "Creation", }, {