[MIG] hr_holidays_leave_auto_approve: Migrated to 10.0 #271
Conversation
|
Please respect commit history following https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-10.0 |
espo-tony
changed the title
There was a problem hiding this comment.
@espo-tony As an improvement to the module, can you make or at least add in readme a roadmap info, which should hide the approve button if the selected holiday status is marked as auto approve...
| #. Go on the leave type configuration menu | ||
| #. Select the leave type you wish to setup | ||
| #. Mark the flag 'Auto Approve'. | ||
|
|
There was a problem hiding this comment.
@espo-tony Can you make readme complaint, usage with Try me on runbot , credits icon, plus Bug Tracker section...
There was a problem hiding this comment.
@feketemihai ,
Thank you for reviewing this module.
In my latest commit I added the missing informations to the readme file.
As for the approve button, I don't think it's a good idea to hide it when the selected status is marked as auto approve, as it would make the request impossible to approve in the following two cases:
- The request was performed, but not approved, before that the module was installed (or after that the module was installed but before that the holiday status was marked as auto approve);
- The administrative user (in contravention with what is suggested in the configuration guide), at the moment of the request creation, doesn't belong to the Holidays/Officer group.
In both cases, as the functionality is intended to work only during the request creation, the approval will never be automatically triggered and, if we hide the button, also the manual approval won't be possible.
To avoid these issues, and because I don't see any drawback in showing the approval button for auto-approval marked requests, I'd prefer to not add this feature. What's your opinion about that?
There was a problem hiding this comment.
@espo-tony This was my fear in the PR for 9.0, but it was merge beside taking this in consideration, #258 , it was about launching the validation on write method...but we can leave like this to keep the same behaviour...Thanks for the changes.
|
What about the commit history? It hasn't been respected |
|
@pedrobaeza |
|
My suggestions are that you start from a clean 10.0 branch, make the steps, and then, copy your final code over the resulting branch, and make a new commit with these changes. Finally, push that branch over this one and you will get everything correct. |
…to define a leave type in order to make the system automatically approving all the leave requests (and leave allocation requests) belonging to that leave type.
|
@pedrobaeza , |
[BSSFL-383] Data all, take S3 file
BSIBSO-1121 Active field in PO, views improvement
Migration of the module 'hr_holidays_leave_auto_approve' from V9 to V10.
This module allows the user to define a leave type in order to make the system automatically approving all the leave requests (and leave allocation requests) belonging to that leave type.