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] Saas 17.1 hr fix same user creation mepe #162602

Conversation

fw-bot
Copy link
Contributor

@fw-bot fw-bot commented Apr 19, 2024

Description of the issue/feature this PR addresses:

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

Forward-Port-Of: #159860

@robodoo
Copy link
Contributor

robodoo commented Apr 19, 2024

@fw-bot
Copy link
Contributor Author

fw-bot commented Apr 19, 2024

@mepe-odoo @sofiagvaladze cherrypicking of pull request #159860 failed.

stdout:

Auto-merging addons/hr/models/hr_employee.py
CONFLICT (content): Merge conflict in addons/hr/models/hr_employee.py

stderr:

14:01:01.948727 git.c:463               trace: built-in: git cherry-pick 73b83c93d806258fc0ec0c77a11e9e9f3f3d0ab6
error: could not apply 73b83c93d806... [FIX] hr : fix double employee linked to the same user (same company)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
----------
status:

Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).

In the former case, you may want to edit this PR message as well.

⚠️ after resolving this conflict, you will need to merge it via @robodoo.

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

@robodoo robodoo added forwardport This PR was created by @fw-bot conflict There was an error while creating this forward-port PR labels Apr 19, 2024
STEP TO REPRODUCE:
==================

    * Go on Employees application
    * Click on new
    * Set a name
    * Click on Hr Settings
    * Set Mitchell Admin as Related User
    * Click on backup cloud

You will have a traceback about an sql constraint violation.

Expected behaviour:
===================
You should get a validation error with the sql constraint message.

Similar behaviour:
==================

When you try to do the same thing but with an existing employee you will
have this validation error and not the traceback.

Issue explanation:
==================

To ensure that only one employee per company is linked to a user; the
sql constraint user_uniq has been created. In the constraint only the fields
user_id and company_id are checked.

But the field user_id is realted to the resource's user_id and stored.
So when creating employee; the record will be created without a user_id.

Right after, a security rule linked to user_id will be checked. This
ir.rule will trigger the computation of user_id. The safe_eval function
will catch the validation error and instead of raising this validation
error will raise a value error with the validation error as the message.
And this, value error will raise the traceback.

Resolution:
===========
If the field user_id is pre-compute, the error will be raise during the
creation and not during the security rule's check.

Task: 3834561
X-original-commit: cce4207
@mepe-odoo mepe-odoo force-pushed the saas-17.2-saas-17.1-hr-fix_same_user_creation-mepe-JGdn-fw branch from a1d4154 to 87cfc6e Compare April 19, 2024 12:21
@mepe-odoo
Copy link
Contributor

@robodoo r+

@C3POdoo C3POdoo requested a review from a team April 19, 2024 12:24
@C3POdoo C3POdoo added the RD research & development, internal work label Apr 19, 2024
robodoo pushed a commit that referenced this pull request Apr 19, 2024
STEP TO REPRODUCE:
==================

    * Go on Employees application
    * Click on new
    * Set a name
    * Click on Hr Settings
    * Set Mitchell Admin as Related User
    * Click on backup cloud

You will have a traceback about an sql constraint violation.

Expected behaviour:
===================
You should get a validation error with the sql constraint message.

Similar behaviour:
==================

When you try to do the same thing but with an existing employee you will
have this validation error and not the traceback.

Issue explanation:
==================

To ensure that only one employee per company is linked to a user; the
sql constraint user_uniq has been created. In the constraint only the fields
user_id and company_id are checked.

But the field user_id is realted to the resource's user_id and stored.
So when creating employee; the record will be created without a user_id.

Right after, a security rule linked to user_id will be checked. This
ir.rule will trigger the computation of user_id. The safe_eval function
will catch the validation error and instead of raising this validation
error will raise a value error with the validation error as the message.
And this, value error will raise the traceback.

Resolution:
===========
If the field user_id is pre-compute, the error will be raise during the
creation and not during the security rule's check.

closes #162602

Task: 3834561
X-original-commit: cce4207
Signed-off-by: Sofie Gvaladze (sgv) <sgv@odoo.com>
Signed-off-by: Mélanie Peyrat (mepe) <mepe@odoo.com>
@robodoo robodoo closed this Apr 19, 2024
@fw-bot fw-bot deleted the saas-17.2-saas-17.1-hr-fix_same_user_creation-mepe-JGdn-fw branch May 3, 2024 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflict There was an error while creating this forward-port PR forwardport This PR was created by @fw-bot RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants