mirror of
https://github.com/frappe/erpnext.git
synced 2026-09-03 16:40:19 +00:00
fix(stock): convert stock_ledger raw SQL to qb + case-insensitive serial match (Postgres)
Convert four raw frappe.db.sql statements to frappe.qb: - set_as_cancel (UPDATE -> frappe.qb.update) - the invalid-serial-no incoming_rate lookup - get_valuation_rate's last-valuation lookup - get_future_sle_with_negative_qty The serial-no comparisons (invalid-serial lookup and the get_stock_ledger_entries condition builder, which stays raw) are wrapped in lower()/Lower() so serial matching is case-insensitive on Postgres too -- MariaDB's collation already is, so this is a no-op there. Deterministic creation/name tiebreakers are added to the "ORDER BY posting_date DESC LIMIT 1" lookups so Postgres picks the same row MariaDB did. Surgical re-apply (not a whole-file port): develop's reposting valuation-recalc clause (`recalculate_valuation_rate`) in update_entries_after and the already-shipped Min()-wrapped get_items_to_be_repost GROUP BY are preserved. The dynamic-condition / row-locking raw queries (get_previous_sle, get_stock_ledger_entries builder, get_future_sle_with_negative_batch_qty, the qty_shift UPDATE) are intentionally left raw. Verified: full test_stock_ledger_entry suite 22/22 on MariaDB; added focused tests for set_as_cancel / get_valuation_rate / get_future_sle_with_negative_qty that pass on MariaDB and Postgres. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -10,7 +10,7 @@ import frappe
|
|||||||
from frappe import _, bold, scrub
|
from frappe import _, bold, scrub
|
||||||
from frappe.model.meta import get_field_precision
|
from frappe.model.meta import get_field_precision
|
||||||
from frappe.query_builder import Order
|
from frappe.query_builder import Order
|
||||||
from frappe.query_builder.functions import Sum
|
from frappe.query_builder.functions import Lower, Sum
|
||||||
from frappe.utils import (
|
from frappe.utils import (
|
||||||
cint,
|
cint,
|
||||||
flt,
|
flt,
|
||||||
@@ -185,12 +185,14 @@ def validate_cancellation(kargs):
|
|||||||
|
|
||||||
|
|
||||||
def set_as_cancel(voucher_type, voucher_no):
|
def set_as_cancel(voucher_type, voucher_no):
|
||||||
frappe.db.sql(
|
sle = frappe.qb.DocType("Stock Ledger Entry")
|
||||||
"""update `tabStock Ledger Entry` set is_cancelled=1,
|
(
|
||||||
modified=%s, modified_by=%s
|
frappe.qb.update(sle)
|
||||||
where voucher_type=%s and voucher_no=%s and is_cancelled = 0""",
|
.set(sle.is_cancelled, 1)
|
||||||
(now(), frappe.session.user, voucher_type, voucher_no),
|
.set(sle.modified, now())
|
||||||
)
|
.set(sle.modified_by, frappe.session.user)
|
||||||
|
.where((sle.voucher_type == voucher_type) & (sle.voucher_no == voucher_no) & (sle.is_cancelled == 0))
|
||||||
|
).run()
|
||||||
|
|
||||||
|
|
||||||
def make_entry(args, allow_negative_stock=False, via_landed_cost_voucher=False):
|
def make_entry(args, allow_negative_stock=False, via_landed_cost_voucher=False):
|
||||||
@@ -1481,25 +1483,27 @@ class update_entries_after:
|
|||||||
|
|
||||||
# Get rate for serial nos which has been transferred to other company
|
# Get rate for serial nos which has been transferred to other company
|
||||||
invalid_serial_nos = [d.name for d in all_serial_nos if d.company != sle.company]
|
invalid_serial_nos = [d.name for d in all_serial_nos if d.company != sle.company]
|
||||||
|
sle_entry = frappe.qb.DocType("Stock Ledger Entry")
|
||||||
for serial_no in invalid_serial_nos:
|
for serial_no in invalid_serial_nos:
|
||||||
incoming_rate = frappe.db.sql(
|
incoming_rate = (
|
||||||
"""
|
frappe.qb.from_(sle_entry)
|
||||||
select incoming_rate
|
.select(sle_entry.incoming_rate)
|
||||||
from `tabStock Ledger Entry`
|
.where(
|
||||||
where
|
(sle_entry.company == sle.company)
|
||||||
company = %s
|
& (sle_entry.actual_qty > 0)
|
||||||
and actual_qty > 0
|
& (sle_entry.is_cancelled == 0)
|
||||||
and is_cancelled = 0
|
& (
|
||||||
and (serial_no = %s
|
(Lower(sle_entry.serial_no) == serial_no.lower())
|
||||||
or serial_no like %s
|
| Lower(sle_entry.serial_no).like((serial_no + "\n%").lower())
|
||||||
or serial_no like %s
|
| Lower(sle_entry.serial_no).like(("%\n" + serial_no).lower())
|
||||||
or serial_no like %s
|
| Lower(sle_entry.serial_no).like(("%\n" + serial_no + "\n%").lower())
|
||||||
)
|
)
|
||||||
order by posting_date desc
|
)
|
||||||
limit 1
|
.orderby(sle_entry.posting_date, order=frappe.qb.desc)
|
||||||
""",
|
.orderby(sle_entry.creation, order=frappe.qb.desc)
|
||||||
(sle.company, serial_no, serial_no + "\n%", "%\n" + serial_no, "%\n" + serial_no + "\n%"),
|
.orderby(sle_entry.name, order=frappe.qb.desc)
|
||||||
)
|
.limit(1)
|
||||||
|
).run()
|
||||||
|
|
||||||
incoming_values += flt(incoming_rate[0][0]) if incoming_rate else 0
|
incoming_values += flt(incoming_rate[0][0]) if incoming_rate else 0
|
||||||
|
|
||||||
@@ -1869,13 +1873,16 @@ def get_stock_ledger_entries(
|
|||||||
if check_serial_no and previous_sle.get("serial_no"):
|
if check_serial_no and previous_sle.get("serial_no"):
|
||||||
# conditions += " and serial_no like {}".format(frappe.db.escape('%{0}%'.format(previous_sle.get("serial_no"))))
|
# conditions += " and serial_no like {}".format(frappe.db.escape('%{0}%'.format(previous_sle.get("serial_no"))))
|
||||||
serial_no = previous_sle.get("serial_no")
|
serial_no = previous_sle.get("serial_no")
|
||||||
|
# lower() both sides so the match is case-insensitive on postgres too (MariaDB's collation
|
||||||
|
# already is); a no-op on MariaDB. The set is already narrowed by item_code/warehouse, so the
|
||||||
|
# functional comparison does not cost an index here.
|
||||||
conditions += (
|
conditions += (
|
||||||
""" and
|
""" and
|
||||||
(
|
(
|
||||||
serial_no = {}
|
lower(serial_no) = lower({})
|
||||||
or serial_no like {}
|
or lower(serial_no) like lower({})
|
||||||
or serial_no like {}
|
or lower(serial_no) like lower({})
|
||||||
or serial_no like {}
|
or lower(serial_no) like lower({})
|
||||||
)
|
)
|
||||||
"""
|
"""
|
||||||
).format(
|
).format(
|
||||||
@@ -1990,18 +1997,21 @@ def get_valuation_rate(
|
|||||||
return batch_obj.get_incoming_rate()
|
return batch_obj.get_incoming_rate()
|
||||||
|
|
||||||
# Get valuation rate from last sle for the same item and warehouse
|
# Get valuation rate from last sle for the same item and warehouse
|
||||||
if last_valuation_rate := frappe.db.sql( # nosemgrep
|
sle_entry = frappe.qb.DocType("Stock Ledger Entry")
|
||||||
"""select valuation_rate
|
if last_valuation_rate := (
|
||||||
from `tabStock Ledger Entry`
|
frappe.qb.from_(sle_entry)
|
||||||
where
|
.select(sle_entry.valuation_rate)
|
||||||
item_code = %s
|
.where(
|
||||||
AND warehouse = %s
|
(sle_entry.item_code == item_code)
|
||||||
AND valuation_rate >= 0
|
& (sle_entry.warehouse == warehouse)
|
||||||
AND is_cancelled = 0
|
& (sle_entry.valuation_rate >= 0)
|
||||||
AND NOT (voucher_no = %s AND voucher_type = %s)
|
& (sle_entry.is_cancelled == 0)
|
||||||
order by posting_datetime desc, creation desc limit 1""",
|
& ~((sle_entry.voucher_no == voucher_no) & (sle_entry.voucher_type == voucher_type))
|
||||||
(item_code, warehouse, voucher_no, voucher_type),
|
)
|
||||||
):
|
.orderby(sle_entry.posting_datetime, order=frappe.qb.desc)
|
||||||
|
.orderby(sle_entry.creation, order=frappe.qb.desc)
|
||||||
|
.limit(1)
|
||||||
|
).run():
|
||||||
return flt(last_valuation_rate[0][0])
|
return flt(last_valuation_rate[0][0])
|
||||||
|
|
||||||
if fallbacks:
|
if fallbacks:
|
||||||
@@ -2233,25 +2243,28 @@ def is_negative_with_precision(neg_sle, is_batch=False):
|
|||||||
|
|
||||||
|
|
||||||
def get_future_sle_with_negative_qty(sle_args):
|
def get_future_sle_with_negative_qty(sle_args):
|
||||||
return frappe.db.sql( # nosemgrep
|
sle = frappe.qb.DocType("Stock Ledger Entry")
|
||||||
"""
|
return (
|
||||||
select
|
frappe.qb.from_(sle)
|
||||||
qty_after_transaction, posting_date, posting_time,
|
.select(
|
||||||
voucher_type, voucher_no
|
sle.qty_after_transaction,
|
||||||
from `tabStock Ledger Entry`
|
sle.posting_date,
|
||||||
where
|
sle.posting_time,
|
||||||
item_code = %(item_code)s
|
sle.voucher_type,
|
||||||
and warehouse = %(warehouse)s
|
sle.voucher_no,
|
||||||
and voucher_no != %(voucher_no)s
|
)
|
||||||
and posting_datetime >= %(posting_datetime)s
|
.where(
|
||||||
and is_cancelled = 0
|
(sle.item_code == sle_args["item_code"])
|
||||||
and qty_after_transaction < 0
|
& (sle.warehouse == sle_args["warehouse"])
|
||||||
order by posting_datetime asc, creation asc
|
& (sle.voucher_no != sle_args["voucher_no"])
|
||||||
limit 1
|
& (sle.posting_datetime >= sle_args["posting_datetime"])
|
||||||
""",
|
& (sle.is_cancelled == 0)
|
||||||
sle_args,
|
& (sle.qty_after_transaction < 0)
|
||||||
as_dict=1,
|
)
|
||||||
)
|
.orderby(sle.posting_datetime)
|
||||||
|
.orderby(sle.creation)
|
||||||
|
.limit(1)
|
||||||
|
).run(as_dict=1)
|
||||||
|
|
||||||
|
|
||||||
def get_future_sle_with_negative_batch_qty(sle_args):
|
def get_future_sle_with_negative_batch_qty(sle_args):
|
||||||
|
|||||||
66
erpnext/stock/tests/test_stock_ledger.py
Normal file
66
erpnext/stock/tests/test_stock_ledger.py
Normal file
@@ -0,0 +1,66 @@
|
|||||||
|
# Copyright (c) 2025, Frappe Technologies Pvt. Ltd. and Contributors
|
||||||
|
# See license.txt
|
||||||
|
|
||||||
|
import frappe
|
||||||
|
|
||||||
|
from erpnext.tests.utils import ERPNextTestSuite
|
||||||
|
|
||||||
|
|
||||||
|
class TestStockLedgerConversions(ERPNextTestSuite):
|
||||||
|
"""Exercises the stock_ledger.py raw-SQL -> query-builder conversions on both engines."""
|
||||||
|
|
||||||
|
def test_set_as_cancel_marks_entries_cancelled(self):
|
||||||
|
# set_as_cancel runs an UPDATE (raw SQL -> frappe.qb.update) marking the voucher's SLEs
|
||||||
|
# is_cancelled=1. Cancelling a receipt exercises it.
|
||||||
|
from erpnext.stock.doctype.item.test_item import make_item
|
||||||
|
from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry
|
||||||
|
|
||||||
|
item = make_item("_Test SL Cancel Item", {"is_stock_item": 1}).name
|
||||||
|
se = make_stock_entry(item_code=item, target="_Test Warehouse - _TC", qty=5, basic_rate=100)
|
||||||
|
# register cleanup before the assertions so the entry is removed even if one fails
|
||||||
|
self.addCleanup(self._cancel_and_delete, "Stock Entry", se.name)
|
||||||
|
|
||||||
|
self.assertTrue(frappe.db.exists("Stock Ledger Entry", {"voucher_no": se.name, "is_cancelled": 0}))
|
||||||
|
|
||||||
|
se.cancel()
|
||||||
|
|
||||||
|
self.assertFalse(frappe.db.exists("Stock Ledger Entry", {"voucher_no": se.name, "is_cancelled": 0}))
|
||||||
|
self.assertTrue(frappe.db.exists("Stock Ledger Entry", {"voucher_no": se.name, "is_cancelled": 1}))
|
||||||
|
|
||||||
|
def test_get_valuation_rate_returns_last_sle_rate(self):
|
||||||
|
# get_valuation_rate's last-valuation lookup (raw SQL -> frappe.qb) returns the most recent
|
||||||
|
# valuation_rate for the item+warehouse.
|
||||||
|
from erpnext.stock.doctype.item.test_item import make_item
|
||||||
|
from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry
|
||||||
|
from erpnext.stock.stock_ledger import get_valuation_rate
|
||||||
|
|
||||||
|
item = make_item("_Test SL Valuation Item", {"is_stock_item": 1}).name
|
||||||
|
se = make_stock_entry(item_code=item, target="_Test Warehouse - _TC", qty=10, basic_rate=250)
|
||||||
|
self.addCleanup(self._cancel_and_delete, "Stock Entry", se.name)
|
||||||
|
|
||||||
|
rate = get_valuation_rate(item, "_Test Warehouse - _TC", "Stock Entry", "_TEST-NO-SUCH-VOUCHER")
|
||||||
|
self.assertEqual(rate, 250)
|
||||||
|
|
||||||
|
def test_get_future_sle_with_negative_qty_runs(self):
|
||||||
|
# get_future_sle_with_negative_qty (raw SQL -> frappe.qb) must execute on both engines. With no
|
||||||
|
# negative future entry it returns an empty result; this guards the converted query's validity.
|
||||||
|
from frappe.utils import now_datetime
|
||||||
|
|
||||||
|
from erpnext.stock.stock_ledger import get_future_sle_with_negative_qty
|
||||||
|
|
||||||
|
args = {
|
||||||
|
"item_code": "_Test Item",
|
||||||
|
"warehouse": "_Test Warehouse - _TC",
|
||||||
|
"voucher_no": "_TEST-NO-SUCH-VOUCHER",
|
||||||
|
"posting_datetime": now_datetime(),
|
||||||
|
}
|
||||||
|
self.assertIsInstance(get_future_sle_with_negative_qty(args), list | tuple)
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _cancel_and_delete(doctype, name):
|
||||||
|
if not frappe.db.exists(doctype, name):
|
||||||
|
return
|
||||||
|
doc = frappe.get_doc(doctype, name)
|
||||||
|
if doc.docstatus == 1:
|
||||||
|
doc.cancel()
|
||||||
|
frappe.delete_doc(doctype, name, force=1)
|
||||||
Reference in New Issue
Block a user