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: total leaves allocated not validated and recalculated on updates post submission #30569

Merged
merged 3 commits into from
Apr 4, 2022

Conversation

ruchamahabal
Copy link
Member

@ruchamahabal ruchamahabal commented Apr 4, 2022

Problem

In Leave Allocation, if New Leaves Allocated field is updated via Data Import or API, Total Leaves Allocated is not recalculated on the server-side post submission. Moreover, the following validations don't run if someone updates allocation after submission:

  • validate_back_dated_allocation: To check if the allocation that is being updated is already carried forward to a future allocation
  • validate_total_leaves_allocated: To check if the total leaves allocated are more than the days in the period, eg: should not allow 30 leaves being allocated for 15 days
  • validate_leave_allocation_days: To check if the total leaves allocated do not exceed max_leaves_allowed within the leave period setup in Leave Type

Fix

  • Recalculate Total Leaves Allocated on server-side
  • Run the above stated validations on on_update_after_submit
  • Add tests for these validations and total calculations for updates done before and after submission

@ruchamahabal ruchamahabal force-pushed the fix-leave-alloc-update branch from 6bf4067 to 5499cec Compare April 4, 2022 13:02
@ruchamahabal ruchamahabal added the CI-failing Unit tests or patch tests are failing. label Apr 4, 2022
@codecov
Copy link

codecov bot commented Apr 4, 2022

Codecov Report

Merging #30569 (793164a) into develop (e8118fc) will increase coverage by 19.40%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##           develop   #30569       +/-   ##
============================================
+ Coverage    41.63%   61.03%   +19.40%     
============================================
  Files         1083     1083               
  Lines        69141    69145        +4     
============================================
+ Hits         28786    42205    +13419     
+ Misses       40355    26940    -13415     
Impacted Files Coverage Δ
...xt/hr/doctype/leave_allocation/leave_allocation.py 97.27% <100.00%> (+24.86%) ⬆️
.../report/bom_operations_time/bom_operations_time.py 89.13% <0.00%> (-2.18%) ⬇️
erpnext/stock/valuation.py 96.69% <0.00%> (-0.11%) ⬇️
erpnext/regional/india/e_invoice/utils.py 42.52% <0.00%> (ø)
erpnext/stock/doctype/pick_list/pick_list.py 72.09% <0.00%> (ø)
erpnext/stock/report/bom_search/bom_search.py 0.00% <0.00%> (ø)
...ccounts/doctype/mode_of_payment/mode_of_payment.py 72.00% <0.00%> (ø)
...e/course_scheduling_tool/course_scheduling_tool.py 0.00% <0.00%> (ø)
...production_plan_summary/production_plan_summary.py 0.00% <0.00%> (ø)
..._time_for_issues/first_response_time_for_issues.py 0.00% <0.00%> (ø)
... and 372 more

@ruchamahabal ruchamahabal removed the CI-failing Unit tests or patch tests are failing. label Apr 4, 2022
@ruchamahabal ruchamahabal merged commit 7675542 into frappe:develop Apr 4, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

1 participant