-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[FIX] onchange_helper: avoid permissions issue #3492
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
[FIX] onchange_helper: avoid permissions issue #3492
Conversation
francesco-ooops
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functional ok!
|
@dreispt can this be merged? thanks! |
twalter-c2c
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I have no suggestion on how to add a generic test for this change, to be honest. I had a specific issue that was solved by this fix, but I would not use it as a test case scenario.
|
@dreispt ping :) |
|
This PR has the |
|
@OCA/tools-maintainers can this be merged? thanks! |
|
/ocabot merge patch Does this need a forward port to other versions? |
|
What a great day to merge this nice PR. Let's do it! |
|
@thomaspaulb it's on 18, would make sense to have it on 16 and 17 |
|
Congratulations, your PR was merged at 9a770c5. Thanks a lot for contributing to OCA. ❤️ |
|
If you have the time could you prepare the PRs |
|
@thomaspaulb will try :) It would be good tho to be consistent on this and have a method to organize backportings as soon as original PRs are merged |
Backporting #3375.
@twalter-c2c and other participants to the linked PR @dreispt @grindtildeath @henrybackman you might be interested.
Could not manage to create a test for this, any suggestion is appreciated.