Skip to content
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

[FW][FIX] hr_timesheet: avoid setting an archived employee to a timesheet in the form view #95016

Conversation

fw-bot
Copy link
Contributor

@fw-bot fw-bot commented Jun 30, 2022

Steps to reproduce:

  • Install timesheet
  • Archive an employee
  • Create a timesheet

Current behavior:
You can select the archived employee

Expected behavior:
You can not select the archived employee

Explanation:
In commit 17b2b07 the employee_id
field of the timesheet was set to have context active_test to false.
To fix this issue we reset the context for this field in every
timesheet form.

A task exist to fix the issue on master id 2884736

opw-2887727
opw-2870739

Second commit:

Previously we had an issue as we created an employee we
also created holiday timesheets for the employee even
if this employee was not active (for example a candidate
in the hiring process) so we had to restrict the creation
of holiday timesheets on employee creation to active
employees.

Doing so we needed for a candidate that succeeded the hiring
process to have his holiday timesheets created. That why we
overwrite the write function and add the behavior that when
an employee is set to active (unactive) his future public
holiday timesheets are created (deleted).

Forward-Port-Of: #94308

@robodoo
Copy link
Contributor

robodoo commented Jun 30, 2022

Pull request status dashboard

@fw-bot
Copy link
Contributor Author

fw-bot commented Jun 30, 2022

This PR targets saas-15.2 and is part of the forward-port chain. Further PRs will be created up to master.

More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port

@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label Jun 30, 2022
@robodoo robodoo added the forwardport This PR was created by @fw-bot label Jun 30, 2022
@mafo-odoo
Copy link
Contributor

The current PR is stuck because of the corresponding PR failing a test (https://github.com/odoo/enterprise/pull/28999). During the hiring process the employee is not active but we already try to create his holiday timesheets. After discussion with the PO we will add a check for employee activity before creating timesheets at employee creation. To have a consistent behavior we will also create/delete the public holiday timesheets when the employee is archived/unarchived.

@fw-bot
Copy link
Contributor Author

fw-bot commented Jul 4, 2022

This PR was modified / updated and has become a normal PR. It should be merged the normal way (via @robodoo)

@C3POdoo C3POdoo requested a review from a team July 4, 2022 11:30
@xavierbol
Copy link
Contributor

Thanks for the changes, but have you check if the issue is not there in 15? 🤔
We probably also have to do it from 15.

@mafo-odoo
Copy link
Contributor

Thanks for the changes, but have you check if the issue is not there in 15? thinking We probably also have to do it from 15.

Yes I just tested on runbot 15.0 and the issue is there so it should be fixed this week or people are going to complain that the salary configurator does not work anymore, it's strange that the test did not fail in 15.0 as it already existed 🤔 I thought we only had the issue in 15.2 because holiday timesheet generation for new employee was added there 😅 but it was added in 15.0. Should I make a new pr to backport the second commit on 15.0 and 15.1?

@xavierbol
Copy link
Contributor

Probably because there is no global time off in the future in the test. Indeed, you have to create a PR in 15 to fix the issue as quick as possible.

… in the form view

Steps to reproduce:
- Install timesheet
- Archive an employee
- Create a timesheet

Current behavior:
You can select the archived employee

Expected behavior:
You can not select the archived employee

Explanation:
In commit 17b2b07 the employee_id
field of the timesheet was set to have context active_test to false.
To fix this issue we reset the context for this field in every
timesheet form and we add user errors if the employee is not active
while the timesheet is created or edited.

opw-2887727
opw-2870739

X-original-commit: d5ac8e0
@xavierbol xavierbol force-pushed the saas-15.2-15.0-opw-2870739-archived-employee-in-timesheet-mafo-Pxcs-fw branch from 19b5e8e to 0fae9e5 Compare July 6, 2022 09:21
…heets

Before this commit, when the employee is generated with the salary
configurator, the employee is archived since the employee will not
directly start in the company. Even if the employee is archived and the
timesheets are generated for this employee based on the global time offs
found in the future. The problem is the timesheets cannot be created
for archived employees and so a user error is raised.
This problem could also happen if the user creates an archived employee.

This commit fixes the issue by preventing the timesheets generation when
the employee created is archived. When this employee will be updated and
active field will be set to True, the timesheets generation will be
applied. In the inverse use case, when the user will archive an employee
and timesheets generated for the future global time offs will be removed.

opw-2887727
opw-2870739
@xavierbol xavierbol force-pushed the saas-15.2-15.0-opw-2870739-archived-employee-in-timesheet-mafo-Pxcs-fw branch from 0fae9e5 to 8e4b6c5 Compare July 6, 2022 09:23
@xavierbol
Copy link
Contributor

@robodoo rebase-ff r+

@robodoo
Copy link
Contributor

robodoo commented Jul 6, 2022

Merge method set to rebase and fast-forward

robodoo pushed a commit that referenced this pull request Jul 6, 2022
… in the form view

Steps to reproduce:
- Install timesheet
- Archive an employee
- Create a timesheet

Current behavior:
You can select the archived employee

Expected behavior:
You can not select the archived employee

Explanation:
In commit 17b2b07 the employee_id
field of the timesheet was set to have context active_test to false.
To fix this issue we reset the context for this field in every
timesheet form and we add user errors if the employee is not active
while the timesheet is created or edited.

opw-2887727
opw-2870739

X-original-commit: d5ac8e0
Part-of: #95016
robodoo pushed a commit that referenced this pull request Jul 6, 2022
…heets

Before this commit, when the employee is generated with the salary
configurator, the employee is archived since the employee will not
directly start in the company. Even if the employee is archived and the
timesheets are generated for this employee based on the global time offs
found in the future. The problem is the timesheets cannot be created
for archived employees and so a user error is raised.
This problem could also happen if the user creates an archived employee.

This commit fixes the issue by preventing the timesheets generation when
the employee created is archived. When this employee will be updated and
active field will be set to True, the timesheets generation will be
applied. In the inverse use case, when the user will archive an employee
and timesheets generated for the future global time offs will be removed.

opw-2887727
opw-2870739

closes #95016

Related: odoo/enterprise#28999
Signed-off-by: Laurent Stukkens (ltu) <ltu@odoo.com>
Signed-off-by: Xavier <xbo@odoo.com>
@robodoo robodoo closed this Jul 6, 2022
@robodoo robodoo temporarily deployed to merge July 6, 2022 13:42 Inactive
@xavierbol xavierbol deleted the saas-15.2-15.0-opw-2870739-archived-employee-in-timesheet-mafo-Pxcs-fw branch July 6, 2022 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
forwardport This PR was created by @fw-bot OE the report is linked to a support ticket (opw-...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants