From 300238fe8360db8205ac4e5e0a9dfd4b8725a3b4 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Sat, 22 Aug 2026 01:52:53 -0400 Subject: [PATCH] [ADD] migration: offer the theme uninstall where the theme is blamed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The question was asked once, at step one, and a flag kept it from ever returning. But a theme only becomes incompatible at a given bump, hours later -- asking at the start could not cover it. Entry [6] appears when TWO conditions hold: a theme is installed, and the recent output names it. One condition alone would offer to wreck the site's design over an error that has nothing to do with it. Also, the 17 to 18 rename OpenUpgrade declares and never applies: forum.post / tag_ids : column1 is now 'forum_post_id' ('forum_id') website_forum/18.0.1.2/ holds only an analysis, no script, so the load dies on a foreign key to a column that does not exist and leaves the database half migrated. --- FR --- La question était posée une fois, à l'étape 1, et un drapeau l'empêchait de revenir. Or un thème ne devient incompatible qu'à un palier donné, des heures plus tard : la poser au départ ne pouvait pas suffire. L'entrée [6] paraît quand DEUX conditions tiennent — un thème installé, et la sortie récente qui le nomme. Une seule offrirait de casser le design du site pour une panne étrangère. Et le renommage 17 → 18 qu'OpenUpgrade déclare sans jamais l'appliquer : forum.post / tag_ids : column1 is now 'forum_post_id' ('forum_id') website_forum/18.0.1.2/ ne porte qu'une analyse, aucun script : le chargement meurt sur une clé étrangère vers une colonne absente et laisse la base à moitié migrée. Assisted-by: Claude Opus 5 --- .../fix_migration_odoo170_to_odoo180.sql | 36 ++++ script/todo/todo_i18n.py | 4 + script/todo/todo_upgrade.py | 59 ++++++ test/test_error_retry_loop.py | 17 +- test/test_theme_on_error.py | 184 ++++++++++++++++++ 5 files changed, 299 insertions(+), 1 deletion(-) create mode 100644 script/odoo/migration/fix_migration_odoo170_to_odoo180.sql create mode 100644 test/test_theme_on_error.py diff --git a/script/odoo/migration/fix_migration_odoo170_to_odoo180.sql b/script/odoo/migration/fix_migration_odoo170_to_odoo180.sql new file mode 100644 index 0000000..65993f4 --- /dev/null +++ b/script/odoo/migration/fix_migration_odoo170_to_odoo180.sql @@ -0,0 +1,36 @@ +-- © 2021-2026 TechnoLibre (http://www.technolibre.ca) +-- License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) +-- +-- Correctifs à appliquer AVANT qu'OpenUpgrade ne migre vers Odoo 18. +-- +-- En SQL et non en Python : ce fichier tourne sur une base encore en +-- 17, que le code de la 18 ne saurait pas charger. + +-- forum_tag_rel : la colonne qui pointe vers forum.post s'appelait +-- `forum_id` — un nom trompeur, elle ne désignait pas un forum. La 18 la +-- nomme `forum_post_id`. +-- +-- OpenUpgrade le DÉCLARE dans son analyse : +-- website_forum / forum.post / tag_ids (many2many) +-- : column1 is now 'forum_post_id' ('forum_id') [forum_tag_rel] +-- mais `website_forum/18.0.1.2/` ne contient aucun script : rien ne +-- l'applique. Le chargement casse alors sur +-- column "forum_post_id" referenced in foreign key constraint does not exist +-- et la base reste à moitié migrée. +-- +-- Les deux conditions rendent l'ordre rejouable : rien à faire si la +-- table n'existe pas (website_forum non installé) ni si le renommage a +-- déjà eu lieu. +DO $$ +BEGIN + IF EXISTS ( + SELECT 1 FROM information_schema.columns + WHERE table_name = 'forum_tag_rel' AND column_name = 'forum_id' + ) AND NOT EXISTS ( + SELECT 1 FROM information_schema.columns + WHERE table_name = 'forum_tag_rel' AND column_name = 'forum_post_id' + ) THEN + ALTER TABLE forum_tag_rel RENAME COLUMN forum_id TO forum_post_id; + RAISE NOTICE 'forum_tag_rel.forum_id renommee en forum_post_id'; + END IF; +END $$; diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index db7d39b..1154f65 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -6171,6 +6171,10 @@ TRANSLATIONS = { "fr": "Les corriger ?", "en": "Correct them?", }, + "Uninstall the theme(s) the error names": { + "fr": "Désinstaller le(s) thème(s) que l'erreur nomme", + "en": "Uninstall the theme(s) the error names", + }, "Census": { "fr": "Recensement", "en": "Census", diff --git a/script/todo/todo_upgrade.py b/script/todo/todo_upgrade.py index e88354a..8dd444c 100755 --- a/script/todo/todo_upgrade.py +++ b/script/todo/todo_upgrade.py @@ -2855,6 +2855,9 @@ class TodoUpgrade: appelante passait le seuil de complexité, et une boucle d'invite se relit mieux seule que noyée dans l'exécution d'une commande. """ + # Une fois, avant la boucle : la détection lit un journal et + # interroge la base, et la boucle peut tourner huit fois. + themes = self.theme_blamed_by_the_error(database_name) tours = 0 while True: # Une borne STRUCTURELLE, et non pas seulement la logique @@ -2880,6 +2883,11 @@ class TodoUpgrade: f"[5] {t('Fix views whose type contradicts their')}" f" {t('inheritance')}" ) + if themes: + print( + f"[6] {t('Uninstall the theme(s) the error names')}" + f" : {', '.join(themes)}" + ) # `self.ask` : une migration automatique s'arrêtait ICI, # sur une invite qui ne demande qu'à continuer, et restait # bloquée sans que rien ne le signale. @@ -2917,6 +2925,17 @@ class TodoUpgrade: # encore et encore, sans fin. defaut = "" continue + if wait_status == "6" and themes: + # Le thème parti, la commande mérite un nouvel essai — + # et `repare` autorise le rejeu automatique borné. + for theme in themes: + self.run_on_terminal( + f"./script/addons/uninstall_addons_theme.sh" + f" {database_name} {theme}" + ) + wait_status = "1" + repare = True + break if wait_status == "5" and database_name: # Rien à voir avec les COW : ici la vue n'est PAS une # copie, c'est son `type` stocké qui ment. Odoo ne le @@ -3193,6 +3212,46 @@ class TodoUpgrade: return [] return [line.strip() for line in (output or []) if line.strip()] + def step_log_tail(self, octets=65536): + """La fin du journal de l'étape en cours, ou "". + + Bornée, et lue depuis la FIN : une mise à jour de modules écrit + des dizaines de milliers de lignes, et charger tout le fichier + pour en regarder vingt coûterait plus que l'erreur qu'on cherche. + """ + chemin = self.log_dir() + etape = getattr(self, "current_step", "") + if not chemin or not etape: + return "" + fichier = os.path.join(chemin, f"{self.step_slug(etape)}.log") + try: + with open(fichier, "rb") as handle: + handle.seek(0, os.SEEK_END) + depart = max(0, handle.tell() - octets) + handle.seek(depart) + return handle.read().decode("utf-8", errors="replace") + except OSError: + return "" + + def theme_blamed_by_the_error(self, database_name): + """Les thèmes installés que la sortie récente MET EN CAUSE. + + Deux conditions, pas une. Un thème doit être installé — sinon il + n'y a rien à retirer — ET son nom doit figurer dans ce que la + commande vient d'écrire. Proposer la désinstallation à chaque + échec reviendrait à offrir de casser le design du site pour une + panne qui n'a rien à voir. + """ + if not database_name: + return [] + installes = self.installed_theme(database_name) + if not installes: + return [] + journal = self.step_log_tail() + if not journal: + return [] + return [nom for nom in installes if nom in journal] + def prompt_uninstall_theme(self, database_name): """Proposer de retirer les thèmes AVANT de monter de version. diff --git a/test/test_error_retry_loop.py b/test/test_error_retry_loop.py index 437ee62..4729f13 100644 --- a/test/test_error_retry_loop.py +++ b/test/test_error_retry_loop.py @@ -54,9 +54,24 @@ class Harness(unittest.TestCase): class FauxExecute: def exec_command_live(_self, cmd, **kw): + # Une interrogation de service — la détection de thème — + # n'est pas un rejeu de la commande : l'inscrire dans + # `lst_run` doublerait tous les comptes de cette suite. + if "ir_module_module" in cmd: + if kw.get("return_status_and_output_and_command"): + return 0, cmd, [] + return 0, cmd self.lst_run.append(cmd) # Toujours en échec : c'est le cas qu'on veut borner. - return (1 if echec else 0), cmd + statut = 1 if echec else 0 + # La FORME du retour suit les drapeaux, comme le vrai : + # un faux qui rend toujours deux valeurs casse dès qu'un + # appelant demande la sortie. + if kw.get("return_status_and_output_and_command"): + return statut, cmd, [] + if kw.get("return_status_and_output"): + return statut, [] + return statut, cmd obj.execute = FauxExecute() suite = iter(resets if resets is not None else []) diff --git a/test/test_theme_on_error.py b/test/test_theme_on_error.py new file mode 100644 index 0000000..1b61111 --- /dev/null +++ b/test/test_theme_on_error.py @@ -0,0 +1,184 @@ +#!/usr/bin/env python3 +# © 2021-2026 TechnoLibre (http://www.technolibre.ca) +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) + +"""Proposer de retirer un thème SEULEMENT quand il est en cause. + +La propriété qui porte tout : deux conditions, jamais une seule. Un +thème doit être installé — sinon il n'y a rien à retirer — ET son nom +doit figurer dans ce que la commande vient d'écrire. Offrir la +désinstallation à chaque échec reviendrait à proposer de casser le +design du site pour une panne qui n'a rien à voir. + +La question était déjà posée à l'étape 1, une fois, et un drapeau +l'empêchait de revenir. Or un thème devient incompatible à un PALIER +précis, des heures plus tard : la poser au départ ne pouvait pas suffire. +""" + +import io +import os +import sys +import tempfile +import unittest + +sys.path.append( + os.path.normpath(os.path.join(os.path.dirname(__file__), "..")) +) + +from script.todo import todo_i18n # noqa: E402 +from script.todo.todo_upgrade import TodoUpgrade # noqa: E402 + + +class TestTheDetection(unittest.TestCase): + def setUp(self): + self.obj = TodoUpgrade.__new__(TodoUpgrade) + self.dossier = tempfile.mkdtemp() + self.obj.log_dir = lambda: self.dossier + self.obj.current_step = "4.5.J - Migrate database" + self.obj.step_slug = staticmethod(lambda msg: "etape") + self.obj.installed_theme = lambda base: ["theme_buzzy"] + + def journal(self, texte): + with io.open( + os.path.join(self.dossier, "etape.log"), "w", encoding="utf-8" + ) as handle: + handle.write(texte) + + def test_a_theme_named_by_the_error_is_blamed(self): + self.journal( + "ERROR ... /addons/odoo_design-themes/theme_buzzy/data/x.xml" + ) + self.assertEqual( + self.obj.theme_blamed_by_the_error("db"), ["theme_buzzy"] + ) + + def test_an_error_that_never_mentions_it_blames_nothing(self): + # C'est LA protection : une erreur de compte ne doit pas faire + # proposer d'effacer le design du site. + self.journal("ERROR relation discuss_channel does not exist") + self.assertEqual(self.obj.theme_blamed_by_the_error("db"), []) + + def test_no_theme_installed_blames_nothing(self): + self.obj.installed_theme = lambda base: [] + self.journal("theme_buzzy partout dans le journal") + self.assertEqual(self.obj.theme_blamed_by_the_error("db"), []) + + def test_no_theme_installed_never_even_reads_the_log(self): + # Le garde n'est pas décoratif : sans lui on ouvrirait et lirait + # 64 ko à CHAQUE erreur d'une migration, pour une liste vide. + lectures = [] + self.obj.installed_theme = lambda base: [] + self.obj.step_log_tail = lambda *a, **k: lectures.append(1) or "" + self.obj.theme_blamed_by_the_error("db") + self.assertEqual(lectures, []) + + def test_no_log_blames_nothing(self): + # Sans journal on ne SAIT pas : accuser au hasard serait pire + # que se taire. + self.assertEqual(self.obj.theme_blamed_by_the_error("db"), []) + + def test_no_database_blames_nothing(self): + self.journal("theme_buzzy") + self.assertEqual(self.obj.theme_blamed_by_the_error(""), []) + + def test_only_the_themes_actually_named_are_returned(self): + self.obj.installed_theme = lambda base: ["theme_buzzy", "theme_zap"] + self.journal("erreur dans theme_zap/data/ir_asset.xml") + self.assertEqual( + self.obj.theme_blamed_by_the_error("db"), ["theme_zap"] + ) + + +class TestReadingTheTail(unittest.TestCase): + def setUp(self): + self.obj = TodoUpgrade.__new__(TodoUpgrade) + self.dossier = tempfile.mkdtemp() + self.obj.log_dir = lambda: self.dossier + self.obj.current_step = "etape" + self.obj.step_slug = staticmethod(lambda msg: "etape") + + def test_it_reads_from_the_END(self): + # Une mise à jour de modules écrit des dizaines de milliers de + # lignes : charger tout le fichier pour en lire vingt coûterait + # plus que l'erreur qu'on cherche. + with io.open( + os.path.join(self.dossier, "etape.log"), "w", encoding="utf-8" + ) as handle: + handle.write("debut\n" + ("x" * 100000) + "\nLA_FIN\n") + fin = self.obj.step_log_tail(octets=1000) + self.assertIn("LA_FIN", fin) + self.assertNotIn("debut", fin) + self.assertLessEqual(len(fin), 1100) + + def test_a_missing_log_is_empty_not_a_crash(self): + self.assertEqual(self.obj.step_log_tail(), "") + + def test_no_step_is_empty(self): + self.obj.current_step = "" + self.assertEqual(self.obj.step_log_tail(), "") + + def test_broken_bytes_do_not_stop_it(self): + # Un journal de migration mêle les encodages : refuser de le lire + # ferait perdre la détection au moment où elle sert. + with open(os.path.join(self.dossier, "etape.log"), "wb") as handle: + handle.write(b"avant \xff\xfe theme_buzzy apres") + self.assertIn("theme_buzzy", self.obj.step_log_tail()) + + +class TestTheMenu(unittest.TestCase): + RACINE = os.path.normpath(os.path.join(os.path.dirname(__file__), "..")) + + def source(self): + with io.open( + os.path.join(self.RACINE, "script", "todo", "todo_upgrade.py"), + encoding="utf-8", + ) as handle: + src = handle.read() + debut = src.index("def _prompt_on_error") + fin = src.index("def prompt_fix_view_type") + return src[debut:fin] + + def test_the_entry_is_conditional(self): + # Affichée sans condition, elle proposerait de retirer le thème + # à chaque erreur, quelle qu'elle soit. + bloc = self.source() + self.assertIn("if themes:", bloc) + self.assertIn('f"[6] ', bloc) + + def test_the_branch_requires_a_blamed_theme(self): + bloc = self.source() + self.assertIn('if wait_status == "6" and themes:', bloc) + + def test_the_detection_runs_once_outside_the_loop(self): + # La boucle peut tourner huit fois ; la détection lit un journal + # et interroge la base. + bloc = self.source() + self.assertEqual(bloc.count("theme_blamed_by_the_error"), 1) + self.assertLess( + bloc.index("theme_blamed_by_the_error"), bloc.index("while True:") + ) + + def test_uninstalling_asks_for_a_retry(self): + bloc = self.source() + debut = bloc.index('if wait_status == "6"') + fin = bloc.index('if wait_status == "5"') + morceau = bloc[debut:fin] + self.assertIn('wait_status = "1"', morceau) + self.assertIn("repare = True", morceau) + + def test_it_goes_through_the_real_terminal(self): + # `uninstall_addons_theme.sh` finit par poser une question ; un + # tube la rendrait invisible et l'on répondrait à l'aveugle. + bloc = self.source() + debut = bloc.index('if wait_status == "6"') + fin = bloc.index('if wait_status == "5"') + self.assertIn("run_on_terminal", bloc[debut:fin]) + + def test_the_label_is_translated(self): + self.assertIn( + "Uninstall the theme(s) the error names", todo_i18n.TRANSLATIONS + ) + + +if __name__ == "__main__": + unittest.main()