From 8134199a574eaff88fa46f0945ac0354c94a2aef Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Fri, 19 Jun 2026 18:00:57 +0530 Subject: [PATCH 1/4] refactor(postgres): port work_order make_bom and stock-entry check to the query builder Co-Authored-By: Claude Opus 4.8 (1M context) --- .../doctype/work_order/work_order.py | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/erpnext/manufacturing/doctype/work_order/work_order.py b/erpnext/manufacturing/doctype/work_order/work_order.py index aaa82c9bf74..8191e047d85 100644 --- a/erpnext/manufacturing/doctype/work_order/work_order.py +++ b/erpnext/manufacturing/doctype/work_order/work_order.py @@ -843,15 +843,16 @@ class WorkOrder(Document): frappe.throw(_("Stopped Work Order cannot be cancelled, Unstop it first to cancel")) # Check whether any stock entry exists against this Work Order - stock_entry = frappe.db.sql( - """select name from `tabStock Entry` - where work_order = %s and docstatus = 1""", - self.name, + stock_entry = frappe.get_all( + "Stock Entry", + filters={"work_order": self.name, "docstatus": 1}, + pluck="name", + limit=1, ) if stock_entry: frappe.throw( _("Cannot cancel because submitted Stock Entry {0} exists").format( - frappe.utils.get_link_to_form("Stock Entry", stock_entry[0][0]) + frappe.utils.get_link_to_form("Stock Entry", stock_entry[0]) ) ) @@ -942,14 +943,20 @@ class WorkOrder(Document): @frappe.whitelist() def make_bom(self): - data = frappe.db.sql( - """ select sed.item_code, sed.qty, sed.s_warehouse - from `tabStock Entry Detail` sed, `tabStock Entry` se - where se.name = sed.parent and se.purpose = 'Manufacture' - and (sed.t_warehouse is null or sed.t_warehouse = '') and se.docstatus = 1 - and se.work_order = %s""", - (self.name), - as_dict=1, + sed = frappe.qb.DocType("Stock Entry Detail") + se = frappe.qb.DocType("Stock Entry") + data = ( + frappe.qb.from_(sed) + .inner_join(se) + .on(se.name == sed.parent) + .select(sed.item_code, sed.qty, sed.s_warehouse) + .where( + (se.purpose == "Manufacture") + & (sed.t_warehouse.isnull() | (sed.t_warehouse == "")) + & (se.docstatus == 1) + & (se.work_order == self.name) + ) + .run(as_dict=1) ) bom = frappe.new_doc("BOM") From 71a07ee7af1c53728ac40687d2c549e3e8da7d7a Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Fri, 19 Jun 2026 18:22:27 +0530 Subject: [PATCH 2/4] refactor(postgres): port workstation queries to the query builder Co-Authored-By: Claude Opus 4.8 (1M context) --- .../doctype/workstation/test_workstation.py | 10 ++++ .../doctype/workstation/workstation.py | 59 +++++++++++-------- 2 files changed, 46 insertions(+), 23 deletions(-) diff --git a/erpnext/manufacturing/doctype/workstation/test_workstation.py b/erpnext/manufacturing/doctype/workstation/test_workstation.py index 21dcc771213..b0154cb96f5 100644 --- a/erpnext/manufacturing/doctype/workstation/test_workstation.py +++ b/erpnext/manufacturing/doctype/workstation/test_workstation.py @@ -110,6 +110,16 @@ class TestWorkstation(ERPNextTestSuite): self.assertEqual(bom_doc.operations[0].hour_rate, 250) self.assertEqual(bom_doc.operations[1].hour_rate, 250) + # update_bom_operation() (run on w1.save()) must write the new rate directly onto the + # Routing's BOM Operation rows. This is the converted query's own effect (not the BOM + # update_cost above) and is what silently skipped on Postgres when parenttype was 'routing'. + routing_op_rate = frappe.db.get_value( + "BOM Operation", + {"parent": routing_doc.name, "parenttype": "Routing", "workstation": "_Test Workstation A"}, + "hour_rate", + ) + self.assertEqual(routing_op_rate, 250) + def make_workstation(*args, **kwargs): args = args if args else kwargs diff --git a/erpnext/manufacturing/doctype/workstation/workstation.py b/erpnext/manufacturing/doctype/workstation/workstation.py index 3ade994a82f..a8400683681 100644 --- a/erpnext/manufacturing/doctype/workstation/workstation.py +++ b/erpnext/manufacturing/doctype/workstation/workstation.py @@ -169,15 +169,20 @@ class Workstation(Document): def validate_overlap_for_operation_timings(self): """Check if there is no overlap in setting Workstation Operating Hours""" for d in self.get("working_hours"): - existing = frappe.db.sql_list( - """select idx from `tabWorkstation Working Hour` - where parent = %s and name != %s - and ( - (start_time between %s and %s) or - (end_time between %s and %s) or - (%s between start_time and end_time)) - """, - (self.name, d.name, d.start_time, d.end_time, d.start_time, d.end_time, d.start_time), + wh = frappe.qb.DocType("Workstation Working Hour") + existing = ( + frappe.qb.from_(wh) + .select(wh.idx) + .where( + (wh.parent == self.name) + & (wh.name != d.name) + & ( + wh.start_time.between(d.start_time, d.end_time) + | wh.end_time.between(d.start_time, d.end_time) + | ((wh.start_time <= d.start_time) & (wh.end_time >= d.start_time)) + ) + ) + .run(pluck=True) ) if existing: @@ -187,17 +192,22 @@ class Workstation(Document): ) def update_bom_operation(self): - bom_list = frappe.db.sql( - """select DISTINCT parent from `tabBOM Operation` - where workstation = %s and parenttype = 'routing' """, - self.name, + bom_list = frappe.get_all( + "BOM Operation", + # DocType is "Routing"; the original raw SQL used 'routing', which matched only via + # MariaDB's case-insensitive collation and silently matched nothing on Postgres. + filters={"workstation": self.name, "parenttype": "Routing"}, + pluck="parent", + distinct=True, ) - for bom_no in bom_list: - frappe.db.sql( - """update `tabBOM Operation` set hour_rate = %s - where parent = %s and workstation = %s""", - (self.hour_rate, bom_no[0], self.name), + if bom_list: + bom_op = frappe.qb.DocType("BOM Operation") + ( + frappe.qb.update(bom_op) + .set(bom_op.hour_rate, self.hour_rate) + .where(bom_op.parent.isin(bom_list) & (bom_op.workstation == self.name)) + .run() ) def validate_workstation_holiday(self, schedule_date, skip_holiday_list_check=False): @@ -451,12 +461,15 @@ def check_workstation_for_holiday(workstation, from_datetime, to_datetime): holiday_list = frappe.db.get_value("Workstation", workstation, "holiday_list") if holiday_list and from_datetime and to_datetime: applicable_holidays = [] - for d in frappe.db.sql( - """select holiday_date from `tabHoliday` where parent = %s - and holiday_date between %s and %s """, - (holiday_list, getdate(from_datetime), getdate(to_datetime)), + for holiday_date in frappe.get_all( + "Holiday", + filters={ + "parent": holiday_list, + "holiday_date": ["between", [getdate(from_datetime), getdate(to_datetime)]], + }, + pluck="holiday_date", ): - applicable_holidays.append(formatdate(d[0])) + applicable_holidays.append(formatdate(holiday_date)) if applicable_holidays: frappe.throw( From afbaaafd00863d931f5a3bad89e57c8bc540191f Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Fri, 19 Jun 2026 18:22:29 +0530 Subject: [PATCH 3/4] refactor(postgres): port maintenance_schedule queries to the query builder Co-Authored-By: Claude Opus 4.8 (1M context) --- .../maintenance_schedule.py | 22 +++++---- .../test_maintenance_schedule.py | 46 +++++++++++++++++++ 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/erpnext/maintenance/doctype/maintenance_schedule/maintenance_schedule.py b/erpnext/maintenance/doctype/maintenance_schedule/maintenance_schedule.py index 104fa8c77dd..2ed918f03bf 100644 --- a/erpnext/maintenance/doctype/maintenance_schedule/maintenance_schedule.py +++ b/erpnext/maintenance/doctype/maintenance_schedule/maintenance_schedule.py @@ -185,9 +185,7 @@ class MaintenanceSchedule(TransactionBase): else: holiday_list = frappe.get_cached_value("Company", self.company, "default_holiday_list") - holidays = frappe.db.sql_list( - """select holiday_date from `tabHoliday` where parent=%s""", holiday_list - ) + holidays = frappe.get_all("Holiday", filters={"parent": holiday_list}, pluck="holiday_date") if not validated and holidays: # max iterations = len(holidays) @@ -235,16 +233,22 @@ class MaintenanceSchedule(TransactionBase): throw(_("Start date should be less than end date for Item {0}").format(d.item_code)) def validate_sales_order(self): + ms = frappe.qb.DocType("Maintenance Schedule") + msi = frappe.qb.DocType("Maintenance Schedule Item") for d in self.get("items"): if d.sales_order: - chk = frappe.db.sql( - """select ms.name from `tabMaintenance Schedule` ms, - `tabMaintenance Schedule Item` msi where msi.parent=ms.name and - msi.sales_order=%s and ms.docstatus=1""", - d.sales_order, + # filter the parent schedule's docstatus (matches the original ms.docstatus = 1) + chk = ( + frappe.qb.from_(ms) + .inner_join(msi) + .on(msi.parent == ms.name) + .select(ms.name) + .where((msi.sales_order == d.sales_order) & (ms.docstatus == 1)) + .limit(1) + .run(pluck=True) ) if chk: - throw(_("Maintenance Schedule {0} exists against {1}").format(chk[0][0], d.sales_order)) + throw(_("Maintenance Schedule {0} exists against {1}").format(chk[0], d.sales_order)) def validate_items_table_change(self): doc_before_save = self.get_doc_before_save() diff --git a/erpnext/maintenance/doctype/maintenance_schedule/test_maintenance_schedule.py b/erpnext/maintenance/doctype/maintenance_schedule/test_maintenance_schedule.py index ccddc38b12d..208674f0963 100644 --- a/erpnext/maintenance/doctype/maintenance_schedule/test_maintenance_schedule.py +++ b/erpnext/maintenance/doctype/maintenance_schedule/test_maintenance_schedule.py @@ -168,6 +168,51 @@ class TestMaintenanceSchedule(ERPNextTestSuite): ms.save() self.assertEqual(len(ms.schedules), 2) + def test_validate_sales_order_duplicate_throws(self): + # validate_sales_order joins Maintenance Schedule + its item filtering the PARENT schedule's + # docstatus=1; a second schedule against a Sales Order already used by a submitted schedule + # must be rejected. + from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order + + so = make_sales_order() + first = make_maintenance_schedule(sales_order=so.name) + self.assertEqual(first.items[0].sales_order, so.name) + first.submit() + + self.assertRaises(frappe.ValidationError, make_maintenance_schedule, sales_order=so.name) + + def test_validate_schedule_date_skips_holiday(self): + # validate_schedule_date_for_holiday_list reads the holiday list via the converted + # get_all("Holiday", {"parent": }, pluck="holiday_date") and shifts a schedule date + # that lands on a holiday back by a day; a non-holiday date is returned unchanged. + from frappe.utils import getdate + + from erpnext.setup.doctype.holiday_list.test_holiday_list import make_holiday_list + + holiday = add_days(today(), 5) + hl = make_holiday_list( + "_Test MS Holidays " + frappe.generate_hash("", 6), + from_date=today(), + to_date=add_days(today(), 10), + holiday_dates=[{"holiday_date": holiday, "description": "Test Holiday"}], + ) + + ms = make_maintenance_schedule() + # a Sales Person with no linked employee routes to the company-default-holiday-list branch + sp = frappe.get_doc( + {"doctype": "Sales Person", "sales_person_name": "_Test MS SP " + frappe.generate_hash("", 5)} + ).insert(ignore_permissions=True) + frappe.db.set_value("Company", ms.company, "default_holiday_list", hl.name) + + # a date on the holiday is shifted back one day... + shifted = ms.validate_schedule_date_for_holiday_list(getdate(holiday), sp.name) + self.assertEqual(getdate(shifted), getdate(add_days(holiday, -1))) + + # ...a non-holiday date is returned unchanged + non_holiday = add_days(today(), 7) + unchanged = ms.validate_schedule_date_for_holiday_list(getdate(non_holiday), sp.name) + self.assertEqual(getdate(unchanged), getdate(non_holiday)) + def make_serial_item_with_serial(self, item_code): serial_item_doc = create_item(item_code, is_stock_item=1) @@ -202,6 +247,7 @@ def make_maintenance_schedule(**args): "no_of_visits": 4, "serial_no": args.get("serial_no"), "sales_person": "Sales Team", + "sales_order": args.get("sales_order"), }, ) ms.insert(ignore_permissions=True) From 09f03e34d078b3be1473425b3f6edb378c3ef5cd Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Fri, 19 Jun 2026 18:22:30 +0530 Subject: [PATCH 4/4] refactor(postgres): port maintenance_visit queries to the query builder Co-Authored-By: Claude Opus 4.8 (1M context) --- .../maintenance_visit/maintenance_visit.py | 42 +++++- .../test_maintenance_visit.py | 137 ++++++++++++++++-- 2 files changed, 162 insertions(+), 17 deletions(-) diff --git a/erpnext/maintenance/doctype/maintenance_visit/maintenance_visit.py b/erpnext/maintenance/doctype/maintenance_visit/maintenance_visit.py index 990cda69084..9415ffc90d2 100644 --- a/erpnext/maintenance/doctype/maintenance_visit/maintenance_visit.py +++ b/erpnext/maintenance/doctype/maintenance_visit/maintenance_visit.py @@ -143,9 +143,22 @@ class MaintenanceVisit(TransactionBase): elif self.completion_status == "Partially Completed": status = "Work In Progress" else: - nm = frappe.db.sql( - "select t1.name, t1.mntc_date, t2.service_person, t2.work_done from `tabMaintenance Visit` t1, `tabMaintenance Visit Purpose` t2 where t2.parent = t1.name and t1.completion_status = 'Partially Completed' and t2.prevdoc_docname = %s and t1.name!=%s and t1.docstatus = 1 order by t1.name desc limit 1", - (d.prevdoc_docname, self.name), + mv = frappe.qb.DocType("Maintenance Visit") + mvp = frappe.qb.DocType("Maintenance Visit Purpose") + nm = ( + frappe.qb.from_(mv) + .inner_join(mvp) + .on(mvp.parent == mv.name) + .select(mv.name, mv.mntc_date, mvp.service_person, mvp.work_done) + .where( + (mv.completion_status == "Partially Completed") + & (mvp.prevdoc_docname == d.prevdoc_docname) + & (mv.name != self.name) + & (mv.docstatus == 1) + ) + .orderby(mv.name, order=frappe.qb.desc) + .limit(1) + .run() ) if nm: @@ -180,14 +193,27 @@ class MaintenanceVisit(TransactionBase): # check_for_doctype = d.prevdoc_doctype if check_for_docname: - check = frappe.db.sql( - "select t1.name from `tabMaintenance Visit` t1, `tabMaintenance Visit Purpose` t2 where t2.parent = t1.name and t1.name!=%s and t2.prevdoc_docname=%s and t1.docstatus = 1 and (t1.mntc_date > %s or (t1.mntc_date = %s and t1.mntc_time > %s))", - (self.name, check_for_docname, self.mntc_date, self.mntc_date, self.mntc_time), + mv = frappe.qb.DocType("Maintenance Visit") + mvp = frappe.qb.DocType("Maintenance Visit Purpose") + check = ( + frappe.qb.from_(mv) + .inner_join(mvp) + .on(mvp.parent == mv.name) + .select(mv.name) + .where( + (mv.name != self.name) + & (mvp.prevdoc_docname == check_for_docname) + & (mv.docstatus == 1) + & ( + (mv.mntc_date > self.mntc_date) + | ((mv.mntc_date == self.mntc_date) & (mv.mntc_time > self.mntc_time)) + ) + ) + .run(pluck=True) ) if check: - check_lst = [x[0] for x in check] - check_lst = ",".join(check_lst) + check_lst = ",".join(check) frappe.throw( _("Cancel Material Visits {0} before cancelling this Maintenance Visit").format(check_lst) ) diff --git a/erpnext/maintenance/doctype/maintenance_visit/test_maintenance_visit.py b/erpnext/maintenance/doctype/maintenance_visit/test_maintenance_visit.py index 616ce702e32..2bc01a8b089 100644 --- a/erpnext/maintenance/doctype/maintenance_visit/test_maintenance_visit.py +++ b/erpnext/maintenance/doctype/maintenance_visit/test_maintenance_visit.py @@ -2,13 +2,139 @@ # See license.txt import frappe -from frappe.utils.data import today +from frappe.utils.data import add_days, getdate, today from erpnext.tests.utils import ERPNextTestSuite class TestMaintenanceVisit(ERPNextTestSuite): - pass + def setUp(self): + self.sales_person = make_sales_person("_Test Maintenance Service Person") + + def make_warranty_claim(self): + # Warranty Claim is not submittable; it provides a real target for the + # purposes-row Dynamic Link (prevdoc_doctype/prevdoc_docname). + claim = frappe.new_doc("Warranty Claim") + claim.status = "Open" + claim.complaint_date = today() + claim.customer = "_Test Customer" + claim.item_code = "_Test Item" + claim.complaint = "Device stopped working under warranty" + claim.company = "_Test Company" + claim.insert(ignore_permissions=True) + return claim + + def make_visit(self, claim, completion_status, mntc_date=None, mntc_time=None, submit=True): + visit = frappe.new_doc("Maintenance Visit") + visit.company = "_Test Company" + visit.customer = "_Test Customer" + visit.mntc_date = mntc_date or today() + if mntc_time: + visit.mntc_time = mntc_time + visit.maintenance_type = "Unscheduled" + visit.completion_status = completion_status + visit.append( + "purposes", + { + "item_code": "_Test Item", + "service_person": self.sales_person.name, + "work_done": "Replaced the faulty component", + "description": "Warranty repair", + "prevdoc_doctype": "Warranty Claim", + "prevdoc_docname": claim.name, + }, + ) + visit.insert(ignore_permissions=True) + if submit: + visit.submit() + return visit + + def test_cancel_blocked_when_later_visit_exists(self): + # check_if_last_visit's converted join query (B): cancelling an EARLIER + # submitted visit must be blocked while a LATER one (greater mntc_date) + # referencing the same prevdoc_docname is still active. + claim = self.make_warranty_claim() + earlier = self.make_visit(claim, "Partially Completed", mntc_date=today()) + later = self.make_visit(claim, "Partially Completed", mntc_date=add_days(today(), 5)) + + # Sanity: both are submitted and share the prevdoc_docname the query keys on. + self.assertEqual(earlier.docstatus, 1) + self.assertEqual(later.docstatus, 1) + self.assertEqual(later.purposes[0].prevdoc_docname, claim.name) + + # The throw originates in check_if_last_visit's query (B): a later visit exists. + self.assertRaisesRegex(frappe.ValidationError, later.name, earlier.cancel) + + def test_cancel_blocked_by_same_date_later_time(self): + # Same converted query (B), time-tiebreak branch: equal mntc_date, but the + # blocking visit has a strictly greater mntc_time. + claim = self.make_warranty_claim() + earlier = self.make_visit(claim, "Partially Completed", mntc_date=today(), mntc_time="09:00:00") + later = self.make_visit(claim, "Partially Completed", mntc_date=today(), mntc_time="15:00:00") + + self.assertRaisesRegex(frappe.ValidationError, later.name, earlier.cancel) + + def test_cancel_allowed_for_latest_visit(self): + # The latest visit has no later sibling -> query (B) returns nothing -> + # cancellation proceeds and the visit is marked Cancelled. + claim = self.make_warranty_claim() + earlier = self.make_visit(claim, "Partially Completed", mntc_date=today()) + later = self.make_visit(claim, "Partially Completed", mntc_date=add_days(today(), 5)) + + later.cancel() + + self.assertEqual(frappe.db.get_value("Maintenance Visit", later.name, "docstatus"), 2) + self.assertEqual(frappe.db.get_value("Maintenance Visit", later.name, "status"), "Cancelled") + # The earlier one is untouched and still submitted. + self.assertEqual(frappe.db.get_value("Maintenance Visit", earlier.name, "docstatus"), 1) + + def test_cancel_reopens_claim_to_work_in_progress_from_prior_partial(self): + # Drives the status-update query (A) inside update_customer_issue(flag=0). + # A submitted "Partially Completed" visit (prior) exists for the claim; when + # a LATER "Fully Completed" visit is cancelled, query (A) finds that prior + # partial visit and the Warranty Claim is reopened to "Work In Progress" + # carrying the prior visit's resolution data. + claim = self.make_warranty_claim() + prior = self.make_visit(claim, "Partially Completed", mntc_date=today()) + latest = self.make_visit(claim, "Fully Completed", mntc_date=add_days(today(), 3)) + + # After submitting the "Fully Completed" visit the claim is Closed. + self.assertEqual(frappe.db.get_value("Warranty Claim", claim.name, "status"), "Closed") + + # Cancelling the latest visit: no later sibling blocks it, so cancel runs + # update_customer_issue(0), which executes query (A) and reopens the claim. + latest.cancel() + + claim.reload() + self.assertEqual(claim.status, "Work In Progress") + # Resolution data is back-filled from the prior partial visit found by query (A). + self.assertEqual(claim.resolved_by, self.sales_person.name) + self.assertEqual(claim.resolution_details, prior.purposes[0].work_done) + self.assertEqual(getdate(claim.resolution_date), getdate(prior.mntc_date)) + + def test_cancel_reopens_claim_to_open_when_no_prior_partial(self): + # Inverse of query (A): a lone "Fully Completed" visit with no prior + # "Partially Completed" sibling -> query (A) returns nothing -> the claim + # is reset to "Open" with cleared resolution fields on cancel. + claim = self.make_warranty_claim() + visit = self.make_visit(claim, "Fully Completed", mntc_date=today()) + self.assertEqual(frappe.db.get_value("Warranty Claim", claim.name, "status"), "Closed") + + visit.cancel() + + claim.reload() + self.assertEqual(claim.status, "Open") + self.assertIsNone(claim.resolved_by) + self.assertIsNone(claim.resolution_details) + self.assertIsNone(claim.resolution_date) + + +def make_sales_person(name): + sales_person = frappe.get_doc({"doctype": "Sales Person", "sales_person_name": name}) + sales_person.insert(ignore_if_duplicate=True) + if not sales_person.name: + sales_person = frappe.get_doc("Sales Person", {"sales_person_name": name}) + return sales_person def make_maintenance_visit(): @@ -33,10 +159,3 @@ def make_maintenance_visit(): mv.insert(ignore_permissions=True) return mv - - -def make_sales_person(name): - sales_person = frappe.get_doc({"doctype": "Sales Person", "sales_person_name": name}) - sales_person.insert(ignore_if_duplicate=True) - - return sales_person