diff --git a/script/odoo/migration/check_cow_views.py b/script/odoo/migration/check_cow_views.py index 8526a2b..525f88f 100755 --- a/script/odoo/migration/check_cow_views.py +++ b/script/odoo/migration/check_cow_views.py @@ -201,6 +201,187 @@ def declared_view_shape(module_dir, template_id): return None +def run_sql(database, sql): + """Le texte brut d'une requête. RuntimeError si la base ne répond pas.""" + result = subprocess.run( + ["psql", "-X", "-w", "-d", database, "-tA", "-c", sql], + capture_output=True, + text=True, + ) + if result.returncode: + raise RuntimeError( + f"Cannot read '{database}': {result.stderr.strip()}" + ) + return result.stdout + + +def query_cow_archs(database): + """[(id, clé, mode, website_id, {langue: arch})] — arch ENTIÈRES. + + `query_cow_views` tronque à 400 caractères : assez pour lire la + première balise, pas pour évaluer un xpath — un fragment coupé n'est + même pas du XML. Et `arch_db` est un jsonb par langue depuis la 17 : + une page cassée en fr_CA et saine en en_US, cela existe, on l'a vu. + """ + jsonb = "jsonb" in run_sql( + database, + "SELECT data_type FROM information_schema.columns" + " WHERE table_name='ir_ui_view' AND column_name='arch_db'", + ) + expr = "arch_db" if jsonb else "json_build_object('', arch_db)" + brut = run_sql( + database, + "SELECT coalesce(json_agg(json_build_object(" + "'id', id, 'key', coalesce(key, ''), 'mode', mode," + f" 'website_id', website_id, 'arch', {expr})), '[]')" + " FROM ir_ui_view WHERE website_id IS NOT NULL", + ) + try: + lignes = json.loads(brut.strip() or "[]") + except ValueError: + return [] + return [ + ( + ligne["id"], + ligne["key"], + ligne["mode"], + ligne["website_id"], + ligne["arch"] if isinstance(ligne["arch"], dict) else {}, + ) + for ligne in lignes + ] + + +def installed_modules(database): + """Les modules installés. Sans eux le balayage des sources est vain. + + Un enfant peut vivre dans N'IMPORTE quel module — `website_crm` hérite + de `website.contactus` — donc il faut balayer large. Mais balayer TOUT + l'arbre cible coûterait des milliers de fichiers pour rien : seuls les + modules installés produiront des vues. + """ + output = run_sql( + database, + "SELECT name FROM ir_module_module WHERE state IN" + " ('installed', 'to upgrade')", + ) + return [ligne.strip() for ligne in output.splitlines() if ligne.strip()] + + +def full_key(module_name, valeur): + """« contactus » dans le module website devient « website.contactus ».""" + valeur = (valeur or "").strip() + if not valeur: + return None + return valeur if "." in valeur else f"{module_name}.{valeur}" + + +def scan_target_views(target_version, lst_module): + """(déclarés, héritages) tels que la version CIBLE les livre. + + `déclarés` : les clés de gabarit que la cible fournit. Sert à repérer + un `t-call` vers un gabarit qui n'existe plus. + + `héritages` : {clé parente: [expressions xpath]}. C'est le manque qui + a coûté quatre paliers de silence — la cible ajoute un ancrage dans + une vue module, sa vue héritière le vise, et la copie de site, qui + n'est jamais réécrite, ne l'a pas. + """ + declares = set() + heritages = {} + for module_name in lst_module: + module_dir = find_module_dir(target_version, module_name) + if module_dir is None: + continue + motif = os.path.join(module_dir, "**", "*.xml") + for file_path in sorted(glob.glob(motif, recursive=True)): + try: + racine = ET.parse(file_path).getroot() + except ET.ParseError: + continue + for element in racine.iter("template"): + cle = full_key(module_name, element.get("id")) + if cle: + declares.add(cle) + parent = full_key(module_name, element.get("inherit_id")) + if not parent: + continue + exprs = [ + noeud.get("expr") + for noeud in element.iter("xpath") + if noeud.get("expr") + ] + if exprs: + heritages.setdefault(parent, []).extend(exprs) + return declares, heritages + + +def will_not_render(database, target_version): + """Les copies qui passeront la migration et rendront 500 ensuite. + + Deux ruptures que `analyse` ne voit pas, parce qu'elles ne touchent + pas la FORME de la copie : + + ancrage manquant un enfant de la CIBLE vise `//t[@t-set='x']` que + la copie n'a pas. + t-call pendant la copie appelle un gabarit que la cible ne + livre plus. + + Celles-là ne se neutralisent PAS : la copie porte une page écrite par + quelqu'un, et la mettre de côté l'effacerait du site. Elles se + réparent — `fix_cow_render.py` remet l'ancrage depuis la vue module + et retire l'appel mort, sans toucher au contenu. + """ + from fix_cow_render import dangling_calls, locates + + copies = query_cow_archs(database) + if not copies: + return [] + declares, heritages = scan_target_views( + target_version, installed_modules(database) + ) + connus = declares | {cle for _i, cle, _m, _w, _a in copies if cle} + risques = [] + for view_id, key, mode, website_id, langues in copies: + if not key: + continue + # Chaque langue : une page peut être cassée en fr_CA et saine en + # en_US, et c'est celle du site qui décide de ce qu'on voit. + manques = set() + pendants = [] + for texte in langues.values(): + for expr in heritages.get(key, []): + if not locates(texte, expr): + manques.add(expr) + for nom in dangling_calls(texte, connus): + if nom not in pendants: + pendants.append(nom) + for expr in sorted(manques): + risques.append( + ( + view_id, + key, + mode, + mode, + website_id, + f"a child of the target needs an anchor this copy lacks:" + f" {expr}", + ) + ) + for nom in pendants: + risques.append( + ( + view_id, + key, + mode, + mode, + website_id, + f"calls a template the target no longer ships: {nom}", + ) + ) + return risques + + def analyse(database, target_version): """Sort COW views into three buckets by comparing with the target sources. @@ -304,9 +485,29 @@ def main(): lst_at_risk, lst_module_absent, lst_no_counterpart = analyse( config.database, config.target_version ) + # L'autre famille de rupture : la copie garde sa forme et cesse de se + # RENDRE. Elle ne se neutralise pas — la copie porte une page écrite + # par quelqu'un — elle se répare. + lst_no_render = will_not_render(config.database, config.target_version) database = config.database target_version = config.target_version + if lst_no_render: + print( + f"⚠️ {len(lst_no_render)}" + f" {t('website COW view(s) will survive the bump and then fail')}" + f" {t('to render:')}" + ) + for view_id, key, _mode, _cible, website_id, raison in lst_no_render: + print(f" - id={view_id} website={website_id} {key}") + print(f" {t(raison)}") + print( + f" {t('Repair rather than neutralize — the copy holds a page')}" + f" {t('someone wrote:')}" + f" ./script/odoo/migration/fix_cow_render.py -d {config.database}" + ) + print("") + if not lst_at_risk: print( f"✅ -> {t('No website COW view changes shape in')}" @@ -378,7 +579,9 @@ def main(): # 0 = rien à signaler, 1 = des copies casseront, 2 = l'outil a échoué. # Le pilote lisait le texte anglais de cette sortie pour savoir s'il # devait poser sa question : traduire le message le rendait aveugle. - return 1 if lst_at_risk else 0 + # Les deux familles comptent : une copie qui rendra 500 est une + # trouvaille, même si la migration, elle, ne s'arrêtera pas. + return 1 if (lst_at_risk or lst_no_render) else 0 if __name__ == "__main__": diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index 911045d..5f96ab5 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -6710,6 +6710,22 @@ TRANSLATIONS = { "fr": "Chaque copie de site sait encore se rendre.", "en": "Every website copy still renders.", }, + "website COW view(s) will survive the bump and then fail": { + "fr": "vue(s) COW de site passeront le palier puis échoueront", + "en": "website COW view(s) will survive the bump and then fail", + }, + "to render:": { + "fr": "à se rendre :", + "en": "to render:", + }, + "Repair rather than neutralize — the copy holds a page": { + "fr": "Réparer plutôt que neutraliser — la copie porte une page", + "en": "Repair rather than neutralize — the copy holds a page", + }, + "someone wrote:": { + "fr": "écrite par quelqu'un :", + "en": "someone wrote:", + }, "Every website copy renders again.": { "fr": "Chaque copie de site sait de nouveau se rendre.", "en": "Every website copy renders again.", diff --git a/script/todo/todo_upgrade.py b/script/todo/todo_upgrade.py index 142750b..276933c 100755 --- a/script/todo/todo_upgrade.py +++ b/script/todo/todo_upgrade.py @@ -2705,6 +2705,10 @@ class TodoUpgrade: f"before_{next_version}", f"after_{next_version}", ) + # ICI et pas avant : la réparation prend l'ancrage manquant + # dans la vue MODULE, qui ne porte l'arch de la cible + # qu'une fois la migration passée. + self.repair_cow_render(database_name_upgrade) lst_upgrade_odoo[index] = cmd_upgrade self.dct_progression["state_4_upgrade_odoo_lst"] = ( @@ -3656,6 +3660,30 @@ class TodoUpgrade: f" -d {database_name} --list" ) + def repair_cow_render(self, database_name): + """Les copies de site qui passent le palier et rendent 500 après. + + Deux ruptures qu'aucune autre étape ne voit : un ancrage que la + vue héritière de la CIBLE réclame et que la copie n'a jamais eu, + et un `t-call` vers un gabarit que la cible ne livre plus. Mesuré + sur une chaîne 12 → 18 : /contact rendait 500 depuis le palier + 14 → 15, et rien ne l'a dit avant le test de fumée final. + + On ne neutralise pas : la copie porte une page écrite par + quelqu'un. On répare, et le contenu reste. + + `wait_at_error=False` : le code 1 de cet outil veut dire « j'ai + trouvé », pas « je suis tombé ». Sans cela le pilote ouvrirait son + menu d'erreur sur une réparation qui s'est bien passée. + """ + outil = os.path.join(PATH_MIGRATION_GLOBAL, "fix_cow_render.py") + if not os.path.isfile(outil): + return + self.todo_upgrade_execute( + f"{PYTHON_BIN} ./{outil} -d {database_name} --apply", + wait_at_error=False, + ) + def diff_cow_views(self, database_name, label_before, label_after): """Print what the version bump did to the website COW views.""" directory = os.path.join( diff --git a/test/test_check_cow_views_render.py b/test/test_check_cow_views_render.py new file mode 100644 index 0000000..c63c391 --- /dev/null +++ b/test/test_check_cow_views_render.py @@ -0,0 +1,266 @@ +#!/usr/bin/env python3 +# © 2021-2026 TechnoLibre (http://www.technolibre.ca) +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) + +"""Prédire, AVANT le palier, la copie qui rendra 500 après. + +`check_cow_views` cherchait la rupture qui ARRÊTE la migration : la forme +que l'arch doit avoir change. Deux autres la laissent finir et ne se +voient qu'à l'ouverture de la page — et personne n'ouvre les pages avant +la fin. + +Mesuré sur une chaîne 12 → 18 réelle : /contact rendait 500 depuis le +palier 14 → 15. Rejouée avec ce contrôle, la prédiction nomme le défaut +dès le palier 13 → 14, cinq paliers avant que quiconque s'en aperçoive. + +Ces copies-là ne se NEUTRALISENT pas : chacune porte une page écrite par +quelqu'un, et la mettre de côté l'effacerait du site. Elles se réparent. +""" + +import os +import shutil +import sys +import tempfile +import unittest + +sys.path.append( + os.path.normpath(os.path.join(os.path.dirname(__file__), "..")) +) +sys.path.append( + os.path.normpath( + os.path.join( + os.path.dirname(__file__), "..", "script", "odoo", "migration" + ) + ) +) + +import check_cow_views as cow # noqa: E402 + +# La copie du client : sa page, sans l'ancrage que la cible attend, et +# avec un appel vers un gabarit que la cible ne livre plus. +ARCH_COPIE = ( + "" + "" + "

