[FIX] migration: reset the stale SCSS after the bump, not before

The fix was offered while the checkout was still on the previous
version. Answering « a » ran reset_asset in an Odoo 12 shell, which has
no web_editor.assets: KeyError, nothing changed, and the migration went
on to break at the next bump — measured on a real run.

Predicting early is right; fixing early is not. The early call is now
--report-only, and a second call comes after the bump, on the upgraded
database, where the checkout can do it. The tool also refuses on its
own: it reads the checkout sources for reset_asset rather than trusting
a version number, and says when to come back.

--- FR ---

[FIX] migration : réinitialiser le SCSS périmé après le palier, pas avant

La correction était proposée alors que le checkout était encore sur la
version précédente. Répondre « a » lançait reset_asset dans un shell
Odoo 12, sans web_editor.assets : KeyError, rien de modifié, et la
migration continuait jusqu'à casser au palier suivant. Mesuré.

Prédire tôt est juste ; corriger tôt ne l'est pas. L'appel précoce est
désormais --report-only, et un second vient après le palier, sur la
base montée de version, là où le checkout sait le faire. L'outil refuse
aussi de lui-même : il cherche reset_asset dans les sources du checkout
plutôt que de se fier à un numéro, et dit quand revenir.

Assisted-by: Claude Opus 5
(cherry picked from commit 13d4d0a33c63efcbafb5fb29313a5377ba5f508e)

script/odoo/migration/check_stale_scss.py

@@ -326,6 +326,54 @@ def render_diff(finding):
     return "\n".join(lines)
 
 
+def running_odoo_dir():
+    """Le répertoire de la version qui répondra au shell, d'après le checkout.
+
+    Ce n'est PAS la cible : au moment où l'on prédit ce que le palier
+    cassera, le checkout est encore sur la version d'avant. C'est elle qui
+    exécutera reset_asset — ou ne le saura pas.
+    """
+    try:
+        with open(".odoo-version", "r", encoding="utf-8") as handle:
+            return "odoo" + handle.read().strip()
+    except OSError:
+        return None
+
+
+def reset_supported(odoo_dir=None):
+    """Cette version sait-elle faire reset_asset ?
+
+    Mesuré : `web_editor.assets` et `reset_asset` n'existent qu'à partir de
+    13.0. Lancé sous odoo12.0, l'appel lève « KeyError: 'web_editor.assets' »
+    et ne change rien — c'est arrivé sur une vraie migration, et le correctif
+    a été cru appliqué alors qu'il avait échoué.
+
+    On regarde les sources plutôt qu'un numéro : c'est ce qui répondra.
+    """
+    odoo_dir = odoo_dir or running_odoo_dir()
+    if not odoo_dir or not os.path.isdir(odoo_dir):
+        return True  # Rien pour trancher : ne pas bloquer sur une supposition.
+    pattern = os.path.join(odoo_dir, "**", "web_editor", "models", "assets.py")
+    for path in glob.iglob(pattern, recursive=True):
+        with open(path, "r", encoding="utf-8", errors="replace") as handle:
+            if "def reset_asset" in handle.read():
+                return True
+    return False
+
+
+def too_early_message(odoo_dir, database):
+    """Pourquoi on ne peut pas encore corriger, et quand on pourra."""
+    return (
+        f"⛔ {t('Resetting needs Odoo 13.0 or later; this checkout is on')}"
+        f" {odoo_dir or '?'}.\n"
+        f"   {t('The prediction stands, the fix does not: run it again once')}"
+        f" {t('the bump is done, on the upgraded database.')}\n"
+        f"   {t('Applying it from here fails with')}"
+        " KeyError: 'web_editor.assets'"
+        f" {t('and changes nothing.')}"
+    )
+
+
 def reset_command(lst_finding, database, config_path="./config.conf"):
     """La commande qui rend les fichiers de module. Rien n'est lancé ici."""
     lines = [f"./odoo_bin.sh shell -c {config_path} -d {database} <<'PY'"]
