Merge pull request #56433 from aerele/backport-56376

fix: skip over-allowance qty validation for non-stock items (backport #56335)

erpnext/__init__.py

@@ -6,7 +6,7 @@ import frappe
 from frappe.model.document import Document
 from frappe.utils.user import is_website_user
 
-__version__ = "16.25.0"
+__version__ = "16.15.1"
 
 
 def get_default_company(user=None):

erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py

@@ -2928,6 +2928,24 @@ class TestPurchaseInvoice(ERPNextTestSuite, StockTestMixin):
 		# Test 4 - Since this PI is overbilled by 130% and only 120% is allowed, it will fail
 		self.assertRaises(frappe.ValidationError, pi.submit)
 
+	@ERPNextTestSuite.change_settings("Accounts Settings", {"over_billing_allowance": 0})
+	def test_non_stock_item_over_billing_against_po_is_blocked(self):
+		service_item = create_item(
+			"_Test Service Item Non Stock PI",
+			is_stock_item=0,
+			is_purchase_item=1,
+		).name
+
+		po = create_purchase_order(item_code=service_item, qty=5, rate=100, do_not_save=False)
+		po.submit()
+
+		pi = make_pi_from_po(po.name)
+		pi.items[0].qty = 10  # overbill by 100 %
+		pi.save()
+
+		with self.assertRaises(frappe.ValidationError):
+			pi.submit()
+
 	def test_discount_percentage_not_set_when_amount_is_manually_set(self):
 		pi = make_purchase_invoice(do_not_save=True)
 		discount_amount = 7

erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py

@@ -3865,6 +3865,51 @@ class TestSalesInvoice(ERPNextTestSuite):
 		self.assertTrue("cannot overbill" in str(err.exception).lower())
 		dn.cancel()
 
+	@ERPNextTestSuite.change_settings("Accounts Settings", {"over_billing_allowance": 0})
+	def test_non_stock_item_over_billing_against_so_is_blocked(self):
+		from erpnext.selling.doctype.sales_order.sales_order import make_sales_invoice as make_si_from_so
+		from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order
+
+		service_item = create_item(
+			"_Test Service Item Non Stock SI",
+			is_stock_item=0,
+		).name
+
+		so = make_sales_order(item_code=service_item, qty=5, rate=100)
+		so.submit()
+
+		si = make_si_from_so(so.name)
+		si.items[0].qty = 10  # overbill by 100 %
+		si.save()
+
+		with self.assertRaises(frappe.ValidationError):
+			si.submit()
+
+	@ERPNextTestSuite.change_settings("Accounts Settings", {"over_billing_allowance": 0})
+	def test_non_stock_item_over_billing_against_so_from_quotation_is_blocked(self):
+		from erpnext.selling.doctype.quotation.quotation import make_sales_order as make_so_from_quotation
+		from erpnext.selling.doctype.quotation.test_quotation import make_quotation
+		from erpnext.selling.doctype.sales_order.sales_order import make_sales_invoice as make_si_from_so
+
+		service_item = create_item(
+			"_Test Service Item Non Stock SI Quot",
+			is_stock_item=0,
+		).name
+
+		quotation = make_quotation(item_code=service_item, qty=5, rate=100)
+
+		so = make_so_from_quotation(quotation.name)
+		so.delivery_date = frappe.utils.add_days(frappe.utils.today(), 7)
+		so.insert()
+		so.submit()
+
+		si = make_si_from_so(so.name)
+		si.items[0].qty = 10  # overbill by 100 %
+		si.save()
+
+		with self.assertRaises(frappe.ValidationError):
+			si.submit()
+
 	@ERPNextTestSuite.change_settings(
 		"Accounts Settings",
 		{

erpnext/controllers/status_updater.py

@@ -383,15 +383,17 @@ class StatusUpdater(Document):
 
 	def fetch_items_with_pending_qty(self, args, item_field, items):
 		doctype = frappe.qb.DocType(args["target_dt"])
-		item_field = doctype[item_field]
+		item_field_col = doctype[item_field]
 		target_ref_field = doctype[args["target_ref_field"]]
 		target_field = doctype[args["target_field"]]
 
-		return (
+		is_qty_check = "qty" in args["target_ref_field"]
+
+		query = (
 			frappe.qb.from_(doctype)
 			.select(
 				doctype.name,
-				item_field.as_("item_code"),
+				item_field_col.as_("item_code"),
 				target_ref_field,
 				target_field,
 				doctype.parenttype,
@@ -400,9 +402,18 @@ class StatusUpdater(Document):
 			.where(target_ref_field < target_field)
 			.where(doctype.name.isin(items))
 			.where(doctype.docstatus == 1)
-			.run(as_dict=True)
 		)
 
+		if is_qty_check:
+			item_table = frappe.qb.DocType("Item")
+			query = (
+				query.join(item_table)
+				.on(item_table.name == item_field_col)
+				.where(item_table.is_stock_item == 1)
+			)
+
+		return query.run(as_dict=True)
+
 	def check_overflow_with_allowance(self, item, args):
 		"""
 		Checks if there is overflow considering a relaxation allowance.

erpnext/controllers/taxes_and_totals.py

@@ -170,7 +170,7 @@ class calculate_taxes_and_totals:
 			return
 
 		if not self.discount_amount_applied:
-			do_not_round_fields = ["valuation_rate", "incoming_rate"]
+			do_not_round_fields = ["valuation_rate", "incoming_rate", "sales_incoming_rate"]
 
 			for item in self.doc.items:
 				self.doc.round_floats_in(item, do_not_round_fields=do_not_round_fields)

erpnext/crm/doctype/lead/lead.py

@@ -448,6 +448,7 @@ def get_lead_details(lead, posting_date=None, company=None, doctype=None):
 	out = frappe._dict()
 
 	lead_doc = frappe.get_doc("Lead", lead)
+	lead_doc.check_permission()
 	lead = lead_doc
 
 	out.update(

erpnext/manufacturing/doctype/job_card/job_card.js

@@ -694,10 +694,11 @@ frappe.ui.form.on("Job Card", {
 		// ── Wire up button click handlers ─────────────────────────────────
 		if (show_start) {
 			wrapper.find(".jcd-btn-start").on("click", () => {
-				const from_time = frappe.datetime.now_datetime();
 				const has_no_employee = !frm.doc.employee || !frm.doc.employee.length;
 
 				if (has_no_employee) {
+					// Capture the start time only when the employee dialog is submitted, not on click,
+					// so the time spent selecting the operator is not counted as worked time.
 					frappe.prompt(
 						{
 							fieldtype: "Table MultiSelect",
@@ -707,11 +708,11 @@ frappe.ui.form.on("Job Card", {
 							reqd: 1,
 							filters: { status: "Active" },
 						},
-						(d) => frm.events.start_timer(frm, from_time, d.employees),
+						(d) => frm.events.start_timer(frm, frappe.datetime.now_datetime(), d.employees),
 						__("Assign Job to Employee")
 					);
 				} else {
-					frm.events.start_timer(frm, from_time, frm.doc.employee);
+					frm.events.start_timer(frm, frappe.datetime.now_datetime(), frm.doc.employee);
 				}
 			});
 		}

erpnext/selling/report/sales_person_wise_transaction_summary/sales_person_wise_transaction_summary.py

@@ -4,7 +4,7 @@
 
 import frappe
 from frappe import _, msgprint, qb
-from frappe.query_builder import Criterion
+from frappe.query_builder import Case, Criterion
 
 from erpnext import get_company_currency
 
@@ -155,50 +155,60 @@ def get_columns(filters):
 
 
 def get_entries(filters):
-	date_field = filters["doc_type"] == "Sales Order" and "transaction_date" or "posting_date"
-	if filters["doc_type"] == "Sales Order":
-		qty_field = "delivered_qty"
-	else:
-		qty_field = "qty"
-	conditions, values = get_conditions(filters, date_field)
+	doc_type = filters["doc_type"]
 
-	entries = frappe.db.sql(
-		"""
-		SELECT
-			dt.name, dt.customer, dt.territory, dt.{} as posting_date, dt_item.item_code,
-			st.sales_person, st.allocated_percentage, dt_item.warehouse,
-		CASE
-			WHEN dt.status = "Closed" THEN dt_item.{} * dt_item.conversion_factor
-			ELSE dt_item.stock_qty
-		END as stock_qty,
-		CASE
-			WHEN dt.status = "Closed" THEN (dt_item.base_net_rate * dt_item.{} * dt_item.conversion_factor)
-			ELSE dt_item.base_net_amount
-		END as base_net_amount,
-		CASE
-			WHEN dt.status = "Closed" THEN ((dt_item.base_net_rate * dt_item.{} * dt_item.conversion_factor) * st.allocated_percentage/100)
-			ELSE dt_item.base_net_amount * st.allocated_percentage/100
-		END as contribution_amt
-		FROM
-			`tab{}` dt, `tab{} Item` dt_item, `tabSales Team` st
-		WHERE
-			st.parent = dt.name and dt.name = dt_item.parent and st.parenttype = {}
-			and dt.docstatus = 1 {} order by st.sales_person, dt.name desc
-		""".format(
-			date_field,
-			qty_field,
-			qty_field,
-			qty_field,
-			filters["doc_type"],
-			filters["doc_type"],
-			"%s",
-			conditions,
-		),
-		tuple([filters["doc_type"], *values]),
-		as_dict=1,
+	date_field = "transaction_date" if doc_type == "Sales Order" else "posting_date"
+	qty_field = "delivered_qty" if doc_type == "Sales Order" else "qty"
+
+	dt = frappe.qb.DocType(doc_type)
+	dt_item = frappe.qb.DocType(f"{doc_type} Item")
+	st = frappe.qb.DocType("Sales Team")
+
+	calc_qty = dt_item[qty_field] * dt_item.conversion_factor
+	calc_net_amount = dt_item.base_net_rate * calc_qty
+
+	stock_qty_case = Case().when(dt.status == "Closed", calc_qty).else_(dt_item.stock_qty).as_("stock_qty")
+
+	base_net_amount_case = (
+		Case()
+		.when(dt.status == "Closed", calc_net_amount)
+		.else_(dt_item.base_net_amount)
+		.as_("base_net_amount")
 	)
 
-	return entries
+	contribution_amt_case = (
+		Case()
+		.when(dt.status == "Closed", (calc_net_amount * st.allocated_percentage / 100))
+		.else_(dt_item.base_net_amount * st.allocated_percentage / 100)
+		.as_("contribution_amt")
+	)
+
+	query = (
+		frappe.get_query(dt, filters=filters, ignore_permissions=False)
+		.join(dt_item)
+		.on(dt.name == dt_item.parent)
+		.join(st)
+		.on(dt.name == st.parent)
+		.select(
+			dt.name,
+			dt.customer,
+			dt.territory,
+			dt[date_field].as_("posting_date"),
+			dt_item.item_code,
+			st.sales_person,
+			st.allocated_percentage,
+			dt_item.warehouse,
+			stock_qty_case,
+			base_net_amount_case,
+			contribution_amt_case,
+		)
+		.where(st.parenttype == doc_type)
+		.where(dt.docstatus == 1)
+	)
+
+	query = query.orderby(st.sales_person).orderby(dt.name, order=frappe.qb.desc)
+
+	return query.run(as_dict=True)
 
 
 def get_conditions(filters, date_field):

erpnext/setup/doctype/transaction_deletion_record/transaction_deletion_record.py

@@ -318,12 +318,23 @@ class TransactionDeletionRecord(Document):
 		Returns:
 		        list: List of child table DocType names (Table field options)
 		"""
-		return frappe.get_all(
+		child_tables = frappe.get_all(
 			"DocField",
 			filters={"parent": doctype_name, "fieldtype": ["in", ["Table", "Table MultiSelect"]]},
 			pluck="options",
 		)
 
+		if not child_tables:
+			return []
+
+		child_tables = frappe.get_all(
+			"DocType",
+			filters={"name": ["in", child_tables], "is_virtual": 0},
+			pluck="name",
+		)
+
+		return child_tables
+
 	def _get_to_delete_row_infos(self, doctype_name, company_field=None, company=None):
 		"""Get child tables and document count for a To Delete list row
 

erpnext/stock/doctype/repost_item_valuation/repost_item_valuation.py

@@ -346,6 +346,9 @@ class RepostItemValuation(Document):
 
 	def _recalculate_valuation_rate(self):
 		doc = frappe.get_doc(self.voucher_type, self.voucher_no)
+		if doc.get("is_internal_supplier"):
+			doc.set_sales_incoming_rate_for_internal_transfer()
+
 		doc.update_valuation_rate()
 		for item in doc.items:
 			item.db_set("valuation_rate", item.valuation_rate)