diff --git a/hrms/hr/doctype/compensatory_leave_request/compensatory_leave_request.py b/hrms/hr/doctype/compensatory_leave_request/compensatory_leave_request.py index 7302e2ddd1..791e5295f0 100644 --- a/hrms/hr/doctype/compensatory_leave_request/compensatory_leave_request.py +++ b/hrms/hr/doctype/compensatory_leave_request/compensatory_leave_request.py @@ -1,6 +1,7 @@ # Copyright (c) 2018, Frappe Technologies Pvt. Ltd. and contributors # For license information, please see license.txt +import datetime import frappe from frappe import _ @@ -78,7 +79,7 @@ def on_submit(self): comp_leave_valid_from = add_days(self.work_end_date, 1) leave_period = get_leave_period(comp_leave_valid_from, comp_leave_valid_from, company) if leave_period: - leave_allocation = self.get_existing_allocation_for_period(leave_period) + leave_allocation = self.get_existing_allocation(comp_leave_valid_from) if leave_allocation: leave_allocation.new_leaves_allocated += date_difference leave_allocation.validate() @@ -122,30 +123,21 @@ def on_cancel(self): leave_allocation, date_difference * -1, add_days(self.work_end_date, 1) ) - def get_existing_allocation_for_period(self, leave_period): - leave_allocation = frappe.db.sql( - """ - select name - from `tabLeave Allocation` - where employee=%(employee)s and leave_type=%(leave_type)s - and docstatus=1 - and (from_date between %(from_date)s and %(to_date)s - or to_date between %(from_date)s and %(to_date)s - or (from_date < %(from_date)s and to_date > %(to_date)s)) - """, - { - "from_date": leave_period[0].from_date, - "to_date": leave_period[0].to_date, + def get_existing_allocation(self, comp_leave_valid_from: datetime.date) -> dict | None: + leave_allocation = frappe.db.get_all( + "Leave Allocation", + filters={ "employee": self.employee, "leave_type": self.leave_type, + "from_date": ("<=", comp_leave_valid_from), + "to_date": (">=", comp_leave_valid_from), + "docstatus": 1, }, - as_dict=1, + limit=1, ) if leave_allocation: return frappe.get_doc("Leave Allocation", leave_allocation[0].name) - else: - return False def create_leave_allocation(self, leave_period, date_difference): is_carry_forward = frappe.db.get_value("Leave Type", self.leave_type, "is_carry_forward") diff --git a/hrms/hr/doctype/compensatory_leave_request/test_compensatory_leave_request.py b/hrms/hr/doctype/compensatory_leave_request/test_compensatory_leave_request.py index e8e52389ce..40b568ed34 100644 --- a/hrms/hr/doctype/compensatory_leave_request/test_compensatory_leave_request.py +++ b/hrms/hr/doctype/compensatory_leave_request/test_compensatory_leave_request.py @@ -2,18 +2,17 @@ # See license.txt import frappe -from frappe.tests import IntegrationTestCase -from frappe.utils import add_days, add_months, today +from frappe.tests.utils import FrappeTestCase +from frappe.utils import add_days, add_months, getdate, today from hrms.hr.doctype.attendance_request.test_attendance_request import get_employee +from hrms.hr.doctype.leave_allocation.test_leave_allocation import create_leave_allocation from hrms.hr.doctype.leave_application.leave_application import get_leave_balance_on from hrms.hr.doctype.leave_period.test_leave_period import create_leave_period from hrms.tests.test_utils import add_date_to_holiday_list -test_dependencies = ["Employee"] - -class TestCompensatoryLeaveRequest(IntegrationTestCase): +class TestCompensatoryLeaveRequest(FrappeTestCase): def setUp(self): frappe.db.delete("Compensatory Leave Request") frappe.db.delete("Leave Ledger Entry") @@ -42,7 +41,7 @@ def test_leave_balance_on_submit(self): before + 1, ) - def test_leave_allocation_update_on_submit(self): + def test_allocation_update_on_submit(self): employee = get_employee() mark_attendance(employee, date=add_days(today(), -1)) compensatory_leave_request = get_compensatory_leave_request( @@ -70,6 +69,54 @@ def test_leave_allocation_update_on_submit(self): ) self.assertEqual(leaves_allocated, 2) + def test_allocation_update_on_submit_on_multiple_allocations(self): + """Tests whether the correct allocation is updated when there are multiple allocations in the same leave period""" + employee = get_employee() + today = getdate() + + first_alloc_start = add_months(today, -3) + first_alloc_end = add_days(today, -1) + second_alloc_start = today + second_alloc_end = add_months(today, 1) + + add_date_to_holiday_list(first_alloc_start, employee.holiday_list) + allocation_1 = create_leave_allocation( + leave_type="Compensatory Off", + employee=employee.name, + from_date=first_alloc_start, + to_date=first_alloc_end, + ) + allocation_1.new_leaves_allocated = 0 + allocation_1.submit() + + add_date_to_holiday_list(second_alloc_start, employee.holiday_list) + allocation_2 = create_leave_allocation( + leave_type="Compensatory Off", + employee=employee.name, + from_date=second_alloc_start, + to_date=second_alloc_end, + ) + allocation_2.new_leaves_allocated = 0 + allocation_2.submit() + + # adds leave balance in first allocation + mark_attendance(employee, date=first_alloc_start) + compensatory_leave_request = get_compensatory_leave_request( + employee.name, leave_date=first_alloc_start + ) + compensatory_leave_request.submit() + allocation_1.reload() + self.assertEqual(allocation_1.total_leaves_allocated, 1) + + # adds leave balance in second allocation + mark_attendance(employee, date=second_alloc_start) + compensatory_leave_request = get_compensatory_leave_request( + employee.name, leave_date=second_alloc_start + ) + compensatory_leave_request.submit() + allocation_2.reload() + self.assertEqual(allocation_2.total_leaves_allocated, 1) + def test_creation_of_leave_ledger_entry_on_submit(self): """check creation of leave ledger entry on submission of leave request""" employee = get_employee() diff --git a/hrms/hr/doctype/leave_ledger_entry/leave_ledger_entry.py b/hrms/hr/doctype/leave_ledger_entry/leave_ledger_entry.py index dd1718aef3..74d467730f 100644 --- a/hrms/hr/doctype/leave_ledger_entry/leave_ledger_entry.py +++ b/hrms/hr/doctype/leave_ledger_entry/leave_ledger_entry.py @@ -4,13 +4,26 @@ import frappe from frappe import _ from frappe.model.document import Document -from frappe.utils import DATE_FORMAT, flt, get_link_to_form, getdate, today +from frappe.utils import DATE_FORMAT, flt, formatdate, get_link_to_form, getdate, today + + +class InvalidLeaveLedgerEntry(frappe.ValidationError): + pass class LeaveLedgerEntry(Document): def validate(self): if getdate(self.from_date) > getdate(self.to_date): - frappe.throw(_("To date needs to be before from date")) + frappe.throw( + _( + "Leave Ledger Entry's To date needs to be after From date. Currently, From Date is {0} and To Date is {1}" + ).format( + frappe.bold(formatdate(self.from_date)), + frappe.bold(formatdate(self.to_date)), + ), + exc=InvalidLeaveLedgerEntry, + title=_("Invalid Leave Ledger Entry"), + ) def on_cancel(self): # allow cancellation of expiry leaves