From 0f3f9fa0d4c873ff493c3c505175bd904023b3b5 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 9 May 2022 12:15:02 +0530 Subject: [PATCH] fix: remove check for already allocated earned leaves (#30931) * fix: remove check for already allocated earned leaves * fix: do not set New Leaves Allocated field as read-only for earned leaves - removing this until there's a better way to update existing allocations (cherry picked from commit f92bc4dd3350ea1b7d465c9791e7051dced0df27) --- .../leave_allocation/leave_allocation.js | 9 --- .../test_leave_application.py | 4 +- .../test_leave_policy_assignment.py | 74 ++----------------- erpnext/hr/utils.py | 33 ++++----- 4 files changed, 21 insertions(+), 99 deletions(-) diff --git a/erpnext/hr/doctype/leave_allocation/leave_allocation.js b/erpnext/hr/doctype/leave_allocation/leave_allocation.js index aef44122513a..9742387c16a6 100755 --- a/erpnext/hr/doctype/leave_allocation/leave_allocation.js +++ b/erpnext/hr/doctype/leave_allocation/leave_allocation.js @@ -34,15 +34,6 @@ frappe.ui.form.on("Leave Allocation", { }); } } - - // make new leaves allocated field read only if allocation is created via leave policy assignment - // and leave type is earned leave, since these leaves would be allocated via the scheduler - if (frm.doc.leave_policy_assignment) { - frappe.db.get_value("Leave Type", frm.doc.leave_type, "is_earned_leave", (r) => { - if (r && cint(r.is_earned_leave)) - frm.set_df_property("new_leaves_allocated", "read_only", 1); - }); - } }, expire_allocation: function(frm) { diff --git a/erpnext/hr/doctype/leave_application/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py index 8924a57708e4..60c0491a5099 100644 --- a/erpnext/hr/doctype/leave_application/test_leave_application.py +++ b/erpnext/hr/doctype/leave_application/test_leave_application.py @@ -744,7 +744,7 @@ def test_earned_leaves_creation(self): i = 0 while i < 14: - allocate_earned_leaves(ignore_duplicates=True) + allocate_earned_leaves() i += 1 self.assertEqual(get_leave_balance_on(employee.name, leave_type, nowdate()), 6) @@ -752,7 +752,7 @@ def test_earned_leaves_creation(self): frappe.db.set_value("Leave Type", leave_type, "max_leaves_allowed", 0) i = 0 while i < 6: - allocate_earned_leaves(ignore_duplicates=True) + allocate_earned_leaves() i += 1 self.assertEqual(get_leave_balance_on(employee.name, leave_type, nowdate()), 9) 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 20249b38ef72..ee61b2bfc918 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,6 +4,7 @@ import unittest import frappe +from frappe.tests.utils import FrappeTestCase from frappe.utils import add_days, add_months, get_first_day, get_last_day, getdate from erpnext.hr.doctype.leave_application.test_leave_application import ( @@ -18,7 +19,7 @@ test_dependencies = ["Employee"] -class TestLeavePolicyAssignment(unittest.TestCase): +class TestLeavePolicyAssignment(FrappeTestCase): def setUp(self): for doctype in [ "Leave Period", @@ -39,6 +40,9 @@ def test_grant_leaves(self): leave_policy = create_leave_policy() leave_policy.submit() + self.employee.date_of_joining = get_first_day(leave_period.from_date) + self.employee.save() + data = { "assignment_based_on": "Leave Period", "leave_policy": leave_policy.name, @@ -187,19 +191,6 @@ def test_earned_leave_alloc_for_passed_months_on_month_end_based_on_leave_period ) 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 test_earned_leave_alloc_for_passed_months_with_cf_leaves_based_on_leave_period(self): from erpnext.hr.doctype.leave_allocation.test_leave_allocation import create_leave_allocation @@ -241,20 +232,6 @@ def test_earned_leave_alloc_for_passed_months_with_cf_leaves_based_on_leave_peri 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 test_earned_leave_alloc_for_passed_months_based_on_joining_date(self): # tests leave alloc for earned leaves for assignment based on joining date in policy assignment leave_type = create_earned_leave_type("Test Earned Leave") @@ -287,19 +264,6 @@ def test_earned_leave_alloc_for_passed_months_based_on_joining_date(self): self.assertEqual(effective_from, self.employee.date_of_joining) self.assertEqual(leaves_allocated, 3) - # to ensure leave is not already allocated to avoid duplication - from erpnext.hr.utils import allocate_earned_leaves - - frappe.flags.current_date = get_last_day(getdate()) - 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 test_grant_leaves_on_doj_for_earned_leaves_based_on_leave_period(self): # tests leave alloc based on leave period for earned leaves with "based on doj" configuration in leave type leave_period, leave_policy = setup_leave_period_and_policy( @@ -329,20 +293,6 @@ def test_grant_leaves_on_doj_for_earned_leaves_based_on_leave_period(self): ) 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 - - frappe.flags.current_date = get_first_day(getdate()) - 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 test_grant_leaves_on_doj_for_earned_leaves_based_on_joining_date(self): # tests leave alloc based on joining date for earned leaves with "based on doj" configuration in leave type leave_type = create_earned_leave_type("Test Earned Leave", based_on_doj=True) @@ -376,21 +326,7 @@ def test_grant_leaves_on_doj_for_earned_leaves_based_on_joining_date(self): self.assertEqual(effective_from, self.employee.date_of_joining) self.assertEqual(leaves_allocated, 3) - # to ensure leave is not already allocated to avoid duplication - from erpnext.hr.utils import allocate_earned_leaves - - frappe.flags.current_date = get_first_day(getdate()) - 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() frappe.db.set_value("Employee", self.employee.name, "date_of_joining", self.original_doj) frappe.flags.current_date = None diff --git a/erpnext/hr/utils.py b/erpnext/hr/utils.py index f1c1608cdd05..d779a5465f5a 100644 --- a/erpnext/hr/utils.py +++ b/erpnext/hr/utils.py @@ -428,7 +428,7 @@ def generate_leave_encashment(): create_leave_encashment(leave_allocation=leave_allocation) -def allocate_earned_leaves(ignore_duplicates=False): +def allocate_earned_leaves(): """Allocate earned leaves to Employees""" e_leave_types = get_earned_leaves() today = getdate() @@ -464,14 +464,10 @@ def allocate_earned_leaves(ignore_duplicates=False): if check_effective_date( from_date, today, e_leave_type.earned_leave_frequency, e_leave_type.based_on_date_of_joining ): - update_previous_leave_allocation( - allocation, annual_allocation, e_leave_type, ignore_duplicates - ) + update_previous_leave_allocation(allocation, annual_allocation, e_leave_type) -def update_previous_leave_allocation( - allocation, annual_allocation, e_leave_type, ignore_duplicates=False -): +def update_previous_leave_allocation(allocation, annual_allocation, e_leave_type): earned_leaves = get_monthly_earned_leave( annual_allocation, e_leave_type.earned_leave_frequency, e_leave_type.rounding ) @@ -485,20 +481,19 @@ def update_previous_leave_allocation( if new_allocation != allocation.total_leaves_allocated: today_date = today() - 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) + allocation.db_set("total_leaves_allocated", new_allocation, update_modified=False) + create_additional_leave_ledger_entry(allocation, earned_leaves, today_date) - if e_leave_type.based_on_date_of_joining: - text = _("allocated {0} leave(s) via scheduler on {1} based on the date of joining").format( - frappe.bold(earned_leaves), frappe.bold(formatdate(today_date)) - ) - else: - text = _("allocated {0} leave(s) via scheduler on {1}").format( - frappe.bold(earned_leaves), frappe.bold(formatdate(today_date)) - ) + if e_leave_type.based_on_date_of_joining: + text = _("allocated {0} leave(s) via scheduler on {1} based on the date of joining").format( + frappe.bold(earned_leaves), frappe.bold(formatdate(today_date)) + ) + else: + text = _("allocated {0} leave(s) via scheduler on {1}").format( + frappe.bold(earned_leaves), frappe.bold(formatdate(today_date)) + ) - allocation.add_comment(comment_type="Info", text=text) + allocation.add_comment(comment_type="Info", text=text) def get_monthly_earned_leave(annual_leaves, frequency, rounding):