mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-04 18:23:05 +00:00
fix: skip redundant reposting of dependent items (#57092)
* fix: skip redundant reposting of dependent items Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: use earliest cascade datetime and batch repost item lookup Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -10,7 +10,7 @@ from frappe.exceptions import QueryDeadlockError, QueryTimeoutError
|
||||
from frappe.model.document import Document
|
||||
from frappe.query_builder import DocType, Interval
|
||||
from frappe.query_builder.functions import CombineDatetime, Max, Now
|
||||
from frappe.utils import cint, get_link_to_form, get_weekday, getdate, now, nowtime
|
||||
from frappe.utils import cint, get_datetime, get_link_to_form, get_weekday, getdate, now, nowtime
|
||||
from frappe.utils.user import get_users_with_role
|
||||
from rq.timeouts import JobTimeoutException
|
||||
|
||||
@@ -19,6 +19,7 @@ from erpnext.accounts.services.gl_validator import validate_accounting_period
|
||||
from erpnext.accounts.utils import get_future_stock_vouchers, repost_gle_for_stock_vouchers
|
||||
from erpnext.stock.stock_ledger import (
|
||||
get_affected_transactions,
|
||||
get_item_wh_first_reposted_from_reposting_data,
|
||||
get_items_to_be_repost,
|
||||
repost_future_sle,
|
||||
)
|
||||
@@ -343,6 +344,21 @@ class RepostItemValuation(Document):
|
||||
)
|
||||
).run()
|
||||
|
||||
def skip_reposts_covered_by_dependents(self):
|
||||
if self.repost_only_accounting_ledgers:
|
||||
return
|
||||
|
||||
coverage = get_item_wh_first_reposted_from_reposting_data(self)
|
||||
if not coverage:
|
||||
return
|
||||
|
||||
source_datetime = get_combine_datetime(self.posting_date, self.posting_time)
|
||||
mark_covered_item_reposts(self.name, coverage, source_datetime)
|
||||
|
||||
affected = get_affected_transactions(self)
|
||||
if affected:
|
||||
mark_covered_transaction_reposts(self, coverage, affected)
|
||||
|
||||
def _recalculate_valuation_rate(self):
|
||||
doc = frappe.get_doc(self.voucher_type, self.voucher_no)
|
||||
if doc.get("is_internal_supplier"):
|
||||
@@ -376,6 +392,130 @@ def bulk_restart_reposting(names: str | list):
|
||||
frappe.msgprint(_("Repost Item Valuation restarted for selected failed records."))
|
||||
|
||||
|
||||
def repost_coverage_cache_key(name):
|
||||
return f"riv_dependent_coverage::{name}"
|
||||
|
||||
|
||||
def get_queued_item_reposts(source_name, item_codes):
|
||||
return frappe.get_all(
|
||||
"Repost Item Valuation",
|
||||
filters={
|
||||
"name": ("!=", source_name),
|
||||
"based_on": "Item and Warehouse",
|
||||
"status": "Queued",
|
||||
"docstatus": 1,
|
||||
"recalculate_valuation_rate": 0,
|
||||
"recreate_stock_ledgers": 0,
|
||||
"via_landed_cost_voucher": 0,
|
||||
"item_code": ("in", item_codes),
|
||||
},
|
||||
fields=["name", "item_code", "warehouse", "posting_date", "posting_time"],
|
||||
)
|
||||
|
||||
|
||||
def mark_covered_item_reposts(source_name, coverage, source_datetime):
|
||||
item_codes = {item_code for item_code, _ in coverage}
|
||||
|
||||
for row in get_queued_item_reposts(source_name, list(item_codes)):
|
||||
from_datetime = coverage.get((row.item_code, row.warehouse))
|
||||
if not from_datetime:
|
||||
continue
|
||||
|
||||
row_datetime = get_combine_datetime(row.posting_date, row.posting_time)
|
||||
if get_datetime(row_datetime) < get_datetime(source_datetime):
|
||||
continue
|
||||
|
||||
if get_datetime(from_datetime) <= get_datetime(row_datetime):
|
||||
frappe.db.set_value("Repost Item Valuation", row.name, "status", "Skipped")
|
||||
|
||||
|
||||
def get_queued_transaction_reposts(source_name, voucher_nos):
|
||||
return frappe.get_all(
|
||||
"Repost Item Valuation",
|
||||
filters={
|
||||
"name": ("!=", source_name),
|
||||
"based_on": "Transaction",
|
||||
"status": "Queued",
|
||||
"docstatus": 1,
|
||||
"repost_only_accounting_ledgers": 0,
|
||||
"recalculate_valuation_rate": 0,
|
||||
"recreate_stock_ledgers": 0,
|
||||
"via_landed_cost_voucher": 0,
|
||||
"voucher_no": ("in", list(voucher_nos)),
|
||||
},
|
||||
fields=["name", "voucher_type", "voucher_no", "posting_date", "posting_time"],
|
||||
)
|
||||
|
||||
|
||||
def accumulate_repost_coverage(row_name, coverage, row_datetime):
|
||||
cache_key = repost_coverage_cache_key(row_name)
|
||||
acc = frappe.cache().get_value(cache_key) or {}
|
||||
|
||||
for key, from_datetime in coverage.items():
|
||||
if get_datetime(from_datetime) > get_datetime(row_datetime):
|
||||
continue
|
||||
|
||||
existing = acc.get(key)
|
||||
if not existing or get_datetime(from_datetime) < get_datetime(existing):
|
||||
acc[key] = from_datetime
|
||||
|
||||
frappe.cache().set_value(cache_key, acc, expires_in_sec=86400)
|
||||
return acc
|
||||
|
||||
|
||||
def get_repost_items_by_voucher(rows):
|
||||
voucher_nos = {row.voucher_no for row in rows}
|
||||
if not voucher_nos:
|
||||
return {}
|
||||
|
||||
items_by_voucher = {}
|
||||
for sle in frappe.get_all(
|
||||
"Stock Ledger Entry",
|
||||
filters={"voucher_no": ("in", list(voucher_nos))},
|
||||
fields=["voucher_type", "voucher_no", "item_code", "warehouse"],
|
||||
distinct=True,
|
||||
):
|
||||
items_by_voucher.setdefault((sle.voucher_type, sle.voucher_no), set()).add(
|
||||
(sle.item_code, sle.warehouse)
|
||||
)
|
||||
|
||||
return items_by_voucher
|
||||
|
||||
|
||||
def is_transaction_repost_covered(items, acc, row_datetime):
|
||||
if not items:
|
||||
return False
|
||||
|
||||
for key in items:
|
||||
covered = acc.get(key)
|
||||
if not covered or get_datetime(covered) > get_datetime(row_datetime):
|
||||
return False
|
||||
|
||||
return True
|
||||
|
||||
|
||||
def mark_covered_transaction_reposts(source, coverage, affected):
|
||||
source_datetime = get_combine_datetime(source.posting_date, source.posting_time)
|
||||
voucher_nos = {voucher_no for _, voucher_no in affected}
|
||||
|
||||
rows = get_queued_transaction_reposts(source.name, voucher_nos)
|
||||
items_by_voucher = get_repost_items_by_voucher(rows)
|
||||
|
||||
for row in rows:
|
||||
if (row.voucher_type, row.voucher_no) not in affected:
|
||||
continue
|
||||
|
||||
row_datetime = get_combine_datetime(row.posting_date, row.posting_time)
|
||||
if get_datetime(row_datetime) < get_datetime(source_datetime):
|
||||
continue
|
||||
|
||||
acc = accumulate_repost_coverage(row.name, coverage, row_datetime)
|
||||
items = items_by_voucher.get((row.voucher_type, row.voucher_no))
|
||||
if is_transaction_repost_covered(items, acc, row_datetime):
|
||||
frappe.db.set_value("Repost Item Valuation", row.name, "status", "Skipped")
|
||||
frappe.cache().delete_value(repost_coverage_cache_key(row.name))
|
||||
|
||||
|
||||
def on_doctype_update():
|
||||
frappe.db.add_index("Repost Item Valuation", ["warehouse", "item_code"], "item_warehouse")
|
||||
|
||||
@@ -407,6 +547,8 @@ def repost(doc):
|
||||
|
||||
repost_gl_entries(doc)
|
||||
|
||||
doc.skip_reposts_covered_by_dependents()
|
||||
|
||||
doc.set_status("Completed")
|
||||
doc.db_set("reposting_data_file", None)
|
||||
remove_attached_file(doc.name)
|
||||
|
||||
@@ -14,10 +14,11 @@ 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.repost_item_valuation.repost_item_valuation import (
|
||||
in_configured_timeslot,
|
||||
mark_covered_transaction_reposts,
|
||||
)
|
||||
from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry
|
||||
from erpnext.stock.tests.test_utils import StockTestMixin
|
||||
from erpnext.stock.utils import PendingRepostingError
|
||||
from erpnext.stock.utils import PendingRepostingError, get_combine_datetime
|
||||
from erpnext.tests.utils import ERPNextTestSuite
|
||||
|
||||
|
||||
@@ -171,6 +172,127 @@ class TestRepostItemValuation(ERPNextTestSuite, StockTestMixin):
|
||||
riv4.set_status("Skipped")
|
||||
riv3.set_status("Skipped")
|
||||
|
||||
def _make_queued_transaction_riv(self, voucher):
|
||||
riv = frappe.get_doc(
|
||||
doctype="Repost Item Valuation",
|
||||
based_on="Transaction",
|
||||
voucher_type=voucher.doctype,
|
||||
voucher_no=voucher.name,
|
||||
posting_date=voucher.posting_date,
|
||||
posting_time="00:00:00",
|
||||
)
|
||||
riv.flags.dont_run_in_test = True
|
||||
riv.submit()
|
||||
return riv
|
||||
|
||||
def test_skip_transaction_repost_covered_by_dependent(self):
|
||||
company = "_Test Company with perpetual inventory"
|
||||
warehouse = "Stores - TCP1"
|
||||
|
||||
covered_pr = make_purchase_receipt(
|
||||
company=company, warehouse=warehouse, item_code="_Test Item", qty=5
|
||||
)
|
||||
other_pr = make_purchase_receipt(
|
||||
company=company, warehouse=warehouse, item_code="_Test Item 2", qty=5
|
||||
)
|
||||
|
||||
covered_riv = self._make_queued_transaction_riv(covered_pr)
|
||||
other_riv = self._make_queued_transaction_riv(other_pr)
|
||||
|
||||
earlier_date = add_days(covered_pr.posting_date, -1)
|
||||
source = frappe._dict(name="__test_source_riv__", posting_date=earlier_date, posting_time="00:00:00")
|
||||
coverage = {("_Test Item", warehouse): get_combine_datetime(earlier_date, "00:00:00")}
|
||||
affected = {("Purchase Receipt", covered_pr.name), ("Purchase Receipt", other_pr.name)}
|
||||
|
||||
mark_covered_transaction_reposts(source, coverage, affected)
|
||||
|
||||
covered_riv.reload()
|
||||
other_riv.reload()
|
||||
self.assertEqual(covered_riv.status, "Skipped")
|
||||
self.assertEqual(other_riv.status, "Queued")
|
||||
|
||||
other_riv.db_set("status", "Skipped")
|
||||
|
||||
def _make_dependent_repack(self, company, consumed_items, source_wh, fg_item, fg_wh, qty, posting_date):
|
||||
se = frappe.new_doc("Stock Entry")
|
||||
se.stock_entry_type = "Repack"
|
||||
se.company = company
|
||||
se.set_posting_time = 1
|
||||
se.posting_date = posting_date
|
||||
for item_code in consumed_items:
|
||||
se.append("items", {"item_code": item_code, "s_warehouse": source_wh, "qty": qty})
|
||||
se.append("items", {"item_code": fg_item, "t_warehouse": fg_wh, "qty": qty, "is_finished_item": 1})
|
||||
se.insert()
|
||||
se.submit()
|
||||
return se
|
||||
|
||||
def test_backdated_manufacture_repost_skips_redundant_dependent(self):
|
||||
from erpnext.stock.doctype.repost_item_valuation.repost_item_valuation import (
|
||||
execute_reposting_entry,
|
||||
)
|
||||
|
||||
frappe.flags.dont_execute_stock_reposts = True
|
||||
self.addCleanup(frappe.flags.pop, "dont_execute_stock_reposts", None)
|
||||
|
||||
original_setting = frappe.db.get_single_value("Stock Reposting Settings", "item_based_reposting")
|
||||
frappe.db.set_single_value("Stock Reposting Settings", "item_based_reposting", 1)
|
||||
self.addCleanup(
|
||||
frappe.db.set_single_value, "Stock Reposting Settings", "item_based_reposting", original_setting
|
||||
)
|
||||
|
||||
company = "_Test Company with perpetual inventory"
|
||||
source_wh = "Stores - TCP1"
|
||||
fg_wh = "Finished Goods - TCP1"
|
||||
|
||||
item_a = make_item(properties={"valuation_method": "FIFO"}).name
|
||||
item_b = make_item(properties={"valuation_method": "FIFO"}).name
|
||||
item_c = make_item(properties={"valuation_method": "FIFO"}).name
|
||||
|
||||
def _day(days):
|
||||
return add_days(nowdate(), days)
|
||||
|
||||
make_stock_entry(
|
||||
item_code=item_a, to_warehouse=source_wh, qty=10, rate=100, posting_date=_day(2), company=company
|
||||
)
|
||||
make_stock_entry(
|
||||
item_code=item_b, to_warehouse=source_wh, qty=10, rate=100, posting_date=_day(3), company=company
|
||||
)
|
||||
self._make_dependent_repack(company, [item_a, item_b], source_wh, item_c, fg_wh, 5, _day(10))
|
||||
|
||||
make_stock_entry(
|
||||
item_code=item_a, to_warehouse=source_wh, qty=10, rate=200, posting_date=_day(1), company=company
|
||||
)
|
||||
make_stock_entry(
|
||||
item_code=item_b, to_warehouse=source_wh, qty=10, rate=200, posting_date=_day(1), company=company
|
||||
)
|
||||
self._make_dependent_repack(company, [item_a, item_b], source_wh, item_c, fg_wh, 5, _day(5))
|
||||
|
||||
rivs = frappe.get_all(
|
||||
"Repost Item Valuation",
|
||||
filters={
|
||||
"docstatus": 1,
|
||||
"based_on": "Item and Warehouse",
|
||||
"status": "Queued",
|
||||
"item_code": ("in", [item_a, item_b, item_c]),
|
||||
},
|
||||
fields=["name", "item_code", "warehouse"],
|
||||
order_by="posting_date asc, posting_time asc, creation asc",
|
||||
)
|
||||
self.assertTrue(
|
||||
any(r.item_code == item_c and r.warehouse == fg_wh for r in rivs),
|
||||
msg="Expected a queued repost for the finished good",
|
||||
)
|
||||
|
||||
for r in rivs:
|
||||
execute_reposting_entry(r.name)
|
||||
|
||||
fg_repost_status = frappe.db.get_value(
|
||||
"Repost Item Valuation",
|
||||
{"based_on": "Item and Warehouse", "item_code": item_c, "warehouse": fg_wh, "docstatus": 1},
|
||||
"status",
|
||||
)
|
||||
self.assertEqual(fg_repost_status, "Skipped")
|
||||
|
||||
def test_stock_freeze_validation(self):
|
||||
today = nowdate()
|
||||
|
||||
|
||||
@@ -306,6 +306,7 @@ def repost_future_sle(
|
||||
resume_item_wh_wise_last_posted_sle = (
|
||||
get_item_wh_wise_last_posted_sle_from_reposting_data(doc, reposting_data) or {}
|
||||
)
|
||||
item_wh_first_reposted = get_item_wh_first_reposted_from_reposting_data(doc, reposting_data) or {}
|
||||
if not items_to_be_repost:
|
||||
return
|
||||
|
||||
@@ -328,6 +329,7 @@ def repost_future_sle(
|
||||
"repost_doc": doc,
|
||||
"repost_affected_transaction": repost_affected_transaction,
|
||||
"item_wh_wise_last_posted_sle": resume_item_wh_wise_last_posted_sle,
|
||||
"item_wh_first_reposted": item_wh_first_reposted,
|
||||
},
|
||||
allow_negative_stock=allow_negative_stock,
|
||||
via_landed_cost_voucher=via_landed_cost_voucher,
|
||||
@@ -337,7 +339,14 @@ def repost_future_sle(
|
||||
|
||||
resume_item_wh_wise_last_posted_sle = {}
|
||||
repost_affected_transaction.update(obj.repost_affected_transaction)
|
||||
update_args_in_repost_item_valuation(doc, index, items_to_be_repost, repost_affected_transaction)
|
||||
item_wh_first_reposted = obj.item_wh_first_reposted
|
||||
update_args_in_repost_item_valuation(
|
||||
doc,
|
||||
index,
|
||||
items_to_be_repost,
|
||||
repost_affected_transaction,
|
||||
item_wh_first_reposted=item_wh_first_reposted,
|
||||
)
|
||||
|
||||
|
||||
def update_args_in_repost_item_valuation(
|
||||
@@ -346,11 +355,15 @@ def update_args_in_repost_item_valuation(
|
||||
items_to_be_repost,
|
||||
repost_affected_transaction,
|
||||
item_wh_wise_last_posted_sle=None,
|
||||
item_wh_first_reposted=None,
|
||||
):
|
||||
file_name = ""
|
||||
if not item_wh_wise_last_posted_sle:
|
||||
item_wh_wise_last_posted_sle = {}
|
||||
|
||||
if not item_wh_first_reposted:
|
||||
item_wh_first_reposted = {}
|
||||
|
||||
if doc.reposting_data_file:
|
||||
file_name = get_reposting_file_name(doc.doctype, doc.name)
|
||||
# frappe.delete_doc("File", file_name, ignore_permissions=True, delete_permanently=True)
|
||||
@@ -360,6 +373,7 @@ def update_args_in_repost_item_valuation(
|
||||
"repost_affected_transaction": repost_affected_transaction,
|
||||
"item_wh_wise_last_posted_sle": {str(k): v for k, v in item_wh_wise_last_posted_sle.items()}
|
||||
or {},
|
||||
"item_wh_first_reposted": {str(k): v for k, v in item_wh_first_reposted.items()},
|
||||
},
|
||||
doc,
|
||||
file_name,
|
||||
@@ -495,6 +509,16 @@ def get_item_wh_wise_last_posted_sle_from_reposting_data(doc, reposting_data=Non
|
||||
return frappe._dict()
|
||||
|
||||
|
||||
def get_item_wh_first_reposted_from_reposting_data(doc, reposting_data=None):
|
||||
if not reposting_data and doc and doc.reposting_data_file:
|
||||
reposting_data = get_reposting_data(doc.reposting_data_file)
|
||||
|
||||
if not reposting_data or not reposting_data.get("item_wh_first_reposted"):
|
||||
return {}
|
||||
|
||||
return {frappe.safe_eval(key): value for key, value in reposting_data.item_wh_first_reposted.items()}
|
||||
|
||||
|
||||
def get_reposting_data(file_path) -> dict:
|
||||
file_name = frappe.db.get_value(
|
||||
"File",
|
||||
@@ -688,6 +712,7 @@ class update_entries_after:
|
||||
self.distinct_sles = set()
|
||||
self.distinct_dependant_item_wh = set()
|
||||
self.prev_sle_dict = frappe._dict({})
|
||||
self.item_wh_first_reposted = dict(self.args.get("item_wh_first_reposted") or {})
|
||||
|
||||
def get_item_wh_wise_last_posted_sle(self):
|
||||
if self.args and self.args.get("item_wh_wise_last_posted_sle"):
|
||||
@@ -738,6 +763,10 @@ class update_entries_after:
|
||||
|
||||
i += 1
|
||||
item_wh_key = (sle.item_code, sle.warehouse)
|
||||
sle_datetime = sle.posting_datetime or get_combine_datetime(sle.posting_date, sle.posting_time)
|
||||
existing_datetime = self.item_wh_first_reposted.get(item_wh_key)
|
||||
if not existing_datetime or get_datetime(sle_datetime) < get_datetime(existing_datetime):
|
||||
self.item_wh_first_reposted[item_wh_key] = sle_datetime
|
||||
if item_wh_key not in self.prev_sle_dict:
|
||||
self.prev_sle_dict[item_wh_key] = get_previous_sle_of_current_voucher(sle)
|
||||
|
||||
@@ -832,6 +861,7 @@ class update_entries_after:
|
||||
self.items_to_be_repost,
|
||||
self.repost_affected_transaction,
|
||||
self.item_wh_wise_last_posted_sle,
|
||||
self.item_wh_first_reposted,
|
||||
)
|
||||
|
||||
if not frappe.in_test:
|
||||
|
||||
Reference in New Issue
Block a user