Parlons de votre projet

" + "
" + "
" +) + +MODULE_ENFANT = """ + + +""" + +MODULE_PARENT = """ + + + +""" + + +class TestFullKey(unittest.TestCase): + def test_a_bare_id_takes_its_module(self): + self.assertEqual( + cow.full_key("website", "contactus"), "website.contactus" + ) + + def test_a_qualified_id_is_left_alone(self): + self.assertEqual( + cow.full_key("website_crm", "website.contactus"), + "website.contactus", + ) + + def test_nothing_stays_nothing(self): + self.assertIsNone(cow.full_key("website", "")) + self.assertIsNone(cow.full_key("website", None)) + + +class TestScanningTheTargetSources(unittest.TestCase): + def setUp(self): + self.racine = tempfile.mkdtemp(prefix="el_cow_") + self.addCleanup(shutil.rmtree, self.racine, True) + for module, contenu in ( + ("website", MODULE_PARENT), + ("website_crm", MODULE_ENFANT), + ): + chemin = os.path.join(self.racine, "odoo", "addons", module) + os.makedirs(chemin) + with open( + os.path.join(chemin, "views.xml"), "w", encoding="utf-8" + ) as f: + f.write(contenu) + + def scan(self, modules=("website", "website_crm")): + return cow.scan_target_views(self.racine, list(modules)) + + def test_it_lists_what_the_target_ships(self): + declares, _ = self.scan() + self.assertIn("website.contactus", declares) + self.assertIn("website.layout", declares) + + def test_it_maps_a_child_xpath_to_the_parent_it_needs(self): + # C'est LE renseignement qui manquait : quelle vue de la cible + # exige quel ancrage, et dans quel parent. + _, heritages = self.scan() + self.assertEqual( + heritages["website.contactus"], + ["//t[@t-set='contactus_form_values']"], + ) + + def test_a_module_absent_from_the_target_is_skipped(self): + declares, _ = self.scan(["website", "jamais_livre"]) + self.assertIn("website.contactus", declares) + + def test_a_template_without_xpath_adds_no_requirement(self): + _, heritages = self.scan(["website"]) + self.assertEqual(heritages, {}) + + +class TestWillNotRender(unittest.TestCase): + """Le verdict, en remplaçant ce qui touche la base et le disque.""" + + def setUp(self): + self.vrai = ( + cow.query_cow_archs, + cow.installed_modules, + cow.scan_target_views, + ) + + def tearDown(self): + ( + cow.query_cow_archs, + cow.installed_modules, + cow.scan_target_views, + ) = self.vrai + + def poser(self, copies, declares, heritages): + cow.query_cow_archs = lambda d: copies + cow.installed_modules = lambda d: ["website"] + cow.scan_target_views = lambda v, m: (declares, heritages) + + def copie(self, arch=ARCH_COPIE, langues=None): + return [ + ( + 1228, + "website.contactus", + "primary", + 1, + langues or {"en_US": arch}, + ) + ] + + def test_it_names_the_anchor_a_target_child_will_need(self): + self.poser( + self.copie(), + { + "website.contactus", + "website.layout", + "website.company_description", + }, + {"website.contactus": ["//t[@t-set='contactus_form_values']"]}, + ) + risques = cow.will_not_render("db", "odoo18.0") + self.assertEqual(1, len(risques), risques) + self.assertIn("contactus_form_values", risques[0][5]) + + def test_an_anchor_the_copy_already_has_is_not_reported(self): + arch = ARCH_COPIE.replace( + "
", + "
", + ) + self.poser( + self.copie(arch), + { + "website.contactus", + "website.layout", + "website.company_description", + }, + {"website.contactus": ["//t[@t-set='contactus_form_values']"]}, + ) + self.assertEqual([], cow.will_not_render("db", "odoo18.0")) + + def test_it_names_a_template_the_target_no_longer_ships(self): + self.poser( + self.copie(), + {"website.contactus", "website.layout"}, + {}, + ) + risques = cow.will_not_render("db", "odoo18.0") + self.assertEqual(1, len(risques), risques) + self.assertIn("company_description", risques[0][5]) + + def test_another_copy_counts_as_a_known_template(self): + # Un gabarit peut n'exister qu'en base : une copie de site qui en + # appelle une autre n'est pas cassée pour autant. + copies = self.copie() + [ + ( + 99, + "website.company_description", + "primary", + 1, + {"en_US": ""}, + ) + ] + self.poser(copies, {"website.contactus", "website.layout"}, {}) + self.assertEqual([], cow.will_not_render("db", "odoo18.0")) + + def test_a_copy_broken_in_one_language_only_is_still_reported(self): + # Le site rend dans SA langue : réparer l'anglais et laisser le + # français cassé donne une page en 500 et un rapport vert. + sain = ARCH_COPIE.replace( + "
", + "
", + ) + self.poser( + self.copie(langues={"en_US": sain, "fr_CA": ARCH_COPIE}), + { + "website.contactus", + "website.layout", + "website.company_description", + }, + {"website.contactus": ["//t[@t-set='contactus_form_values']"]}, + ) + risques = cow.will_not_render("db", "odoo18.0") + self.assertEqual(1, len(risques), risques) + + def test_a_copy_without_a_key_is_left_alone(self): + self.poser( + [(7, "", "primary", 1, {"en_US": ARCH_COPIE})], + {"website.company_description"}, + {}, + ) + self.assertEqual([], cow.will_not_render("db", "odoo18.0")) + + def test_no_copy_at_all_is_not_an_error(self): + self.poser([], set(), {}) + self.assertEqual([], cow.will_not_render("db", "odoo18.0")) + + +class TestItStaysOutOfTheNeutralizer(unittest.TestCase): + """Ces copies se réparent ; les neutraliser effacerait la page.""" + + def test_the_neutralizer_only_acts_on_the_shape_bucket(self): + with open( + os.path.join( + os.path.dirname(__file__), + "..", + "script", + "odoo", + "migration", + "neutralize_cow_views.py", + ), + encoding="utf-8", + ) as handle: + source = handle.read() + self.assertIn("lst_at_risk, _, _ = analyse(", source) + self.assertNotIn("will_not_render", source) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_fix_cow_render.py b/test/test_fix_cow_render.py index 2534bab..d74b21e 100644 --- a/test/test_fix_cow_render.py +++ b/test/test_fix_cow_render.py @@ -351,5 +351,89 @@ class TestTheReport(unittest.TestCase): self.assertIn("⚠", texte) +class TestItRunsInsideTheMigration(unittest.TestCase): + """La réparation doit tourner APRÈS OpenUpgrade, pas avant. + + Elle prend l'ancrage manquant dans la vue MODULE, qui ne porte l'arch + de la version cible qu'une fois la migration passée. Lancée avant, + elle recopierait l'ancien ancrage — ou rien. + """ + + def pilote(self): + chemin = os.path.join( + os.path.dirname(__file__), + "..", + "script", + "todo", + "todo_upgrade.py", + ) + with open(chemin, encoding="utf-8") as handle: + return handle.read() + + def corps_de_la_reparation(self): + """Le CODE de la méthode, docstring retirée. + + La docstring nomme `wait_at_error=False` pour l'expliquer : un + test qui lit tout le texte le trouve même quand l'argument a + disparu, et croit garder ce qu'il a laissé partir. Mesuré. + """ + import ast + + for noeud in ast.walk(ast.parse(self.pilote())): + if ( + isinstance(noeud, ast.FunctionDef) + and noeud.name == "repair_cow_render" + ): + sans_texte = [ + n + for n in noeud.body + if not ( + isinstance(n, ast.Expr) + and isinstance(n.value, ast.Constant) + and isinstance(n.value.value, str) + ) + ] + return ast.dump(ast.Module(body=sans_texte, type_ignores=[])) + return "" + + def test_the_scan_finds_the_method_at_all(self): + # Sans cette borne, les tests suivants passeraient sur une chaîne + # vide le jour où la méthode est renommée. + self.assertTrue(self.corps_de_la_reparation()) + + def test_the_driver_calls_the_repair(self): + self.assertIn("repair_cow_render", self.pilote()) + + def test_it_applies_rather_than_only_reporting(self): + corps = self.corps_de_la_reparation() + self.assertIn("fix_cow_render.py", corps) + self.assertIn("--apply", corps) + + def test_a_finding_does_not_open_the_error_menu(self): + # Le code 1 de cet outil veut dire « j'ai trouvé », pas « je suis + # tombé ». Sans ce drapeau le pilote s'arrête sur une réparation + # réussie. + corps = self.corps_de_la_reparation() + self.assertIn("wait_at_error", corps) + self.assertIn("value=False", corps) + + def test_it_is_called_after_the_data_migration(self): + source = self.pilote() + migration = source.index("cmd_upgrade,\n new_env=") + appel = source.index("self.repair_cow_render(") + self.assertLess( + migration, + appel, + "la réparation doit suivre OpenUpgrade : avant, la vue module" + " porte encore l'arch de l'ancienne version", + ) + + def test_a_missing_tool_is_not_a_crash(self): + # Un vieux checkout n'a pas le fichier ; la migration ne doit pas + # s'arrêter pour autant. + corps = self.corps_de_la_reparation() + self.assertIn("isfile", corps) + + if __name__ == "__main__": unittest.main() diff --git a/test/test_migration_tools_i18n.py b/test/test_migration_tools_i18n.py index 6d07c4a..8b0b643 100644 --- a/test/test_migration_tools_i18n.py +++ b/test/test_migration_tools_i18n.py @@ -218,11 +218,30 @@ class TestTheExitCodesTheDriverRelieson(unittest.TestCase): def test_check_cow_views_reports_findings_with_1(self): # Le chemin qui compte demande une base ; on vérifie la décision # elle-même, à la source. + # + # On lit le RETURN, pas une ligne recopiée : l'outil a gagné une + # seconde famille de trouvailles (les copies qui rendront 500), + # et figer le texte faisait échouer le test sur un ajout juste. + # Ce qui doit rester vrai, c'est que CHAQUE famille pèse sur le + # code de sortie. + import ast + path = os.path.join( REPO, "script", "odoo", "migration", "check_cow_views.py" ) with open(path) as handle: - self.assertIn("return 1 if lst_at_risk else 0", handle.read()) + arbre = ast.parse(handle.read()) + retours = [ + ast.dump(noeud) + for fonction in ast.walk(arbre) + if isinstance(fonction, ast.FunctionDef) + and fonction.name == "main" + for noeud in ast.walk(fonction) + if isinstance(noeud, ast.Return) + ] + decision = [r for r in retours if "lst_at_risk" in r] + self.assertEqual(1, len(decision), retours) + self.assertIn("lst_no_render", decision[0]) class TestAllToolsKeepWorkingStandalone(unittest.TestCase):