Skip to content

Commit

Permalink
fix: dependent gle reposting (backport #30726) (#30772)
Browse files Browse the repository at this point in the history
* refactor: repost error handling

(cherry picked from commit afc5a55)

* test: dependent GL entry reposting

(cherry picked from commit a2af2da)

# Conflicts:
#	erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py

* fix: dependent GLE reposting

(cherry picked from commit ecdb493)

# Conflicts:
#	erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.json
#	erpnext/stock/stock_ledger.py

* test: repost queue progress

(cherry picked from commit 8f51954)

* test: use disposable item codes in tests

dependency causes flake

(cherry picked from commit d2882ea)

* fix: correct sorting while updating bin

(cherry picked from commit b24920c)

# Conflicts:
#	erpnext/stock/doctype/bin/bin.py

* fix: sort stock vouchers before reposting GLE

(cherry picked from commit 700e864)

* test: discard local future SLE cache between tests

(cherry picked from commit 9734329)

* chore: conflicts

Co-authored-by: Ankush Menat <ankush@frappe.io>
  • Loading branch information
mergify[bot] and ankush authored Apr 21, 2022
1 parent 32cff94 commit a6d0938
Show file tree
Hide file tree
Showing 9 changed files with 219 additions and 51 deletions.
28 changes: 27 additions & 1 deletion erpnext/accounts/test/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
import unittest

import frappe
from frappe.test_runner import make_test_objects

from erpnext.accounts.party import get_party_shipping_address
from erpnext.accounts.utils import get_future_stock_vouchers, get_voucherwise_gl_entries
from erpnext.accounts.utils import (
get_future_stock_vouchers,
get_voucherwise_gl_entries,
sort_stock_vouchers_by_posting_date,
)
from erpnext.stock.doctype.item.test_item import make_item
from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_purchase_receipt
from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry


class TestUtils(unittest.TestCase):
Expand Down Expand Up @@ -47,6 +54,25 @@ def test_get_voucher_wise_gl_entry(self):
msg="get_voucherwise_gl_entries not returning expected GLes",
)

def test_stock_voucher_sorting(self):
vouchers = []

item = make_item().name

stock_entry = {"item": item, "to_warehouse": "_Test Warehouse - _TC", "qty": 1, "rate": 10}

se1 = make_stock_entry(posting_date="2022-01-01", **stock_entry)
se2 = make_stock_entry(posting_date="2022-02-01", **stock_entry)
se3 = make_stock_entry(posting_date="2022-03-01", **stock_entry)

for doc in (se1, se2, se3):
vouchers.append((doc.doctype, doc.name))

vouchers.append(("Stock Entry", "Wat"))

sorted_vouchers = sort_stock_vouchers_by_posting_date(list(reversed(vouchers)))
self.assertEqual(sorted_vouchers, vouchers)


