refactor: validate total weightage in all appraisal forms (backport #…

…1976) (#1978)

(cherry picked from commit bb1b916)

Co-authored-by: Rucha Mahabal <[email protected]>

hrms/hr/doctype/appraisal/appraisal.py

@@ -9,10 +9,11 @@
 
 from hrms.hr.doctype.appraisal_cycle.appraisal_cycle import validate_active_appraisal_cycle
 from hrms.hr.utils import validate_active_employee
+from hrms.mixins.appraisal import AppraisalMixin
 from hrms.payroll.utils import sanitize_expression
 
 
-class Appraisal(Document):
+class Appraisal(Document, AppraisalMixin):
 	def validate(self):
 		if not self.status:
 			self.status = "Draft"
@@ -22,6 +23,8 @@ def validate(self):
 		validate_active_employee(self.employee)
 		validate_active_appraisal_cycle(self.appraisal_cycle)
 		self.validate_duplicate()
+		self.validate_total_weightage("appraisal_kra", "KRAs")
+		self.validate_total_weightage("self_ratings", "Self Ratings")
 
 		self.set_goal_score()
 		self.calculate_self_appraisal_score()

hrms/hr/doctype/appraisal_template/appraisal_template.py

@@ -7,23 +7,10 @@
 from frappe.model.document import Document
 from frappe.utils import flt
 
+from hrms.mixins.appraisal import AppraisalMixin
 
-class AppraisalTemplate(Document):
-	def validate(self):
-		self.validate_total_weightage("goals")
-		self.validate_total_weightage("rating_criteria")
-
-	def validate_total_weightage(self, table_name):
-		if not self.get(table_name):
-			return
 
-		total_weightage = sum(flt(d.per_weightage) for d in self.get(table_name))
-
-		if flt(total_weightage, 2) != 100.0:
-			table = _("KRAs") if table_name == "goals" else _("Criteria")
-			frappe.throw(
-				_("Total weightage for all {0} must add up to 100. Currently, it is {1}%").format(
-					frappe.bold(table), total_weightage
-				),
-				title=_("Incorrect Weightage Allocation"),
-			)
+class AppraisalTemplate(Document, AppraisalMixin):
+	def validate(self):
+		self.validate_total_weightage("goals", "KRAs")
+		self.validate_total_weightage("rating_criteria", "Criteria")

hrms/hr/doctype/employee_performance_feedback/employee_performance_feedback.py

@@ -8,15 +8,16 @@
 
 from hrms.hr.doctype.appraisal_cycle.appraisal_cycle import validate_active_appraisal_cycle
 from hrms.hr.utils import validate_active_employee
+from hrms.mixins.appraisal import AppraisalMixin
 
 
-class EmployeePerformanceFeedback(Document):
+class EmployeePerformanceFeedback(Document, AppraisalMixin):
 	def validate(self):
 		validate_active_appraisal_cycle(self.appraisal_cycle)
 
 		self.validate_employee()
 		self.validate_appraisal()
-		self.validate_total_weightage()
+		self.validate_total_weightage("feedback_ratings", "Feedback Ratings")
 		self.set_total_score()
 
 	def on_submit(self):
@@ -44,17 +45,6 @@ def validate_appraisal(self):
 				_("Appraisal {0} does not belong to Employee {1}").format(self.appraisal, self.employee)
 			)
 
-	def validate_total_weightage(self):
-		total_weightage = sum(flt(d.per_weightage) for d in self.feedback_ratings)
-
-		if flt(total_weightage, 2) != 100.0:
-			frappe.throw(
-				_("Total weightage for all criteria must add up to 100. Currently, it is {0}%").format(
-					total_weightage
-				),
-				title=_("Incorrect Weightage Allocation"),
-			)
-
 	def set_total_score(self):
 		total = 0
 		for entry in self.feedback_ratings:

hrms/mixins/appraisal.py

@@ -0,0 +1,21 @@
+import frappe
+from frappe import _
+from frappe.utils import flt
+
+
+class AppraisalMixin:
+	"""Mixin class for common validations in Appraisal doctypes"""
+
+	def validate_total_weightage(self, table_name: str, table_label: str) -> None:
+		if not self.get(table_name):
+			return
+
+		total_weightage = sum(flt(d.per_weightage) for d in self.get(table_name))
+
+		if flt(total_weightage, 2) != 100.0:
+			frappe.throw(
+				_("Total weightage for all {0} must add up to 100. Currently, it is {1}%").format(
+					frappe.bold(_(table_label)), total_weightage
+				),
+				title=_("Incorrect Weightage Allocation"),
+			)