From 48e5128a3f58f3aa81cbd62151fc08cfdd67bea7 Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Fri, 10 Jan 2025 14:17:31 +0530 Subject: [PATCH 1/5] fix: create single leave ledger entry for encashment if leave is carry forward type. (cherry picked from commit acd618f84c5d6458b1c41947bd995305e5b56a71) # Conflicts: # hrms/hr/doctype/leave_encashment/leave_encashment.py --- hrms/hr/doctype/leave_encashment/leave_encashment.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hrms/hr/doctype/leave_encashment/leave_encashment.py b/hrms/hr/doctype/leave_encashment/leave_encashment.py index 66481f6bea..e25fad8bb6 100644 --- a/hrms/hr/doctype/leave_encashment/leave_encashment.py +++ b/hrms/hr/doctype/leave_encashment/leave_encashment.py @@ -205,14 +205,17 @@ def create_leave_ledger_entry(self, submit=True): to_date = leave_allocation.get("to_date") +<<<<<<< HEAD can_expire = not frappe.db.get_value("Leave Type", self.leave_type, "is_carry_forward") +======= + can_expire = not frappe.db.get_value("Leave Type",self.leave_type,"is_carry_forward") +>>>>>>> acd618f8 (fix: create single leave ledger entry for encashment if leave is carry forward type.) if to_date < getdate() and can_expire: args = frappe._dict( leaves=self.encashment_days, from_date=to_date, to_date=to_date, is_carry_forward=0 ) create_leave_ledger_entry(self, args, submit) - def create_leave_encashment(leave_allocation): """Creates leave encashment for the given allocations""" for allocation in leave_allocation: From dea0135914302fe20aa03b127da808a8b9bf2a52 Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Fri, 10 Jan 2025 15:01:05 +0530 Subject: [PATCH 2/5] chore: fixed linter issue (cherry picked from commit de06a25ad4668fe24015c24d390d8e861ebd0898) # Conflicts: # hrms/hr/doctype/leave_encashment/leave_encashment.py --- hrms/hr/doctype/leave_encashment/leave_encashment.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hrms/hr/doctype/leave_encashment/leave_encashment.py b/hrms/hr/doctype/leave_encashment/leave_encashment.py index e25fad8bb6..6db6b1425a 100644 --- a/hrms/hr/doctype/leave_encashment/leave_encashment.py +++ b/hrms/hr/doctype/leave_encashment/leave_encashment.py @@ -205,17 +205,22 @@ def create_leave_ledger_entry(self, submit=True): to_date = leave_allocation.get("to_date") +<<<<<<< HEAD <<<<<<< HEAD can_expire = not frappe.db.get_value("Leave Type", self.leave_type, "is_carry_forward") ======= can_expire = not frappe.db.get_value("Leave Type",self.leave_type,"is_carry_forward") >>>>>>> acd618f8 (fix: create single leave ledger entry for encashment if leave is carry forward type.) +======= + can_expire = not frappe.db.get_value("Leave Type", self.leave_type, "is_carry_forward") +>>>>>>> de06a25a (chore: fixed linter issue) if to_date < getdate() and can_expire: args = frappe._dict( leaves=self.encashment_days, from_date=to_date, to_date=to_date, is_carry_forward=0 ) create_leave_ledger_entry(self, args, submit) + def create_leave_encashment(leave_allocation): """Creates leave encashment for the given allocations""" for allocation in leave_allocation: From 853ddadc0ce0e99f68d97b8b2ae175a8a5d5da54 Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Mon, 13 Jan 2025 17:42:27 +0530 Subject: [PATCH 3/5] chore: test creation of leave ledger entries depending on leave type while creating leave encashment (cherry picked from commit 2a1632e6e54ad59e0916555034ce70d3c50574ce) --- .../leave_encashment/test_leave_encashment.py | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/hrms/hr/doctype/leave_encashment/test_leave_encashment.py b/hrms/hr/doctype/leave_encashment/test_leave_encashment.py index a315fcf846..28c555d79d 100644 --- a/hrms/hr/doctype/leave_encashment/test_leave_encashment.py +++ b/hrms/hr/doctype/leave_encashment/test_leave_encashment.py @@ -259,3 +259,91 @@ def test_creation_of_leave_ledger_entry_on_submit(self): frappe.db.delete("Additional Salary", {"ref_docname": leave_encashment.name}) leave_encashment.cancel() self.assertFalse(frappe.db.exists("Leave Ledger Entry", {"transaction_name": leave_encashment.name})) + + @set_holiday_list("_Test Leave Encashment", "_Test Company") + def test_creation_of_leave_ledger_entry_after_leave_period_expiry(self): + frappe.db.delete("Leave Period", {"name": self.leave_period.name}) + # create new leave period that has end date of yesterday + start_date = add_days(getdate(), -30) + end_date = add_days(getdate(), -1) + self.leave_period = create_leave_period(start_date, end_date, "_Test Company") + + # case 1: leave type carry forwards + frappe.db.set_value( + "Leave Type", + self.leave_type, + { + "is_carry_forward": 1, + }, + ) + + leave_policy = frappe.get_value("Leave Policy", {"title": "Test Leave Policy"}, "name") + data = { + "assignment_based_on": "Leave Period", + "leave_policy": leave_policy, + "leave_period": self.leave_period.name, + } + employee = make_employee("test_employee2_encashment@example.com", company="_Test Company") + create_assignment_for_multiple_employees([employee], frappe._dict(data)) + + make_salary_structure( + "Salary Structure for Encashment", + "Monthly", + employee, + other_details={"leave_encashment_amount_per_day": 50}, + ) + + leave_encashment = frappe.get_doc( + { + "doctype": "Leave Encashment", + "employee": employee, + "leave_type": self.leave_type, + "leave_period": self.leave_period.name, + "encashment_date": self.leave_period.to_date, + "currency": "INR", + } + ).insert() + leave_encashment.submit() + # check if single leave ledger entry is created of negative value + self.assertEqual(frappe.get_value("Leave Type", self.leave_type, "is_carry_forward"), 1) + leave_ledger_entry = frappe.get_all( + "Leave Ledger Entry", fields="*", filters={"transaction_name": leave_encashment.name} + ) + self.assertEqual(len(leave_ledger_entry), 1) + self.assertEqual(leave_ledger_entry[0].leaves, leave_encashment.encashment_days * -1) + + # case 2: leave type does not carry forward + # cancel previous encashment + frappe.db.delete("Additional Salary", {"ref_docname": leave_encashment.name}) + leave_encashment.cancel() + # set leave type to not carry forward + frappe.db.set_value( + "Leave Type", + self.leave_type, + { + "is_carry_forward": 0, + }, + ) + # create leave encashment + new_leave_encashment = frappe.get_doc( + { + "doctype": "Leave Encashment", + "employee": employee, + "leave_type": self.leave_type, + "leave_period": self.leave_period.name, + "encashment_date": self.leave_period.to_date, + "currency": "INR", + } + ).insert() + new_leave_encashment.submit() + # check if two leave ledger entries are created + self.assertEqual(frappe.get_value("Leave Type", self.leave_type, "is_carry_forward"), 0) + leave_ledger_entry = frappe.get_all( + "Leave Ledger Entry", + fields="*", + filters={"transaction_name": new_leave_encashment.name}, + order_by="leaves", + ) + self.assertEqual(len(leave_ledger_entry), 2) + self.assertEqual(leave_ledger_entry[0].leaves, new_leave_encashment.encashment_days * -1) + self.assertEqual(leave_ledger_entry[1].leaves, new_leave_encashment.encashment_days * 1) From 2e0c35ea8c6c7c98103bffffa806e3df80b57c9a Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Wed, 15 Jan 2025 15:17:20 +0530 Subject: [PATCH 4/5] chore: better test scenarios for encashments created after leave period (cherry picked from commit ebf0545cfbb5bc5dbf31b07dfdecda7b1fa69a1c) --- .../leave_encashment/test_leave_encashment.py | 114 ++++++++++-------- 1 file changed, 66 insertions(+), 48 deletions(-) diff --git a/hrms/hr/doctype/leave_encashment/test_leave_encashment.py b/hrms/hr/doctype/leave_encashment/test_leave_encashment.py index 28c555d79d..d43b2ac50f 100644 --- a/hrms/hr/doctype/leave_encashment/test_leave_encashment.py +++ b/hrms/hr/doctype/leave_encashment/test_leave_encashment.py @@ -8,6 +8,8 @@ from erpnext.setup.doctype.employee.test_employee import make_employee from erpnext.setup.doctype.holiday_list.test_holiday_list import set_holiday_list +from hrms.hr.doctype.leave_allocation.leave_allocation import get_unused_leaves +from hrms.hr.doctype.leave_ledger_entry.leave_ledger_entry import process_expired_allocation from hrms.hr.doctype.leave_period.test_leave_period import create_leave_period from hrms.hr.doctype.leave_policy.test_leave_policy import create_leave_policy from hrms.hr.doctype.leave_policy_assignment.leave_policy_assignment import ( @@ -261,19 +263,77 @@ def test_creation_of_leave_ledger_entry_on_submit(self): self.assertFalse(frappe.db.exists("Leave Ledger Entry", {"transaction_name": leave_encashment.name})) @set_holiday_list("_Test Leave Encashment", "_Test Company") - def test_creation_of_leave_ledger_entry_after_leave_period_expiry(self): + def test_unused_leaves_after_leave_encashment_for_carry_forwarding_leave_type(self): + employee = make_employee("test_employee2_encashment@example.com", company="_Test Company") + # allocated 10 leaves, encashed 5 + leave_encashment = self.get_encashment_created_after_leave_period( + employee, is_carry_forward=1, encashment_days=5 + ) + # check if unused leaves are 5 before processing expired allocation runs + unused_leaves = get_unused_leaves( + employee, self.leave_type, self.leave_period.from_date, self.leave_period.to_date + ) + self.assertEqual(unused_leaves, 5) + + # check if a single leave ledger entry is created + self.assertEqual(frappe.get_value("Leave Type", self.leave_type, "is_carry_forward"), 1) + leave_ledger_entry = frappe.get_all( + "Leave Ledger Entry", fields=["leaves"], filters={"transaction_name": leave_encashment.name} + ) + self.assertEqual(len(leave_ledger_entry), 1) + self.assertEqual(leave_ledger_entry[0].leaves, leave_encashment.encashment_days * -1) + + # check if unused leaves are 5 after processing expired allocation runs + process_expired_allocation() + unused_leaves = get_unused_leaves( + employee, self.leave_type, self.leave_period.from_date, self.leave_period.to_date + ) + self.assertEqual(unused_leaves, 5) + + @set_holiday_list("_Test Leave Encashment", "_Test Company") + def test_leave_expiry_after_leave_encashment_for_non_carry_forwarding_leave_type(self): + employee = make_employee("test_employee3_encashment@example.com", company="_Test Company") + # allocated 10 leaves, encashed 3 + + leave_encashment = self.get_encashment_created_after_leave_period( + employee, is_carry_forward=0, encashment_days=3 + ) + # when leave encashment is created after leave allocation period is over, + # it's assumed that process expired allocation has expired the leaves, + # hence a reverse ledger entry should be created for the encashment + # check if two leave ledger entries are created + self.assertEqual(frappe.get_value("Leave Type", self.leave_type, "is_carry_forward"), 0) + leave_ledger_entry = frappe.get_all( + "Leave Ledger Entry", + fields="*", + filters={"transaction_name": leave_encashment.name}, + order_by="leaves", + ) + self.assertEqual(len(leave_ledger_entry), 2) + self.assertEqual(leave_ledger_entry[0].leaves, leave_encashment.encashment_days * -1) + self.assertEqual(leave_ledger_entry[1].leaves, leave_encashment.encashment_days * 1) + + # check if 10 leaves are expired after processing expired allocation runs + process_expired_allocation() + + expired_leaves = frappe.get_value( + "Leave Ledger Entry", + {"employee": employee, "leave_type": self.leave_type, "is_expired": 1}, + "leaves", + ) + self.assertEqual(expired_leaves, -10) + + def get_encashment_created_after_leave_period(self, employee, is_carry_forward, encashment_days): frappe.db.delete("Leave Period", {"name": self.leave_period.name}) # create new leave period that has end date of yesterday start_date = add_days(getdate(), -30) end_date = add_days(getdate(), -1) self.leave_period = create_leave_period(start_date, end_date, "_Test Company") - - # case 1: leave type carry forwards frappe.db.set_value( "Leave Type", self.leave_type, { - "is_carry_forward": 1, + "is_carry_forward": is_carry_forward, }, ) @@ -283,7 +343,6 @@ def test_creation_of_leave_ledger_entry_after_leave_period_expiry(self): "leave_policy": leave_policy, "leave_period": self.leave_period.name, } - employee = make_employee("test_employee2_encashment@example.com", company="_Test Company") create_assignment_for_multiple_employees([employee], frappe._dict(data)) make_salary_structure( @@ -300,50 +359,9 @@ def test_creation_of_leave_ledger_entry_after_leave_period_expiry(self): "leave_type": self.leave_type, "leave_period": self.leave_period.name, "encashment_date": self.leave_period.to_date, + "encashment_days": encashment_days, "currency": "INR", } ).insert() leave_encashment.submit() - # check if single leave ledger entry is created of negative value - self.assertEqual(frappe.get_value("Leave Type", self.leave_type, "is_carry_forward"), 1) - leave_ledger_entry = frappe.get_all( - "Leave Ledger Entry", fields="*", filters={"transaction_name": leave_encashment.name} - ) - self.assertEqual(len(leave_ledger_entry), 1) - self.assertEqual(leave_ledger_entry[0].leaves, leave_encashment.encashment_days * -1) - - # case 2: leave type does not carry forward - # cancel previous encashment - frappe.db.delete("Additional Salary", {"ref_docname": leave_encashment.name}) - leave_encashment.cancel() - # set leave type to not carry forward - frappe.db.set_value( - "Leave Type", - self.leave_type, - { - "is_carry_forward": 0, - }, - ) - # create leave encashment - new_leave_encashment = frappe.get_doc( - { - "doctype": "Leave Encashment", - "employee": employee, - "leave_type": self.leave_type, - "leave_period": self.leave_period.name, - "encashment_date": self.leave_period.to_date, - "currency": "INR", - } - ).insert() - new_leave_encashment.submit() - # check if two leave ledger entries are created - self.assertEqual(frappe.get_value("Leave Type", self.leave_type, "is_carry_forward"), 0) - leave_ledger_entry = frappe.get_all( - "Leave Ledger Entry", - fields="*", - filters={"transaction_name": new_leave_encashment.name}, - order_by="leaves", - ) - self.assertEqual(len(leave_ledger_entry), 2) - self.assertEqual(leave_ledger_entry[0].leaves, new_leave_encashment.encashment_days * -1) - self.assertEqual(leave_ledger_entry[1].leaves, new_leave_encashment.encashment_days * 1) + return leave_encashment From fc1c4bafaaf20894696d5067f94dceaed91f0bc1 Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Fri, 17 Jan 2025 11:12:27 +0530 Subject: [PATCH 5/5] chore: resolved merge conflicts --- hrms/hr/doctype/leave_encashment/leave_encashment.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/hrms/hr/doctype/leave_encashment/leave_encashment.py b/hrms/hr/doctype/leave_encashment/leave_encashment.py index 6db6b1425a..66481f6bea 100644 --- a/hrms/hr/doctype/leave_encashment/leave_encashment.py +++ b/hrms/hr/doctype/leave_encashment/leave_encashment.py @@ -205,15 +205,7 @@ def create_leave_ledger_entry(self, submit=True): to_date = leave_allocation.get("to_date") -<<<<<<< HEAD -<<<<<<< HEAD can_expire = not frappe.db.get_value("Leave Type", self.leave_type, "is_carry_forward") -======= - can_expire = not frappe.db.get_value("Leave Type",self.leave_type,"is_carry_forward") ->>>>>>> acd618f8 (fix: create single leave ledger entry for encashment if leave is carry forward type.) -======= - can_expire = not frappe.db.get_value("Leave Type", self.leave_type, "is_carry_forward") ->>>>>>> de06a25a (chore: fixed linter issue) if to_date < getdate() and can_expire: args = frappe._dict( leaves=self.encashment_days, from_date=to_date, to_date=to_date, is_carry_forward=0