@@ -418,14 +466,20 @@ def prompt(lst_finding, database, config_path="./config.conf", ask=input):
     perdre on ne sait quoi. L'invite revient donc après chaque lecture :
     regarder ne répond pas à la question.
     """
+    odoo_dir = running_odoo_dir()
+    can_reset = reset_supported(odoo_dir)
+    if not can_reset:
+        print(too_early_message(odoo_dir, database))
     while True:
+        choix = (
+            f" a = {t('reset them onto the module file')}" if can_reset else ""
+        )
         answer = (
             ask(
                 f"💬 {t('What do you want to do with these customizations?')}"
                 f" ({t('Enter = nothing')},"
                 f" v = {t('what the copy changed')},"
-                f" w = {t('full screen')},"
-                f" a = {t('reset them onto the module file')}) : "
+                f" w = {t('full screen')}{choix}) : "
             )
             .strip()
             .lower()
@@ -445,7 +499,7 @@ def prompt(lst_finding, database, config_path="./config.conf", ask=input):
                 for finding in lst_finding:
                     print(render_diff(finding))
             continue
-        if answer == "a":
+        if answer == "a" and can_reset:
             lst_path = backup_custom(lst_finding, database)
             print(f"📦 {t('Saved before resetting')} :")
             for path in lst_path:
@@ -498,6 +552,11 @@ def main(argv=None):
         default="./config.conf",
         help="Odoo config used by the shell for --apply",
     )
+    parser.add_argument(
+        "--report-only",
+        action="store_true",
+        help="never ask anything, even in front of a terminal",
+    )
     parser.add_argument(
         "--diff",
         action="store_true",
@@ -544,6 +603,10 @@ def main(argv=None):
             for finding in lst_finding:
                 print(render_diff(finding))
     if config.apply:
+        odoo_dir = running_odoo_dir()
+        if not reset_supported(odoo_dir):
+            print(too_early_message(odoo_dir, config.database))
+            return 2
         lst_path = backup_custom(lst_finding, config.database)
         print(f"📦 {t('Saved before resetting')} :")
         for path in lst_path:
@@ -562,7 +625,10 @@ def main(argv=None):
     # laisser retrouver soi-même les deux arguments de reset_asset. Mais
     # seulement devant un terminal — dans un tube, une invite bloquerait
     # l'appelant sans que personne ne voie la question.
-    if not (config.diff or config.tui) and sys.stdin.isatty():
+    if (
+        not (config.diff or config.tui or config.report_only)
+        and sys.stdin.isatty()
+    ):
         if prompt(lst_finding, config.database, config.config):
             return 0
     return 1

script/todo/todo_i18n.py

@@ -4906,6 +4906,26 @@ TRANSLATIONS = {
         "fr": "Commande de réinitialisation copiée.",
         "en": "Reset command copied.",
     },
+    "Resetting needs Odoo 13.0 or later; this checkout is on": {
+        "fr": "La réinitialisation exige Odoo 13.0 ou plus ; ce checkout est sur",
+        "en": "Resetting needs Odoo 13.0 or later; this checkout is on",
+    },
+    "The prediction stands, the fix does not: run it again once": {
+        "fr": "La prédiction tient, pas la correction : relancer une fois",
+        "en": "The prediction stands, the fix does not: run it again once",
+    },
+    "the bump is done, on the upgraded database.": {
+        "fr": "le palier passé, sur la base montée de version.",
+        "en": "the bump is done, on the upgraded database.",
+    },
+    "Applying it from here fails with": {
+        "fr": "L'appliquer d'ici échoue sur",
+        "en": "Applying it from here fails with",
+    },
+    "and changes nothing.": {
+        "fr": "et ne change rien.",
+        "en": "and changes nothing.",
+    },
     "Nothing to decide yet": {
         "fr": "Rien à décider pour l'instant",
         "en": "Nothing to decide yet",

script/todo/todo_upgrade.py

@@ -1645,12 +1645,14 @@ class TodoUpgrade:
         # une copie figée, et un palier renomme aussi des variables. Celle-ci
         # ne se voit qu'à l'écran d'une page — « Style error », sans dire quel
         # fichier ni depuis quand — et seulement une fois le palier passé.
-        # Sur le VRAI terminal : l'outil pose lui-même ses questions — voir
-        # l'écart, l'ouvrir en plein écran, réinitialiser — et un tube les
-        # rendrait toutes injoignables, comme il fermait déjà la TUI.
+        # Ici on PRÉDIT, on ne corrige pas : le checkout est encore sur la
+        # version d'avant, et reset_asset n'existe qu'à partir de 13.0.
+        # Appliquer depuis là lève « KeyError: 'web_editor.assets' » — c'est
+        # arrivé sur une vraie migration, et l'échec est passé pour un
+        # succès. La correction est proposée après le palier, plus bas.
         self.run_on_terminal(
             f"{PYTHON_BIN} ./script/odoo/migration/check_stale_scss.py"
-            f" -d {database_name} -t odoo{start_version + 1}.0"
+            f" -d {database_name} -t odoo{start_version + 1}.0 --report-only"
         )
 
         msg = "3 - Clean up database before data migration"
@@ -2507,6 +2509,17 @@ class TodoUpgrade:
                 )
                 self.write_config()
 
+                # ICI seulement la correction est possible : le checkout est
+                # passé à la version cible, donc le shell sait faire
+                # reset_asset. Et c'est le dernier moment utile — la mise à
+                # jour des modules qui suit est ce qui compile les bundles,
+                # donc ce qui échoue si un SCSS figé reste en place.
+                self.run_on_terminal(
+                    f"{PYTHON_BIN}"
+                    " ./script/odoo/migration/check_stale_scss.py"
+                    f" -d {database_name_upgrade} -t odoo{next_version}.0"
+                )
+
                 str_wait_next_version = (
                     " (or wait next version 🤖)"
                     if next_version != lst_next_version[-1]

test/test_check_stale_scss.py

@@ -303,6 +303,113 @@ class TestTheMigrationRunsIt(unittest.TestCase):
         )
 
 
+class TestTheFixCannotRunTooEarly(unittest.TestCase):
+    """Corriger exige la version d'ARRIVÉE, pas celle de départ.
+
+    Mesuré sur une vraie migration : la question a été posée avant le palier,
+    alors que le checkout était encore sur odoo12.0. Répondre « a » a lancé
+    reset_asset dans un shell Odoo 12, qui ne connaît pas `web_editor.assets`
+    — KeyError, rien de modifié, et la migration a continué jusqu'à casser au
+    palier suivant.
+
+    Prédire tôt reste juste. C'est corriger tôt qui ne l'est pas.
+    """
+
+    def setUp(self):
+        from script.todo import todo_i18n
+
+        self.addCleanup(
+            setattr, todo_i18n, "_current_lang", todo_i18n._current_lang
+        )
+        todo_i18n._current_lang = "en"
+
+    def test_a_version_without_reset_asset_is_refused(self):
+        import os
+        import tempfile
+
+        root = tempfile.mkdtemp()
+        old = os.path.join(root, "odoo12.0", "addons", "web_editor", "models")
+        os.makedirs(old)
+        with open(os.path.join(old, "assets.py"), "w") as handle:
+            handle.write("class Assets:\n    pass\n")
+        self.assertFalse(scss.reset_supported(os.path.join(root, "odoo12.0")))
+
+    def test_a_version_with_it_is_allowed(self):
+        import os
+        import tempfile
+
+        root = tempfile.mkdtemp()
+        new = os.path.join(root, "odoo13.0", "addons", "web_editor", "models")
+        os.makedirs(new)
+        with open(os.path.join(new, "assets.py"), "w") as handle:
+            handle.write("def reset_asset(self, url, bundle):\n    pass\n")
+        self.assertTrue(scss.reset_supported(os.path.join(root, "odoo13.0")))
+
+    def test_an_unknown_checkout_does_not_block(self):
+        # Rien pour trancher : refuser sur une supposition empêcherait de
+        # corriger là où c'est possible.
+        self.assertTrue(scss.reset_supported("odoo_no_such_dir_zz"))
+
+    def test_the_prompt_hides_the_fix_when_it_cannot_work(self):
+        # Offrir un choix qui échouera, c'est le faire prendre.
+        import contextlib
+        import io
+
+        original = scss.reset_supported
+        scss.reset_supported = lambda odoo_dir=None: False
+        self.addCleanup(setattr, scss, "reset_supported", original)
+        finding = {
+            "id": 1,
+            "url": "/a/b.custom.web.assets_frontend.scss",
+            "missing": ["x"],
+            "custom": "a",
+            "base_url": "/a/b.scss",
+            "bundle": "web.assets_frontend",
+            "module_path": None,
+            "module_content": "",
+            "version_dir": "odoo13.0",
+            "database": "db",
+        }
+        out = io.StringIO()
+        with contextlib.redirect_stdout(out):
+            wrote = scss.prompt([finding], "db", ask=lambda p: "a")
+        self.assertFalse(wrote)
+        self.assertIn("KeyError", out.getvalue())
+        self.assertNotIn("a = reset", out.getvalue())
+
+
+class TestTheMigrationAsksAtTheRightMoment(unittest.TestCase):
+    """Prédire avant le palier, corriger après — jamais l'inverse."""
+
+    def source(self):
+        import inspect
+
+        from script.todo.todo_upgrade import TodoUpgrade
+
+        return inspect.getsource(TodoUpgrade.execute_odoo_upgrade)
+
+    def test_the_early_call_cannot_offer_the_fix(self):
+        source = self.source()
+        premier = source.index("check_stale_scss.py")
+        fenetre = source[premier : premier + 400]
+        self.assertIn("--report-only", fenetre)
+
+    def test_a_second_call_comes_after_the_bump(self):
+        # Sans elle, la prédiction n'aurait jamais de suite : on saurait ce
+        # qui va casser sans jamais pouvoir le réparer.
+        source = self.source()
+        self.assertEqual(source.count("check_stale_scss.py"), 2)
+        second = source.rindex("check_stale_scss.py")
+        self.assertGreater(second, source.index("state_4_upgrade_odoo_lst"))
+
+    def test_the_second_call_targets_the_upgraded_database(self):
+        source = self.source()
+        second = source.rindex("check_stale_scss.py")
+        fenetre = source[second : second + 300]
+        self.assertIn("database_name_upgrade", fenetre)
+        self.assertNotIn("--report-only", fenetre)
+
+
 class TestThePromptStaysOutOfAPipe(unittest.TestCase):
     def test_it_only_asks_in_front_of_a_terminal(self):
         # Une invite dans un tube bloquerait l'appelant sur une question que