[17.0] [MIG] web_ir_actions_act_multi #2704
Conversation
51a3825 to
f4bab88
Compare
|
/ocabot migration web_ir_actions_act_multi |
|
@augusto-weiss are you close to merge this pr ? |
|
Hi @lbisiach, sorry i lost the notification... this pr must have review |
9db8993 to
de306e7
Compare
|
@chienandalu i update the table name, so now it's posible to uninstall the module without have a crashed db |
| _inherit = "ir.actions.actions" | ||
| _table = "ir_actions" | ||
| _table = "ir_actions_act_multi" |
There was a problem hiding this comment.
Yes, but this change very likely needs migration scripts, so that upgrading from Odoo 16, data is correctly separated and cleaned up from the ir_actions table and added to ir_actions_act_multi accordingly.
de306e7 to
088ddad
Compare
There was a problem hiding this comment.
The goal of using the same table is to avoid populating too many relations for models that aren't used that much (if any)
To avoid deleting that system relation you could override the _drop_table method and to ignore it when the model table is that one of the system.
Another option is to make the model an abstract one, although that would avoid that you could declare those actions as data (which maybe isn't done anyway...)
|
@chienandalu IMO make the model Abstract is a better option... i am not sure about a case where users want to declare data |
I think so, but in any case, I think a migration script should be made to avoid dropping the table when the ir.model gets unlinked. I think OU handles this with its non destructive policy but maybe in other contexts it isn't |
|
Hello everyone @chienandalu @gaikaz @lbisiach @pedrobaeza |
There was a problem hiding this comment.
@augusto-weiss @chienandalu could you please update the status of this PR
|
Hi @carlos-lopez-tecnativa i'm waiting for some feedback. |
There was a problem hiding this comment.
Hi @augusto-weiss sorry I had to grok the issue again 😅
Your option of using another table is safe enough, although at the cost of an extra table that won't be used. Not big deal anyway :)
Another option would be just using the model as abstract to avoid the uninstall problems, as that would ignore dropping the table: https://github.com/odoo/odoo/blob/88d0d1e15b7f3efeab39017512585248fd8c1d94/odoo/addons/base/models/ir_model.py#L297
And the third one to create an uninstall hook, which would probably kinda complex as we'd need to avoid dropping the table somehow...
So add your changes for the final review :)
ping @augusto-weiss |
|
Sorry @chienandalu i lost the notification! Thank @carlos-lopez-tecnativa for ping me |
|
@augusto-weiss please include #3119 and squash any additional administrative commits. |
ping @augusto-weiss |
1 similar comment
ping @augusto-weiss |
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: web-16.0/web-16.0-web_ir_actions_act_multi Translate-URL: https://translation.odoo-community.org/projects/web-16-0/web-16-0-web_ir_actions_act_multi/
Currently translated at 100.0% (17 of 17 strings) Translation: web-16.0/web-16.0-web_ir_actions_act_multi Translate-URL: https://translation.odoo-community.org/projects/web-16-0/web-16-0-web_ir_actions_act_multi/es/
Currently translated at 100.0% (17 of 17 strings) Translation: web-16.0/web-16.0-web_ir_actions_act_multi Translate-URL: https://translation.odoo-community.org/projects/web-16-0/web-16-0-web_ir_actions_act_multi/it/
088ddad to
1c42727
Compare
|
Hi @carlos-lopez-tecnativa thanks about that. I included it. |
There was a problem hiding this comment.
I expected that my commit would be included, keeping the original author—just do a cherry-pick. But anyway, LGTM.
CC @chienandalu @pedrobaeza, could you review this, please?
|
/ocabot merge nobump |
|
This PR looks fantastic, let's merge it! |
|
Congratulations, your PR was merged at 0e5e4bf. Thanks a lot for contributing to OCA. ❤️ |
No description provided.