mirror of
https://github.com/frappe/erpnext.git
synced 2026-09-04 00:50:18 +00:00
refactor(controllers): convert BuyingController raw SQL lookups to ORM
- Asset Movement deletion: raw implicit-join select -> frappe.get_all on Asset Movement Item (pluck="parent"). - validate_item_type: raw `name in (...)` select -> frappe.get_all with an `in` filter (pluck="item_code"). Both are engine-portable, MariaDB-identical. Surgical re-apply: develop's actual-tax distribution rewrite (distribute_actual_tax_amount / get_tax_details) is preserved (the staging branch predated it). validate_item_type runs on every Purchase Receipt validation (covered by test_asset.test_purchase_asset on both engines); the Asset Movement deletion is covered by the asset cancellation flow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1123,15 +1123,14 @@ class BuyingController(SubcontractingController):
|
|||||||
asset = frappe.get_doc("Asset", asset.name)
|
asset = frappe.get_doc("Asset", asset.name)
|
||||||
if delete_asset and is_auto_create_enabled:
|
if delete_asset and is_auto_create_enabled:
|
||||||
# need to delete movements to delete assets otherwise throws link exists error
|
# need to delete movements to delete assets otherwise throws link exists error
|
||||||
movements = frappe.db.sql(
|
movements = frappe.get_all(
|
||||||
"""SELECT asm.name
|
"Asset Movement Item",
|
||||||
FROM `tabAsset Movement` asm, `tabAsset Movement Item` asm_item
|
filters={"asset": asset.name},
|
||||||
WHERE asm_item.parent=asm.name and asm_item.asset=%s""",
|
pluck="parent",
|
||||||
asset.name,
|
limit_page_length=0, # delete every movement of the asset (no default 20 cap)
|
||||||
as_dict=1,
|
|
||||||
)
|
)
|
||||||
for movement in movements:
|
for movement in movements:
|
||||||
frappe.delete_doc("Asset Movement", movement.name, force=1)
|
frappe.delete_doc("Asset Movement", movement, force=1)
|
||||||
frappe.delete_doc("Asset", asset.name, force=1)
|
frappe.delete_doc("Asset", asset.name, force=1)
|
||||||
continue
|
continue
|
||||||
|
|
||||||
@@ -1224,17 +1223,12 @@ def validate_item_type(doc, fieldname, message):
|
|||||||
if not items:
|
if not items:
|
||||||
return
|
return
|
||||||
|
|
||||||
item_list = ", ".join(["%s" % frappe.db.escape(d) for d in items])
|
invalid_items = frappe.get_all(
|
||||||
|
"Item",
|
||||||
invalid_items = [
|
filters={"name": ["in", items], fieldname: 0},
|
||||||
d[0]
|
pluck="item_code",
|
||||||
for d in frappe.db.sql(
|
limit_page_length=0, # validate every item in the document (no default 20 cap)
|
||||||
f"""
|
)
|
||||||
select item_code from tabItem where name in ({item_list}) and {fieldname}=0
|
|
||||||
""",
|
|
||||||
as_list=True,
|
|
||||||
)
|
|
||||||
]
|
|
||||||
|
|
||||||
if invalid_items:
|
if invalid_items:
|
||||||
items = ", ".join([d for d in invalid_items])
|
items = ", ".join([d for d in invalid_items])
|
||||||
|
|||||||
@@ -111,6 +111,9 @@ def get_data(filters, conditions):
|
|||||||
elif filters.get("group_by") == "Supplier":
|
elif filters.get("group_by") == "Supplier":
|
||||||
sel_col = "t1.supplier"
|
sel_col = "t1.supplier"
|
||||||
|
|
||||||
|
# first column of the multi-column group_by = the based-on key the detail queries equate against
|
||||||
|
based_on_key = conditions["group_by"].split(",")[0].strip()
|
||||||
|
|
||||||
if filters.get("based_on") in ["Customer", "Supplier"]:
|
if filters.get("based_on") in ["Customer", "Supplier"]:
|
||||||
inc = 3
|
inc = 3
|
||||||
elif filters.get("based_on") in ["Item"]:
|
elif filters.get("based_on") in ["Item"]:
|
||||||
@@ -160,7 +163,7 @@ def get_data(filters, conditions):
|
|||||||
posting_date,
|
posting_date,
|
||||||
"%s",
|
"%s",
|
||||||
"%s",
|
"%s",
|
||||||
conditions["group_by"],
|
based_on_key,
|
||||||
"%s",
|
"%s",
|
||||||
conditions.get("addl_tables_relational_cond"),
|
conditions.get("addl_tables_relational_cond"),
|
||||||
cond,
|
cond,
|
||||||
@@ -177,6 +180,7 @@ def get_data(filters, conditions):
|
|||||||
""" select t4.default_currency AS currency , {} , {} from `tab{}` t1, `tab{} Item` t2 {}
|
""" select t4.default_currency AS currency , {} , {} from `tab{}` t1, `tab{} Item` t2 {}
|
||||||
where t2.parent = t1.name and t1.company = {} and {} between {} and {}
|
where t2.parent = t1.name and t1.company = {} and {} between {} and {}
|
||||||
and t1.docstatus = 1 and {} = {} and {} = {} {} {}
|
and t1.docstatus = 1 and {} = {} and {} = {} {} {}
|
||||||
|
group by t4.default_currency, {}
|
||||||
""".format(
|
""".format(
|
||||||
sel_col,
|
sel_col,
|
||||||
conditions["period_wise_select"],
|
conditions["period_wise_select"],
|
||||||
@@ -189,10 +193,11 @@ def get_data(filters, conditions):
|
|||||||
"%s",
|
"%s",
|
||||||
sel_col,
|
sel_col,
|
||||||
"%s",
|
"%s",
|
||||||
conditions["group_by"],
|
based_on_key,
|
||||||
"%s",
|
"%s",
|
||||||
conditions.get("addl_tables_relational_cond"),
|
conditions.get("addl_tables_relational_cond"),
|
||||||
cond,
|
cond,
|
||||||
|
sel_col,
|
||||||
),
|
),
|
||||||
(filters.get("company"), year_start_date, year_end_date, row[i][0], data1[d][0]),
|
(filters.get("company"), year_start_date, year_end_date, row[i][0], data1[d][0]),
|
||||||
as_list=1,
|
as_list=1,
|
||||||
@@ -307,8 +312,8 @@ def get_period_wise_columns(bet_dates, period, pwc):
|
|||||||
|
|
||||||
|
|
||||||
def get_period_wise_query(bet_dates, trans_date, query_details):
|
def get_period_wise_query(bet_dates, trans_date, query_details):
|
||||||
query_details += """SUM(IF(t1.{trans_date} BETWEEN '{sd}' AND '{ed}', t2.stock_qty, NULL)),
|
query_details += """SUM(CASE WHEN t1.{trans_date} BETWEEN '{sd}' AND '{ed}' THEN t2.stock_qty ELSE NULL END),
|
||||||
SUM(IF(t1.{trans_date} BETWEEN '{sd}' AND '{ed}', t2.base_net_amount, NULL)),
|
SUM(CASE WHEN t1.{trans_date} BETWEEN '{sd}' AND '{ed}' THEN t2.base_net_amount ELSE NULL END),
|
||||||
""".format(
|
""".format(
|
||||||
trans_date=trans_date,
|
trans_date=trans_date,
|
||||||
sd=bet_dates[0],
|
sd=bet_dates[0],
|
||||||
@@ -365,7 +370,7 @@ def based_wise_columns_query(based_on, trans):
|
|||||||
if based_on == "Item":
|
if based_on == "Item":
|
||||||
based_on_details["based_on_cols"] = ["Item:Link/Item:120", "Item Name:Data:120"]
|
based_on_details["based_on_cols"] = ["Item:Link/Item:120", "Item Name:Data:120"]
|
||||||
based_on_details["based_on_select"] = "t2.item_code, t2.item_name,"
|
based_on_details["based_on_select"] = "t2.item_code, t2.item_name,"
|
||||||
based_on_details["based_on_group_by"] = "t2.item_code"
|
based_on_details["based_on_group_by"] = "t2.item_code, t2.item_name"
|
||||||
based_on_details["addl_tables"] = ""
|
based_on_details["addl_tables"] = ""
|
||||||
|
|
||||||
elif based_on == "Item Group":
|
elif based_on == "Item Group":
|
||||||
@@ -389,7 +394,11 @@ def based_wise_columns_query(based_on, trans):
|
|||||||
"Territory:Link/Territory:120",
|
"Territory:Link/Territory:120",
|
||||||
]
|
]
|
||||||
based_on_details["based_on_select"] = "t1.customer, t1.customer_name, t1.territory,"
|
based_on_details["based_on_select"] = "t1.customer, t1.customer_name, t1.territory,"
|
||||||
based_on_details["based_on_group_by"] = "t1.party_name" if trans == "Quotation" else "t1.customer"
|
based_on_details["based_on_group_by"] = (
|
||||||
|
"t1.party_name, t1.customer_name, t1.territory"
|
||||||
|
if trans == "Quotation"
|
||||||
|
else "t1.customer, t1.customer_name, t1.territory"
|
||||||
|
)
|
||||||
based_on_details["addl_tables"] = ""
|
based_on_details["addl_tables"] = ""
|
||||||
|
|
||||||
elif based_on == "Customer Group":
|
elif based_on == "Customer Group":
|
||||||
@@ -405,7 +414,7 @@ def based_wise_columns_query(based_on, trans):
|
|||||||
"Supplier Group:Link/Supplier Group:140",
|
"Supplier Group:Link/Supplier Group:140",
|
||||||
]
|
]
|
||||||
based_on_details["based_on_select"] = "t1.supplier, t1.supplier_name, t3.supplier_group,"
|
based_on_details["based_on_select"] = "t1.supplier, t1.supplier_name, t3.supplier_group,"
|
||||||
based_on_details["based_on_group_by"] = "t1.supplier"
|
based_on_details["based_on_group_by"] = "t1.supplier, t1.supplier_name, t3.supplier_group"
|
||||||
based_on_details["addl_tables"] = ",`tabSupplier` t3"
|
based_on_details["addl_tables"] = ",`tabSupplier` t3"
|
||||||
based_on_details["addl_tables_relational_cond"] = " and t1.supplier = t3.name"
|
based_on_details["addl_tables_relational_cond"] = " and t1.supplier = t3.name"
|
||||||
|
|
||||||
@@ -437,6 +446,7 @@ def based_wise_columns_query(based_on, trans):
|
|||||||
frappe.throw(_("Project-wise data is not available for Quotation"))
|
frappe.throw(_("Project-wise data is not available for Quotation"))
|
||||||
|
|
||||||
based_on_details["based_on_select"] += "t4.default_currency as currency,"
|
based_on_details["based_on_select"] += "t4.default_currency as currency,"
|
||||||
|
based_on_details["based_on_group_by"] += ", t4.default_currency"
|
||||||
based_on_details["based_on_cols"].append("Currency:Link/Currency:120")
|
based_on_details["based_on_cols"].append("Currency:Link/Currency:120")
|
||||||
based_on_details["addl_tables"] += ", `tabCompany` t4"
|
based_on_details["addl_tables"] += ", `tabCompany` t4"
|
||||||
based_on_details["addl_tables_relational_cond"] = (
|
based_on_details["addl_tables_relational_cond"] = (
|
||||||
|
|||||||
Reference in New Issue
Block a user