ADDRESS_RECORDS = [
{
Expand Down
27 changes: 27 additions & 0 deletions erpnext/accounts/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@


from json import loads
from typing import List, Tuple

import frappe
import frappe.defaults
Expand Down Expand Up @@ -1123,13 +1124,18 @@ def update_gl_entries_after(
def repost_gle_for_stock_vouchers(
stock_vouchers, posting_date, company=None, warehouse_account=None
):
if not stock_vouchers:
return

def _delete_gl_entries(voucher_type, voucher_no):
frappe.db.sql(
"""delete from `tabGL Entry`
where voucher_type=%s and voucher_no=%s""",
(voucher_type, voucher_no),
)

stock_vouchers = sort_stock_vouchers_by_posting_date(stock_vouchers)

if not warehouse_account:
warehouse_account = get_warehouse_account_map(company)

Expand All @@ -1150,6 +1156,27 @@ def _delete_gl_entries(voucher_type, voucher_no):
_delete_gl_entries(voucher_type, voucher_no)


def sort_stock_vouchers_by_posting_date(
stock_vouchers: List[Tuple[str, str]]
) -> List[Tuple[str, str]]:
sle = frappe.qb.DocType("Stock Ledger Entry")
voucher_nos = [v[1] for v in stock_vouchers]

sles = (
frappe.qb.from_(sle)
.select(sle.voucher_type, sle.voucher_no, sle.posting_date, sle.posting_time, sle.creation)
.where((sle.is_cancelled == 0) & (sle.voucher_no.isin(voucher_nos)))
.groupby(sle.voucher_type, sle.voucher_no)
).run(as_dict=True)
sorted_vouchers = [(sle.voucher_type, sle.voucher_no) for sle in sles]

unknown_vouchers = set(stock_vouchers) - set(sorted_vouchers)
if unknown_vouchers:
sorted_vouchers.extend(unknown_vouchers)

return sorted_vouchers


def get_future_stock_vouchers(
posting_date, posting_time, for_warehouses=None, for_items=None, company=None
):
Expand Down
27 changes: 14 additions & 13 deletions erpnext/stock/doctype/bin/bin.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

import frappe
from frappe.model.document import Document
from frappe.query_builder import Order
from frappe.query_builder.functions import CombineDatetime
from frappe.utils import flt


Expand Down Expand Up @@ -134,24 +136,23 @@ def update_qty(bin_name, args):

bin_details = get_bin_details(bin_name)
# actual qty is already updated by processing current voucher
actual_qty = bin_details.actual_qty
actual_qty = bin_details.actual_qty or 0.0
sle = frappe.qb.DocType("Stock Ledger Entry")

# actual qty is not up to date in case of backdated transaction
if future_sle_exists(args):
actual_qty = (
frappe.db.get_value(
"Stock Ledger Entry",
filters={
"item_code": args.get("item_code"),
"warehouse": args.get("warehouse"),
"is_cancelled": 0,
},
fieldname="qty_after_transaction",
order_by="posting_date desc, posting_time desc, creation desc",
)
or 0.0
last_sle_qty = (
frappe.qb.from_(sle)
.select(sle.qty_after_transaction)
.where((sle.item_code == args.get("item_code")) & (sle.warehouse == args.get("warehouse")))
.orderby(CombineDatetime(sle.posting_date, sle.posting_time), order=Order.desc)
.orderby(sle.creation, order=Order.desc)
.run()
)

if last_sle_qty:
actual_qty = last_sle_qty[0][0]

ordered_qty = flt(bin_details.ordered_qty) + flt(args.get("ordered_qty"))
reserved_qty = flt(bin_details.reserved_qty) + flt(args.get("reserved_qty"))
indented_qty = flt(bin_details.indented_qty) + flt(args.get("indented_qty"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"error_section",
"error_log",
"items_to_be_repost",
"affected_transactions",
"distinct_item_and_warehouse",
"current_index"
],
Expand Down Expand Up @@ -172,12 +173,20 @@
"no_copy": 1,
"print_hide": 1,
"read_only": 1
},
{
"fieldname": "affected_transactions",
"fieldtype": "Code",
"hidden": 1,
"label": "Affected Transactions",
"no_copy": 1,
"read_only": 1
}
],
"index_web_pages_for_search": 1,
"is_submittable": 1,
"links": [],
"modified": "2022-01-18 10:57:33.450907",
"modified": "2022-04-18 14:08:08.821602",
"modified_by": "Administrator",
"module": "Stock",
"name": "Repost Item Valuation",
Expand Down Expand Up @@ -229,4 +238,4 @@
"sort_field": "modified",
"sort_order": "DESC",
"states": []
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@
from frappe.model.document import Document
from frappe.utils import cint, get_link_to_form, get_weekday, now, nowtime
from frappe.utils.user import get_users_with_role
from rq.timeouts import JobTimeoutException

import erpnext
from erpnext.accounts.utils import update_gl_entries_after
from erpnext.stock.stock_ledger import get_items_to_be_repost, repost_future_sle
from erpnext.accounts.utils import get_future_stock_vouchers, repost_gle_for_stock_vouchers
from erpnext.stock.stock_ledger import (
get_affected_transactions,
get_items_to_be_repost,
repost_future_sle,
)


class RepostItemValuation(Document):
Expand Down Expand Up @@ -129,12 +132,12 @@ def repost(doc):

doc.set_status("Completed")

except (Exception, JobTimeoutException):
except Exception:
frappe.db.rollback()
traceback = frappe.get_traceback()
frappe.log_error(traceback)

message = frappe.message_log.pop()
message = frappe.message_log.pop() if frappe.message_log else ""
if traceback:
message += "<br>" + "Traceback: <br>" + traceback
frappe.db.set_value(doc.doctype, doc.name, "error_log", message)
Expand Down Expand Up @@ -170,34 +173,54 @@ def repost_sl_entries(doc):
],
allow_negative_stock=doc.allow_negative_stock,
via_landed_cost_voucher=doc.via_landed_cost_voucher,
doc=doc,
)


def repost_gl_entries(doc):
if not cint(erpnext.is_perpetual_inventory_enabled(doc.company)):
return

# directly modified transactions
directly_dependent_transactions = _get_directly_dependent_vouchers(doc)
repost_affected_transaction = get_affected_transactions(doc)
repost_gle_for_stock_vouchers(
directly_dependent_transactions + list(repost_affected_transaction),
doc.posting_date,
doc.company,
)


def _get_directly_dependent_vouchers(doc):
"""Get stock vouchers that are directly affected by reposting
i.e. any one item-warehouse is present in the stock transaction"""

items = set()
warehouses = set()

if doc.based_on == "Transaction":
ref_doc = frappe.get_doc(doc.voucher_type, doc.voucher_no)
doc_items, doc_warehouses = ref_doc.get_items_and_warehouses()
items.update(doc_items)
warehouses.update(doc_warehouses)

sles = get_items_to_be_repost(doc.voucher_type, doc.voucher_no)
sle_items = [sle.item_code for sle in sles]
sle_warehouse = [sle.warehouse for sle in sles]

items = list(set(doc_items).union(set(sle_items)))
warehouses = list(set(doc_warehouses).union(set(sle_warehouse)))
sle_items = {sle.item_code for sle in sles}
sle_warehouses = {sle.warehouse for sle in sles}
items.update(sle_items)
warehouses.update(sle_warehouses)
else:
items = [doc.item_code]
warehouses = [doc.warehouse]

update_gl_entries_after(
doc.posting_date,
doc.posting_time,
for_warehouses=warehouses,
for_items=items,
items.add(doc.item_code)
warehouses.add(doc.warehouse)

affected_vouchers = get_future_stock_vouchers(
posting_date=doc.posting_date,
posting_time=doc.posting_time,
for_warehouses=list(warehouses),
for_items=list(items),
company=doc.company,
)
return affected_vouchers


def notify_error_to_stock_managers(doc, traceback):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,3 +186,10 @@ def test_prevention_of_cancelled_transaction_riv(self):
riv.db_set("status", "Skipped")
riv.reload()
riv.cancel() # it should cancel now

def test_queue_progress_serialization(self):
# Make sure set/tuple -> list behaviour is retained.
self.assertEqual(
[["a", "b"], ["c", "d"]],
sorted(frappe.parse_json(frappe.as_json(set([("a", "b"), ("c", "d")])))),
)
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from frappe.custom.doctype.property_setter.property_setter import make_property_setter
from frappe.tests.utils import FrappeTestCase, change_settings
from frappe.utils import add_days, today
from frappe.utils.data import add_to_date

from erpnext.accounts.doctype.gl_entry.gl_entry import rename_gle_sle_docs
from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note
Expand Down Expand Up @@ -624,6 +625,64 @@ def test_negative_fifo_valuation(self):
receipt2 = make_stock_entry(item_code=item, target=warehouse, qty=15, rate=15)
self.assertSLEs(receipt2, [{"stock_queue": [[5, 15]], "stock_value_difference": 175}])

def test_dependent_gl_entry_reposting(self):
def _get_stock_credit(doc):
return frappe.db.get_value(
"GL Entry",
{
"voucher_no": doc.name,
"voucher_type": doc.doctype,
"is_cancelled": 0,
"account": "Stock In Hand - TCP1",
},
"sum(credit)",
)

def _day(days):
return add_to_date(date=today(), days=days)

item = make_item().name
A = "Stores - TCP1"
B = "Work In Progress - TCP1"
C = "Finished Goods - TCP1"

make_stock_entry(item_code=item, to_warehouse=A, qty=5, rate=10, posting_date=_day(0))
make_stock_entry(item_code=item, from_warehouse=A, to_warehouse=B, qty=5, posting_date=_day(1))
depdendent_consumption = make_stock_entry(
item_code=item, from_warehouse=B, qty=5, posting_date=_day(2)
)
self.assertEqual(50, _get_stock_credit(depdendent_consumption))

# backdated receipt - should trigger GL repost of all previous stock entries
bd_receipt = make_stock_entry(
item_code=item, to_warehouse=A, qty=5, rate=20, posting_date=_day(-1)
)
self.assertEqual(100, _get_stock_credit(depdendent_consumption))

# cancelling receipt should reset it back
bd_receipt.cancel()
self.assertEqual(50, _get_stock_credit(depdendent_consumption))

bd_receipt2 = make_stock_entry(
item_code=item, to_warehouse=A, qty=2, rate=20, posting_date=_day(-2)
)
# total as per FIFO -> 2 * 20 + 3 * 10 = 70
self.assertEqual(70, _get_stock_credit(depdendent_consumption))

# transfer WIP material to final destination and consume it all
depdendent_consumption.cancel()
make_stock_entry(item_code=item, from_warehouse=B, to_warehouse=C, qty=5, posting_date=_day(3))
final_consumption = make_stock_entry(
item_code=item, from_warehouse=C, qty=5, posting_date=_day(4)
)
# exact amount gets consumed
self.assertEqual(70, _get_stock_credit(final_consumption))

# cancel original backdated receipt - should repost A -> B -> C
bd_receipt2.cancel()
# original amount
self.assertEqual(50, _get_stock_credit(final_consumption))


def create_repack_entry(**args):
args = frappe._dict(args)
Expand Down
Loading

0 comments on commit a6d0938

Please sign in to comment.