Skip to content

[15.0][FIX] auditlog rule: Control the write function in the models when executed by an onchange function #2912

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: 15.0
Choose a base branch
from

Conversation

angelvilaplana
Copy link

Changes to fix the following issue: #2343

The goal of these changes is to fix the issue that when you activate the audit rule for the products, and then you try to create or modify the price, an access error appears.

From what I've been able to see, this is because in version 15 added the onchange decorator in the _set_product_lst_price function. As within this function a write is being executed for the same product. This causes it to go through the logic of the AuditlogRule with an object that has the id as "NewId". Causing the error to appear when checking the security rules.

I've modified the AuditlogRule class and modify the _make_write function, where inside there, I check if the record id is an object that instance of NewId. If instance of NewId, What it does is ignore the original logic to prevent it from executing the read function. It'll only run the write_full.origin to do the base logic. If this is not the case, and it's a normal id, it'll execute the usual logic.

@cuongnmtm
Copy link

The same issue happens on 16.0 and 17.0, blocking tracking write changes on the product.

Copy link

@DucTruongKomit DucTruongKomit left a comment

Choose a reason for hiding this comment

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

Did few tests and code LGTM.

Copy link
Contributor

@LoisRForgeFlow LoisRForgeFlow left a comment

Choose a reason for hiding this comment

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

👍

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 15.0-ocabot-merge-pr-2912-by-LoisRForgeFlow-bump-patch, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Jun 26, 2025
Signed-off-by LoisRForgeFlow
@OCA-git-bot
Copy link
Contributor

@LoisRForgeFlow your merge command was aborted due to failed check(s), which you can inspect on this commit of 15.0-ocabot-merge-pr-2912-by-LoisRForgeFlow-bump-patch.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@LoisRForgeFlow
Copy link
Contributor

@angelvilaplana you will need to rebase this PR to be able to merge.

@angelvilaplana angelvilaplana force-pushed the 15.0-auditlog_prevent_acess_error_newid branch from ea77145 to 7f872da Compare June 26, 2025 13:39
@LoisRForgeFlow
Copy link
Contributor

@angelvilaplana looks like branch 15.0 is not very active in the last 2 commits of v15 (single commits in the last 6 months) the pre-commit is already failing so I guess we need a copier update to update the pre-commit configuration and all the dotfiles. Could you open a separated PR with that and ping me?

@MateuGForgeFlow
Copy link

Hi @angelvilaplana,
I see you have 2 commits in v15 and just one commit in v16.
What do you think about unifying criteria? Maybe squashing commits here?

I am doing the forward port for v17.

Thank you!

@angelvilaplana angelvilaplana force-pushed the 15.0-auditlog_prevent_acess_error_newid branch from 7f872da to 2d008b0 Compare June 26, 2025 17:01
@angelvilaplana
Copy link
Author

Hi @MateuGForgeFlow,

Thank you very much for the message and the suggestion!
I've squashed the commits to unify the criteria as you recommended.

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.

7 participants