OCA
[17.0] [MIG] web_ir_actions_act_multi
Hi @lbisiach, sorry i lost the notification... this pr must have review
@chienandalu i update the table name, so now it's posible to uninstall the module without have a crashed db
@chienandalu IMO make the model Abstract is a better option... i am not sure about a case where users want to declare data
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 I'm working on migration script for the issue . I pushed a new PR with a commit which fix it. If you think its ok, i can include it here. https://github.com/OCA/web/pull/2997/commits/a01cfad69a25fc55115c0f013c12d0c785023e25 Let me know your opinions
Hi @carlos-lopez-tecnativa i'm waiting for some feedback. Last changes could be: https://github.com/OCA/web/commit/a01cfad69a25fc55115c0f013c12d0c785023e25
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 https://github.com/OCA/web/pull/3119 and squash any additional administrative commits.
@augusto-weiss please include #3119 and squash any additional administrative commits.
ping @augusto-weiss
@augusto-weiss please include #3119 and squash any additional administrative commits.
ping @augusto-weiss
Hi @carlos-lopez-tecnativa thanks about that. I included it. cc @chienandalu @gaikaz
This PR looks fantastic, let's merge it! Prepared branch 17.0-ocabot-merge-pr-2704-by-pedrobaeza-bump-nobump, awaiting test results.
Congratulations, your PR was merged at 0e5e4bf58918e610f0e4e52e55ed17ec93fa14ef. Thanks a lot for contributing to OCA. ❤️