From b6e85a14a8c5be4e241134b5f21ef78ff8afd4ac Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Wed, 4 Sep 2024 09:54:12 +0530 Subject: [PATCH 1/3] fix(Payroll): multiline condition & formula eval failing on salary slip creation from payroll entry (cherry picked from commit c45f553bdaf04f1d5bc56b2812038fd1db1c9c5b) --- hrms/payroll/doctype/salary_slip/salary_slip.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hrms/payroll/doctype/salary_slip/salary_slip.py b/hrms/payroll/doctype/salary_slip/salary_slip.py index 5f2787fded..c623a708ce 100644 --- a/hrms/payroll/doctype/salary_slip/salary_slip.py +++ b/hrms/payroll/doctype/salary_slip/salary_slip.py @@ -329,7 +329,7 @@ def get_emp_and_working_day_details(self): struct = self.check_sal_struct() if struct: - self._salary_structure_doc = frappe.get_cached_doc("Salary Structure", struct) + self.set_salary_structure_doc() self.salary_slip_based_on_timesheet = ( self._salary_structure_doc.salary_slip_based_on_timesheet or 0 ) From 309a18400714b276864c000d73cda0bd0794e067 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Wed, 4 Sep 2024 10:15:42 +0530 Subject: [PATCH 2/3] test: multiline formula (cherry picked from commit 0a878c701e33dcbdc7141e72b2c829da019fc57a) --- hrms/payroll/doctype/salary_slip/test_salary_slip.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hrms/payroll/doctype/salary_slip/test_salary_slip.py b/hrms/payroll/doctype/salary_slip/test_salary_slip.py index 0305859d18..e24d5002cf 100644 --- a/hrms/payroll/doctype/salary_slip/test_salary_slip.py +++ b/hrms/payroll/doctype/salary_slip/test_salary_slip.py @@ -1884,7 +1884,8 @@ def make_earning_salary_component( "salary_component": "Special Allowance", "abbr": "SA", "condition": "H < 10000", - "formula": "BS*.5", + # intentional to test multiline formula + "formula": "BS\n*.5", "type": "Earning", "amount_based_on_formula": 1, "depends_on_payment_days": 0, From 587a7b4b6100c5277844de7c6ef070508e11d6b5 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 4 Sep 2024 14:49:00 +0530 Subject: [PATCH 3/3] chore: fix payroll tests (backport #2150) (#2151) (cherry picked from commit 4e55a0c788bcb27d5ed8c69521e4313287949ea0) Co-authored-by: Rucha Mahabal --- .../salary_component/test_salary_component.py | 19 ++++++++++-------- .../salary_structure/test_salary_structure.py | 20 +++++++++---------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/hrms/payroll/doctype/salary_component/test_salary_component.py b/hrms/payroll/doctype/salary_component/test_salary_component.py index 9ce17ceb9a..8bc503d70a 100644 --- a/hrms/payroll/doctype/salary_component/test_salary_component.py +++ b/hrms/payroll/doctype/salary_component/test_salary_component.py @@ -19,23 +19,26 @@ def test_update_salary_structures(self): salary_structure3 = make_salary_structure("Salary Structure 3", "Monthly") salary_structure3.cancel() # Details should not update for cancelled Salary Structures + OLD_FORMULA = "BS\n*.5" + OLD_CONDITION = "H < 10000" + ss1_detail = next( (d for d in salary_structure1.earnings if d.salary_component == "Special Allowance"), None ) - self.assertEqual(ss1_detail.condition, "H < 10000") - self.assertEqual(ss1_detail.formula, "BS*.5") + self.assertEqual(ss1_detail.condition, OLD_CONDITION) + self.assertEqual(ss1_detail.formula, OLD_FORMULA) ss2_detail = next( (d for d in salary_structure2.earnings if d.salary_component == "Special Allowance"), None ) - self.assertEqual(ss2_detail.condition, "H < 10000") - self.assertEqual(ss2_detail.formula, "BS*.5") + self.assertEqual(ss2_detail.condition, OLD_CONDITION) + self.assertEqual(ss2_detail.formula, OLD_FORMULA) ss3_detail = next( (d for d in salary_structure3.earnings if d.salary_component == "Special Allowance"), None ) - self.assertEqual(ss3_detail.condition, "H < 10000") - self.assertEqual(ss3_detail.formula, "BS*.5") + self.assertEqual(ss3_detail.condition, OLD_CONDITION) + self.assertEqual(ss3_detail.formula, OLD_FORMULA) salary_component.update_salary_structures("condition", "H < 8000") ss1_detail.reload() @@ -43,7 +46,7 @@ def test_update_salary_structures(self): ss2_detail.reload() self.assertEqual(ss2_detail.condition, "H < 8000") ss3_detail.reload() - self.assertEqual(ss3_detail.condition, "H < 10000") + self.assertEqual(ss3_detail.condition, OLD_CONDITION) salary_component.update_salary_structures("formula", "BS*.3") ss1_detail.reload() @@ -51,7 +54,7 @@ def test_update_salary_structures(self): ss2_detail.reload() self.assertEqual(ss2_detail.formula, "BS*.3") ss3_detail.reload() - self.assertEqual(ss3_detail.formula, "BS*.5") + self.assertEqual(ss3_detail.formula, OLD_FORMULA) def create_salary_component(component_name, **args): diff --git a/hrms/payroll/doctype/salary_structure/test_salary_structure.py b/hrms/payroll/doctype/salary_structure/test_salary_structure.py index a52741df56..5e0861942f 100644 --- a/hrms/payroll/doctype/salary_structure/test_salary_structure.py +++ b/hrms/payroll/doctype/salary_structure/test_salary_structure.py @@ -3,7 +3,7 @@ import frappe from frappe.tests.utils import FrappeTestCase -from frappe.utils import add_years, date_diff, get_first_day, nowdate +from frappe.utils import add_years, cstr, date_diff, get_first_day, nowdate from frappe.utils.make_random import get_random import erpnext @@ -91,23 +91,23 @@ def test_amount_totals(self): self.assertEqual(salary_slip.get("net_pay"), 78000 - salary_slip.get("total_deduction")) def test_whitespaces_in_formula_conditions_fields(self): - salary_structure = make_salary_structure("Salary Structure Sample", "Monthly", dont_submit=True) - - for row in salary_structure.earnings: + def add_whitespaces(row): row.formula = "\n%s\n\n" % row.formula row.condition = "\n%s\n\n" % row.condition - for row in salary_structure.deductions: - row.formula = "\n%s\n\n" % row.formula - row.condition = "\n%s\n\n" % row.condition + salary_structure = make_salary_structure("Salary Structure Sample", "Monthly", dont_submit=True) + for table in ("earnings", "deductions"): + for row in salary_structure.get(table): + add_whitespaces(row) - salary_structure.save() + # sanitized before validate and reset to original state to maintain readability + salary_structure.sanitize_condition_and_formula_fields() for row in salary_structure.earnings: - self.assertFalse("\n" in row.formula or "\n" in row.condition) + self.assertFalse("\n" in cstr(row.formula) or "\n" in cstr(row.condition)) for row in salary_structure.deductions: - self.assertFalse(("\n" in row.formula) or ("\n" in row.condition)) + self.assertFalse("\n" in cstr(row.formula) or "\n" in cstr(row.condition)) def test_salary_structures_assignment(self): company_currency = erpnext.get_default_currency()