Skip to content

[17.0][FIX] hr_timesheet_sheet: prevent deletion of timesheet entries when changing sheet period#848

Open
nihelgabsi-acsone wants to merge 1 commit intoOCA:17.0from
acsone:17.0-hr_timesheet_sheet-nga
Open

[17.0][FIX] hr_timesheet_sheet: prevent deletion of timesheet entries when changing sheet period#848
nihelgabsi-acsone wants to merge 1 commit intoOCA:17.0from
acsone:17.0-hr_timesheet_sheet-nga

Conversation

@nihelgabsi-acsone
Copy link
Copy Markdown

When creating a new Timesheet Sheet, changing the period will trigger the method _onchange_scope() which calls _compute_timesheet_ids() that at every execution sets the timesheets : Delete the existing ones and Link new timesheets.
After that timesheets will be removed from the project too.

Issue Ref #847

Copy link
Copy Markdown

@sbejaoui sbejaoui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code review + functional test

… when changing sheet period

Changing a timesheet sheet’s period triggered a recomputation that removed
lines whose dates fall outside the new range. This caused data loss, since
existing timesheet entries were deleted instead of simply being detached
from the sheet.

refs OCA#847
@sbejaoui sbejaoui force-pushed the 17.0-hr_timesheet_sheet-nga branch from 27caac4 to 002a56c Compare January 13, 2026 09:12
@sbejaoui
Copy link
Copy Markdown

Please, could you rebase? I want to test it in the runboat.

rebased

@MiquelRForgeFlow
Copy link
Copy Markdown
Contributor

MiquelRForgeFlow commented Jan 13, 2026

Thanks. Tested. There is an "issue" with this approach, that is, you need first to save the sheet before starting adding sheet lines or analytic lines. If not, it can lead to confusion, when you add the first sheet line, and then other sheet lines (for example time-offs) instantly appear.

If you want to go with the approach of this PR, then my vote is to make invisible the "Select Project" button and the Details tab when the sheet is not saved yet. And also a little message "Please save the sheet before proceeding." instead of "Nothing to display." in the Summary tab when the sheet has not been saved yet.

Otherwise, I still prefer #860, which simply solves the issue (deleting timesheets).

Copy link
Copy Markdown
Contributor

@alexey-pelykh alexey-pelykh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. Moving timesheet linkage from _onchange_scope into create() is the right call -- onchange-based data manipulation is unreliable for server-side operations. The test simplifications removing the Form-based timesheet assignment workarounds are a welcome cleanup. LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants