[16.0][ADD] hr_holidays_tier_validation: New module - #205
Conversation
|
Depends on OCA/server-ux#1125
|
cbe047e to
e69083a
Compare
66ec2c3 to
e92d830
Compare
0b9e87d to
46ab9c0
Compare
New module to overrides default holidays validation process to allow holidays validation based on a custom tier system.
46ab9c0 to
33d3408
Compare
marcos-mendez
left a comment
There was a problem hiding this comment.
Automated Review -- Tests Failed
1. Root cause of the test failure
The test failure occurs because the database connection fails during the Odoo server startup, likely due to an incorrect or missing database configuration in the test environment (Runboat). This is not a code issue but a test infrastructure problem.
2. Suggested fix
No fix is needed for the code itself. The error originates from the test environment setup and not from the module's logic.
3. Additional code issues
- In
hr_holidays_tier_validation/models/hr_leave.py, the_compute_need_validationmethod overrides the parent logic only whenleave_validation_type == "tier_validation", but this may lead to inconsistent behavior if other validation types are used. Ensure all validation paths are consistently handled. - The
_notify_review_availablemethod includes a commented-out condition (and r.validation_status == "pending"), which should be either removed or uncommented and made functional to ensure notifications are sent only for pending reviews.
4. Test improvements
To improve test coverage, add the following test cases in a TransactionCase or SavepointCase:
- Test tier validation on leave creation: Create a leave with
leave_validation_type = 'tier_validation'and assert thatneed_validationis set and validation is requested. - Test tier validation on confirm: Confirm a leave and verify that
request_validation()is called. - Test tier approval flow: Approve a tier review and assert that
action_validate()is triggered. - Test tier rejection flow: Reject a tier review and assert that
action_refuse()is triggered. - Test state conditions bypass: Use
skip_check_state_conditioncontext and ensure state transitions work correctly.
Use @tagged('post_install', 'manual') for tests that require manual setup or are performance-heavy.
⚠️ PR Aging Alert: CRITICAL
This PR by @maisim has been waiting for 235 days — that is over 7 months without being merged or closed.
🔴 Zero human reviews in 235 days. This contributor invested their time to improve this module. The PSC owes them at least a response — even a "needs changes" is better than silence.
💤 No activity for 90 days. Has this PR been forgotten?
Every ignored PR is a contributor who might not come back. Review time matters. (OCA Aging Report)
Reciprocal Review Request
Hi everyone! I found some test failures on this PR and left detailed feedback above. I am happy to discuss or help debug. In the meantime, if any of you get a chance, I would appreciate a look at my open PR(s):
My open PRs across OCA:
- server-tools#3554 [MIG] datetime_formatter: Migration to 18.0
- server-tools#3548 [18.0][MIG] base_kanban_stage: Migration to 18.0
- hr-attendance#262 [16.0][ADD] Hr_attendance_idsecure: iDSecure (ControliD) attendance integration
- stock-logistics-workflow#2276 [16.0][ADD] stock_move_line_devaluation
- stock-logistics-workflow#2275 [16.0][ADD] Stock move line analytic account
- stock-logistics-workflow#2268 [16.0][ADD] stock_move_line_picking_partner
- purchase-workflow#2694 [16.0][IMP]Purchase workflow added to review state & exception fix
Reviewing each other's work helps the whole community move forward. Thank you!
Environment via OCA Neural Reviewer: Minikube + K8s Job + oca-ci/py3.10-odoo16.0 | Odoo 16.0
Automated review by OCA Neural Reviewer + qwen3-coder:30b
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
New module to overrides default holidays validation process to allow holidays validation based on a custom tier system.
Depends on a module wich allow to disallow 😅 validations restart because if the leave is refused we want to keep trace of this.
OCA/server-ux#1125