diff --git a/erpnext/hr/doctype/leave_allocation/leave_allocation.py b/erpnext/hr/doctype/leave_allocation/leave_allocation.py index 8fae2a9a888..09484d33755 100755 --- a/erpnext/hr/doctype/leave_allocation/leave_allocation.py +++ b/erpnext/hr/doctype/leave_allocation/leave_allocation.py @@ -90,6 +90,7 @@ class LeaveAllocation(Document): if self.carry_forward: self.set_carry_forwarded_leaves_in_previous_allocation(on_cancel=True) + # nosemgrep: frappe-semgrep-rules.rules.frappe-modifying-but-not-comitting def on_update_after_submit(self): if self.has_value_changed("new_leaves_allocated"): self.validate_against_leave_applications() @@ -99,7 +100,11 @@ class LeaveAllocation(Document): # run required validations again since total leaves are being updated self.validate_leave_days_and_dates() - leaves_to_be_added = self.new_leaves_allocated - self.get_existing_leave_count() + leaves_to_be_added = flt( + (self.new_leaves_allocated - self.get_existing_leave_count()), + self.precision("new_leaves_allocated"), + ) + args = { "leaves": leaves_to_be_added, "from_date": self.from_date, @@ -118,14 +123,13 @@ class LeaveAllocation(Document): "employee": self.employee, "company": self.company, "leave_type": self.leave_type, + "is_carry_forward": 0, + "docstatus": 1, }, - pluck="leaves", + fields=["SUM(leaves) as total_leaves"], ) - total_existing_leaves = 0 - for entry in ledger_entries: - total_existing_leaves += entry - return total_existing_leaves + return ledger_entries[0].total_leaves if ledger_entries else 0 def validate_against_leave_applications(self): leaves_taken = get_approved_leaves_for_period( diff --git a/erpnext/hr/doctype/leave_allocation/test_leave_allocation.py b/erpnext/hr/doctype/leave_allocation/test_leave_allocation.py index 48953003000..07792b5ea54 100644 --- a/erpnext/hr/doctype/leave_allocation/test_leave_allocation.py +++ b/erpnext/hr/doctype/leave_allocation/test_leave_allocation.py @@ -18,6 +18,7 @@ class TestLeaveAllocation(FrappeTestCase): def setUp(self): frappe.db.delete("Leave Period") frappe.db.delete("Leave Allocation") + frappe.db.delete("Leave Ledger Entry") emp_id = make_employee("test_emp_leave_allocation@salary.com", company="_Test Company") self.employee = frappe.get_doc("Employee", emp_id) @@ -69,7 +70,6 @@ class TestLeaveAllocation(FrappeTestCase): def test_validation_for_over_allocation(self): leave_type = create_leave_type(leave_type_name="Test Over Allocation", is_carry_forward=1) - leave_type.save() doc = frappe.get_doc( { @@ -137,9 +137,9 @@ class TestLeaveAllocation(FrappeTestCase): ) ).insert() - leave_type = create_leave_type(leave_type_name="_Test Allocation Validation", is_carry_forward=1) - leave_type.max_leaves_allowed = 25 - leave_type.save() + leave_type = create_leave_type( + leave_type_name="_Test Allocation Validation", is_carry_forward=1, max_leaves_allowed=25 + ) # 15 leaves allocated in this period allocation = create_leave_allocation( @@ -174,9 +174,9 @@ class TestLeaveAllocation(FrappeTestCase): ) ).insert() - leave_type = create_leave_type(leave_type_name="_Test Allocation Validation", is_carry_forward=1) - leave_type.max_leaves_allowed = 30 - leave_type.save() + leave_type = create_leave_type( + leave_type_name="_Test Allocation Validation", is_carry_forward=1, max_leaves_allowed=30 + ) # 15 leaves allocated allocation = create_leave_allocation( @@ -207,7 +207,6 @@ class TestLeaveAllocation(FrappeTestCase): def test_validate_back_dated_allocation_update(self): leave_type = create_leave_type(leave_type_name="_Test_CF_leave", is_carry_forward=1) - leave_type.save() # initial leave allocation = 15 leave_allocation = create_leave_allocation( @@ -235,10 +234,12 @@ class TestLeaveAllocation(FrappeTestCase): self.assertRaises(BackDatedAllocationError, leave_allocation.save) def test_carry_forward_calculation(self): - leave_type = create_leave_type(leave_type_name="_Test_CF_leave", is_carry_forward=1) - leave_type.maximum_carry_forwarded_leaves = 10 - leave_type.max_leaves_allowed = 30 - leave_type.save() + leave_type = create_leave_type( + leave_type_name="_Test_CF_leave", + is_carry_forward=1, + maximum_carry_forwarded_leaves=10, + max_leaves_allowed=30, + ) # initial leave allocation = 15 leave_allocation = create_leave_allocation( @@ -286,7 +287,6 @@ class TestLeaveAllocation(FrappeTestCase): is_carry_forward=1, expire_carry_forwarded_leaves_after_days=90, ) - leave_type.save() # initial leave allocation leave_allocation = create_leave_allocation( @@ -352,12 +352,51 @@ class TestLeaveAllocation(FrappeTestCase): ) leave_allocation.submit() leave_allocation.reload() - self.assertTrue(leave_allocation.total_leaves_allocated, 15) + self.assertEqual(leave_allocation.total_leaves_allocated, 15) leave_allocation.new_leaves_allocated = 40 - leave_allocation.submit() + leave_allocation.save() leave_allocation.reload() - self.assertTrue(leave_allocation.total_leaves_allocated, 40) + + updated_entry = frappe.db.get_all( + "Leave Ledger Entry", + {"transaction_name": leave_allocation.name}, + pluck="leaves", + order_by="creation desc", + limit=1, + ) + + self.assertEqual(updated_entry[0], 25) + self.assertEqual(leave_allocation.total_leaves_allocated, 40) + + def test_leave_addition_after_submit_with_carry_forward(self): + from erpnext.hr.doctype.leave_application.test_leave_application import ( + create_carry_forwarded_allocation, + ) + + leave_type = create_leave_type( + leave_type_name="_Test_CF_leave_expiry", + is_carry_forward=1, + include_holiday=True, + ) + + leave_allocation = create_carry_forwarded_allocation(self.employee, leave_type) + # 15 new leaves, 15 carry forwarded leaves + self.assertEqual(leave_allocation.total_leaves_allocated, 30) + + leave_allocation.new_leaves_allocated = 32 + leave_allocation.save() + leave_allocation.reload() + + updated_entry = frappe.db.get_all( + "Leave Ledger Entry", + {"transaction_name": leave_allocation.name}, + pluck="leaves", + order_by="creation desc", + limit=1, + ) + self.assertEqual(updated_entry[0], 17) + self.assertEqual(leave_allocation.total_leaves_allocated, 47) def test_leave_subtraction_after_submit(self): leave_allocation = create_leave_allocation( @@ -365,12 +404,49 @@ class TestLeaveAllocation(FrappeTestCase): ) leave_allocation.submit() leave_allocation.reload() - self.assertTrue(leave_allocation.total_leaves_allocated, 15) + self.assertEqual(leave_allocation.total_leaves_allocated, 15) leave_allocation.new_leaves_allocated = 10 leave_allocation.submit() leave_allocation.reload() - self.assertTrue(leave_allocation.total_leaves_allocated, 10) + + updated_entry = frappe.db.get_all( + "Leave Ledger Entry", + {"transaction_name": leave_allocation.name}, + pluck="leaves", + order_by="creation desc", + limit=1, + ) + + self.assertEqual(updated_entry[0], -5) + self.assertEqual(leave_allocation.total_leaves_allocated, 10) + + def test_leave_subtraction_after_submit_with_carry_forward(self): + from erpnext.hr.doctype.leave_application.test_leave_application import ( + create_carry_forwarded_allocation, + ) + + leave_type = create_leave_type( + leave_type_name="_Test_CF_leave_expiry", + is_carry_forward=1, + include_holiday=True, + ) + + leave_allocation = create_carry_forwarded_allocation(self.employee, leave_type) + self.assertEqual(leave_allocation.total_leaves_allocated, 30) + + leave_allocation.new_leaves_allocated = 8 + leave_allocation.save() + + updated_entry = frappe.db.get_all( + "Leave Ledger Entry", + {"transaction_name": leave_allocation.name}, + pluck="leaves", + order_by="creation desc", + limit=1, + ) + self.assertEqual(updated_entry[0], -7) + self.assertEqual(leave_allocation.total_leaves_allocated, 23) def test_validation_against_leave_application_after_submit(self): from erpnext.payroll.doctype.salary_slip.test_salary_slip import make_holiday_list diff --git a/erpnext/hr/doctype/leave_application/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py index 231b14f6163..e30b84bbf34 100644 --- a/erpnext/hr/doctype/leave_application/test_leave_application.py +++ b/erpnext/hr/doctype/leave_application/test_leave_application.py @@ -702,7 +702,7 @@ class TestLeaveApplication(unittest.TestCase): leave_type_name="_Test_CF_leave_expiry", is_carry_forward=1, expire_carry_forwarded_leaves_after_days=90, - ).insert() + ) create_carry_forwarded_allocation(employee, leave_type) details = get_leave_balance_on( @@ -774,7 +774,6 @@ class TestLeaveApplication(unittest.TestCase): employee = get_employee() leave_type = create_leave_type(leave_type_name="Test Leave Type 1") - leave_type.save() leave_allocation = create_leave_allocation( employee=employee.name, employee_name=employee.employee_name, leave_type=leave_type.name @@ -817,7 +816,6 @@ class TestLeaveApplication(unittest.TestCase): expire_carry_forwarded_leaves_after_days=90, include_holiday=True, ) - leave_type.submit() create_carry_forwarded_allocation(employee, leave_type) @@ -856,7 +854,6 @@ class TestLeaveApplication(unittest.TestCase): is_carry_forward=1, expire_carry_forwarded_leaves_after_days=90, ) - leave_type.submit() create_carry_forwarded_allocation(employee, leave_type) @@ -1005,7 +1002,7 @@ class TestLeaveApplication(unittest.TestCase): leave_type_name="_Test_CF_leave_expiry", is_carry_forward=1, expire_carry_forwarded_leaves_after_days=90, - ).insert() + ) leave_alloc = create_carry_forwarded_allocation(employee, leave_type) cf_expiry = frappe.db.get_value( @@ -1038,7 +1035,7 @@ class TestLeaveApplication(unittest.TestCase): leave_type_name="_Test_CF_leave_expiry", is_carry_forward=1, expire_carry_forwarded_leaves_after_days=90, - ).insert() + ) leave_alloc = create_carry_forwarded_allocation(employee, leave_type) cf_expiry = frappe.db.get_value( @@ -1072,7 +1069,7 @@ class TestLeaveApplication(unittest.TestCase): leave_type_name="_Test_CF_leave_expiry", is_carry_forward=1, expire_carry_forwarded_leaves_after_days=90, - ).insert() + ) leave_alloc = create_carry_forwarded_allocation(employee, leave_type) cf_expiry = frappe.db.get_value( @@ -1114,7 +1111,7 @@ class TestLeaveApplication(unittest.TestCase): leave_type_name="_Test_CF_leave_expiry", is_carry_forward=1, expire_carry_forwarded_leaves_after_days=90, - ).insert() + ) leave_alloc = create_carry_forwarded_allocation(employee, leave_type) cf_expiry = frappe.db.get_value( @@ -1148,7 +1145,7 @@ class TestLeaveApplication(unittest.TestCase): leave_type_name="_Test_CF_leave_expiry", is_carry_forward=1, expire_carry_forwarded_leaves_after_days=90, - ).insert() + ) leave_alloc = create_carry_forwarded_allocation(employee, leave_type) cf_expiry = frappe.db.get_value( diff --git a/erpnext/hr/doctype/leave_type/test_leave_type.py b/erpnext/hr/doctype/leave_type/test_leave_type.py index 69f9e125203..56bf641d261 100644 --- a/erpnext/hr/doctype/leave_type/test_leave_type.py +++ b/erpnext/hr/doctype/leave_type/test_leave_type.py @@ -9,7 +9,8 @@ test_records = frappe.get_test_records("Leave Type") def create_leave_type(**args): args = frappe._dict(args) if frappe.db.exists("Leave Type", args.leave_type_name): - return frappe.get_doc("Leave Type", args.leave_type_name) + frappe.delete_doc("Leave Type", args.leave_type_name, force=True) + leave_type = frappe.get_doc( { "doctype": "Leave Type", @@ -23,10 +24,14 @@ def create_leave_type(**args): "expire_carry_forwarded_leaves_after_days": args.expire_carry_forwarded_leaves_after_days or 0, "encashment_threshold_days": args.encashment_threshold_days or 5, "earning_component": "Leave Encashment", + "max_leaves_allowed": args.max_leaves_allowed, + "maximum_carry_forwarded_leaves": args.maximum_carry_forwarded_leaves, } ) if leave_type.is_ppl: leave_type.fraction_of_daily_salary_per_leave = args.fraction_of_daily_salary_per_leave or 0.5 + leave_type.insert() + return leave_type diff --git a/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py b/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py index d167d1d86c9..75cb4991299 100644 --- a/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py +++ b/erpnext/hr/report/employee_leave_balance/test_employee_leave_balance.py @@ -154,7 +154,6 @@ class TestEmployeeLeaveBalance(unittest.TestCase): @set_holiday_list("_Test Emp Balance Holiday List", "_Test Company") def test_opening_balance_considers_carry_forwarded_leaves(self): leave_type = create_leave_type(leave_type_name="_Test_CF_leave_expiry", is_carry_forward=1) - leave_type.insert() # 30 leaves allocated for first half of the year allocation1 = make_allocation_record( diff --git a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py index 69cfce6badc..5ad549fea28 100644 --- a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py @@ -267,7 +267,6 @@ class TestSalarySlip(FrappeTestCase): make_leave_application(emp_id, first_sunday, add_days(first_sunday, 3), "Leave Without Pay") leave_type_ppl = create_leave_type(leave_type_name="Test Partially Paid Leave", is_ppl=1) - leave_type_ppl.save() alloc = create_leave_allocation( employee=emp_id,