From 447c978757ffeef8ebaeedc2dd34124ed9b45fff Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Fri, 28 May 2021 21:28:42 +0530 Subject: [PATCH 1/5] fix: choose correct Salary Structure Assignment when getting data for formula eval --- .../doctype/salary_slip/salary_slip.py | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/erpnext/payroll/doctype/salary_slip/salary_slip.py b/erpnext/payroll/doctype/salary_slip/salary_slip.py index afdf081ac8..cc9e8d1043 100644 --- a/erpnext/payroll/doctype/salary_slip/salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/salary_slip.py @@ -493,8 +493,28 @@ class SalarySlip(TransactionBase): '''Returns data for evaluating formula''' data = frappe._dict() + salary_structure_assignment = frappe.get_value( + "Salary Structure Assignment", + { + "employee": self.employee, + "salary_structure": self.salary_structure, + "from_date": ("<=", self.start_date), + "docstatus": 1, + }, + order_by="from_date desc", + ) + + if not salary_structure_assignment: + frappe.throw( + _("Please assign a Salary Structure for Employee {0} " + "applicable from or before {1} first").format( + frappe.bold(self.employee_name), + frappe.bold(self.start_date) + ) + ) + data.update(frappe.get_doc("Salary Structure Assignment", - {"employee": self.employee, "salary_structure": self.salary_structure}).as_dict()) + salary_structure_assignment).as_dict()) data.update(frappe.get_doc("Employee", self.employee).as_dict()) data.update(self.as_dict()) From 74818c7b624534a39b647476587abb02f84b1e88 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Sat, 29 May 2021 00:04:26 +0530 Subject: [PATCH 2/5] fix: improve filter for `from_date`; validation for joining and relieving date --- .../doctype/salary_slip/salary_slip.py | 40 +++++++++++++++---- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/erpnext/payroll/doctype/salary_slip/salary_slip.py b/erpnext/payroll/doctype/salary_slip/salary_slip.py index cc9e8d1043..2b35d94dfc 100644 --- a/erpnext/payroll/doctype/salary_slip/salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/salary_slip.py @@ -115,10 +115,23 @@ class SalarySlip(TransactionBase): status = "Cancelled" return status - def validate_dates(self): + def validate_dates(self, joining_date=None, relieving_date=None): if date_diff(self.end_date, self.start_date) < 0: frappe.throw(_("To date cannot be before From date")) + if not joining_date: + joining_date, relieving_date = frappe.get_cached_value( + "Employee", + self.employee, + ("date_of_joining", "relieving_date") + ) + + if date_diff(self.end_date, joining_date) < 0: + frappe.throw(_("Cannot create Salary Slip for Employee joining after Payroll Period")) + + if relieving_date and date_diff(relieving_date, self.start_date) < 0: + frappe.throw(_("Cannot create Salary Slip for Employee who has left before Payroll Period")) + def is_rounding_total_disabled(self): return cint(frappe.db.get_single_value("Payroll Settings", "disable_rounded_total")) @@ -154,9 +167,14 @@ class SalarySlip(TransactionBase): if not self.salary_slip_based_on_timesheet: self.get_date_details() - self.validate_dates() - joining_date, relieving_date = frappe.get_cached_value("Employee", self.employee, - ["date_of_joining", "relieving_date"]) + + joining_date, relieving_date = frappe.get_cached_value( + "Employee", + self.employee, + ("date_of_joining", "relieving_date") + ) + + self.validate_dates(joining_date, relieving_date) #getin leave details self.get_working_days_details(joining_date, relieving_date) @@ -492,13 +510,21 @@ class SalarySlip(TransactionBase): def get_data_for_eval(self): '''Returns data for evaluating formula''' data = frappe._dict() + employee = frappe.get_doc("Employee", self.employee).as_dict() + + start_date = getdate(self.start_date) + date_to_validate = ( + employee.date_of_joining + if employee.date_of_joining > start_date + else start_date + ) salary_structure_assignment = frappe.get_value( "Salary Structure Assignment", { "employee": self.employee, "salary_structure": self.salary_structure, - "from_date": ("<=", self.start_date), + "from_date": ("<=", date_to_validate), "docstatus": 1, }, order_by="from_date desc", @@ -509,14 +535,14 @@ class SalarySlip(TransactionBase): _("Please assign a Salary Structure for Employee {0} " "applicable from or before {1} first").format( frappe.bold(self.employee_name), - frappe.bold(self.start_date) + frappe.bold(formatdate(date_to_validate)), ) ) data.update(frappe.get_doc("Salary Structure Assignment", salary_structure_assignment).as_dict()) - data.update(frappe.get_doc("Employee", self.employee).as_dict()) + data.update(employee) data.update(self.as_dict()) # set values for components From 0e5e1350b241e9ac0b1c93b92da1d8999cd9723c Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Fri, 4 Jun 2021 13:51:45 +0530 Subject: [PATCH 3/5] perf: use frappe.get_value with wildcard instead of another frappe.get_doc call --- erpnext/payroll/doctype/salary_slip/salary_slip.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/erpnext/payroll/doctype/salary_slip/salary_slip.py b/erpnext/payroll/doctype/salary_slip/salary_slip.py index 2b35d94dfc..877503b41c 100644 --- a/erpnext/payroll/doctype/salary_slip/salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/salary_slip.py @@ -527,7 +527,9 @@ class SalarySlip(TransactionBase): "from_date": ("<=", date_to_validate), "docstatus": 1, }, + "*", order_by="from_date desc", + as_dict=True, ) if not salary_structure_assignment: @@ -539,9 +541,7 @@ class SalarySlip(TransactionBase): ) ) - data.update(frappe.get_doc("Salary Structure Assignment", - salary_structure_assignment).as_dict()) - + data.update(salary_structure_assignment) data.update(employee) data.update(self.as_dict()) From ca205be5ac62ff25815df81bde6e2a8a7852b26c Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Mon, 7 Jun 2021 18:40:54 +0530 Subject: [PATCH 4/5] fix: tests --- .../doctype/salary_slip/test_salary_slip.py | 51 +++++++++++-------- .../salary_structure/test_salary_structure.py | 39 +++++++++----- 2 files changed, 57 insertions(+), 33 deletions(-) diff --git a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py index 01e4170d31..9e7db977ab 100644 --- a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py @@ -8,7 +8,6 @@ import erpnext import calendar import random from erpnext.accounts.utils import get_fiscal_year -from frappe.utils.make_random import get_random from frappe.utils import getdate, nowdate, add_days, add_months, flt, get_first_day, get_last_day, cstr from erpnext.payroll.doctype.salary_structure.salary_structure import make_salary_slip from erpnext.payroll.doctype.payroll_entry.payroll_entry import get_month_details @@ -155,12 +154,14 @@ class TestSalarySlip(unittest.TestCase): self.assertEqual(ss.gross_pay, 78000) def test_payment_days(self): + from erpnext.payroll.doctype.salary_structure.test_salary_structure import create_salary_structure_assignment + no_of_days = self.get_no_of_days() # Holidays not included in working days frappe.db.set_value("Payroll Settings", None, "include_holidays_in_total_working_days", 1) # set joinng date in the same month - make_employee("test_payment_days@salary.com") + employee = make_employee("test_payment_days@salary.com") if getdate(nowdate()).day >= 15: relieving_date = getdate(add_days(nowdate(),-10)) date_of_joining = getdate(add_days(nowdate(),-10)) @@ -174,25 +175,30 @@ class TestSalarySlip(unittest.TestCase): date_of_joining = getdate(nowdate()) relieving_date = getdate(nowdate()) - frappe.db.set_value("Employee", frappe.get_value("Employee", - {"employee_name":"test_payment_days@salary.com"}, "name"), "date_of_joining", date_of_joining) - frappe.db.set_value("Employee", frappe.get_value("Employee", - {"employee_name":"test_payment_days@salary.com"}, "name"), "relieving_date", None) - frappe.db.set_value("Employee", frappe.get_value("Employee", - {"employee_name":"test_payment_days@salary.com"}, "name"), "status", "Active") + frappe.db.set_value("Employee", employee, { + "date_of_joining": date_of_joining, + "relieving_date": None, + "status": "Active" + }) - ss = make_employee_salary_slip("test_payment_days@salary.com", "Monthly", "Test Payment Days") + salary_structure = "Test Payment Days" + ss = make_employee_salary_slip("test_payment_days@salary.com", "Monthly", salary_structure) self.assertEqual(ss.total_working_days, no_of_days[0]) self.assertEqual(ss.payment_days, (no_of_days[0] - getdate(date_of_joining).day + 1)) # set relieving date in the same month - frappe.db.set_value("Employee",frappe.get_value("Employee", - {"employee_name":"test_payment_days@salary.com"}, "name"), "date_of_joining", (add_days(nowdate(),-60))) - frappe.db.set_value("Employee", frappe.get_value("Employee", - {"employee_name":"test_payment_days@salary.com"}, "name"), "relieving_date", relieving_date) - frappe.db.set_value("Employee", frappe.get_value("Employee", - {"employee_name":"test_payment_days@salary.com"}, "name"), "status", "Left") + frappe.db.set_value("Employee", employee, { + "date_of_joining": add_days(nowdate(),-60), + "relieving_date": relieving_date, + "status": "Left" + }) + + if date_of_joining.day > 1: + self.assertRaises(frappe.ValidationError, ss.save) + + create_salary_structure_assignment(employee, salary_structure) + ss.reload() ss.save() self.assertEqual(ss.total_working_days, no_of_days[0]) @@ -285,6 +291,7 @@ class TestSalarySlip(unittest.TestCase): def test_multi_currency_salary_slip(self): from erpnext.payroll.doctype.salary_structure.test_salary_structure import make_salary_structure + applicant = make_employee("test_multi_currency_salary_slip@salary.com", company="_Test Company") frappe.db.sql("""delete from `tabSalary Structure` where name='Test Multi Currency Salary Slip'""") salary_structure = make_salary_structure("Test Multi Currency Salary Slip", "Monthly", employee=applicant, company="_Test Company", currency='USD') @@ -325,7 +332,8 @@ class TestSalarySlip(unittest.TestCase): def test_component_wise_year_to_date_computation(self): from erpnext.payroll.doctype.salary_structure.test_salary_structure import make_salary_structure - applicant = make_employee("test_ytd@salary.com", company="_Test Company") + employee_name = "test_component_wise_ytd@salary.com" + applicant = make_employee(employee_name, company="_Test Company") payroll_period = create_payroll_period(name="_Test Payroll Period 1", company="_Test Company") @@ -336,13 +344,13 @@ class TestSalarySlip(unittest.TestCase): "Monthly", employee=applicant, company="_Test Company", currency="INR", payroll_period=payroll_period) # clear salary slip for this employee - frappe.db.sql("DELETE FROM `tabSalary Slip` where employee_name = 'test_ytd@salary.com'") + frappe.db.sql("DELETE FROM `tabSalary Slip` where employee_name = '%s'" % employee_name) create_salary_slips_for_payroll_period(applicant, salary_structure.name, payroll_period, deduct_random=False, num=3) salary_slips = frappe.get_all("Salary Slip", fields=["name"], filters={"employee_name": - "test_ytd@salary.com"}, order_by = "posting_date") + employee_name}, order_by="posting_date") year_to_date = dict() for slip in salary_slips: @@ -380,10 +388,10 @@ class TestSalarySlip(unittest.TestCase): from erpnext.payroll.doctype.salary_structure.test_salary_structure import \ make_salary_structure, create_salary_structure_assignment + salary_structure = make_salary_structure("Stucture to test tax", "Monthly", - other_details={"max_benefits": 100000}, test_tax=True) - create_salary_structure_assignment(employee, salary_structure.name, - payroll_period.start_date) + other_details={"max_benefits": 100000}, test_tax=True, + employee=employee, payroll_period=payroll_period) # create salary slip for whole period deducting tax only on last period # to find the total tax amount paid @@ -469,6 +477,7 @@ class TestSalarySlip(unittest.TestCase): def make_employee_salary_slip(user, payroll_frequency, salary_structure=None): from erpnext.payroll.doctype.salary_structure.test_salary_structure import make_salary_structure + if not salary_structure: salary_structure = payroll_frequency + " Salary Structure Test for Salary Slip" diff --git a/erpnext/payroll/doctype/salary_structure/test_salary_structure.py b/erpnext/payroll/doctype/salary_structure/test_salary_structure.py index 36387f23df..26cd9922e4 100644 --- a/erpnext/payroll/doctype/salary_structure/test_salary_structure.py +++ b/erpnext/payroll/doctype/salary_structure/test_salary_structure.py @@ -6,7 +6,7 @@ import frappe import unittest import erpnext from frappe.utils.make_random import get_random -from frappe.utils import nowdate, add_days, add_years, getdate, add_months +from frappe.utils import nowdate, add_days, add_years, getdate, add_months, get_first_day, date_diff from erpnext.payroll.doctype.salary_structure.salary_structure import make_salary_slip from erpnext.payroll.doctype.salary_slip.test_salary_slip import make_earning_salary_component,\ make_deduction_salary_component, make_employee_salary_slip, create_tax_slab @@ -113,8 +113,9 @@ class TestSalaryStructure(unittest.TestCase): sal_struct = make_salary_structure("Salary Structure Multi Currency", "Monthly", currency='USD') self.assertEqual(sal_struct.currency, 'USD') -def make_salary_structure(salary_structure, payroll_frequency, employee=None, dont_submit=False, other_details=None, - test_tax=False, company=None, currency=erpnext.get_default_currency(), payroll_period=None): +def make_salary_structure(salary_structure, payroll_frequency, employee=None, + from_date=None, dont_submit=False, other_details=None,test_tax=False, + company=None, currency=erpnext.get_default_currency(), payroll_period=None): if test_tax: frappe.db.sql("""delete from `tabSalary Structure` where name=%s""",(salary_structure)) @@ -139,10 +140,23 @@ def make_salary_structure(salary_structure, payroll_frequency, employee=None, do else: salary_structure_doc = frappe.get_doc("Salary Structure", salary_structure) + filters = {'employee':employee, 'docstatus': 1} + if not from_date and payroll_period: + from_date = payroll_period.start_date + + if from_date: + filters['from_date'] = from_date + if employee and not frappe.db.get_value("Salary Structure Assignment", - {'employee':employee, 'docstatus': 1}) and salary_structure_doc.docstatus==1: - create_salary_structure_assignment(employee, salary_structure, company=company, currency=currency, - payroll_period=payroll_period) + filters) and salary_structure_doc.docstatus==1: + create_salary_structure_assignment( + employee, + salary_structure, + from_date=from_date, + company=company, + currency=currency, + payroll_period=payroll_period + ) return salary_structure_doc @@ -165,12 +179,13 @@ def create_salary_structure_assignment(employee, salary_structure, from_date=Non salary_structure_assignment.base = 50000 salary_structure_assignment.variable = 5000 - if getdate(nowdate()).day == 1: - date = from_date or nowdate() - else: - date = from_date or add_days(nowdate(), -1) + if not from_date: + from_date = get_first_day(nowdate()) + joining_date = frappe.get_cached_value("Employee", employee, "date_of_joining") + if date_diff(joining_date, from_date) > 0: + from_date = joining_date - salary_structure_assignment.from_date = date + salary_structure_assignment.from_date = from_date salary_structure_assignment.salary_structure = salary_structure salary_structure_assignment.currency = currency salary_structure_assignment.payroll_payable_account = get_payable_account(company) @@ -183,4 +198,4 @@ def create_salary_structure_assignment(employee, salary_structure, from_date=Non def get_payable_account(company=None): if not company: company = erpnext.get_default_company() - return frappe.db.get_value("Company", company, "default_payroll_payable_account") \ No newline at end of file + return frappe.db.get_value("Company", company, "default_payroll_payable_account") From 062e247353a2277727050c648a4633a2d24fb1b8 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Mon, 7 Jun 2021 19:34:02 +0530 Subject: [PATCH 5/5] test: remove unused imports --- .../payroll/doctype/salary_structure/test_salary_structure.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/payroll/doctype/salary_structure/test_salary_structure.py b/erpnext/payroll/doctype/salary_structure/test_salary_structure.py index 26cd9922e4..dce6b7aa3d 100644 --- a/erpnext/payroll/doctype/salary_structure/test_salary_structure.py +++ b/erpnext/payroll/doctype/salary_structure/test_salary_structure.py @@ -6,7 +6,7 @@ import frappe import unittest import erpnext from frappe.utils.make_random import get_random -from frappe.utils import nowdate, add_days, add_years, getdate, add_months, get_first_day, date_diff +from frappe.utils import nowdate, add_years, get_first_day, date_diff from erpnext.payroll.doctype.salary_structure.salary_structure import make_salary_slip from erpnext.payroll.doctype.salary_slip.test_salary_slip import make_earning_salary_component,\ make_deduction_salary_component, make_employee_salary_slip, create_tax_slab