refactor: Employee Leave Balance

[ruchamahabal]

Problems

• Incorrect opening balances on leave allocation boundary dates
• Carry forwarded leaves accounted in leaves allocated column, should be part of opening balance
• Leaves Allocated column also adds expired leave allocations causing negative balances
• Fix Column Labels
• Leaves Taken also adds leaves expiring on boundary date as leaves taken, causing ambiguity
• Expired Leaves not showing up properly
• Leave Balance shows minimum leaves remaining after comparing with remaining days for allocation expiry causing ambiguity
• Leave Balance on the Leave Application dashboard
• Tests
• In the case of "Allow Negative Balance", Leave Applications spanning over two allocations create a single ledger entry, causing problems in balance calculation. Split ledger entries for such applications to have cleaner data.
• Test for splitting ledger entries
• Intermediate Leave ledger entries for carry forwarded leaves, do not take holidays into account

---

1. Leave Balance Problems

1.1 Incorrect Opening Balances

Problem

• Create Leave Allocation for last year (1-1-2021 to 31-12-2021)

  

• Check Leave Balance Summary for this year:

  

• Opening Balance is incorrect while checking leave balance on leave allocation period's start date. Opening Balance is the leave balance 1 day before the filter start date. i.e. it fetches leave balance on 31st Dec 2021. However, the leaves are not carried forward to the next leave allocation, hence the opening balance should be 0, not 1 since it expires on 1st.

• These carry forwarded leaves were instead accounted in leaves allocated column, should be part of the opening balance.

Fix

• If the previous allocation's expiry is the same as the opening balance date (filter from date - 1), then fetch the carry forwarded leave balance on that day. If not carry-forwarded then it should be 0.

  eg: Here Casual Leave was not carry-forwarded, balance is 0. 10 Sick Leaves were carry forwarded, so opening balance is 10.

  

• If the balance is not being checked on allocation boundary, fetch opening balance as usual.

1.2 Leaves Allocated column also adds expired leave allocations causing negative balances

Problem:
When the allocation period ends, it creates a separate Leave Ledger Entry with (-1 * leaves) to expire and reduce the leave balance. Such leave ledgers were not being ignored while calculation leaves allocated in the leave balance report.

Fix:

Ignore leave ledger entries with is_expired = 1 as they are not part of leave allocation.

1.3 Leaves Taken also adds leaves expiring on boundary date as leaves taken, causing ambiguity

Problem

• Leave Allocation for 10 leaves each has been created for 1st Jan 2021 - 31st Dec 2021
• Since the balance is being checked on 31st Dec and these leaves expire on 31st Dec 2021, this is accounted for in the Leaves Taken column, although no leave applications have been created. 🤦🏻‍♀️

Fix

• Leaves Expiring should not be accounted for in Leaves Taken.
• Closing Balance should be 10
• Opening Balance for the next allocation depends on whether these leaves get carry-forwarded to the next allocation or not, which is already handled above.

1.4 Incorrect Expired Leaves Calculation

• When the allocation period ends, it creates a separate Leave Ledger Entry with (-1 * leaves) to expire to reduce the leave balance. These negative entries were being added in the expiry leaves calculation, giving incorrect values.
• Expired Leaves here means leaves that are expiring before the To Date filter
• This should also reduce leaves taken within that period since such leaves won't expire as they are already consumed.

More details here:

https://github.com/frappe/erpnext/pull/29439/files#diff-574a6584c831cb44ab451aea33aedce4f2c59e9896221b441c5f24ecb24b4061R200-R211

2. Leave Application Problems

2.1 Leave Summary Table fixes

Before:

• Pending Leaves here means Leaves Pending Approval, but it's confusing when seeing this.
• Total leaves allocated considering canceled leaves
• Show dashboard only once from date is set, else it fetches all allocations till date and generates an incorrect balance

After:

• Leave -> Leave(s)
• Change 'Pending Leaves' to 'Leaves Pending Approval' for better context
• Update labels in Salary Slip Leave Details table too

2.2 Split Leave Ledger Entries for Leave Application created on boundary

Background:

When User creates a leave application spanning across 2 allocations:

• If Allow Negative Balance is disabled in Leave Type:
  • Validation for application across allocations is shown, things stop right there
• If Allow Negative Balance is enabled in Leave Type:
  • eg: 2 allocations are present: 1st Jan 2021 - 31st Dec 2021 and 1st Jan 2022 - 31st Dec 2022
  • User applies for a 5 day leave from 29th Dec 2021 to 2nd Jan 2022
  • Leave Ledger creates a single entry of 5 leaves:
    • From Date: 29th Dec 2021
    • To Date: 2nd Jan 2022
  • While checking for balances, they are usually checked for a single allocation. Now this will show up as 5 leaves taken in both the allocations.
  • This causes problems in balance calculations. Also helps in cleaner negative balance booking.

Fix:

Split ledger entries for such applications to have cleaner data.

2.3 Leave Balance in Leave Application

Problem:

Example:

• If an allocation of 14 leaves is created for 1-1-2022 to 31-12-2022.
• User applies for leave on 28th December, the leave balance will be shown as 4 even though they have 14 leaves which is super confusing for the user. Leave Balance shows minimum leaves remaining after comparing with remaining days for allocation expiry, since you cannot consume 14 leaves in 4 days. 🥲

Fix:

This problem is interlinked to 2.2
Show the balance as 14 for the user. Show appropriate validation message:

[ruchamahabal]

erpnext/erpnext/hr/report/employee_leave_balance/employee_leave_balance.py

Lines 121 to 122 in b8fbfed

	# not be shown on the basis of days left it create in user mind for carry_forward leave
	row.closing_balance = (new_allocation + opening - (row.leaves_expired + leaves_taken))

I don't understand what the comment means here 🙃

[codecov]

Codecov Report

Merging #29439 (211916b) into develop (835c029) will increase coverage by 0.39%.
The diff coverage is 99.23%.

@@             Coverage Diff             @@
##           develop   #29439      +/-   ##
===========================================
+ Coverage    60.18%   60.57%   +0.39%     
===========================================
  Files         1084     1084              
  Lines        68547    68609      +62     
===========================================
+ Hits         41253    41561     +308     
+ Misses       27294    27048     -246     

Impacted Files	Coverage Δ
...r/doctype/leave_ledger_entry/leave_ledger_entry.py	91.80% <ø> (ø)
erpnext/payroll/doctype/salary_slip/salary_slip.py	85.60% <ø> (ø)
.../hr/doctype/leave_application/leave_application.py	82.00% <98.92%> (+7.65%)	⬆️
...t/employee_leave_balance/employee_leave_balance.py	94.44% <100.00%> (+94.44%)	⬆️
...payroll/doctype/income_tax_slab/income_tax_slab.py	83.33% <0.00%> (-16.67%)	⬇️
...unts/report/purchase_register/purchase_register.py	73.28% <0.00%> (-8.22%)	⬇️
...pnext/setup/doctype/sales_partner/sales_partner.py	65.21% <0.00%> (-4.35%)	⬇️
...e_purchase_register/item_wise_purchase_register.py	75.00% <0.00%> (-4.00%)	⬇️
...ctype/woocommerce_settings/woocommerce_settings.py	80.76% <0.00%> (-3.85%)	⬇️
erpnext/crm/doctype/prospect/prospect.py	55.22% <0.00%> (-1.50%)	⬇️
... and 38 more