Merge pull request #2131 from frappe/mergify/bp/version-15-hotfix/pr-…

…2088

fix(Salary Structure Assignment: Preview Salary Slip): Calculation of earnings whose formula is dependent on deductions and so on (backport #2088)

hrms/payroll/doctype/salary_slip/salary_slip.py

@@ -2,6 +2,7 @@
 # License: GNU General Public License v3. See license.txt
 
 
+import ast
 import unicodedata
 from datetime import date
 
@@ -765,6 +766,12 @@ def set_salary_structure_assignment(self):
 			)
 
 	def calculate_net_pay(self):
+		def set_gross_pay_and_base_gross_pay():
+			self.gross_pay = self.get_component_totals("earnings", depends_on_payment_days=1)
+			self.base_gross_pay = flt(
+				flt(self.gross_pay) * flt(self.exchange_rate), self.precision("base_gross_pay")
+			)
+
 		if self.salary_structure:
 			self.calculate_component_amounts("earnings")
 
@@ -780,20 +787,39 @@ def calculate_net_pay(self):
 				relieving_date=self.relieving_date,
 			)[1]
 
-		self.gross_pay = self.get_component_totals("earnings", depends_on_payment_days=1)
-		self.base_gross_pay = flt(
-			flt(self.gross_pay) * flt(self.exchange_rate), self.precision("base_gross_pay")
-		)
+		set_gross_pay_and_base_gross_pay()
 
 		if self.salary_structure:
 			self.calculate_component_amounts("deductions")
 
+			deduction_abbrs = [d.abbr for d in self.deductions]
+			self.update_dependent_components_recursively("earnings", deduction_abbrs)
+
+		set_gross_pay_and_base_gross_pay()
+		self.update_dependent_components_recursively("deductions", ["gross_pay", "base_gross_pay"])
+
 		set_loan_repayment(self)
 
 		self.set_precision_for_component_amounts()
 		self.set_net_pay()
 		self.compute_income_tax_breakup()
 
+	def update_dependent_components_recursively(
+		self, component_type: str, updated_var: str | list[str]
+	) -> None:
+		def is_var_updated(var: str | list[str]) -> bool:
+			return var == updated_var if isinstance(updated_var, str) else var in updated_var
+
+		other_component_type = "deductions" if component_type == "earnings" else "earnings"
+
+		for d in self._salary_structure_doc.get(component_type):
+			if not d.amount_based_on_formula:
+				continue
+			for var in get_variables_from_formula(d.formula):
+				if is_var_updated(var):
+					self.add_structure_component(d, component_type)
+					self.update_dependent_components_recursively(other_component_type, d.abbr)
+
 	def set_net_pay(self):
 		self.total_deduction = self.get_component_totals("deductions")
 		self.base_total_deduction = flt(
@@ -1082,49 +1108,54 @@ def calculate_component_amounts(self, component_type):
 
 	def add_structure_components(self, component_type):
 		self.data, self.default_data = self.get_data_for_eval()
-		timesheet_component = self._salary_structure_doc.salary_component
 
 		for struct_row in self._salary_structure_doc.get(component_type):
-			if self.salary_slip_based_on_timesheet and struct_row.salary_component == timesheet_component:
-				continue
+			self.add_structure_component(struct_row, component_type)
 
-			amount = self.eval_condition_and_formula(struct_row, self.data)
-			if struct_row.statistical_component:
-				# update statitical component amount in reference data based on payment days
-				# since row for statistical component is not added to salary slip
-
-				self.default_data[struct_row.abbr] = flt(amount)
-				if struct_row.depends_on_payment_days:
-					payment_days_amount = (
-						flt(amount) * flt(self.payment_days) / cint(self.total_working_days)
-						if self.total_working_days
-						else 0
-					)
-					self.data[struct_row.abbr] = flt(payment_days_amount, struct_row.precision("amount"))
+	def add_structure_component(self, struct_row, component_type):
+		if (
+			self.salary_slip_based_on_timesheet
+			and struct_row.salary_component == self._salary_structure_doc.salary_component
+		):
+			return
 
-			else:
-				# default behavior, the system does not add if component amount is zero
-				# if remove_if_zero_valued is unchecked, then ask system to add component row
-				remove_if_zero_valued = frappe.get_cached_value(
-					"Salary Component", struct_row.salary_component, "remove_if_zero_valued"
+		amount = self.eval_condition_and_formula(struct_row, self.data)
+		if struct_row.statistical_component:
+			# update statitical component amount in reference data based on payment days
+			# since row for statistical component is not added to salary slip
+
+			self.default_data[struct_row.abbr] = flt(amount)
+			if struct_row.depends_on_payment_days:
+				payment_days_amount = (
+					flt(amount) * flt(self.payment_days) / cint(self.total_working_days)
+					if self.total_working_days
+					else 0
 				)
+				self.data[struct_row.abbr] = flt(payment_days_amount, struct_row.precision("amount"))
 
-				default_amount = 0
+		else:
+			# default behavior, the system does not add if component amount is zero
+			# if remove_if_zero_valued is unchecked, then ask system to add component row
+			remove_if_zero_valued = frappe.get_cached_value(
+				"Salary Component", struct_row.salary_component, "remove_if_zero_valued"
+			)
 
-				if (
-					amount
-					or (struct_row.amount_based_on_formula and amount is not None)
-					or (not remove_if_zero_valued and amount is not None and not self.data[struct_row.abbr])
-				):
-					default_amount = self.eval_condition_and_formula(struct_row, self.default_data)
-					self.update_component_row(
-						struct_row,
-						amount,
-						component_type,
-						data=self.data,
-						default_amount=default_amount,
-						remove_if_zero_valued=remove_if_zero_valued,
-					)
+			default_amount = 0
+
+			if (
+				amount
+				or (struct_row.amount_based_on_formula and amount is not None)
+				or (not remove_if_zero_valued and amount is not None and not self.data[struct_row.abbr])
+			):
+				default_amount = self.eval_condition_and_formula(struct_row, self.default_data)
+				self.update_component_row(
+					struct_row,
+					amount,
+					component_type,
+					data=self.data,
+					default_amount=default_amount,
+					remove_if_zero_valued=remove_if_zero_valued,
+				)
 
 	def get_data_for_eval(self):
 		"""Returns data for evaluating formula"""
@@ -2274,8 +2305,6 @@ def _safe_eval(code: str, eval_globals: dict | None = None, eval_locals: dict |
 
 
 def _check_attributes(code: str) -> None:
-	import ast
-
 	from frappe.utils.safe_exec import UNSAFE_ATTRIBUTES
 
 	unsafe_attrs = set(UNSAFE_ATTRIBUTES).union(["__"]) - {"format"}
@@ -2314,3 +2343,7 @@ def email_salary_slips(names) -> None:
 	for name in names:
 		salary_slip = frappe.get_doc("Salary Slip", name)
 		salary_slip.email_salary_slip()
+
+
+def get_variables_from_formula(formula: str) -> list[str]:
+	return [node.id for node in ast.walk(ast.parse(formula)) if isinstance(node, ast.Name)]

hrms/payroll/doctype/salary_slip/test_salary_slip.py

@@ -1610,6 +1610,69 @@ def test_variable_tax_component(self):
 		self.assertEqual(test_tds.accounts[0].company, salary_slip.company)
 		self.assertListEqual(tax_component, ["_Test TDS"])
 
+	def test_circular_dependency_in_formula(self):
+		from hrms.payroll.doctype.salary_structure.test_salary_structure import (
+			create_salary_structure_assignment,
+		)
+
+		earnings = [
+			{
+				"salary_component": "Dependent Earning",
+				"abbr": "DE",
+				"type": "Earning",
+				"depends_on_payment_days": 0,
+				"amount_based_on_formula": 1,
+				"formula": "ID * 10",
+			},
+		]
+		make_salary_component(earnings, False, company_list=[])
+
+		deductions = [
+			{
+				"salary_component": "Independent Deduction",
+				"abbr": "ID",
+				"type": "Deduction",
+				"amount": 500,
+			},
+			{
+				"salary_component": "Dependent Deduction",
+				"abbr": "DD",
+				"type": "Deduction",
+				"amount_based_on_formula": 1,
+				"formula": "DE / 5",
+			},
+		]
+		make_salary_component(deductions, False, company_list=[])
+
+		details = {
+			"doctype": "Salary Structure",
+			"name": "Test Salary Structure for Circular Dependency",
+			"company": "_Test Company",
+			"payroll_frequency": "Monthly",
+			"payment_account": get_random("Account", filters={"account_currency": "USD"}),
+			"currency": "INR",
+		}
+		salary_structure = frappe.get_doc(details)
+
+		for entry in earnings:
+			salary_structure.append("earnings", entry)
+		for entry in deductions:
+			salary_structure.append("deductions", entry)
+
+		salary_structure.insert()
+		salary_structure.submit()
+
+		emp = make_employee("[email protected]", company="_Test Company")
+
+		create_salary_structure_assignment(emp, salary_structure.name, currency="INR")
+		salary_slip = make_salary_slip(
+			salary_structure.name, employee=emp, posting_date=getdate(), for_preview=1
+		)
+
+		self.assertEqual(salary_slip.gross_pay, 5000)
+		self.assertEqual(salary_slip.earnings[0].amount, 5000)
+		self.assertEqual(salary_slip.deductions[1].amount, 1000)
+
 
 class TestSalarySlipSafeEval(FrappeTestCase):
 	def test_safe_eval_for_salary_slip(self):