[IMP] Add FS Storage Dynamic Routing Rules - #662
Conversation
|
Hi @lmignon, |
OCA-git-bot
added
mod:fs_storage
0d0bdee to
26e70a2
Compare
lmignon
left a comment
lmignon
left a comment
There was a problem hiding this comment.
@baimont Could you add some user-oriented documentation explaining this new feature in the usage section?
I think this is particularly important here, as without this explanation it is quite difficult to understand what the code is doing, and someone discovering the module would otherwise have no way of knowing that this feature exists.
26e70a2 to
a70e6cc
Compare
hey @lmignon, done |
| Note | ||
|
|
||
| A rule's domain is only evaluated when the attachment is created or when its | ||
| content is rewritten (e.g. via `open(..., new_version=True)`). It is **not** |
There was a problem hiding this comment.
new_version=True is the default. IMO, it's important to be precise here so that users don't think that a regular call to open() without this parameter will behave differently.
lmignon
left a comment
lmignon
left a comment
There was a problem hiding this comment.
Some little comments regarding the way you design your cache....
| @api.model | ||
| @tools.ormcache("model_name") | ||
| def _get_storage_rule_ids_for_model(self, model_name): | ||
| """Static/cacheable part: which rules exist for this model.""" | ||
| return tuple( | ||
| self.env["fs.storage.rule"] | ||
| .sudo() | ||
| .search([("model_id.model", "=", model_name)]) | ||
| .ids | ||
| ) | ||
|
|
There was a problem hiding this comment.
If you create a cache of rule ids for a given model_name, you should query the database only once for all the model_name and keep a dictionary of ids by model_name into the cache
| rule_ids = self._get_storage_rule_ids_for_model(model_name) | ||
| if rule_ids: | ||
| record = self.env[model_name].sudo().browse(res_id).exists() | ||
| if record: | ||
| for rule in self.env["fs.storage.rule"].sudo().browse(rule_ids): | ||
| if rule.field_id and rule.field_id.name != field_name: | ||
| continue | ||
| if record.filtered_domain(safe_eval(rule.domain or "[]")): |
There was a problem hiding this comment.
The cache of ids for a model_name is useless if you need to browse the records to get the field name and the domain. Why not a cache of [(field_name, storage_code, domain) by model_name?
4 participants
No description provided.