From 5363b1922ac2812ed2920f106ce1ed9c13cd2768 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Sat, 5 Feb 2022 16:05:46 +0530 Subject: [PATCH 1/7] fix: earned leaves not allocated if assignment is created on month-end (cherry picked from commit 25c7f850b14f1f423631225725ad7d2e9647049f) # Conflicts: # erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.py --- .../leave_policy_assignment.py | 23 +++++++++++++++++-- erpnext/hr/utils.py | 22 ++++++++++++++++-- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.py b/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.py index c79216a275d..0ae368f9df4 100644 --- a/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.py +++ b/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.py @@ -8,8 +8,12 @@ from math import ceil import frappe from frappe import _, bold from frappe.model.document import Document +<<<<<<< HEAD from frappe.utils import date_diff, flt, formatdate, get_datetime, getdate from six import string_types +======= +from frappe.utils import date_diff, flt, formatdate, get_datetime, get_last_day, getdate +>>>>>>> 25c7f850b1 (fix: earned leaves not allocated if assignment is created on month-end) class LeavePolicyAssignment(Document): @@ -109,8 +113,8 @@ class LeavePolicyAssignment(Document): def get_leaves_for_passed_months(self, leave_type, new_leaves_allocated, leave_type_details, date_of_joining): from erpnext.hr.utils import get_monthly_earned_leave - current_month = get_datetime().month - current_year = get_datetime().year + current_month = get_datetime(frappe.flags.current_date).month or get_datetime().month + current_year = get_datetime(frappe.flags.current_date).year or get_datetime().year from_date = frappe.db.get_value("Leave Period", self.leave_period, "from_date") if getdate(date_of_joining) > getdate(from_date): @@ -120,10 +124,14 @@ class LeavePolicyAssignment(Document): from_date_year = get_datetime(from_date).year months_passed = 0 + if current_year == from_date_year and current_month > from_date_month: months_passed = current_month - from_date_month + months_passed = add_current_month_if_applicable(months_passed) + elif current_year > from_date_year: months_passed = (12 - from_date_month) + current_month + months_passed = add_current_month_if_applicable(months_passed) if months_passed > 0: monthly_earned_leave = get_monthly_earned_leave(new_leaves_allocated, @@ -135,6 +143,17 @@ class LeavePolicyAssignment(Document): return new_leaves_allocated +def add_current_month_if_applicable(months_passed): + date = getdate(frappe.flags.current_date) or getdate() + last_day_of_month = get_last_day(date) + + # if its the last day of the month, then that month should also be considered + if last_day_of_month == date: + months_passed += 1 + + return months_passed + + @frappe.whitelist() def create_assignment_for_multiple_employees(employees, data): diff --git a/erpnext/hr/utils.py b/erpnext/hr/utils.py index 0b2f99c358e..6cf7df371e7 100644 --- a/erpnext/hr/utils.py +++ b/erpnext/hr/utils.py @@ -393,9 +393,12 @@ def update_previous_leave_allocation(allocation, annual_allocation, e_leave_type new_allocation = e_leave_type.max_leaves_allowed if new_allocation != allocation.total_leaves_allocated: - allocation.db_set("total_leaves_allocated", new_allocation, update_modified=False) today_date = today() - create_additional_leave_ledger_entry(allocation, earned_leaves, today_date) + + if not is_earned_leave_already_allocated(allocation, annual_allocation): + allocation.db_set("total_leaves_allocated", new_allocation, update_modified=False) + create_additional_leave_ledger_entry(allocation, earned_leaves, today_date) + def get_monthly_earned_leave(annual_leaves, frequency, rounding): earned_leaves = 0.0 @@ -413,6 +416,21 @@ def get_monthly_earned_leave(annual_leaves, frequency, rounding): return earned_leaves +def is_earned_leave_already_allocated(allocation, annual_allocation): + from erpnext.hr.doctype.leave_policy_assignment.leave_policy_assignment import get_leave_type_details + + leave_type_details = get_leave_type_details() + date_of_joining = frappe.db.get_value("Employee", allocation.employee, "date_of_joining") + + assignment = frappe.get_doc("Leave Policy Assignment", allocation.leave_policy_assignment) + leaves_for_passed_months = assignment.get_leaves_for_passed_months(allocation.leave_type, + annual_allocation, leave_type_details, date_of_joining) + + if allocation.total_leaves_allocated >= leaves_for_passed_months: + return True + return False + + def get_leave_allocations(date, leave_type): return frappe.db.sql("""select name, employee, from_date, to_date, leave_policy_assignment, leave_policy from `tabLeave Allocation` From d26a8d47e4a4ad6faa47bcf67700dba28399efae Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Sat, 5 Feb 2022 16:05:54 +0530 Subject: [PATCH 2/7] test: earned leave allocation for passed months and allocation on month-end (cherry picked from commit 63ee4f1b64b0110d6d97f4114605db2732dcb224) --- .../test_leave_policy_assignment.py | 73 +++++++++++++++++-- 1 file changed, 68 insertions(+), 5 deletions(-) diff --git a/erpnext/hr/doctype/leave_policy_assignment/test_leave_policy_assignment.py b/erpnext/hr/doctype/leave_policy_assignment/test_leave_policy_assignment.py index 8953a51e8bb..6dd589182fc 100644 --- a/erpnext/hr/doctype/leave_policy_assignment/test_leave_policy_assignment.py +++ b/erpnext/hr/doctype/leave_policy_assignment/test_leave_policy_assignment.py @@ -4,7 +4,7 @@ import unittest import frappe -from frappe.utils import add_months, get_first_day, getdate +from frappe.utils import add_months, get_first_day, get_last_day, getdate from erpnext.hr.doctype.leave_application.test_leave_application import ( get_employee, @@ -124,6 +124,69 @@ class TestLeavePolicyAssignment(unittest.TestCase): }, "total_leaves_allocated") self.assertEqual(leaves_allocated, 0) + def test_earned_leave_allocation_for_passed_months(self): + employee = get_employee() + leave_type = create_earned_leave_type("Test Earned Leave") + leave_period = create_leave_period("Test Earned Leave Period", + start_date=get_first_day(add_months(getdate(), -1))) + leave_policy = frappe.get_doc({ + "doctype": "Leave Policy", + "title": "Test Leave Policy", + "leave_policy_details": [{"leave_type": leave_type.name, "annual_allocation": 12}] + }).insert() + + # Case 1: assignment created one month after the leave period, should allocate 1 leave + frappe.flags.current_date = get_first_day(getdate()) + data = { + "assignment_based_on": "Leave Period", + "leave_policy": leave_policy.name, + "leave_period": leave_period.name + } + leave_policy_assignments = create_assignment_for_multiple_employees([employee.name], frappe._dict(data)) + + leaves_allocated = frappe.db.get_value("Leave Allocation", { + "leave_policy_assignment": leave_policy_assignments[0] + }, "total_leaves_allocated") + self.assertEqual(leaves_allocated, 1) + + def test_earned_leave_allocation_for_passed_months_on_month_end(self): + employee = get_employee() + leave_type = create_earned_leave_type("Test Earned Leave") + leave_period = create_leave_period("Test Earned Leave Period", + start_date=get_first_day(add_months(getdate(), -2))) + leave_policy = frappe.get_doc({ + "doctype": "Leave Policy", + "title": "Test Leave Policy", + "leave_policy_details": [{"leave_type": leave_type.name, "annual_allocation": 12}] + }).insert() + + # Case 2: assignment created on the last day of the leave period's latter month + # should allocate 1 leave for current month even though the month has not ended + # since the daily job might have already executed + frappe.flags.current_date = get_last_day(getdate()) + + data = { + "assignment_based_on": "Leave Period", + "leave_policy": leave_policy.name, + "leave_period": leave_period.name + } + leave_policy_assignments = create_assignment_for_multiple_employees([employee.name], frappe._dict(data)) + + leaves_allocated = frappe.db.get_value("Leave Allocation", { + "leave_policy_assignment": leave_policy_assignments[0] + }, "total_leaves_allocated") + self.assertEqual(leaves_allocated, 3) + + # if the daily job is not completed yet, there is another check present + # to ensure leave is not already allocated to avoid duplication + from erpnext.hr.utils import allocate_earned_leaves + allocate_earned_leaves() + + leaves_allocated = frappe.db.get_value("Leave Allocation", { + "leave_policy_assignment": leave_policy_assignments[0] + }, "total_leaves_allocated") + self.assertEqual(leaves_allocated, 3) + def tearDown(self): frappe.db.rollback() @@ -136,14 +199,14 @@ def create_earned_leave_type(leave_type): doctype="Leave Type", is_earned_leave=1, earned_leave_frequency="Monthly", - rounding=0.5, - max_leaves_allowed=6 + rounding=0.5 )).insert() -def create_leave_period(name): +def create_leave_period(name, start_date=None): frappe.delete_doc_if_exists("Leave Period", name, force=1) - start_date = get_first_day(getdate()) + if not start_date: + start_date = get_first_day(getdate()) return frappe.get_doc(dict( name=name, From ce26759a95b21d776e3cf24efd92d3b99ca776ff Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Sat, 5 Feb 2022 16:40:55 +0530 Subject: [PATCH 3/7] fix: linter (cherry picked from commit a52ba0a5447df1998b7900230ce1cdb4a0a3dace) --- erpnext/hr/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/erpnext/hr/utils.py b/erpnext/hr/utils.py index 6cf7df371e7..4ddfaafd104 100644 --- a/erpnext/hr/utils.py +++ b/erpnext/hr/utils.py @@ -417,7 +417,9 @@ def get_monthly_earned_leave(annual_leaves, frequency, rounding): def is_earned_leave_already_allocated(allocation, annual_allocation): - from erpnext.hr.doctype.leave_policy_assignment.leave_policy_assignment import get_leave_type_details + from erpnext.hr.doctype.leave_policy_assignment.leave_policy_assignment import ( + get_leave_type_details, + ) leave_type_details = get_leave_type_details() date_of_joining = frappe.db.get_value("Employee", allocation.employee, "date_of_joining") From aa04e027107a2a5652a6b2b61fc54ca137b767f5 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Sun, 6 Feb 2022 20:30:46 +0530 Subject: [PATCH 4/7] fix(test): add ignore duplicates flag to allocation function (cherry picked from commit e25544f94e20f675befd71e58df8156649cbf1f0) --- .../doctype/leave_application/test_leave_application.py | 4 ++-- erpnext/hr/utils.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py index 93f4176bd5f..39356bdcf18 100644 --- a/erpnext/hr/doctype/leave_application/test_leave_application.py +++ b/erpnext/hr/doctype/leave_application/test_leave_application.py @@ -545,7 +545,7 @@ class TestLeaveApplication(unittest.TestCase): from erpnext.hr.utils import allocate_earned_leaves i = 0 while(i<14): - allocate_earned_leaves() + allocate_earned_leaves(ignore_duplicates=True) i += 1 self.assertEqual(get_leave_balance_on(employee.name, leave_type, nowdate()), 6) @@ -553,7 +553,7 @@ class TestLeaveApplication(unittest.TestCase): frappe.db.set_value('Leave Type', leave_type, 'max_leaves_allowed', 0) i = 0 while(i<6): - allocate_earned_leaves() + allocate_earned_leaves(ignore_duplicates=True) i += 1 self.assertEqual(get_leave_balance_on(employee.name, leave_type, nowdate()), 9) diff --git a/erpnext/hr/utils.py b/erpnext/hr/utils.py index 4ddfaafd104..622ee3f9389 100644 --- a/erpnext/hr/utils.py +++ b/erpnext/hr/utils.py @@ -353,7 +353,7 @@ def generate_leave_encashment(): create_leave_encashment(leave_allocation=leave_allocation) -def allocate_earned_leaves(): +def allocate_earned_leaves(ignore_duplicates=False): '''Allocate earned leaves to Employees''' e_leave_types = get_earned_leaves() today = getdate() @@ -381,9 +381,9 @@ def allocate_earned_leaves(): from_date = frappe.db.get_value("Employee", allocation.employee, "date_of_joining") if check_effective_date(from_date, today, e_leave_type.earned_leave_frequency, e_leave_type.based_on_date_of_joining_date): - update_previous_leave_allocation(allocation, annual_allocation, e_leave_type) + update_previous_leave_allocation(allocation, annual_allocation, e_leave_type, ignore_duplicates) -def update_previous_leave_allocation(allocation, annual_allocation, e_leave_type): +def update_previous_leave_allocation(allocation, annual_allocation, e_leave_type, ignore_duplicates=False): earned_leaves = get_monthly_earned_leave(annual_allocation, e_leave_type.earned_leave_frequency, e_leave_type.rounding) allocation = frappe.get_doc('Leave Allocation', allocation.name) @@ -395,7 +395,7 @@ def update_previous_leave_allocation(allocation, annual_allocation, e_leave_type if new_allocation != allocation.total_leaves_allocated: today_date = today() - if not is_earned_leave_already_allocated(allocation, annual_allocation): + if ignore_duplicates or not is_earned_leave_already_allocated(allocation, annual_allocation): allocation.db_set("total_leaves_allocated", new_allocation, update_modified=False) create_additional_leave_ledger_entry(allocation, earned_leaves, today_date) From a5f016a7cd7ac952fde70f7127d52a61703bb22f Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 8 Feb 2022 14:36:31 +0530 Subject: [PATCH 5/7] fix: handle carry forwarded leaves while checking for duplicate allocation (cherry picked from commit bd1555bd230c0932bc0b7476f1ca68092a697e51) --- .../test_leave_policy_assignment.py | 55 ++++++++++++++++++- erpnext/hr/utils.py | 7 ++- 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/erpnext/hr/doctype/leave_policy_assignment/test_leave_policy_assignment.py b/erpnext/hr/doctype/leave_policy_assignment/test_leave_policy_assignment.py index 6dd589182fc..26821f58e2d 100644 --- a/erpnext/hr/doctype/leave_policy_assignment/test_leave_policy_assignment.py +++ b/erpnext/hr/doctype/leave_policy_assignment/test_leave_policy_assignment.py @@ -187,6 +187,58 @@ class TestLeavePolicyAssignment(unittest.TestCase): }, "total_leaves_allocated") self.assertEqual(leaves_allocated, 3) + def test_earned_leave_allocation_for_passed_months_with_carry_forwarded_leaves(self): + from erpnext.hr.doctype.leave_allocation.test_leave_allocation import create_leave_allocation + + employee = get_employee() + leave_type = create_earned_leave_type("Test Earned Leave") + leave_period = create_leave_period("Test Earned Leave Period", + start_date=get_first_day(add_months(getdate(), -2))) + leave_policy = frappe.get_doc({ + "doctype": "Leave Policy", + "title": "Test Leave Policy", + "leave_policy_details": [{"leave_type": leave_type.name, "annual_allocation": 12}] + }).insert() + + # initial leave allocation = 5 + leave_allocation = create_leave_allocation( + employee=employee.name, + employee_name=employee.employee_name, + leave_type=leave_type.name, + from_date=add_months(getdate(), -12), + to_date=add_months(getdate(), -3), + new_leaves_allocated=5, + carry_forward=0) + leave_allocation.submit() + + # Case 3: assignment created on the last day of the leave period's latter month with carry forwarding + frappe.flags.current_date = get_last_day(add_months(getdate(), -1)) + + data = { + "assignment_based_on": "Leave Period", + "leave_policy": leave_policy.name, + "leave_period": leave_period.name, + "carry_forward": 1 + } + # carry forwarded leaves = 5, 3 leaves allocated for passed months + leave_policy_assignments = create_assignment_for_multiple_employees([employee.name], frappe._dict(data)) + + details = frappe.db.get_value("Leave Allocation", { + "leave_policy_assignment": leave_policy_assignments[0] + }, ["total_leaves_allocated", "new_leaves_allocated", "unused_leaves", "name"], as_dict=True) + self.assertEqual(details.new_leaves_allocated, 2) + self.assertEqual(details.unused_leaves, 5) + self.assertEqual(details.total_leaves_allocated, 7) + + # if the daily job is not completed yet, there is another check present + # to ensure leave is not already allocated to avoid duplication + from erpnext.hr.utils import is_earned_leave_already_allocated + frappe.flags.current_date = get_last_day(getdate()) + + allocation = frappe.get_doc('Leave Allocation', details.name) + # 1 leave is still pending to be allocated, irrespective of carry forwarded leaves + self.assertFalse(is_earned_leave_already_allocated(allocation, leave_policy.leave_policy_details[0].annual_allocation)) + def tearDown(self): frappe.db.rollback() @@ -199,7 +251,8 @@ def create_earned_leave_type(leave_type): doctype="Leave Type", is_earned_leave=1, earned_leave_frequency="Monthly", - rounding=0.5 + rounding=0.5, + is_carry_forward=1 )).insert() diff --git a/erpnext/hr/utils.py b/erpnext/hr/utils.py index 622ee3f9389..6032d0c1bd3 100644 --- a/erpnext/hr/utils.py +++ b/erpnext/hr/utils.py @@ -428,7 +428,12 @@ def is_earned_leave_already_allocated(allocation, annual_allocation): leaves_for_passed_months = assignment.get_leaves_for_passed_months(allocation.leave_type, annual_allocation, leave_type_details, date_of_joining) - if allocation.total_leaves_allocated >= leaves_for_passed_months: + # exclude carry-forwarded leaves while checking for leave allocation for passed months + num_allocations = allocation.total_leaves_allocated + if allocation.unused_leaves: + num_allocations -= allocation.unused_leaves + + if num_allocations >= leaves_for_passed_months: return True return False From cc867d6338e42c76c976e0632d926ec7eda2b024 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 8 Feb 2022 15:55:44 +0530 Subject: [PATCH 6/7] fix: conflicts --- .../leave_policy_assignment/leave_policy_assignment.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.py b/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.py index 0ae368f9df4..fe96f3db4f2 100644 --- a/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.py +++ b/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.py @@ -8,12 +8,7 @@ from math import ceil import frappe from frappe import _, bold from frappe.model.document import Document -<<<<<<< HEAD -from frappe.utils import date_diff, flt, formatdate, get_datetime, getdate -from six import string_types -======= from frappe.utils import date_diff, flt, formatdate, get_datetime, get_last_day, getdate ->>>>>>> 25c7f850b1 (fix: earned leaves not allocated if assignment is created on month-end) class LeavePolicyAssignment(Document): From e3246efa5db6c9de199083f6973c1aabf64d1f9f Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 8 Feb 2022 16:12:24 +0530 Subject: [PATCH 7/7] fix: conflicts --- .../doctype/leave_policy_assignment/leave_policy_assignment.py | 1 + 1 file changed, 1 insertion(+) diff --git a/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.py b/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.py index fe96f3db4f2..2115b05e92f 100644 --- a/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.py +++ b/erpnext/hr/doctype/leave_policy_assignment/leave_policy_assignment.py @@ -9,6 +9,7 @@ import frappe from frappe import _, bold from frappe.model.document import Document from frappe.utils import date_diff, flt, formatdate, get_datetime, get_last_day, getdate +from six import string_types class LeavePolicyAssignment(Document):