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: honour 'include holidays' setting while marking attendance for leave application #29425

Merged
merged 4 commits into from
Jan 24, 2022

Conversation

ruchamahabal
Copy link
Member

@ruchamahabal ruchamahabal commented Jan 24, 2022

Steps to replicate:

  • Create a Leave Type with Include holidays within leaves as leaves disabled

    holidays
  • Create a Leave Application that has holidays in between.

    leaves
  • Since the above setting is disabled, the holidays in between should not be considered as leaves but holidays.

  • Even though the Total Leave Days are calculated correctly (after excluding holidays), attendance is marked for 6 days (holidays included) with the status "On Leave"

Fix:

Honour Include holidays within leaves as leaves while marking attendance too, and not just while calculating Total Leave Days.

leave-das

P.S: These had no side effects like affecting leave balance because total leave days are/were calculated fine. Just attendance marking was corrected.

@codecov
Copy link

codecov bot commented Jan 24, 2022

Codecov Report

Merging #29425 (f145664) into develop (108d104) will increase coverage by 0.05%.
The diff coverage is 93.93%.

@@             Coverage Diff             @@
##           develop   #29425      +/-   ##
===========================================
+ Coverage    57.99%   58.04%   +0.05%     
===========================================
  Files         1091     1091              
  Lines        67868    67880      +12     
===========================================
+ Hits         39357    39402      +45     
+ Misses       28511    28478      -33     
Impacted Files Coverage Δ
.../hr/doctype/leave_application/leave_application.py 73.82% <93.93%> (+0.95%) ⬆️
...work_order_stock_report/work_order_stock_report.py 50.00% <0.00%> (-50.00%) ⬇️
.../manufacturing/report/bom_explorer/bom_explorer.py 94.44% <0.00%> (-5.56%) ⬇️
erpnext/stock/report/stock_ledger/stock_ledger.py 74.79% <0.00%> (-3.26%) ⬇️
erpnext/stock/report/stock_ageing/stock_ageing.py 91.32% <0.00%> (-1.74%) ⬇️
...ctype/accounting_dimension/accounting_dimension.py 64.34% <0.00%> (-1.56%) ⬇️
...e/period_closing_voucher/period_closing_voucher.py 88.23% <0.00%> (-1.48%) ⬇️
erpnext/portal/utils.py 28.98% <0.00%> (-1.45%) ⬇️
...t/accounts/report/general_ledger/general_ledger.py 59.93% <0.00%> (-1.35%) ⬇️
.../report/stock_projected_qty/stock_projected_qty.py 86.84% <0.00%> (-1.32%) ⬇️
... and 25 more

@ruchamahabal ruchamahabal marked this pull request as ready for review January 24, 2022 13:34
@ruchamahabal ruchamahabal merged commit a1f0cb4 into frappe:develop Jan 24, 2022
@ruchamahabal ruchamahabal deleted the fix-holiday branch January 24, 2022 13:35
@ruchamahabal
Copy link
Member Author

@Mergifyio backport version-13-hotfix

@mergify
Copy link
Contributor

mergify bot commented Jan 24, 2022

backport version-13-hotfix

✅ Backports have been created

ruchamahabal added a commit that referenced this pull request Jan 24, 2022
…eave application (backport #29425) (#29432)

Co-authored-by: Rucha Mahabal <ruchamahabal2@gmail.com>
KrithiRamani pushed a commit to KrithiRamani/erpnext that referenced this pull request Mar 14, 2022
…eave application (backport frappe#29425) (frappe#29432)

Co-authored-by: Rucha Mahabal <ruchamahabal2@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

1 participant