Skip to content

Commit

Permalink
fix: remove check for already allocated earned leaves (#30931) (#30932)
Browse files Browse the repository at this point in the history 
* 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 f92bc4d)

Co-authored-by: Rucha Mahabal <ruchamahabal2@gmail.com>
  • Loading branch information
@mergify @ruchamahabal
mergify[bot] and ruchamahabal committed May 9, 2022
1 parent 9189653 commit 80f7d66
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 99 deletions.
9 changes: 0 additions & 9 deletions erpnext/hr/doctype/leave_allocation/leave_allocation.js
View file
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions erpnext/hr/doctype/leave_application/test_leave_application.py
View file
Expand Up @@ -744,15 +744,15 @@ 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)

# validate earned leaves creation without maximum leaves
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)

Expand Down
74 changes: 5 additions & 69 deletions erpnext/hr/doctype/leave_policy_assignment/test_leave_policy_assignment.py
View file
Expand Up @@ -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 (
Expand All @@ -18,7 +19,7 @@
test_dependencies = ["Employee"]


class TestLeavePolicyAssignment(unittest.TestCase):
class TestLeavePolicyAssignment(FrappeTestCase):
def setUp(self):
for doctype in [
"Leave Period",
Expand All @@ -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,
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down
33 changes: 14 additions & 19 deletions erpnext/hr/utils.py
View file
Expand Up @@ -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()
Expand Down Expand Up @@ -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
)
Expand All @@ -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):
Expand Down

0 comments on commit 80f7d66

Please sign in to comment.