Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove check for already allocated earned leaves #30931

Merged
merged 2 commits into from
May 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions erpnext/hr/doctype/leave_allocation/leave_allocation.js
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -745,15 +745,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
Original file line number Diff line number Diff line change
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 @@ -188,19 +192,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 @@ -242,20 +233,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 @@ -288,19 +265,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 @@ -330,20 +294,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 @@ -377,21 +327,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
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,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 @@ -305,14 +305,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 @@ -326,20 +322,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