[FIX] base: do not overwrite context when upgrading

[MiquelRForgeFlow]

Just allow to pass flags to the context of _process_end method.

(the same is done in https://github.com/odoo/odoo/blob/14.0/odoo/addons/base/models/ir_model.py#L2087)

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

[robodoo]

Pull request status dashboard.

[MiquelRForgeFlow]

please @rco-odoo could you check? we need this and it's very simple

[rco-odoo]

Why? Can you please explain the problem you are trying to solve?

[MiquelRForgeFlow]

Well, we want to pas some context info when calling the _process_end method, but this method is overwriting the context :S

[rco-odoo]

Well, we want to pas some context info when calling the _process_end method, but this method is overwriting the context :S

This is what you want to do, not why you want to do it.

Besides, _process_end() is designed to be called from loading.py after upgrading some modules. By design the calling context is empty, because loading the registry is a "neutral" operation (no lang, no timezone, no current company, whatsoever).

[MiquelRForgeFlow]

As to why we want to pass flags in context in this method, it's to be able to do nice hooks instead of superugly hooks. It happens that when doing _process_end(), it may try to delete some model data and we are in a project where we don't want that deletion somehow in some cases.

[rco-odoo]

I really don't get it. As I wrote before, _process_end() is called from loading.py with the superuser and an empty context. If you want to prevent the deletion in some circumstances, those circumstances cannot depend on the current user or context, but can only depend on the database. That should be doable with a simple override of unlink(), isn't it?

[MiquelRForgeFlow]

Well, let's go with a broader explanation:

We in OpenUpgrade (v13) project (a fork which contains some core patches to avoid deleting obsolete data) added this FIX to avoid an error. But in OpenUpgrade (v14) project we decided to avoid doing a fork, and we proceed with clean monkeypatches approach. That fix was forward ported here (in a clean but a bit convoluted way), because we don't want to do ugly monkeypatches with verbatim copies of odoo methods. But we found that this patch only fixes the uninstall case, but not the upgrading case. We detected the upgrading case error in base_automation migration, that when doing _process_end() it gives the error of obsolete base.automation.lead.test model not being in the environment. I added in that PR a commit to solve that by passing a flag, but the overwriting in the context in _process_end() evades my fix.

Well, I know you have your migration process and maybe don't like our free community approach, that's why I didn't want to say it at the beginning. I hope you understand all I said, and feel free to ask anything.

[rco-odoo]

Well, let's go with a broader explanation:

We in OpenUpgrade (v13) project (a fork which contains some core patches to avoid deleting obsolete data) added this FIX to avoid an error. But in OpenUpgrade (v14) project we decided to avoid doing a fork, and we proceed with clean monkeypatches approach. That fix was forward ported here (in a clean but a bit convoluted way), because we don't want to do ugly monkeypatches with verbatim copies of odoo methods. But we found that this patch only fixes the uninstall case, but not the upgrading case. We detected the upgrading case error in base_automation migration, that when doing _process_end() it gives the error of obsolete base.automation.lead.test model not being in the environment. I added in that PR a commit to solve that by passing a flag, but the overwriting in the context in _process_end() evades my fix.

Ouch! I understand how the monkeypatch works, but it looks so fragile: any fix in the original code can make it break.

We solve such cases in a different way. Our migration framework runs scripts that adapts data in plain SQL most of the time. This avoids the problem of the missing model, and is usually much more efficient. If the default process fails, we adapt the scripts to solve the problem ahead of the failure.

I will ask for other opinions before making a decision.

[rco-odoo]

BTW I noticed that

• _module_data_uninstall() extends the context here, while
• _process_end() forces the context here

[MiquelRForgeFlow]

@rco-odoo Yeah. I hope we receive your green-light soon.

[MiquelRForgeFlow]

BTW I noticed that

_module_data_uninstall() extends the context [here], while
_process_end() forces the context [here]

@rco-odoo any chance this minor patch can be accepted? 🙏 I mean, I don't see why one method has to extend and the other has to force. Let's make both extend to overall "harmonize" it?

I can edit the commit message if you prefer and put something more suitable.

[C3POdoo]

Dear @MiquelRForgeFlow,

Thank you for your contribution but the version 14.0 is no longer supported.
We only support the last 3 stable versions so no longer accepts patches into this branch.

We apology if we could not look at your request in time.
If the contribution still makes sense for the upper version, please let us know and do not hesitate to recreate one for the recent versions. We will try to check it as soon as possible.

This is an automated message.