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

feat(poc): holiday assignment #2768

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

rtdany10
Copy link
Contributor

@rtdany10 rtdany10 commented Feb 8, 2025

This is a PR with bare minimum functionality of holiday list assignment, like shift assignment.
An extra parameter as_on is introduced in the existing function with None as default.
The existing function is also made to call the new function with the help of a hook.

Related PR on erpnext: frappe/erpnext#45803

I'll work on the validations and rest of the clean up if this approach seems acceptable to the maintainers.

@rtdany10 rtdany10 marked this pull request as draft February 8, 2025 14:09
@rtdany10
Copy link
Contributor Author

rtdany10 commented Feb 8, 2025

@asmitahase @AyshaHakeem @ruchamahabal @nabinhait
Sorry to tag, but a review would be nice

@rtdany10
Copy link
Contributor Author

Hi @rtdany10, I'm not clear as to what problem does this solve. A little more context would be helpful!

Hi @asmitahase,
Currently, holiday list is not "assigned", rather set in employee/shift/company.
Assume you have a holiday list: 2025, starting from 01/01/2025 and ending on 31/12/2025.

I assign this holiday list inside employee. Everything works fine till end of the year. 2026 starts and a similar new Holiday list is created and set inside the employee on 01/01/2026.

Now, lets say I'm running the payroll for December 2025 by 5th of Jan, 2026. System will show all holidays as absent for the employee as their holiday list has been updated to 2026. To tackle this, I'd like to propose a Holiday Assignment, much like Shift Assignment.

@asmitahase
Copy link
Collaborator

Thanks @rtdany10, we'll get this reviewed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants