-
-
Notifications
You must be signed in to change notification settings - Fork 536
[13.0] Refactor storage of job records with "sudo" support #281
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
[13.0] Refactor storage of job records with "sudo" support #281
Conversation
These fields contain the current model and record ids and user id the job works on. Storing them as separate fields does not allow to take benefits on the "JobSerialized" field, which keeps the original user, and later the 'su' flag. We could store the context there as well (or a cleaned one). Use computed fields to keep the model_name, record_ids, user_id fields for backward compatibility. The model_name should stay anyway for the searches on the UI, and the user_id for the same reason + possibility to edit the user.
Which means that the record the job is working on, or any record argument of the job keeps the 'su' flag.
simahawk
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.
LG (just code review)
| list: "[]", | ||
| tuple: "[]", | ||
| models.BaseModel: lambda env: json.dumps( | ||
| {"_type": "odoo_recordset", "model": "base", "ids": [], "uid": env.uid} |
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.
shall we store record.context too?
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.
This clearly opens this possibility, this is definitely where the context has to be stored. I mentioned it in the commit message :)
However, there are some questions about what should be allowed or not to store in the context (cf #121), also whether we should or not always store it (can take useless space?), so it should not be part of this PR.
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.
created #283
|
/ocabot merge nobump (version is changed in the PR already) |
|
What a great day to merge this nice PR. Let's do it! |
|
Congratulations, your PR was merged at 644a4a5. Thanks a lot for contributing to OCA. ❤️ |
Following changes of OCA#281 The initial sudo() is lost when we call "with_env()" with a False su flag. Ensure the read job.record keeps a su flag.
Following changes of OCA#281 The initial sudo() is lost when we call "with_env()" with a False su flag. Ensure the read job.record keeps a su flag.
Following changes of OCA#281 The initial sudo() is lost when we call "with_env()" with a False su flag. Ensure the read job.record keeps a su flag.
Following changes of OCA#281 The initial sudo() is lost when we call "with_env()" with a False su flag. Ensure the read job.record keeps a su flag.
Following changes of OCA#281 The initial sudo() is lost when we call "with_env()" with a False su flag. Ensure the read job.record keeps a su flag.
Following changes of OCA#281 The initial sudo() is lost when we call "with_env()" with a False su flag. Ensure the read job.record keeps a su flag.
Following changes of OCA#281 The initial sudo() is lost when we call "with_env()" with a False su flag. Ensure the read job.record keeps a su flag.
Following changes of OCA#281 The initial sudo() is lost when we call "with_env()" with a False su flag. Ensure the read job.record keeps a su flag.
Following changes of OCA#281 The initial sudo() is lost when we call "with_env()" with a False su flag. Ensure the read job.record keeps a su flag.
Following changes of OCA#281 The initial sudo() is lost when we call "with_env()" with a False su flag. Ensure the read job.record keeps a su flag.
Replaces #272
Previously, the "main" record, the one on which the job works, was stored in 3 fields:
model_name,user_idandrecord_ids(and the support for any new field such as sudo or context would need a new field). The records ofargsandkwargsare serialized inJobSerializedfields.This PR unifies the way they are stored: the "main" record is now stored in a
JobSerializedfield as well.The support of the
suflag (which means the record wassudo()-ed), is added in the second commit, which shows how we benefit from the refactor.The original fields are kept:
model_nameis stored for the UI (used in search or groupby)user_idis stored for the same reason, but also has ainversemethod that updates the serialized recordrecord_idsis kept only for backward compatibility, but could be removed in 14.0