[IMP] project_timesheet_holidays: prevent modifying leave timesheets#30863
[IMP] project_timesheet_holidays: prevent modifying leave timesheets#30863alexey-pelykh wants to merge 1 commit into
Conversation
|
You have making several changes in the same PR. Better to address little things each time. You are adding a hook and also avoiding data modification. Do only one here and the other in another PR (although hooks are not usually accepted in stable). |
|
@pedrobaeza no hooks here - not sure I'm on the same page, this PR only refactors code and adds |
There was a problem hiding this comment.
This is the hook I'm mentioning.
There was a problem hiding this comment.
Ah, got it. Well, I can move it back, though other modules in Odoo use same approach.
There was a problem hiding this comment.
Let me rephrase - if this going to be the reason of rejection - I'll move it back, since it was added w/o any particular usage in mind at least right now
There was a problem hiding this comment.
Yes, as said, 1 change each time.
3700f68 to
3fb053d
Compare
3fb053d to
b7e3aaf
Compare
jem-odoo
left a comment
jem-odoo
left a comment
There was a problem hiding this comment.
Coding review. Interesting feature for master.
Please detail a little the use case motivation and consequence in your commit message. The commit title should not be "ensure data integrity" but "prevent modifying leave timesheets"
b7e3aaf to
d2e887f
Compare
d2e887f to
7cac70a
Compare
alexey-pelykh
changed the title
7cac70a to
fdd8d5b
Compare
|
Note for myself: prevent leave timesheet modification can cause problem when adjust a timesheet in grid view. We will need to exlude those kind of timesheet, which implies a bridge module between timesheet_grid and project_timesheet_holidays. |
|
@jem-odoo do I need to fix the conflicts? |
|
@alexey-pelykh I will have to discuss with the Product Owner if it is worthy to handle that use case. |
|
@jem-odoo any feedback available? |
|
It's impressive how a PR can be unattended for 6 years without any feedback |
7 participants
Description of the issue/feature this PR addresses:
#30850
Current behavior before PR:
Desired behavior after PR is merged:
--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr