refactor(Timesheet): move methods to Timesheet Detail (#43794)

erpnext/projects/doctype/timesheet/timesheet.py

@@ -7,7 +7,8 @@
 import frappe
 from frappe import _
 from frappe.model.document import Document
-from frappe.utils import add_to_date, flt, get_datetime, getdate, time_diff_in_hours
+from frappe.utils import flt, get_datetime, getdate
+from frappe.utils.deprecations import deprecated
 
 from erpnext.controllers.queries import get_match_cond
 from erpnext.setup.utils import get_exchange_rate
@@ -75,8 +76,9 @@ def validate(self):
 	def calculate_hours(self):
 		for row in self.time_logs:
 			if row.to_time and row.from_time:
-				row.hours = time_diff_in_hours(row.to_time, row.from_time)
-				self.update_billing_hours(row)
+				row.calculate_hours()
+				row.validate_billing_hours()
+				row.update_billing_hours()
 
 	def calculate_total_amounts(self):
 		self.total_hours = 0.0
@@ -87,7 +89,7 @@ def calculate_total_amounts(self):
 		self.total_billed_amount = self.base_total_billed_amount = 0.0
 
 		for d in self.get("time_logs"):
-			self.update_billing_hours(d)
+			d.update_billing_hours()
 			self.update_time_rates(d)
 
 			self.total_hours += flt(d.hours)
@@ -108,18 +110,9 @@ def calculate_percentage_billed(self):
 		elif self.total_billed_hours > 0 and self.total_billable_hours > 0:
 			self.per_billed = (self.total_billed_hours * 100) / self.total_billable_hours
 
-	def update_billing_hours(self, args):
-		if args.is_billable:
-			if flt(args.billing_hours) == 0.0:
-				args.billing_hours = args.hours
-			elif flt(args.billing_hours) > flt(args.hours):
-				frappe.msgprint(
-					_("Warning - Row {0}: Billing Hours are more than Actual Hours").format(args.idx),
-					indicator="orange",
-					alert=True,
-				)
-		else:
-			args.billing_hours = 0
+	@deprecated
+	def update_billing_hours(self, args: "TimesheetDetail"):
+		args.update_billing_hours()
 
 	def set_status(self):
 		self.status = {"0": "Draft", "1": "Submitted", "2": "Cancelled"}[str(self.docstatus or 0)]
@@ -181,40 +174,29 @@ def update_task_and_project(self):
 				projects.append(data.project)
 
 	def validate_dates(self):
-		for data in self.time_logs:
-			if data.from_time and data.to_time and time_diff_in_hours(data.to_time, data.from_time) < 0:
-				frappe.throw(_("To date cannot be before from date"))
+		for time_log in self.time_logs:
+			time_log.validate_dates()
 
 	def validate_time_logs(self):
-		for data in self.get("time_logs"):
-			self.set_to_time(data)
-			self.validate_overlap(data)
-			self.set_project(data)
-			self.validate_project(data)
-
-	def set_to_time(self, data):
-		if not (data.from_time and data.hours):
-			return
-
-		_to_time = get_datetime(add_to_date(data.from_time, hours=data.hours, as_datetime=True))
-		if data.to_time != _to_time:
-			data.to_time = _to_time
+		for time_log in self.time_logs:
+			time_log.set_to_time()
+			self.validate_overlap(time_log)
+			time_log.set_project()
+			time_log.validate_parent_project(self.parent_project)
+			time_log.validate_task_project()
 
 	def validate_overlap(self, data):
 		settings = frappe.get_single("Projects Settings")
 		self.validate_overlap_for("user", data, self.user, settings.ignore_user_time_overlap)
 		self.validate_overlap_for("employee", data, self.employee, settings.ignore_employee_time_overlap)
 
-	def set_project(self, data):
-		data.project = data.project or frappe.db.get_value("Task", data.task, "project")
+	@deprecated
+	def set_project(self, data: "TimesheetDetail"):
+		data.set_project()
 
-	def validate_project(self, data):
-		if self.parent_project and self.parent_project != data.project:
-			frappe.throw(
-				_("Row {0}: Project must be same as the one set in the Timesheet: {1}.").format(
-					data.idx, self.parent_project
-				)
-			)
+	@deprecated
+	def validate_project(self, data: "TimesheetDetail"):
+		data.validate_parent_project(self.parent_project)
 
 	def validate_overlap_for(self, fieldname, args, value, ignore_validation=False):
 		if not value or ignore_validation:
@@ -284,20 +266,8 @@ def check_internal_overlap(self, fieldname, args):
 		return False
 
 	def update_cost(self):
-		for data in self.time_logs:
-			if data.activity_type or data.is_billable:
-				rate = get_activity_cost(self.employee, data.activity_type)
-				hours = data.billing_hours or 0
-				costing_hours = data.billing_hours or data.hours or 0
-				if rate:
-					data.billing_rate = (
-						flt(rate.get("billing_rate")) if flt(data.billing_rate) == 0 else data.billing_rate
-					)
-					data.costing_rate = (
-						flt(rate.get("costing_rate")) if flt(data.costing_rate) == 0 else data.costing_rate
-					)
-					data.billing_amount = data.billing_rate * hours
-					data.costing_amount = data.costing_rate * costing_hours
+		for time_log in self.time_logs:
+			time_log.update_cost(self.employee)
 
 	def update_time_rates(self, ts_detail):
 		if not ts_detail.is_billable:

erpnext/projects/doctype/timesheet_detail/timesheet_detail.py

@@ -2,7 +2,10 @@
 # For license information, please see license.txt
 
 
+import frappe
+from frappe import _
 from frappe.model.document import Document
+from frappe.utils import add_to_date, flt, get_datetime, time_diff_in_hours
 
 
 class TimesheetDetail(Document):
@@ -40,4 +43,83 @@ class TimesheetDetail(Document):
 		to_time: DF.Datetime | None
 	# end: auto-generated types
 
-	pass
+	def set_to_time(self):
+		"""Set to_time based on from_time and hours."""
+		if not (self.from_time and self.hours):
+			return
+
+		self.to_time = get_datetime(add_to_date(self.from_time, hours=self.hours, as_datetime=True))
+
+	def set_project(self):
+		"""Set project based on task."""
+		if self.task and not self.project:
+			self.project = frappe.db.get_value("Task", self.task, "project")
+
+	def calculate_hours(self):
+		"""Calculate hours based on from_time and to_time."""
+		if self.to_time and self.from_time:
+			self.hours = time_diff_in_hours(self.to_time, self.from_time)
+
+	def update_billing_hours(self):
+		"""Update billing hours based on hours."""
+		if not self.is_billable:
+			self.billing_hours = 0
+			return
+
+		if flt(self.billing_hours) == 0.0:
+			self.billing_hours = self.hours
+
+	def update_cost(self, employee: str):
+		"""Update costing and billing rates based on activity type."""
+		from erpnext.projects.doctype.timesheet.timesheet import get_activity_cost
+
+		if not self.is_billable and not self.activity_type:
+			return
+
+		rate = get_activity_cost(employee, self.activity_type)
+		if not rate:
+			return
+
+		self.billing_rate = (
+			flt(rate.get("billing_rate")) if flt(self.billing_rate) == 0 else self.billing_rate
+		)
+		self.costing_rate = (
+			flt(rate.get("costing_rate")) if flt(self.costing_rate) == 0 else self.costing_rate
+		)
+
+		self.billing_amount = self.billing_rate * (self.billing_hours or 0)
+		self.costing_amount = self.costing_rate * (self.billing_hours or self.hours or 0)
+
+	def validate_dates(self):
+		"""Validate that to_time is not before from_time."""
+		if self.from_time and self.to_time and time_diff_in_hours(self.to_time, self.from_time) < 0:
+			frappe.throw(_("To Time cannot be before from date"))
+
+	def validate_parent_project(self, parent_project: str):
+		"""Validate that project is same as Timesheet's parent project."""
+		if parent_project and parent_project != self.project:
+			frappe.throw(
+				_("Row {0}: Project must be same as the one set in the Timesheet: {1}.").format(
+					self.idx, parent_project
+				)
+			)
+
+	def validate_task_project(self):
+		"""Validate that the the task belongs to the project specified in the timesheet detail."""
+		if self.task and self.project:
+			task_project = frappe.db.get_value("Task", self.task, "project")
+			if task_project and task_project != self.project:
+				frappe.throw(
+					_("Row {0}: Task {1} does not belong to Project {2}").format(
+						self.idx, frappe.bold(self.task), frappe.bold(self.project)
+					)
+				)
+
+	def validate_billing_hours(self):
+		"""Warn if billing hours are more than actual hours."""
+		if flt(self.billing_hours) > flt(self.hours):
+			frappe.msgprint(
+				_("Warning - Row {0}: Billing Hours are more than Actual Hours").format(self.idx),
+				indicator="orange",
+				alert=True,
+			)