Override of migrate_module not working as expected

[jcdrubay]

Module

openupgrade_framework

Describe the bug

This function is not honoring it's docstring.

def migrate_module(self, pkg, stage):
    """In openupgrade, also run migration scripts upon installation.
    We want to always pass in pre and post migration files and use a new
    argument in the migrate decorator (explained in the docstring)
    to decide if we want to do something if a new module is installed
    during the migration.
    We trick Odoo into running the scripts by setting the update attribute if necessary.
    """
    has_update = hasattr(pkg, "update")
    if not has_update:
        pkg.update = True
    MigrationManager.migrate_module._original_method(self, pkg, stage)
    if not has_update:
        delattr(pkg, "update")

To Reproduce

Version 15.0 (probably earlier version as 13.0 and 14.0 of Odoo has the same if statement)

Steps to reproduce the behavior:

1. Run upgrade for a module which was renamed, the pre-migration is not executed.

Expected behavior
Run the pre-migration script.

Additional context

In https://github.com/odoo/odoo/blob/15.0/odoo/modules/migration.py#L101
The if statement of original function migrate_module is positive because the state of the module is "to install".

        if not (hasattr(pkg, 'update') or state == 'to upgrade') or state == 'to install':
            return

Suggested new code:

--- a/openupgrade_framework/odoo_patch/odoo/modules/migration.py
+++ b/openupgrade_framework/odoo_patch/odoo/modules/migration.py
@@ -14,9 +14,15 @@ def migrate_module(self, pkg, stage):
     has_update = hasattr(pkg, "update")
     if not has_update:
         pkg.update = True
+
+    to_install = pkg.state == "to install"
+    pkg.state = "to ugprade"
     MigrationManager.migrate_module._original_method(self, pkg, stage)
     if not has_update:
         delattr(pkg, "update")
+    if to_install:
+        pkg.state = "to install"
+

[pedrobaeza]

The question is that the rename is done after this code. What I see that we have lost is the recursive upgrade cycle that install/upgrade remaining modules that comes for this kind of things to do. I rose this problem in #3061, and it seems solved for the specific case I put, but it seems it's not enough for others where the dependency chain has several levels.

cc @StefanRijnhart

[StefanRijnhart]

This was fixed for 14.0 in 01a23e6. I forgot to port it to 15, sorry!

[StefanRijnhart]

Here you go: #3286

[pedrobaeza]

And what about the other point, Stefan? Have you experienced it in v14 migrations?

[StefanRijnhart]

@pedrobaeza Can you create a separate issue for that with a concrete example?

[StefanRijnhart]

Or reopen the original ticket.

[pedrobaeza]

OK, I was just telling if you have directly the idea of what I'm talking. I will prepare an example of the problem in a new issue.

[jcdrubay]

Since #3286 was merged, I think that we can close the issue.

@pedrobaeza I trust that you don't need this issue to follow-up on preparing the new one you are preparing.