feat: Credit present on holiday against absence #3425
Conversation
Consider Present on Holidays
Consider Present on Holidays
Consider Present on Holiday
WalkthroughThe changes refine payroll attendance handling for holidays. The payroll settings JSON updates the description and removes a dependency for a holiday attendance field. In the salary slip Python logic, the single variable for marking attendance on holidays is split into two: one for considering absences and another for crediting presence, with corresponding updates to method signatures and internal logic. Changes
Sequence Diagram(s)sequenceDiagram
participant Employee
participant SalarySlip
participant PayrollSettings
Employee->>SalarySlip: Request working days details
SalarySlip->>PayrollSettings: Fetch holiday attendance settings
PayrollSettings-->>SalarySlip: Return consider_absent_on_holidays, credit_present_on_holidays
SalarySlip->>SalarySlip: Get attendance records (all statuses)
SalarySlip->>SalarySlip: Calculate absent and present days
alt credit_present_on_holidays is true
SalarySlip->>SalarySlip: Credit present/half-day attendance on holidays
else consider_absent_on_holidays is true
SalarySlip->>SalarySlip: Deduct pay for absent on holidays
else
SalarySlip->>SalarySlip: Skip holidays for absent calculation
end
SalarySlip-->>Employee: Return working days and absence details
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🪛 Ruff (0.12.2)hrms/payroll/doctype/salary_slip/salary_slip.py718-718: Blank line contains whitespace Remove whitespace from blank line (W293) 727-727: Blank line contains whitespace Remove whitespace from blank line (W293) 🪛 GitHub Actions: Lintershrms/payroll/doctype/salary_slip/salary_slip.py[error] 461-730: pre-commit hook 'trailing-whitespace' and 'ruff-format' failed and automatically fixed trailing whitespace and formatting issues in this file. Run 'pre-commit run --all-files' locally to reproduce. ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (5)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
hrms/payroll/doctype/salary_slip/salary_slip.py (1)
461-730: Fix code formatting issues.The pre-commit hook 'ruff-format' failed. Please run
pre-commit run --all-fileslocally to apply the required formatting fixes before pushing.
🧹 Nitpick comments (1)
hrms/payroll/doctype/salary_slip/salary_slip.py (1)
723-739: Holiday attendance logic correctly implements the PR objectives.The implementation properly handles:
- Crediting present attendance on holidays against absent days
- Accounting for half-day attendance with appropriate fractions
- Skipping certain attendance types when appropriate
Consider adding validation to prevent the absent count from going negative:
if d.status == "Present": - absent -= 1 + absent = max(0, absent - 1) elif d.status == "Half Day": - absent -= (1 - daily_wages_fraction_for_half_day) + absent = max(0, absent - (1 - daily_wages_fraction_for_half_day))This ensures that excessive holiday work doesn't result in negative absent days.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hrms/payroll/doctype/payroll_settings/payroll_settings.json(2 hunks)hrms/payroll/doctype/salary_slip/salary_slip.py(8 hunks)
🧰 Additional context used
🪛 GitHub Actions: Linters
hrms/payroll/doctype/salary_slip/salary_slip.py
[error] 461-730: pre-commit hook 'ruff-format' failed: 1 file reformatted automatically. Run 'pre-commit run --all-files' locally to apply fixes.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Python Unit Tests (2)
- GitHub Check: Python Unit Tests (1)
- GitHub Check: Summary
🔇 Additional comments (5)
hrms/payroll/doctype/payroll_settings/payroll_settings.json (2)
148-148: Description update clarifies dual behavior effectively.The updated description clearly explains how the field behaves differently based on the "Include holidays in total working days" setting. This improves user understanding of the feature.
148-152: AI summary mentions "depends_on" removal, but it's not reflected in the code.The AI summary states that the "depends_on" attribute was removed from this field, but the annotated code only shows a description change. This field doesn't appear to have had a "depends_on" attribute to begin with.
Likely an incorrect or invalid review comment.
hrms/payroll/doctype/salary_slip/salary_slip.py (3)
431-440: Variable naming is clear and logic separation is appropriate.The separation of
consider_absent_on_holidaysandcredit_present_on_holidaysbased on theinclude_holidays_in_total_working_dayssetting is logical and aligns with the PR objectives.
525-525: Parameter rename is consistent with the new naming convention.The method signature and logic have been properly updated to use
consider_absent_on_holidays.Also applies to: 539-540
696-699: Removal of status filter is appropriate for the new logic.Fetching all attendance records is necessary to process both "Present" and "Absent" statuses on holidays according to the new business logic.
Description
This pull request introduces a feature to automatically compensate employees who work on a holiday by offsetting an absent day within the same payroll period.
Problem
Currently, when "Include Holidays in Total Working Days" is disabled in Payroll Settings, there is no automated way to credit an employee who is marked "Present" on a holiday. The standard workaround is to create a manual Compensatory Leave Request, which adds administrative overhead. This PR solves that by making the process automatic.
Solution
This feature enhances the functionality of the existing "Consider Marked Attendance on Holidays" checkbox in Payroll Settings, giving it a dual purpose based on its context:
When "Include Holidays in Total Working Days" is DISABLED: If this checkbox is checked, any employee marked "Present" on a holiday will have their
absent_dayscount reduced by 1 in the Salary Slip calculation. This effectively treats working on a holiday as a compensatory day.When "Include Holidays in Total Working Days" is ENABLED: The original functionality is preserved. If checked, an employee marked "Absent" on a holiday will have their payment days deducted.
To ensure clarity for users, the description of the "Consider Marked Attendance on Holidays" checkbox has been updated to explain both behaviors.
Changes Made
hrms/payroll/doctype/salary_slip/salary_slip.py: Modified the attendance calculation logic to handle the new compensatory credit scenario.hrms/payroll/doctype/payroll_settings/payroll_settings.json: Updated the description of theconsider_marked_attendance_on_holidaysfield to reflect its dual functionality.Summary by CodeRabbit