refactor(postgres): address Greptile review on Assets conversions

- cancel_movement_entries: filter the parent Asset Movement's docstatus via a qb join
  (as the original SQL did), instead of the child Asset Movement Item.docstatus. Behaviour
  is identical in normal flows (child docstatus is synced) but this is exactly faithful.
- get_maintenance_log: add a both-engine test for this previously-untested whitelisted
  endpoint. Confirms the frappe v16 dict aggregate field spec ({"COUNT": ...}) runs and
  returns correct per-status counts (no runtime crash).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Mihir Kandoi
2026-06-19 10:02:59 +05:30
parent fc9608d14d
commit 4bc3420b21
2 changed files with 40 additions and 4 deletions

View File

@@ -735,10 +735,16 @@ class Asset(AccountsController):
frappe.throw(_("Asset cannot be cancelled, as it is already {0}").format(self.status))
def cancel_movement_entries(self):
movements = frappe.get_all(
"Asset Movement Item",
filters={"asset": self.name, "docstatus": 1},
fields=["parent as name"],
# filter the parent Asset Movement's docstatus (as the original SQL did), not the child row's
asm = frappe.qb.DocType("Asset Movement")
asm_item = frappe.qb.DocType("Asset Movement Item")
movements = (
frappe.qb.from_(asm_item)
.inner_join(asm)
.on(asm_item.parent == asm.name)
.select(asm.name)
.where((asm_item.asset == self.name) & (asm.docstatus == 1))
.run(as_dict=True)
)
for movement in movements:

View File

@@ -18,6 +18,36 @@ class TestAssetMaintenance(ERPNextTestSuite):
self.asset_name = frappe.db.get_value("Asset", {"purchase_receipt": self.pr.name}, "name")
self.asset_doc = frappe.get_doc("Asset", self.asset_name)
def test_get_maintenance_log_counts_by_status(self):
"""get_maintenance_log uses a v16 dict aggregate field spec
({"COUNT": "asset_name", "as": "count"}); confirm it runs and returns correct per-status counts
on both engines (the whitelisted endpoint was previously untested)."""
from erpnext.assets.doctype.asset_maintenance.asset_maintenance import get_maintenance_log
self.asset_doc.available_for_use_date = nowdate()
self.asset_doc.purchase_date = nowdate()
self.asset_doc.save()
frappe.get_doc(
{
"doctype": "Asset Maintenance",
"asset_name": self.asset_name,
"maintenance_team": "Team Awesome",
"company": "_Test Company",
"asset_maintenance_tasks": get_maintenance_tasks(),
}
).insert()
rows = get_maintenance_log(self.asset_name)
# the dict aggregate spec did not crash and returned grouped rows...
self.assertTrue(rows)
self.assertTrue(all("maintenance_status" in r for r in rows))
# ...and the per-status counts sum to the total number of logs for this asset
self.assertEqual(
sum(r["count"] for r in rows),
frappe.db.count("Asset Maintenance Log", {"asset_name": self.asset_name}),
)
def test_create_asset_maintenance_with_log(self):
month_end_date = get_last_day(nowdate())