feat: add `total_billing_hours` to Sales Invoice (fp #26783) (#27742)

* feat: add `total_billing_hours` to Sales Invoice

* fix: re-save doctypes

* fix: indentation

* fix: replace reference to old function
diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.js b/erpnext/accounts/doctype/sales_invoice/sales_invoice.js
index 828d5bd..73e1284 100644
--- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.js
+++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.js
@@ -453,7 +453,7 @@
 				let row = frappe.get_doc(d.doctype, d.name)
 				set_timesheet_detail_rate(row.doctype, row.name, me.frm.doc.currency, row.timesheet_detail)
 			});
-			calculate_total_billing_amount(this.frm);
+			frm.trigger("calculate_timesheet_totals");
 		}
 	}
 };
@@ -725,19 +725,6 @@
 		}
 	},
 
-	project: function(frm){
-		if (!frm.doc.is_return) {
-			frm.call({
-				method: "add_timesheet_data",
-				doc: frm.doc,
-				callback: function(r, rt) {
-					refresh_field(['timesheets'])
-				}
-			})
-			frm.refresh();
-		}
-	},
-
 	onload: function(frm) {
 		frm.redemption_conversion_factor = null;
 	},
@@ -848,25 +835,92 @@
 		}
 	},
 
-	add_timesheet_row: function(frm, row, exchange_rate) {
-		frm.add_child('timesheets', {
-			'activity_type': row.activity_type,
-			'description': row.description,
-			'time_sheet': row.parent,
-			'billing_hours': row.billing_hours,
-			'billing_amount': flt(row.billing_amount) * flt(exchange_rate),
-			'timesheet_detail': row.name,
-			'project_name': row.project_name
+	project: function(frm) {
+		if (frm.doc.project) {
+			frm.events.add_timesheet_data(frm, {
+				project: frm.doc.project
+			});
+		}
+	},
+
+	async add_timesheet_data(frm, kwargs) {
+		if (kwargs === "Sales Invoice") {
+			// called via frm.trigger()
+			kwargs = Object();
+		}
+
+		if (!kwargs.hasOwnProperty("project") && frm.doc.project) {
+			kwargs.project = frm.doc.project;
+		}
+
+		const timesheets = await frm.events.get_timesheet_data(frm, kwargs);
+		return frm.events.set_timesheet_data(frm, timesheets);
+	},
+
+	async get_timesheet_data(frm, kwargs) {
+		return frappe.call({
+			method: "erpnext.projects.doctype.timesheet.timesheet.get_projectwise_timesheet_data",
+			args: kwargs
+		}).then(r => {
+			if (!r.exc && r.message.length > 0) {
+				return r.message
+			} else {
+				return []
+			}
 		});
-		frm.refresh_field('timesheets');
-		calculate_total_billing_amount(frm);
+	},
+
+	set_timesheet_data: function(frm, timesheets) {
+		frm.clear_table("timesheets")
+		timesheets.forEach(timesheet => {
+			if (frm.doc.currency != timesheet.currency) {
+				frappe.call({
+					method: "erpnext.setup.utils.get_exchange_rate",
+					args: {
+						from_currency: timesheet.currency,
+						to_currency: frm.doc.currency
+					},
+					callback: function(r) {
+						if (r.message) {
+							exchange_rate = r.message;
+							frm.events.append_time_log(frm, timesheet, exchange_rate);
+						}
+					}
+				});
+			} else {
+				frm.events.append_time_log(frm, timesheet, 1.0);
+			}
+		});
+	},
+
+	append_time_log: function(frm, time_log, exchange_rate) {
+		const row = frm.add_child("timesheets");
+		row.activity_type = time_log.activity_type;
+		row.description = time_log.description;
+		row.time_sheet = time_log.time_sheet;
+		row.from_time = time_log.from_time;
+		row.to_time = time_log.to_time;
+		row.billing_hours = time_log.billing_hours;
+		row.billing_amount = flt(time_log.billing_amount) * flt(exchange_rate);
+		row.timesheet_detail = time_log.name;
+    row.project_name = time_log.project_name;
+
+		frm.refresh_field("timesheets");
+		frm.trigger("calculate_timesheet_totals");
+	},
+
+	calculate_timesheet_totals: function(frm) {
+		frm.set_value("total_billing_amount",
+			frm.doc.timesheets.reduce((a, b) => a + (b["billing_amount"] || 0.0), 0.0));
+		frm.set_value("total_billing_hours",
+			frm.doc.timesheets.reduce((a, b) => a + (b["billing_hours"] || 0.0), 0.0));
 	},
 
 	refresh: function(frm) {
 		if (frm.doc.docstatus===0 && !frm.doc.is_return) {
-			frm.add_custom_button(__('Fetch Timesheet'), function() {
+			frm.add_custom_button(__("Fetch Timesheet"), function() {
 				let d = new frappe.ui.Dialog({
-					title: __('Fetch Timesheet'),
+					title: __("Fetch Timesheet"),
 					fields: [
 						{
 							"label" : __("From"),
@@ -875,8 +929,8 @@
 							"reqd": 1,
 						},
 						{
-							fieldtype: 'Column Break',
-							fieldname: 'col_break_1',
+							fieldtype: "Column Break",
+							fieldname: "col_break_1",
 						},
 						{
 							"label" : __("To"),
@@ -893,48 +947,18 @@
 						},
 					],
 					primary_action: function() {
-						let data = d.get_values();
-						frappe.call({
-							method: "erpnext.projects.doctype.timesheet.timesheet.get_projectwise_timesheet_data",
-							args: {
-								from_time: data.from_time,
-								to_time: data.to_time,
-								project: data.project
-							},
-							callback: function(r) {
-								if (!r.exc && r.message.length > 0) {
-									frm.clear_table('timesheets')
-									r.message.forEach((d) => {
-										let exchange_rate = 1.0;
-										if (frm.doc.currency != d.currency) {
-											frappe.call({
-												method: 'erpnext.setup.utils.get_exchange_rate',
-												args: {
-													from_currency: d.currency,
-													to_currency: frm.doc.currency
-												},
-												callback: function(r) {
-													if (r.message) {
-														exchange_rate = r.message;
-														frm.events.add_timesheet_row(frm, d, exchange_rate);
-													}
-												}
-											});
-										} else {
-											frm.events.add_timesheet_row(frm, d, exchange_rate);
-										}
-									});
-								} else {
-									frappe.msgprint(__('No Timesheets found with the selected filters.'))
-								}
-								d.hide();
-							}
+						const data = d.get_values();
+						frm.events.add_timesheet_data(frm, {
+							from_time: data.from_time,
+							to_time: data.to_time,
+							project: data.project
 						});
+						d.hide();
 					},
-					primary_action_label: __('Get Timesheets')
+					primary_action_label: __("Get Timesheets")
 				});
 				d.show();
-			})
+			});
 		}
 
 		if (frm.doc.is_debit_note) {
@@ -967,26 +991,20 @@
 			frm: frm
 		});
 	},
+
 	create_dunning: function(frm) {
 		frappe.model.open_mapped_doc({
 			method: "erpnext.accounts.doctype.sales_invoice.sales_invoice.create_dunning",
 			frm: frm
 		});
 	}
-})
+});
 
