Skip to content

[16.0][FIX]hr_timesheet_sheet: handle new analityc line when previous sheet was confirmed #754

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: 16.0
Choose a base branch
from

Conversation

PicchiSeba
Copy link

@PicchiSeba PicchiSeba commented Mar 18, 2025

When creating a new line on a project task via the "Start Work", if there is another line which is present in a hr_timesheet_sheet in state confirm or done, the new line will be created by duplicating the last one.

Which means creating a new line with the same sheet_id.
The new line is then updated with a new sheet (the corresponding one if there is one, None otherwise).
This sheet change triggers the _check_state method before assigning the new sheet, which raises the error.

Steps to reproduce on runboat:

  1. log as admin into baseonly db
  2. install hr_timesheet_sheet and project_timesheet_time_control modules
  3. create a new timesheet for the current week (also stop tasks that are currently active)
  4. select a task and change one of the lines' day to the last working day of this week
  5. confirm timesheet
  6. go on the same task where you changed day, click on "Start Work" to create a new line. Let it start in the following week
  7. observe error "You cannot modify an entry in a confirmed timesheet sheet"

Expected behavior: you should be able to create a line on the following week, like in version 14.0

@PicchiSeba PicchiSeba changed the title [14.0][FIX]hr_timesheet_sheet: handle new analityc line when previous sheet was confirmed [16.0][FIX]hr_timesheet_sheet: handle new analityc line when previous sheet was confirmed Mar 18, 2025
@PicchiSeba PicchiSeba marked this pull request as draft March 18, 2025 12:06
@PicchiSeba PicchiSeba force-pushed the 16.0-fix-timesheet-error branch 2 times, most recently from 625a869 to e79175f Compare March 18, 2025 12:24
@PicchiSeba PicchiSeba marked this pull request as ready for review March 18, 2025 12:27
@aleuffre
Copy link

I'm not entirely sure how it worked on v14, but I think a more elegant solution would be set copy=False on field sheet_id of account.analytic.line so that the duplicated line is not immediately linked to the old sheet..

What do you think? Would that work?

@PicchiSeba PicchiSeba force-pushed the 16.0-fix-timesheet-error branch from e79175f to f9e10f9 Compare March 21, 2025 09:17
@PicchiSeba
Copy link
Author

PicchiSeba commented Mar 21, 2025

I'm not entirely sure how it worked on v14, but I think a more elegant solution would be set copy=False on field sheet_id of account.analytic.line so that the duplicated line is not immediately linked to the old sheet..

What do you think? Would that work?

Yes, that would also work. But I'm not sure if some other module in this repo in relying on the copy of sheet_id.
I will go on with your solution as it is indeed more elegant.

Copy link

@aleuffre aleuffre left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants