From bab644a249de4355d6700f53a7bfbf0114ebb30c Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 13 Sep 2021 13:24:27 +0530 Subject: [PATCH] fix(Payroll): incorrect component amount calculation if dependent on another payment days based component (#27349) * fix(Payroll): incorrect component amount calculation if dependent on another payment days based component * fix: set component amount precision at the end * fix: consider default amount during taxt calculations * test: component amount dependent on another payment days based component * fix: test --- .../doctype/salary_slip/salary_slip.py | 33 ++-- .../doctype/salary_slip/test_salary_slip.py | 148 ++++++++++++++++++ 2 files changed, 167 insertions(+), 14 deletions(-) diff --git a/erpnext/payroll/doctype/salary_slip/salary_slip.py b/erpnext/payroll/doctype/salary_slip/salary_slip.py index 8c48345d8fd6..d113e7e56974 100644 --- a/erpnext/payroll/doctype/salary_slip/salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/salary_slip.py @@ -487,7 +487,7 @@ def calculate_net_pay(self): self.calculate_component_amounts("deductions") self.set_loan_repayment() - self.set_component_amounts_based_on_payment_days() + self.set_precision_for_component_amounts() self.set_net_pay() def set_net_pay(self): @@ -709,6 +709,17 @@ def update_component_row(self, component_data, amount, component_type, additiona component_row.amount = amount + self.update_component_amount_based_on_payment_days(component_row) + + def update_component_amount_based_on_payment_days(self, component_row): + joining_date, relieving_date = self.get_joining_and_relieving_dates() + component_row.amount = self.get_amount_based_on_payment_days(component_row, joining_date, relieving_date)[0] + + def set_precision_for_component_amounts(self): + for component_type in ("earnings", "deductions"): + for component_row in self.get(component_type): + component_row.amount = flt(component_row.amount, component_row.precision("amount")) + def calculate_variable_based_on_taxable_salary(self, tax_component, payroll_period): if not payroll_period: frappe.msgprint(_("Start and end dates not in a valid Payroll Period, cannot calculate {0}.") @@ -866,14 +877,7 @@ def get_tax_paid_in_period(self, start_date, end_date, tax_component): return total_tax_paid def get_taxable_earnings(self, allow_tax_exemption=False, based_on_payment_days=0): - joining_date, relieving_date = frappe.get_cached_value("Employee", self.employee, - ["date_of_joining", "relieving_date"]) - - if not relieving_date: - relieving_date = getdate(self.end_date) - - if not joining_date: - frappe.throw(_("Please set the Date Of Joining for employee {0}").format(frappe.bold(self.employee_name))) + joining_date, relieving_date = self.get_joining_and_relieving_dates() taxable_earnings = 0 additional_income = 0 @@ -884,7 +888,10 @@ def get_taxable_earnings(self, allow_tax_exemption=False, based_on_payment_days= if based_on_payment_days: amount, additional_amount = self.get_amount_based_on_payment_days(earning, joining_date, relieving_date) else: - amount, additional_amount = earning.amount, earning.additional_amount + if earning.additional_amount: + amount, additional_amount = earning.amount, earning.additional_amount + else: + amount, additional_amount = earning.default_amount, earning.additional_amount if earning.is_tax_applicable: if additional_amount: @@ -1055,7 +1062,7 @@ def get_component_totals(self, component_type, depends_on_payment_days=0): total += amount return total - def set_component_amounts_based_on_payment_days(self): + def get_joining_and_relieving_dates(self): joining_date, relieving_date = frappe.get_cached_value("Employee", self.employee, ["date_of_joining", "relieving_date"]) @@ -1065,9 +1072,7 @@ def set_component_amounts_based_on_payment_days(self): if not joining_date: frappe.throw(_("Please set the Date Of Joining for employee {0}").format(frappe.bold(self.employee_name))) - for component_type in ("earnings", "deductions"): - for d in self.get(component_type): - d.amount = flt(self.get_amount_based_on_payment_days(d, joining_date, relieving_date)[0], d.precision("amount")) + return joining_date, relieving_date def set_loan_repayment(self): self.total_loan_repayment = 0 diff --git a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py index 480daa259543..bff36a414902 100644 --- a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py @@ -17,6 +17,7 @@ getdate, nowdate, ) +from frappe.utils.make_random import get_random import erpnext from erpnext.accounts.utils import get_fiscal_year @@ -134,6 +135,65 @@ def test_payment_days_based_on_leave_application(self): frappe.db.set_value("Payroll Settings", None, "payroll_based_on", "Leave") + def test_component_amount_dependent_on_another_payment_days_based_component(self): + from erpnext.hr.doctype.attendance.attendance import mark_attendance + from erpnext.payroll.doctype.salary_structure.test_salary_structure import ( + create_salary_structure_assignment, + ) + + no_of_days = self.get_no_of_days() + # Payroll based on attendance + frappe.db.set_value("Payroll Settings", None, "payroll_based_on", "Attendance") + + salary_structure = make_salary_structure_for_payment_days_based_component_dependency() + employee = make_employee("test_payment_days_based_component@salary.com", company="_Test Company") + + # base = 50000 + create_salary_structure_assignment(employee, salary_structure.name, company="_Test Company", currency="INR") + + # mark employee absent for a day since this case works fine if payment days are equal to working days + month_start_date = get_first_day(nowdate()) + month_end_date = get_last_day(nowdate()) + + first_sunday = frappe.db.sql(""" + select holiday_date from `tabHoliday` + where parent = 'Salary Slip Test Holiday List' + and holiday_date between %s and %s + order by holiday_date + """, (month_start_date, month_end_date))[0][0] + + mark_attendance(employee, add_days(first_sunday, 1), 'Absent', ignore_validate=True) # counted as absent + + # make salary slip and assert payment days + ss = make_salary_slip_for_payment_days_dependency_test("test_payment_days_based_component@salary.com", salary_structure.name) + self.assertEqual(ss.absent_days, 1) + + days_in_month = no_of_days[0] + no_of_holidays = no_of_days[1] + + self.assertEqual(ss.payment_days, days_in_month - no_of_holidays - 1) + + ss.reload() + payment_days_based_comp_amount = 0 + for component in ss.earnings: + if component.salary_component == "HRA - Payment Days": + payment_days_based_comp_amount = flt(component.amount, component.precision("amount")) + break + + # check if the dependent component is calculated using the amount updated after payment days + actual_amount = 0 + precision = 0 + for component in ss.deductions: + if component.salary_component == "P - Employee Provident Fund": + precision = component.precision("amount") + actual_amount = flt(component.amount, precision) + break + + expected_amount = flt((flt(ss.gross_pay) - payment_days_based_comp_amount) * 0.12, precision) + + self.assertEqual(actual_amount, expected_amount) + frappe.db.set_value("Payroll Settings", None, "payroll_based_on", "Leave") + def test_salary_slip_with_holidays_included(self): no_of_days = self.get_no_of_days() frappe.db.set_value("Payroll Settings", None, "include_holidays_in_total_working_days", 1) @@ -864,3 +924,91 @@ def make_holiday_list(): holiday_list = holiday_list.name return holiday_list + +def make_salary_structure_for_payment_days_based_component_dependency(): + earnings = [ + { + "salary_component": "Basic Salary - Payment Days", + "abbr": "P_BS", + "type": "Earning", + "formula": "base", + "amount_based_on_formula": 1 + }, + { + "salary_component": "HRA - Payment Days", + "abbr": "P_HRA", + "type": "Earning", + "depends_on_payment_days": 1, + "amount_based_on_formula": 1, + "formula": "base * 0.20" + } + ] + + make_salary_component(earnings, False, company_list=["_Test Company"]) + + deductions = [ + { + "salary_component": "P - Professional Tax", + "abbr": "P_PT", + "type": "Deduction", + "depends_on_payment_days": 1, + "amount": 200.00 + }, + { + "salary_component": "P - Employee Provident Fund", + "abbr": "P_EPF", + "type": "Deduction", + "exempted_from_income_tax": 1, + "amount_based_on_formula": 1, + "depends_on_payment_days": 0, + "formula": "(gross_pay - P_HRA) * 0.12" + } + ] + + make_salary_component(deductions, False, company_list=["_Test Company"]) + + salary_structure = "Salary Structure with PF" + if frappe.db.exists("Salary Structure", salary_structure): + frappe.db.delete("Salary Structure", salary_structure) + + details = { + "doctype": "Salary Structure", + "name": salary_structure, + "company": "_Test Company", + "payroll_frequency": "Monthly", + "payment_account": get_random("Account", filters={"account_currency": "INR"}), + "currency": "INR" + } + + salary_structure_doc = frappe.get_doc(details) + + for entry in earnings: + salary_structure_doc.append("earnings", entry) + + for entry in deductions: + salary_structure_doc.append("deductions", entry) + + salary_structure_doc.insert() + salary_structure_doc.submit() + + return salary_structure_doc + +def make_salary_slip_for_payment_days_dependency_test(employee, salary_structure): + employee = frappe.db.get_value("Employee", { + "user_id": employee + }, + ["name", "company", "employee_name"], + as_dict=True) + + salary_slip_name = frappe.db.get_value("Salary Slip", {"employee": frappe.db.get_value("Employee", {"user_id": employee})}) + + if not salary_slip_name: + salary_slip = make_salary_slip(salary_structure, employee=employee.name) + salary_slip.employee_name = employee.employee_name + salary_slip.payroll_frequency = "Monthly" + salary_slip.posting_date = nowdate() + salary_slip.insert() + else: + salary_slip = frappe.get_doc("Salary Slip", salary_slip_name) + + return salary_slip \ No newline at end of file