-var calculate_total_billing_amount =  function(frm) {
-	var doc = frm.doc;
-
-	doc.total_billing_amount = 0.0
-	if (doc.timesheets) {
-		doc.timesheets.forEach((d) => {
-			doc.total_billing_amount += flt(d.billing_amount)
-		});
+frappe.ui.form.on("Sales Invoice Timesheet", {
+	timesheets_remove(frm) {
+		frm.trigger("calculate_timesheet_totals");
 	}
-
-	refresh_field('total_billing_amount')
-}
+});
 
 var set_timesheet_detail_rate = function(cdt, cdn, currency, timelog) {
 	frappe.call({
diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.json b/erpnext/accounts/doctype/sales_invoice/sales_invoice.json
index 2d6c04e..f3adb89 100644
--- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.json
+++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.json
@@ -74,6 +74,7 @@
   "time_sheet_list",
   "timesheets",
   "total_billing_amount",
+  "total_billing_hours",
   "section_break_30",
   "total_qty",
   "base_total",
@@ -2011,6 +2012,13 @@
    "hidden": 1,
    "label": "Ignore Default Payment Terms Template",
    "read_only": 1
+  },
+  {
+   "fieldname": "total_billing_hours",
+   "fieldtype": "Float",
+   "label": "Total Billing Hours",
+   "print_hide": 1,
+   "read_only": 1
   }
  ],
  "icon": "fa fa-file-text",
@@ -2023,7 +2031,7 @@
    "link_fieldname": "consolidated_invoice"
   }
  ],
- "modified": "2021-09-28 13:09:34.391799",
+ "modified": "2021-10-02 03:36:10.251715",
  "modified_by": "Administrator",
  "module": "Accounts",
  "name": "Sales Invoice",
diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py
index 100d943..eb26aa2 100644
--- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py
+++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py
@@ -758,7 +758,7 @@
 		if self.project:
 			for data in get_projectwise_timesheet_data(self.project):
 				self.append('timesheets', {
-						'time_sheet': data.parent,
+						'time_sheet': data.time_sheet,
 						'billing_hours': data.billing_hours,
 						'billing_amount': data.billing_amount,
 						'timesheet_detail': data.name,
@@ -769,12 +769,11 @@
 			self.calculate_billing_amount_for_timesheet()
 
 	def calculate_billing_amount_for_timesheet(self):
-		total_billing_amount = 0.0
-		for data in self.timesheets:
-			if data.billing_amount:
-				total_billing_amount += data.billing_amount
+		def timesheet_sum(field):
+			return sum((ts.get(field) or 0.0) for ts in self.timesheets)
 
-		self.total_billing_amount = total_billing_amount
+		self.total_billing_amount = timesheet_sum("billing_amount")
+		self.total_billing_hours = timesheet_sum("billing_hours")
 
 	def get_warehouse(self):
 		user_pos_profile = frappe.db.sql("""select name, warehouse from `tabPOS Profile`
diff --git a/erpnext/accounts/doctype/sales_invoice_timesheet/sales_invoice_timesheet.json b/erpnext/accounts/doctype/sales_invoice_timesheet/sales_invoice_timesheet.json
index c902973..69b7c12 100644
--- a/erpnext/accounts/doctype/sales_invoice_timesheet/sales_invoice_timesheet.json
+++ b/erpnext/accounts/doctype/sales_invoice_timesheet/sales_invoice_timesheet.json
@@ -7,12 +7,19 @@
  "field_order": [
   "activity_type",
   "description",
-  "billing_hours",
-  "billing_amount",
+  "section_break_3",
+  "from_time",
   "column_break_5",
+  "to_time",
+  "section_break_7",
+  "billing_hours",
+  "column_break_9",
+  "billing_amount",
+  "section_break_11",
   "time_sheet",
-  "project_name",
-  "timesheet_detail"
+  "timesheet_detail",
+  "column_break_13",
+  "project_name"
  ],
  "fields": [
   {
@@ -65,19 +72,52 @@
    "read_only": 1
   },
   {
+   "fieldname": "from_time",
+   "fieldtype": "Datetime",
+   "label": "From Time"
+  },
+  {
+   "fieldname": "to_time",
+   "fieldtype": "Datetime",
+   "label": "To Time"
+  },
+  {
+   "fieldname": "section_break_3",
+   "fieldtype": "Section Break",
+   "label": "Time"
+  },
+  {
    "fieldname": "column_break_5",
    "fieldtype": "Column Break"
   },
   {
+   "fieldname": "section_break_7",
+   "fieldtype": "Section Break",
+   "label": "Totals"
+  },
+  {
+   "fieldname": "column_break_9",
+   "fieldtype": "Column Break"
+  },
+  {
+   "fieldname": "section_break_11",
+   "fieldtype": "Section Break",
+   "label": "Reference"
+  },
+  {
    "fieldname": "project_name",
    "fieldtype": "Data",
    "label": "Project Name",
    "read_only": 1
+  },
+  {
+   "fieldname": "column_break_13",
+   "fieldtype": "Column Break"
   }
  ],
  "istable": 1,
  "links": [],
- "modified": "2021-06-08 14:43:02.748981",
+ "modified": "2021-10-02 03:48:44.979777",
  "modified_by": "Administrator",
  "module": "Accounts",
  "name": "Sales Invoice Timesheet",
diff --git a/erpnext/projects/doctype/timesheet/timesheet.py b/erpnext/projects/doctype/timesheet/timesheet.py
index e144e82..363c3b6 100644
--- a/erpnext/projects/doctype/timesheet/timesheet.py
+++ b/erpnext/projects/doctype/timesheet/timesheet.py
@@ -215,25 +215,47 @@
 
 @frappe.whitelist()
 def get_projectwise_timesheet_data(project=None, parent=None, from_time=None, to_time=None):
-	condition = ''
+	condition = ""
 	if project:
-		condition += "and tsd.project = %(project)s"
+		condition += "AND tsd.project = %(project)s "
 	if parent:
-		condition += "AND tsd.parent = %(parent)s"
+		condition += "AND tsd.parent = %(parent)s "
 	if from_time and to_time:
 		condition += "AND CAST(tsd.from_time as DATE) BETWEEN %(from_time)s AND %(to_time)s"
 
-	return frappe.db.sql("""SELECT tsd.name as name,
-				tsd.parent as parent, tsd.billing_hours as billing_hours,
-				tsd.billing_amount as billing_amount, tsd.activity_type as activity_type,
-				tsd.description as description, ts.currency as currency,
-				tsd.project_name as project_name
-			FROM `tabTimesheet Detail` tsd
-			INNER JOIN `tabTimesheet` ts ON ts.name = tsd.parent
-			WHERE tsd.parenttype = 'Timesheet'
-				and tsd.docstatus=1 {0}
-				and tsd.is_billable = 1
-				and tsd.sales_invoice is null""".format(condition), {'project': project, 'parent': parent, 'from_time': from_time, 'to_time': to_time}, as_dict=1)
+	query = f"""
+		SELECT
+			tsd.name as name,
+			tsd.parent as time_sheet,
+			tsd.from_time as from_time,
+			tsd.to_time as to_time,
+			tsd.billing_hours as billing_hours,
+			tsd.billing_amount as billing_amount,
+			tsd.activity_type as activity_type,
+			tsd.description as description,
+			ts.currency as currency,
+			tsd.project_name as project_name
+		FROM `tabTimesheet Detail` tsd
+			INNER JOIN `tabTimesheet` ts
+			ON ts.name = tsd.parent
+		WHERE
+			tsd.parenttype = 'Timesheet'
+			AND tsd.docstatus = 1
+			AND tsd.is_billable = 1
+			AND tsd.sales_invoice is NULL
+			{condition}
+		ORDER BY tsd.from_time ASC
+	"""
+
+	filters = {
+		"project": project,
+		"parent": parent,
+		"from_time": from_time,
+		"to_time": to_time
+	}
+
+	return frappe.db.sql(query, filters, as_dict=1)
+
 
 @frappe.whitelist()
 def get_timesheet_detail_rate(timelog, currency):