From 9c911438f107a8441852d56af5f2bd14f00b510f Mon Sep 17 00:00:00 2001 From: rohitwaghchaure Date: Fri, 3 Jul 2026 17:17:38 +0530 Subject: [PATCH] fix: do not rebook standard cost variance on non-update-stock purchase invoice (#56799) --- .../purchase_invoice/services/gl_composer.py | 135 +----- .../item_standard_cost.json | 4 +- .../item_standard_cost/item_standard_cost.py | 78 +++- .../test_item_standard_cost.py | 390 ++++++++++++++++-- .../stock_entry/services/gl_composer.py | 132 ++++-- .../stock_reconciliation.js | 2 +- .../stock_reconciliation.py | 128 +++++- 7 files changed, 666 insertions(+), 203 deletions(-) diff --git a/erpnext/accounts/doctype/purchase_invoice/services/gl_composer.py b/erpnext/accounts/doctype/purchase_invoice/services/gl_composer.py index f776994a29b..8524783b033 100644 --- a/erpnext/accounts/doctype/purchase_invoice/services/gl_composer.py +++ b/erpnext/accounts/doctype/purchase_invoice/services/gl_composer.py @@ -3,7 +3,6 @@ import frappe from frappe import _ -from frappe.query_builder.functions import Sum from frappe.utils import cint, flt, get_link_to_form import erpnext @@ -131,7 +130,6 @@ class PurchaseInvoiceGLComposer(BaseGLComposer): from erpnext.accounts.doctype.purchase_invoice.purchase_invoice import ( get_purchase_document_details, ) - from erpnext.stock.utils import get_valuation_method doc = self.doc tax_service = TaxService(doc) @@ -331,33 +329,25 @@ class PurchaseInvoiceGLComposer(BaseGLComposer): self.make_provisional_gl_entry(gl_entries, item) if not doc.is_internal_transfer(): - handled = False - if ( - item.item_code - and item.item_code in stock_items - and item.get("purchase_receipt") - and not doc.is_return - and get_valuation_method(item.item_code, doc.company) == "Standard Cost" - ): - handled = self.make_standard_cost_srbnb_split( - gl_entries, item, expense_account, account_currency, base_amount - ) - - if not handled: - gl_entries.append( - self.get_gl_dict( - { - "account": expense_account, - "against": doc.supplier, - "debit": base_amount, - "debit_in_transaction_currency": amount, - "cost_center": item.cost_center, - "project": item.project or doc.project, - }, - account_currency, - item=item, - ) + # When Update Stock is disabled, this invoice has no stock impact: the linked + # Purchase Receipt already booked the stock (at standard) and the Purchase Price + # Variance. Here we only clear "Stock Received But Not Billed" at the full billed + # amount against the supplier - booking PPV again would double count it and leave + # SRBNB partially uncleared. + gl_entries.append( + self.get_gl_dict( + { + "account": expense_account, + "against": doc.supplier, + "debit": base_amount, + "debit_in_transaction_currency": amount, + "cost_center": item.cost_center, + "project": item.project or doc.project, + }, + account_currency, + item=item, ) + ) # check if the exchange rate has changed if ( @@ -530,95 +520,6 @@ class PurchaseInvoiceGLComposer(BaseGLComposer): }, ) - def make_standard_cost_srbnb_split( - self, gl_entries, item, expense_account, account_currency, base_amount - ): - """For a Standard Cost item billed against a Purchase Receipt, clear SRBNB at the standard - value the receipt actually booked and post the (Net Amount - standard) difference to the - Purchase Price Variance account. Returns False (caller falls back) if the receipt value - can't be resolved.""" - from erpnext.stock.doctype.item_standard_cost.item_standard_cost import ( - get_purchase_price_variance_account, - ) - - doc = self.doc - precision = item.precision("base_net_amount") - standard_value = flt(self.get_pr_stock_value(item), precision) - if not standard_value: - return False - - gl_entries.append( - self.get_gl_dict( - { - "account": expense_account, - "against": doc.supplier, - "debit": standard_value, - "debit_in_transaction_currency": flt(standard_value / doc.conversion_rate, precision), - "remarks": doc.get("remarks") or _("Accounting Entry for Stock"), - "cost_center": item.cost_center, - "project": item.project or doc.project, - }, - account_currency, - item=item, - ) - ) - - variance = flt(base_amount - standard_value, precision) - if variance: - gl_entries.append( - self.get_gl_dict( - { - "account": get_purchase_price_variance_account(item.item_code, doc.company), - "against": doc.supplier, - "debit": variance, - "debit_in_transaction_currency": flt(variance / doc.conversion_rate, precision), - "remarks": doc.get("remarks") or _("Purchase Price Variance"), - "cost_center": item.cost_center, - "project": item.project or doc.project, - }, - item=item, - ) - ) - - return True - - def get_pr_stock_value(self, item): - """Stock value (at standard) the linked Purchase Receipt booked for the quantity this invoice - row is billing. - - Accepted and rejected stock for the same receipt row share `voucher_detail_no`, so the - warehouse filter is required: without it the accepted warehouse's SRBNB would be cleared at - accepted + rejected value and post the wrong Purchase Price Variance amount. The accepted - warehouse is read from the receipt row itself (not the invoice row, which may be unset on a - non-stock invoice). - - The receipt's full accepted value is pro-rated to the invoiced quantity, so a partial bill - clears SRBNB (and posts PPV) for only the units it covers, not the whole receipt row.""" - pr_detail = frappe.db.get_value( - "Purchase Receipt Item", item.pr_detail, ["warehouse", "stock_qty"], as_dict=True - ) - if not pr_detail or not pr_detail.warehouse: - return 0.0 - - sle = frappe.qb.DocType("Stock Ledger Entry") - result = ( - frappe.qb.from_(sle) - .select(Sum(sle.stock_value_difference)) - .where( - (sle.voucher_type == "Purchase Receipt") - & (sle.voucher_no == item.purchase_receipt) - & (sle.voucher_detail_no == item.pr_detail) - & (sle.warehouse == pr_detail.warehouse) - & (sle.is_cancelled == 0) - ) - ).run() - accepted_value = flt(result[0][0]) if result and result[0][0] else 0.0 - if not accepted_value or not flt(pr_detail.stock_qty): - return accepted_value - - # Pro-rate to the quantity being billed by this invoice row (handles partial billing). - return accepted_value * flt(item.stock_qty) / flt(pr_detail.stock_qty) - def get_stock_variance_account(self, item): """For Standard Cost items the purchase-price-vs-standard difference is a Purchase Price Variance; for all other items it keeps the existing behaviour (default expense account).""" diff --git a/erpnext/stock/doctype/item_standard_cost/item_standard_cost.json b/erpnext/stock/doctype/item_standard_cost/item_standard_cost.json index 7a0e8ab85c9..10b1cbeecaa 100644 --- a/erpnext/stock/doctype/item_standard_cost/item_standard_cost.json +++ b/erpnext/stock/doctype/item_standard_cost/item_standard_cost.json @@ -73,7 +73,7 @@ "label": "Revaluation" }, { - "description": "Stock Reconciliation auto-created to revalue on-hand stock to the new standard rate.", + "description": "Stock Reconciliation that revalues on-hand stock to this standard rate: auto-created when the rate is changed here, or the reconciliation that captured this rate (opening entry or rate change).", "fieldname": "revaluation_entry", "fieldtype": "Link", "label": "Revaluation Entry", @@ -95,7 +95,7 @@ "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2026-06-26 11:00:00.000000", + "modified": "2026-07-02 11:00:00.000000", "modified_by": "Administrator", "module": "Stock", "name": "Item Standard Cost", diff --git a/erpnext/stock/doctype/item_standard_cost/item_standard_cost.py b/erpnext/stock/doctype/item_standard_cost/item_standard_cost.py index 8450d10791e..62fb8e02903 100644 --- a/erpnext/stock/doctype/item_standard_cost/item_standard_cost.py +++ b/erpnext/stock/doctype/item_standard_cost/item_standard_cost.py @@ -91,13 +91,75 @@ class ItemStandardCost(Document): # previous (or missing) rate earlier in the request, so the revaluation below — and anything # else in this request — reads the newly submitted rate. clear_item_standard_rate_cache() + + # When a Stock Reconciliation captured this rate (opening entry or rate change), it has set + # revaluation_entry to itself and performs the revaluation. Don't spawn another one. + if self.revaluation_entry: + return + self.create_revaluation_entry() def before_cancel(self): - frappe.throw( - _("Item Standard Cost cannot be cancelled. Submit a new record to change the standard rate.") + self.validate_no_stock_activity_on_or_after_effective_date() + + def has_stock_activity_on_or_after_effective_date(self): + """Is there any live stock transaction for this item on or after the effective date, other than + the revaluation Stock Reconciliation this record created? Such transactions are valued at this + standard rate, so this record cannot be safely cancelled while they exist.""" + sle = frappe.qb.DocType("Stock Ledger Entry") + query = ( + frappe.qb.from_(sle) + .select(sle.name) + .where( + (sle.item_code == self.item_code) + & (sle.company == self.company) + & (sle.is_cancelled == 0) + # posting_datetime (indexed) is preferred over posting_date; get_datetime on the date gives + # the start of the effective date, so this matches everything on or after it. + & (sle.posting_datetime >= get_datetime(self.effective_date)) + ) + .limit(1) ) + # The revaluation reco this record created posts on the effective date; exclude it, it is + # reversed together with this document in on_cancel. + if self.revaluation_entry: + query = query.where(sle.voucher_no != self.revaluation_entry) + + return bool(query.run()) + + def validate_no_stock_activity_on_or_after_effective_date(self): + """A submitted Item Standard Cost can be cancelled only when no stock transaction exists for the + item on or after its effective date, other than the revaluation Stock Reconciliation it created. + Later transactions are valued at this standard rate, so cancelling it would corrupt their + valuation and force a repost.""" + if self.has_stock_activity_on_or_after_effective_date(): + frappe.throw( + _( + "Item Standard Cost cannot be cancelled because stock transactions exist for Item {0} on or after the Effective Date {1}. Cancel those transactions first." + ).format( + get_link_to_form("Item", self.item_code), + frappe.bold(frappe.format(self.effective_date, "Date")), + ) + ) + + def on_cancel(self): + # Drop the cached standard rate first: this record is now cancelled, so the revaluation reversal + # below (and anything else in this request) must re-read the previous effective rate, not this one. + clear_item_standard_rate_cache() + + # Set when this cancellation was triggered by the source reconciliation itself (it is already + # cancelling); reversing revaluation_entry would loop back into that same reconciliation. + if self.flags.from_source_reconciliation: + return + + # Reverse the revaluation this record created. + if self.revaluation_entry: + reco = frappe.get_doc("Stock Reconciliation", self.revaluation_entry) + if reco.docstatus == 1: + reco.flags.via_item_standard_cost = True + reco.cancel() + def create_revaluation_entry(self): """Revalue on-hand stock to the new standard rate via a Stock Reconciliation. @@ -230,6 +292,18 @@ def get_item_standard_rate(item_code, company, posting_date=None): return flt(rate[0]) if rate else None +def has_item_standard_cost(item_code, company): + """True if a submitted Item Standard Cost exists for the item in the company (any effective date). + Used to tell an opening Stock Reconciliation (no standard rate yet, rate is editable) apart from a + later one (standard rate owned by Item Standard Cost, only quantity may be adjusted).""" + return bool( + frappe.db.exists( + "Item Standard Cost", + {"item_code": item_code, "company": company, "docstatus": 1}, + ) + ) + + def clear_item_standard_rate_cache(): """Drop the request-cached results of `get_item_standard_rate` so reads after a new Item Standard Cost is submitted see the fresh rate instead of a value cached earlier in the same request.""" diff --git a/erpnext/stock/doctype/item_standard_cost/test_item_standard_cost.py b/erpnext/stock/doctype/item_standard_cost/test_item_standard_cost.py index 597b978b17b..a4029cffe18 100644 --- a/erpnext/stock/doctype/item_standard_cost/test_item_standard_cost.py +++ b/erpnext/stock/doctype/item_standard_cost/test_item_standard_cost.py @@ -232,29 +232,252 @@ class TestItemStandardCost(ERPNextTestSuite): self.assertFalse(frappe.db.exists("Repost Item Valuation", {"voucher_no": se0.name})) - def test_cannot_cancel(self): + def test_cancel_allowed_without_stock_activity(self): + # No stock transaction on/after the effective date -> the standard cost can be cancelled. item = create_standard_cost_item() isc = create_item_standard_cost(item.name, rate=100) + isc.cancel() + self.assertEqual(isc.docstatus, 2) + + def test_cancel_blocked_with_stock_activity(self): + # A stock transaction on/after the effective date is valued at this standard rate, so the + # standard cost cannot be cancelled while it exists. + item = create_standard_cost_item() + isc = create_item_standard_cost(item.name, rate=100) + make_stock_entry(item_code=item.name, target=TEST_WAREHOUSE, qty=5, basic_rate=100) self.assertRaises(frappe.ValidationError, isc.cancel) - def test_direct_stock_reconciliation_blocked(self): + def test_cancel_reverses_revaluation(self): + # Cancelling a rate change reverses the revaluation Stock Reconciliation it created, restoring + # the previous stock value (the movement that triggered it predates the effective date). + item = create_standard_cost_item() + create_item_standard_cost(item.name, rate=100, effective_date=add_days(today(), -10)) + make_stock_entry( + item_code=item.name, + target=TEST_WAREHOUSE, + qty=10, + basic_rate=100, + posting_date=add_days(today(), -5), + ) + + isc2 = create_item_standard_cost(item.name, rate=130, effective_date=today()) + self.assertTrue(isc2.revaluation_entry) + + def stock_value(): + return flt( + frappe.db.get_value( + "Bin", {"item_code": item.name, "warehouse": TEST_WAREHOUSE}, "stock_value" + ) + ) + + self.assertEqual(stock_value(), 1300) + + isc2.cancel() + self.assertEqual(frappe.db.get_value("Stock Reconciliation", isc2.revaluation_entry, "docstatus"), 2) + self.assertEqual(stock_value(), 1000) + + def test_stock_reconciliation_rate_change_creates_standard_cost(self): + # Editing the rate on a reconciliation creates a new Item Standard Cost and revalues on-hand + # stock to it - the reconciliation is a shortcut into the standard cost, not a manual override. + from erpnext.stock.doctype.item_standard_cost.item_standard_cost import get_item_standard_rate from erpnext.stock.doctype.stock_reconciliation.test_stock_reconciliation import ( create_stock_reconciliation, ) item = create_standard_cost_item() - create_item_standard_cost(item.name, rate=100) - make_stock_entry(item_code=item.name, target=TEST_WAREHOUSE, qty=10, basic_rate=100) + create_item_standard_cost( + item.name, rate=100, company=PI_COMPANY, effective_date=add_days(today(), -10) + ) + make_stock_entry( + item_code=item.name, + to_warehouse=PI_STORES, + company=PI_COMPANY, + qty=10, + basic_rate=100, + posting_date=add_days(today(), -5), + ) + # Same quantity, new rate 130: a new standard cost is set and the 10 on-hand units revalue to 1300. + reco = create_stock_reconciliation( + item_code=item.name, warehouse=PI_STORES, qty=10, rate=130, company=PI_COMPANY + ) + self.assertEqual(reco.docstatus, 1) + + self.assertEqual(flt(get_item_standard_rate(item.name, PI_COMPANY)), 130) + stock_value = frappe.db.get_value( + "Bin", {"item_code": item.name, "warehouse": PI_STORES}, "stock_value" + ) + self.assertEqual(flt(stock_value), 1300) + + def test_stock_reconciliation_qty_change_allowed(self): + # A reconciliation may adjust the quantity of a Standard Cost item: stock stays valued at the + # standard rate and the value difference is booked to the Stock Adjustment account. + from erpnext.stock.doctype.stock_reconciliation.test_stock_reconciliation import ( + create_stock_reconciliation, + ) + + item = create_standard_cost_item() + create_item_standard_cost(item.name, rate=100, company=PI_COMPANY) + make_stock_entry( + item_code=item.name, to_warehouse=PI_STORES, company=PI_COMPANY, qty=10, basic_rate=100 + ) + + # Count down to 8 at the standard rate: value 800, a 200 reduction against Stock Adjustment. + reco = create_stock_reconciliation( + item_code=item.name, warehouse=PI_STORES, qty=8, rate=100, company=PI_COMPANY + ) + self.assertEqual(reco.docstatus, 1) + + bin_data = frappe.db.get_value( + "Bin", + {"item_code": item.name, "warehouse": PI_STORES}, + ["actual_qty", "stock_value"], + as_dict=True, + ) + self.assertEqual(flt(bin_data.actual_qty), 8) + self.assertEqual(flt(bin_data.stock_value), 800) + + sle = frappe.db.get_value( + "Stock Ledger Entry", {"voucher_no": reco.name, "is_cancelled": 0}, "valuation_rate" + ) + self.assertEqual(flt(sle), 100) + + stock_adjustment = frappe.get_cached_value("Company", PI_COMPANY, "stock_adjustment_account") + booked = flt( + frappe.db.sql( + "select sum(debit - credit) from `tabGL Entry` where voucher_no=%s and account=%s and is_cancelled=0", + (reco.name, stock_adjustment), + )[0][0] + ) + self.assertEqual(booked, 200) + + def test_opening_reconciliation_creates_standard_cost(self): + # With no Item Standard Cost yet, an opening Stock Reconciliation may set the rate; that rate is + # captured into an Item Standard Cost record so the resulting stock (and later transactions) are + # valued at it. + from erpnext.stock.doctype.item_standard_cost.item_standard_cost import ( + get_item_standard_rate, + has_item_standard_cost, + ) + from erpnext.stock.doctype.stock_reconciliation.test_stock_reconciliation import ( + create_stock_reconciliation, + ) + + item = create_standard_cost_item() + self.assertFalse(has_item_standard_cost(item.name, PI_COMPANY)) + + reco = create_stock_reconciliation( + item_code=item.name, warehouse=PI_STORES, qty=5, rate=100, company=PI_COMPANY + ) + self.assertEqual(reco.docstatus, 1) + + # The opening rate is now the item's standard cost. + self.assertTrue(has_item_standard_cost(item.name, PI_COMPANY)) + self.assertEqual(flt(get_item_standard_rate(item.name, PI_COMPANY)), 100) + + bin_data = frappe.db.get_value( + "Bin", + {"item_code": item.name, "warehouse": PI_STORES}, + ["actual_qty", "stock_value"], + as_dict=True, + ) + self.assertEqual(flt(bin_data.actual_qty), 5) + self.assertEqual(flt(bin_data.stock_value), 500) + + def test_opening_reconciliation_requires_rate(self): + # An opening reconciliation for a Standard Cost item with no standard rate yet must carry a + # positive rate - there is nothing to value the stock at otherwise. + from erpnext.stock.doctype.stock_reconciliation.test_stock_reconciliation import ( + create_stock_reconciliation, + ) + + item = create_standard_cost_item() self.assertRaises( frappe.ValidationError, create_stock_reconciliation, item_code=item.name, - warehouse=TEST_WAREHOUSE, - qty=8, - rate=120, + warehouse=PI_STORES, + qty=5, + rate=0, + company=PI_COMPANY, ) + def test_opening_reconciliation_points_standard_cost_to_itself(self): + # The captured Item Standard Cost records the reconciliation as its revaluation entry (the reco is + # the revaluation) instead of spawning a second one. + from erpnext.stock.doctype.stock_reconciliation.test_stock_reconciliation import ( + create_stock_reconciliation, + ) + + item = create_standard_cost_item() + reco = create_stock_reconciliation( + item_code=item.name, warehouse=PI_STORES, qty=5, rate=100, company=PI_COMPANY + ) + + isc = frappe.db.get_value( + "Item Standard Cost", + {"item_code": item.name, "company": PI_COMPANY, "docstatus": 1}, + "revaluation_entry", + ) + self.assertEqual(isc, reco.name) + + def test_opening_reconciliation_cancel_cancels_standard_cost(self): + # Cancelling an opening reconciliation removes the stock it created, so the Item Standard Cost it + # introduced is cancelled too. + from erpnext.stock.doctype.item_standard_cost.item_standard_cost import has_item_standard_cost + from erpnext.stock.doctype.stock_reconciliation.test_stock_reconciliation import ( + create_stock_reconciliation, + ) + + item = create_standard_cost_item() + reco = create_stock_reconciliation( + item_code=item.name, warehouse=PI_STORES, qty=5, rate=100, company=PI_COMPANY + ) + isc_name = frappe.db.get_value( + "Item Standard Cost", {"item_code": item.name, "company": PI_COMPANY, "docstatus": 1}, "name" + ) + + reco.cancel() + + self.assertEqual(frappe.db.get_value("Item Standard Cost", isc_name, "docstatus"), 2) + self.assertFalse(has_item_standard_cost(item.name, PI_COMPANY)) + + def test_rate_change_reconciliation_cancel_reverts_standard_cost(self): + # A rate-change reconciliation revalues on-hand stock only on/after its effective date, and that + # revaluation is reversed on cancel. The pre-existing stock sits before the effective date, so no + # live SLE is valued at the new rate once the reco's own entry is reversed. Cancelling therefore + # cancels the Item Standard Cost it created and the item falls back to the previous standard rate. + from erpnext.stock.doctype.item_standard_cost.item_standard_cost import get_item_standard_rate + from erpnext.stock.doctype.stock_reconciliation.test_stock_reconciliation import ( + create_stock_reconciliation, + ) + + item = create_standard_cost_item() + create_item_standard_cost( + item.name, rate=100, company=PI_COMPANY, effective_date=add_days(today(), -10) + ) + make_stock_entry( + item_code=item.name, + to_warehouse=PI_STORES, + company=PI_COMPANY, + qty=10, + basic_rate=100, + posting_date=add_days(today(), -5), + ) + + reco = create_stock_reconciliation( + item_code=item.name, warehouse=PI_STORES, qty=10, rate=130, company=PI_COMPANY + ) + isc_name = frappe.db.get_value("Item Standard Cost", {"revaluation_entry": reco.name}, "name") + self.assertTrue(isc_name) + self.assertEqual(flt(get_item_standard_rate(item.name, PI_COMPANY)), 130) + + reco.cancel() + + # The revaluation is reverted, so the standard cost it created is cancelled and the rate reverts. + self.assertEqual(frappe.db.get_value("Item Standard Cost", isc_name, "docstatus"), 2) + self.assertEqual(flt(get_item_standard_rate(item.name, PI_COMPANY)), 100) + def test_backdated_transaction_blocked(self): item = create_standard_cost_item() create_item_standard_cost(item.name, rate=100, effective_date=today()) @@ -364,6 +587,40 @@ class TestItemStandardCost(ERPNextTestSuite): # The additional cost is credited out of its source account (it flowed into the variance). self.assertEqual(gl_net(additional_cost_account), -30) + def test_manufacturing_variance_no_stock_adjustment_entry(self): + # With an additional cost in the mix, the net-zero Stock Adjustment reclassification must not + # survive as a debit == credit entry: only the real accounts (variance, additional cost source, + # stock) should be booked. + ensure_mfg_variance_account(PI_COMPANY) + additional_cost_account = "Expenses Included In Valuation - TCP1" + stock_adjustment = frappe.get_cached_value("Company", PI_COMPANY, "stock_adjustment_account") + rm = create_standard_cost_item() + fg = create_standard_cost_item() + create_item_standard_cost(rm.name, rate=50, company=PI_COMPANY) + create_item_standard_cost(fg.name, rate=200, company=PI_COMPANY) + + make_stock_entry(item_code=rm.name, to_warehouse=PI_STORES, company=PI_COMPANY, qty=10, basic_rate=50) + + se = frappe.new_doc("Stock Entry") + se.purpose = "Repack" + se.stock_entry_type = "Repack" + se.company = PI_COMPANY + se.append("items", {"item_code": rm.name, "s_warehouse": PI_STORES, "qty": 5}) + se.append("items", {"item_code": fg.name, "t_warehouse": PI_FG, "qty": 1, "is_finished_item": 1}) + se.append( + "additional_costs", + {"expense_account": additional_cost_account, "description": "Freight", "amount": 30}, + ) + se.insert() + se.submit() + + # No Stock Adjustment entry at all - the difference is entirely the manufacturing variance. + self.assertFalse( + frappe.db.exists( + "GL Entry", {"voucher_no": se.name, "account": stock_adjustment, "is_cancelled": 0} + ) + ) + def test_manufacturing_variance_account_required(self): # Without a Manufacturing Variance account, submitting a Standard Cost Manufacture/Repack must fail. previous = frappe.get_cached_value("Company", PI_COMPANY, "default_manufacturing_variance_account") @@ -484,43 +741,6 @@ class TestItemStandardCost(ERPNextTestSuite): # The submit must have invalidated the cache, so this reads the freshly submitted rate. self.assertEqual(flt(get_item_standard_rate(item.name, TEST_COMPANY)), 100) - def test_pr_stock_value_excludes_rejected_warehouse(self): - # Accepted and rejected stock for one receipt row share voucher_detail_no. The standard-cost - # SRBNB split must clear only the accepted warehouse's value, not accepted + rejected. - from erpnext.accounts.doctype.purchase_invoice.services.gl_composer import ( - PurchaseInvoiceGLComposer, - ) - from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_purchase_receipt - from erpnext.stock.doctype.warehouse.test_warehouse import create_warehouse - - item = create_standard_cost_item() - create_item_standard_cost(item.name, rate=100, company=PI_COMPANY) - - rejected_warehouse = create_warehouse("_Test SC Rejected Warehouse", company=PI_COMPANY) - - # Receive 10 accepted + 2 rejected at a billed rate of 150; both SLEs value at the standard 100. - pr = make_purchase_receipt( - item_code=item.name, - company=PI_COMPANY, - warehouse=PI_STORES, - qty=10, - rejected_qty=2, - rejected_warehouse=rejected_warehouse, - rate=150, - ) - - # Method body uses only `item`, so it can be called unbound. - def pr_value(stock_qty): - mock_item = frappe._dict( - purchase_receipt=pr.name, pr_detail=pr.items[0].name, stock_qty=stock_qty - ) - return flt(PurchaseInvoiceGLComposer.get_pr_stock_value(None, mock_item)) - - # Billing all 10: accepted only (10 * 100), not accepted + rejected (12 * 100). - self.assertEqual(pr_value(10), 1000) - # Billing only 4 of the 10 accepted units: pro-rated to the invoiced qty (4 * 100). - self.assertEqual(pr_value(4), 400) - def test_pr_books_variance_to_ppv_account(self): # Receiving a Standard Cost item at a rate above the standard must book the difference to the # Purchase Price Variance account, not the default expense (COGS) account. @@ -571,6 +791,88 @@ class TestItemStandardCost(ERPNextTestSuite): frappe.db.set_value("Company", PI_COMPANY, "default_purchase_price_variance_account", previous) frappe.clear_cache(doctype="Company") + def test_pi_without_update_stock_does_not_rebook_variance(self): + # The Purchase Receipt already booked the 70 variance to PPV. Billing it with a Purchase Invoice + # that has Update Stock disabled must only clear "Stock Received But Not Billed" at the full billed + # amount (200) - it must NOT re-book the variance to the Purchase Price Variance account. + from erpnext.stock.doctype.purchase_receipt.mapper import make_purchase_invoice + from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_purchase_receipt + + ppv_account = ensure_ppv_account(PI_COMPANY) + srbnb_account = frappe.get_cached_value("Company", PI_COMPANY, "stock_received_but_not_billed") + + item = create_standard_cost_item() + create_item_standard_cost(item.name, rate=130, company=PI_COMPANY) + + pr = make_purchase_receipt( + item_code=item.name, company=PI_COMPANY, warehouse=PI_STORES, qty=1, rate=200 + ) + + pi = make_purchase_invoice(pr.name) + self.assertEqual(pi.update_stock, 0) + pi.submit() + + def booked(account): + return flt( + frappe.db.sql( + "select sum(debit - credit) from `tabGL Entry` where voucher_no=%s and account=%s and is_cancelled=0", + (pi.name, account), + )[0][0] + ) + + # No variance re-booked on the invoice; SRBNB is fully cleared at the billed value. + self.assertEqual(booked(ppv_account), 0) + self.assertEqual(booked(srbnb_account), 200) + + def test_material_receipt_books_variance_to_ppv(self): + # Receiving a Standard Cost item via Material Receipt at a manual basic rate (200) plus an + # additional cost (100) must value stock at the standard 130 and book the rest to the Purchase + # Price Variance account: (200*10 + 100) - 130*10 = 800. + ppv_account = ensure_ppv_account(PI_COMPANY) + additional_cost_account = "Expenses Included In Valuation - TCP1" + item = create_standard_cost_item() + create_item_standard_cost(item.name, rate=130, company=PI_COMPANY) + + se = frappe.new_doc("Stock Entry") + se.purpose = "Material Receipt" + se.stock_entry_type = "Material Receipt" + se.company = PI_COMPANY + se.append( + "items", + { + "item_code": item.name, + "t_warehouse": PI_STORES, + "qty": 10, + "basic_rate": 200, + }, + ) + se.append( + "additional_costs", + {"expense_account": additional_cost_account, "description": "Freight", "amount": 100}, + ) + se.insert() + se.submit() + + # Stock is valued at the standard rate, not the manual 200 + additional cost. + sle = frappe.db.get_value( + "Stock Ledger Entry", + {"voucher_no": se.name, "is_cancelled": 0}, + ["valuation_rate", "stock_value_difference"], + as_dict=True, + ) + self.assertEqual(flt(sle.valuation_rate), 130) + self.assertEqual(flt(sle.stock_value_difference), 1300) + + def booked(account): + return flt( + frappe.db.sql( + "select sum(debit - credit) from `tabGL Entry` where voucher_no=%s and account=%s and is_cancelled=0", + (se.name, account), + )[0][0] + ) + + self.assertEqual(booked(ppv_account), 800) + def test_revaluation_posted_after_same_day_movement(self): # A movement earlier on the effective date must not end up after the revaluation, otherwise the # reco would backdate the current quantity ahead of it. diff --git a/erpnext/stock/doctype/stock_entry/services/gl_composer.py b/erpnext/stock/doctype/stock_entry/services/gl_composer.py index f8b93d765ec..2893a239329 100644 --- a/erpnext/stock/doctype/stock_entry/services/gl_composer.py +++ b/erpnext/stock/doctype/stock_entry/services/gl_composer.py @@ -39,11 +39,15 @@ class StockEntryGLComposer(BaseStockGLComposer): self._append_lcv_gl_entries(gl_entries, inventory_account_map) if doc.purpose in ("Repack", "Manufacture"): - self._append_manufacturing_variance_gl_entries(gl_entries) + self._append_manufacturing_variance_gl_entries(gl_entries, inventory_account_map) + elif doc.purpose == "Material Receipt": + self._append_receipt_variance_gl_entries(gl_entries) return process_gl_map(gl_entries, from_repost=frappe.flags.through_repost_item_valuation) - def _append_manufacturing_variance_gl_entries(self, gl_entries: list) -> None: + def _append_manufacturing_variance_gl_entries( + self, gl_entries: list, inventory_account_map: dict + ) -> None: """For Standard Cost finished goods produced via Manufacture/Repack, stock is booked at the item's standard rate, while the entry consumes raw-material (plus additional/landed) cost. The difference is a manufacturing variance and is reclassified from the finished good's expense account to the @@ -52,10 +56,62 @@ class StockEntryGLComposer(BaseStockGLComposer): # Reuse the SLE map the base composer already fetched in compose() to avoid a second identical query. sle_map = self._sle_map + from erpnext.stock.doctype.item_standard_cost.item_standard_cost import ( + get_manufacturing_variance_account, + ) + for d in self.doc.get("items"): variance = self._get_finished_good_variance(d, sle_map, precision) if variance: - self._append_manufacturing_variance_pair(gl_entries, d, variance) + account = get_manufacturing_variance_account(d.item_code, self.doc.company) + remarks = self.doc.get("remarks") or _("Manufacturing Variance for {0}").format(d.item_code) + self._append_standard_cost_variance_pair( + gl_entries, d, variance, account, remarks, inventory_account_map + ) + + def _append_receipt_variance_gl_entries(self, gl_entries: list) -> None: + """For a Standard Cost item received via Material Receipt, stock is booked at the item's standard + rate while the row may carry a manually-set basic rate plus additional/landed cost. The gap + between that intended cost and the standard value is a purchase price variance, reclassified from + the item's expense account to the Purchase Price Variance account.""" + from erpnext.stock.doctype.item_standard_cost.item_standard_cost import ( + get_purchase_price_variance_account, + ) + + precision = self.get_debit_field_precision() + sle_map = self._sle_map + + for d in self.doc.get("items"): + variance = self._get_receipt_variance(d, sle_map, precision) + if variance: + account = get_purchase_price_variance_account(d.item_code, self.doc.company) + remarks = self.doc.get("remarks") or _("Purchase Price Variance for {0}").format(d.item_code) + self._append_standard_cost_variance_pair(gl_entries, d, variance, account, remarks) + + def _get_receipt_variance(self, item, sle_map, precision) -> float: + """Purchase price variance for a Standard Cost item on a Material Receipt: the gap between the full + computed incoming cost (basic amount + additional cost + LCV, i.e. ``amount``) and the standard + value booked into stock. 0 for anything that is not a plain Standard Cost receipt row.""" + from erpnext.stock.utils import get_valuation_method + + if not item.t_warehouse or item.s_warehouse: + return 0.0 + + if ( + item.get("is_finished_item") + or item.get("secondary_item_type") + or item.get("is_legacy_scrap_item") + ): + return 0.0 + + if get_valuation_method(item.item_code, self.doc.company) != "Standard Cost": + return 0.0 + + standard_value = sum( + flt(sle.stock_value_difference) for sle in sle_map.get(item.name, []) if flt(sle.actual_qty) > 0 + ) + + return flt(flt(item.amount) - standard_value, precision) def _get_finished_good_variance(self, item, sle_map, precision) -> float: """Manufacturing variance for a Standard Cost finished good: the gap between the full computed @@ -77,19 +133,27 @@ class StockEntryGLComposer(BaseStockGLComposer): return flt(flt(item.amount) - standard_value, precision) - def _append_manufacturing_variance_pair(self, gl_entries: list, item, variance: float) -> None: - """Reclassify ``variance`` from the finished good's expense account to its Manufacturing Variance - account, restoring the expense account to the value it would carry without Standard Cost.""" - from erpnext.stock.doctype.item_standard_cost.item_standard_cost import ( - get_manufacturing_variance_account, - ) - + def _append_standard_cost_variance_pair( + self, + gl_entries: list, + item, + variance: float, + variance_account: str, + remarks: str, + inventory_account_map: dict | None = None, + ) -> None: + """Reclassify ``variance`` from the item's expense account to the given variance account, + restoring the expense account to the value it would carry without Standard Cost.""" doc = self.doc - variance_account = get_manufacturing_variance_account(item.item_code, doc.company) cost_center = item.cost_center or frappe.get_cached_value("Company", doc.company, "cost_center") - remarks = doc.get("remarks") or _("Manufacturing Variance for {0}").format(item.item_code) project = item.project or doc.get("project") + inventory_account = None + if inventory_account_map: + inventory_account = doc.get_inventory_account_dict(item, inventory_account_map, "t_warehouse")[ + "account" + ] + gl_entries.append( self.get_gl_dict( { @@ -107,7 +171,7 @@ class StockEntryGLComposer(BaseStockGLComposer): self.get_gl_dict( { "account": item.expense_account, - "against": variance_account, + "against": inventory_account or variance_account, "cost_center": cost_center, "remarks": remarks, "debit": -1 * variance, @@ -142,6 +206,11 @@ class StockEntryGLComposer(BaseStockGLComposer): return item_account_wise_additional_cost + def get_valuation_method(self, item_code: str) -> str: + from erpnext.stock.utils import get_valuation_method + + return get_valuation_method(item_code, self.doc.company) + def _append_additional_cost_gl_entries( self, gl_entries: list, item_account_wise_additional_cost: dict ) -> None: @@ -170,18 +239,33 @@ class StockEntryGLComposer(BaseStockGLComposer): ) ) - gl_entries.append( - self.get_gl_dict( - { - "account": d.expense_account, - "against": account, - "cost_center": d.cost_center, - "remarks": doc.get("remarks") or _("Accounting Entry for Stock"), - "credit": -1 * amount["base_amount"], - }, - item=d, + if self.get_valuation_method(d.item_code) == "Standard Cost": + gl_entries.append( + self.get_gl_dict( + { + "account": d.expense_account, + "against": account, + "cost_center": d.cost_center, + "remarks": doc.get("remarks") or _("Accounting Entry for Stock"), + "debit": flt(amount["base_amount"]), + }, + item=d, + ) + ) + + else: + gl_entries.append( + self.get_gl_dict( + { + "account": d.expense_account, + "against": account, + "cost_center": d.cost_center, + "remarks": doc.get("remarks") or _("Accounting Entry for Stock"), + "credit": -1 * flt(amount["base_amount"]), + }, + item=d, + ) ) - ) def _append_lcv_gl_entries(self, gl_entries: list, inventory_account_map: dict) -> None: doc = self.doc diff --git a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.js b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.js index ef4672899cc..3cbd52ffa22 100644 --- a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.js +++ b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.js @@ -6,7 +6,7 @@ frappe.provide("erpnext.accounts.dimensions"); frappe.ui.form.on("Stock Reconciliation", { setup(frm) { - frm.ignore_doctypes_on_cancel_all = ["Serial and Batch Bundle"]; + frm.ignore_doctypes_on_cancel_all = ["Serial and Batch Bundle", "Item Standard Cost"]; frm.barcode_scanner = new erpnext.utils.BarcodeScanner({ frm: frm, uom_field: "stock_uom", diff --git a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py index 5bba06f9a67..e63a6334829 100644 --- a/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py +++ b/erpnext/stock/doctype/stock_reconciliation/stock_reconciliation.py @@ -114,12 +114,73 @@ class StockReconciliation(StockController): ) def on_submit(self): + self.set_standard_cost_from_reconciliation() self.make_bundle_for_current_qty() self.make_bundle_using_old_serial_batch_fields() self.update_stock_ledger() self.make_gl_entries() self.repost_future_sle_and_gle() + def set_standard_cost_from_reconciliation(self): + if self.flags.via_item_standard_cost: + return + + from erpnext.stock.doctype.item_standard_cost.item_standard_cost import ( + get_item_standard_rate, + has_item_standard_cost, + ) + + created = set() + for item in self.items: + if not item.item_code or item.item_code in created: + continue + if not is_standard_cost_item(item.item_code, self.company) or not flt(item.valuation_rate): + continue + + if has_item_standard_cost(item.item_code, self.company): + standard_rate = get_item_standard_rate(item.item_code, self.company, self.posting_date) + precision = item.precision("valuation_rate") + if flt(item.valuation_rate, precision) == flt(standard_rate, precision): + # Rate unchanged: a plain quantity adjustment, valued at the existing standard rate. + continue + + isc = frappe.new_doc("Item Standard Cost") + isc.item_code = item.item_code + isc.company = self.company + isc.effective_date = self.posting_date + isc.standard_rate = item.valuation_rate + isc.revaluation_entry = self.name + isc.insert() + isc.submit() + created.add(item.item_code) + + def cancel_created_item_standard_cost(self): + if self.flags.via_item_standard_cost: + return + + records = frappe.get_all( + "Item Standard Cost", + filters={ + "revaluation_entry": self.name, + "docstatus": 1, + "creation": [">", self.creation], + }, + fields=["name", "item_code", "company"], + ) + for record in records: + isc = frappe.get_doc("Item Standard Cost", record.name) + + # This runs after make_sle_on_cancel has already marked this reco's SLEs is_cancelled=1, so + # the only remaining activity on/after the effective date is genuine later stock (receipts, + # issues) valued at this standard rate. Skip those — cancelling would corrupt their valuation. + # Checking on-hand Bin qty instead would falsely skip a plain rate change, whose on-hand qty + # reverts on cancellation, silently leaving the standard rate out of sync with every SLE. + if isc.has_stock_activity_on_or_after_effective_date(): + continue + + isc.flags.from_source_reconciliation = True + isc.cancel() + def on_cancel(self): self.validate_reserved_stock() self.ignore_linked_doctypes = ( @@ -127,11 +188,14 @@ class StockReconciliation(StockController): "Stock Ledger Entry", "Repost Item Valuation", "Serial and Batch Bundle", + "Item Standard Cost", ) + self.make_sle_on_cancel() self.make_gl_entries_on_cancel() self.repost_future_sle_and_gle() self.delete_auto_created_batches() + self.cancel_created_item_standard_cost() def make_bundle_for_current_qty(self): from erpnext.stock.serial_batch_bundle import SerialBatchCreation @@ -174,19 +238,45 @@ class StockReconciliation(StockController): ) def validate_standard_cost_items(self): - """Stock Reconciliation is not allowed for Standard Cost items — their rate is changed - only through the Item Standard Cost doctype (which creates the revaluation reco itself).""" + """Validate the Standard Cost rows of the reconciliation. + + For a Standard Cost item the valuation rate is owned by Item Standard Cost, so a reconciliation + is primarily a quantity adjustment (the value difference is booked to Stock Adjustment at the + standard rate). Two things it may do with the rate, handled on submit: + - opening entry (no Item Standard Cost yet): the entered rate sets the item's standard cost + (set_standard_cost_for_opening_items); + - rate change (rate differs from the current standard): a new Item Standard Cost is created, + which revalues on-hand stock (apply_standard_cost_rate_changes). + + Here we only guard the inputs: an opening row needs a positive rate, and because a standard cost + is company-wide, all rows for the same item must carry the same rate.""" if self.flags.via_item_standard_cost: return + from erpnext.stock.doctype.item_standard_cost.item_standard_cost import has_item_standard_cost + + rates = {} for item in self.items: - if item.item_code and is_standard_cost_item(item.item_code, self.company): + if not item.item_code or not is_standard_cost_item(item.item_code, self.company): + continue + + if not has_item_standard_cost(item.item_code, self.company) and flt(item.valuation_rate) <= 0: + # Opening entry with no standard cost yet: there is no rate to value the stock at. frappe.throw( _( - "Row #{0}: Stock Reconciliation is not allowed for Item {1}, which uses the Standard Cost valuation method. Change its rate through Item Standard Cost instead." + "Row #{0}: Enter a Valuation Rate for Item {1} to set up its opening Standard Cost." ).format(item.idx, get_link_to_form("Item", item.item_code)) ) + if flt(item.valuation_rate): + rate = flt(item.valuation_rate, item.precision("valuation_rate")) + if rates.setdefault(item.item_code, rate) != rate: + frappe.throw( + _( + "Row #{0}: Valuation Rate for Item {1} must be the same across all rows, as it is the item's company-wide Standard Cost." + ).format(item.idx, get_link_to_form("Item", item.item_code)) + ) + def set_current_serial_and_batch_bundle(self, voucher_detail_no=None, save=False) -> None: """Set Serial and Batch Bundle for each item""" for item in self.items: @@ -196,9 +286,9 @@ class StockReconciliation(StockController): if not item.item_code: continue - # Standard Cost revaluation recos are pure value changes: qty is unchanged and the SLE is - # revalued at the standard rate, so no serial/batch bundle is created (see update_stock_ledger, - # which routes these rows through the single revaluation SLE path). + # A Standard Cost item is valued at the standard rate regardless of serial/batch, so no + # serial/batch bundle is created; update_stock_ledger routes these rows through the single + # SLE path (qty may change, valuation always comes from the standard rate). if is_standard_cost_item(item.item_code, self.company): continue @@ -452,7 +542,7 @@ class StockReconciliation(StockController): if not item.item_code: continue - # Standard Cost revaluation recos are pure value changes; no serial/batch bundle needed. + # Standard Cost items are valued at the standard rate; no serial/batch bundle needed. if is_standard_cost_item(item.item_code, self.company): continue @@ -576,8 +666,8 @@ class StockReconciliation(StockController): if item.valuation_rate is None: item.valuation_rate = item_dict.get("rate") - # Standard Cost items are revalued by rate only; don't pull serial nos onto the row, or a - # serial/batch bundle would be built for what must stay a pure value-change SLE. + # Standard Cost items are valued at the standard rate; don't pull serial nos onto the row, + # or a serial/batch bundle would be built for what stays a single standard-rate SLE. if item_dict.get("serial_nos") and not is_standard_cost_item(item.item_code, self.company): item.current_serial_no = item_dict.get("serial_nos") if self.purpose == "Stock Reconciliation" and not item.serial_no and item.qty: @@ -794,9 +884,9 @@ class StockReconciliation(StockController): "Item", row.item_code, ["has_serial_no", "has_batch_no"], as_dict=1 ) - # A Standard Cost item is revalued by rate alone (qty unchanged, valuation from the standard - # rate), so even a serialized/batched one is posted through the single revaluation SLE path - # without a serial/batch bundle, the same as a non-serial item. + # A Standard Cost item is always valued at the standard rate (qty may change, the rate does + # not), so even a serialized/batched one is posted through the single SLE path without a + # serial/batch bundle, the same as a non-serial item. if (item.has_serial_no or item.has_batch_no) and not is_standard_cost_item( row.item_code, self.company ): @@ -1460,6 +1550,18 @@ def get_stock_balance_for( ) ) + # For a Standard Cost item with no on-hand stock to derive a rate from (an opening entry, or an + # empty warehouse), default the rate to the standard rate effective on the posting date so the form + # shows it. When stock already exists, the balance rate is already the standard rate, so it is left + # alone - overriding it with the (possibly just-changed) standard rate would hide a real value change + # from remove_items_with_no_change. + if not rate and company and is_standard_cost_item(item_code, company): + from erpnext.stock.doctype.item_standard_cost.item_standard_cost import get_item_standard_rate + + standard_rate = get_item_standard_rate(item_code, company, posting_date) + if standard_rate is not None: + rate = standard_rate + return { "qty": qty, "rate": rate,