Merge pull request #2615 from ruchamahabal/fix-comp-leave-allocation

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")

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()

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