diff --git a/script/analyse/check_migration_quality.py b/script/analyse/check_migration_quality.py index bfdb363..b1c5f62 100755 --- a/script/analyse/check_migration_quality.py +++ b/script/analyse/check_migration_quality.py @@ -333,6 +333,158 @@ def render_missing(etat, colour=False, limit=60): return "\n".join(lignes) +# Ce qu'Odoo déplace ou retire de lui-même, d'une version à l'autre. +# +# Sans cette carte, l'outil annonçait « 81 tables ont perdu des lignes » +# et la plus grosse d'entre elles — ir_translation, 32 984 lignes — était +# une refonte voulue par l'éditeur. Les vraies questions se noyaient dans +# les fausses, ce qui est la façon la plus sûre de ne pas les voir. +# +# Chaque entrée a été VÉRIFIÉE sur une migration réelle 12 → 18, en +# comptant les deux côtés. Une carte écrite de mémoire vaudrait moins que +# pas de carte : elle expliquerait des pertes qui n'en sont pas. +SEMANTIC_MAP = ( + # Fusions : les enregistrements continuent, ailleurs et autrement. + { + "since": 13, + "table": "account_invoice", + "into": "account_move", + "kind": "merged", + "why": "invoices became journal entries", + }, + { + "since": 13, + "table": "account_invoice_line", + "into": "account_move_line", + "kind": "merged", + "why": "invoices became journal entries", + }, + { + "since": 13, + "table": "account_invoice_tax", + "into": "account_move_line", + "kind": "merged", + "why": "invoices became journal entries", + }, + # Renommages : les mêmes enregistrements, sous un autre nom. + { + "since": 17, + "table": "mail_channel", + "into": "discuss_channel", + "kind": "renamed", + "why": "Discuss was renamed", + }, + { + "since": 17, + "table": "mail_channel_partner", + "into": "discuss_channel_member", + "kind": "renamed", + "why": "Discuss was renamed", + }, + { + "since": 13, + "table": "website_redirect", + "into": "website_rewrite", + "kind": "renamed", + "why": "redirections were reworked", + }, + { + "since": 13, + "table": "crm_lead_tag", + "into": "crm_tag", + "kind": "renamed", + "why": "tags became shared", + }, + # Retraits : la fonction a quitté la base pour du code. + { + "since": 16, + "table": "ir_translation", + "into": None, + "kind": "retired", + "why": "translations moved into jsonb columns", + }, + { + "since": 17, + "table": "account_tax_template", + "into": None, + "kind": "retired", + "why": "chart templates left the database", + }, + { + "since": 17, + "table": "account_account_template", + "into": None, + "kind": "retired", + "why": "chart templates left the database", + }, + { + "since": 17, + "table": "account_chart_template", + "into": None, + "kind": "retired", + "why": "chart templates left the database", + }, + { + "since": 17, + "table": "account_fiscal_position_template", + "into": None, + "kind": "retired", + "why": "chart templates left the database", + }, + { + "since": 17, + "table": "account_fiscal_position_tax_template", + "into": None, + "kind": "retired", + "why": "chart templates left the database", + }, + { + "since": 17, + "table": "account_fiscal_position_account_template", + "into": None, + "kind": "retired", + "why": "chart templates left the database", + }, + { + "since": 15, + "table": "stock_inventory", + "into": "stock_quant", + "kind": "merged", + "why": "inventory adjustments became quants", + }, + { + "since": 15, + "table": "stock_inventory_line", + "into": "stock_quant", + "kind": "merged", + "why": "inventory adjustments became quants", + }, +) + + +def as_version(etat): + """« 18.0 » -> 18. None si l'on ne sait pas.""" + try: + return int(float((etat or {}).get("odoo") or 0)) or None + except (TypeError, ValueError): + return None + + +def explain_loss(table, version): + """Ce qu'Odoo a fait de cette table à cette version, ou None. + + `version` est celle d'ARRIVÉE : une refonte de la 17 n'explique rien + d'un palier 13 → 14, et l'accepter ferait taire une vraie perte sous + prétexte qu'elle porte le nom d'une table refondue plus tard. + """ + if not version: + return None + for entree in SEMANTIC_MAP: + if entree["table"] == table and entree["since"] <= version: + return entree + return None + + def compare(avant, apres): """Ce qui a été gagné et ce qui a été perdu entre deux paliers.""" if not avant.get("exists") or not apres.get("exists"): @@ -341,18 +493,34 @@ def compare(avant, apres): mod_avant, mod_apres = set(avant["model"]), set(apres["model"]) tbl_avant, tbl_apres = avant["table"], apres["table"] + # TOUJOURS quatre éléments, le dernier étant l'explication ou None. + # Un tuple de taille variable obligerait chaque lecteur à s'en méfier. lignes_perdues = [] for table, nombre in sorted(tbl_avant.items()): reste = tbl_apres.get(table) if reste is None and nombre: - lignes_perdues.append((table, nombre, 0)) + lignes_perdues.append((table, nombre, 0, None)) elif reste is not None and reste < nombre: - lignes_perdues.append((table, nombre, reste)) + lignes_perdues.append((table, nombre, reste, None)) lignes_gagnees = [ (table, tbl_avant.get(table, 0), nombre) for table, nombre in sorted(tbl_apres.items()) if nombre > tbl_avant.get(table, 0) ] + version = as_version(apres) + for index, (table, debut, fin, _rien) in enumerate(lignes_perdues): + connu = explain_loss(table, version) + if not connu: + continue + # Une fusion qui n'a PAS grossi la table d'accueil n'explique + # rien : on garde l'explication et on dit qu'elle ne tient pas. + arrivee = connu["into"] + recue = ( + apres["table"].get(arrivee, 0) - avant["table"].get(arrivee, 0) + if arrivee + else None + ) + lignes_perdues[index] = (table, debut, fin, {**connu, "gained": recue}) return { "modules_lost": sorted(inst_avant - inst_apres), "modules_gained": sorted(inst_apres - inst_avant), @@ -384,7 +552,9 @@ def probable_renames(perdues, gagnees): cherche un dégât là où il n'y en a pas. """ disparues = { - table: avant for table, avant, apres in perdues if apres == 0 and avant + table: avant + for table, avant, apres, _connu in perdues + if apres == 0 and avant } apparues = { table: apres for table, avant, apres in gagnees if avant == 0 and apres @@ -528,21 +698,51 @@ def render_compare(diff, colour, limit=8): # LE signal qui compte : un module en moins se voit, une table qui # passe de quatre mille lignes à zéro ne se voit nulle part. # - # Une table probablement renommée reste dans la LISTE, annotée. L'en - # retirer était le vrai danger : un rapprochement faux — et il y en - # a eu — aurait fait disparaître une perte réelle du rapport. + # Les pertes EXPLIQUÉES sont séparées des autres, jamais retirées. + # Sans cette séparation on lisait « 81 tables ont perdu des lignes » + # dont la plus grosse — ir_translation, 32 984 lignes — était une + # refonte voulue par Odoo : les vraies questions se noyaient dans + # les fausses, ce qui est la façon la plus sûre de ne pas les voir. vers = {a: b for a, b, _n in diff["renamed"]} - lignes.append( - f" {paint('▼', 'fail', colour)} {len(perdues)}" - f" {t('table(s) lost rows')} :" - ) - for table, avant, apres in perdues[:limit]: - note = ( - f" ↻ {t('probably renamed to')} {vers[table]}" - if table in vers - else "" + ouvertes = [item for item in perdues if not item[3]] + connues = [item for item in perdues if item[3]] + if ouvertes: + lignes.append( + f" {paint('▼', 'fail', colour)} {len(ouvertes)}" + f" {t('table(s) lost rows, unexplained')} :" ) - lignes.append(f" {table:<40} {avant:>8} → {apres}{note}") + for table, avant, apres, _rien in ouvertes[:limit]: + note = ( + f" ↻ {t('probably renamed to')} {vers[table]}" + if table in vers + else "" + ) + lignes.append(f" {table:<40} {avant:>8} → {apres}{note}") + if len(ouvertes) > limit: + lignes.append(f" … {len(ouvertes) - limit} {t('more')}") + if connues: + lignes.append( + f" {paint('▽', 'dim', colour)} {len(connues)}" + f" {t('table(s) Odoo moved or retired')} :" + ) + for table, avant, apres, connu in connues[:limit]: + if connu["into"]: + recue = connu.get("gained") + # Une fusion dont la table d'accueil n'a PAS grossi + # n'explique rien : le dire, plutôt que de classer la + # perte comme attendue et passer à autre chose. + accueil = ( + f"+{recue} {t('there')}" + if recue and recue > 0 + else paint(t("but it gained nothing"), "fail", colour) + ) + ou = f"→ {connu['into']} ({accueil})" + else: + ou = t("retired from the database") + lignes.append(f" {table:<40} {avant:>8} → {apres} {ou}") + lignes.append(f" {connu['why']}") + if len(connues) > limit: + lignes.append(f" … {len(connues) - limit} {t('more')}") delta = diff["delta"] lignes.append( " " diff --git a/script/analyse/check_migration_quality_tui.py b/script/analyse/check_migration_quality_tui.py index fee2360..4328936 100644 --- a/script/analyse/check_migration_quality_tui.py +++ b/script/analyse/check_migration_quality_tui.py @@ -67,10 +67,13 @@ def rows(lst_snapshot): # Le précédent voyage avec la ligne : le panneau en a besoin pour # écrire « 171 (−43) », et le recalculer là-bas ferait deux # sources pour la même comparaison. + # La colonne compte les pertes INEXPLIQUÉES : afficher 81 quand + # 79 sont des refontes voulues par Odoo ferait fuir le lecteur du + # seul chiffre qui demande une réponse. perdu = ( 0 if diff is None or diff.get("unavailable") - else len(diff["rows_lost"]) + else len([item for item in diff["rows_lost"] if not item[3]]) ) lst.append( { diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index aa3218e..c8c45d3 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -5783,6 +5783,26 @@ TRANSLATIONS = { "fr": "Choisissez un palier pour voir ses fichiers absents.", "en": "Pick a step to see its missing files.", }, + "table(s) lost rows, unexplained": { + "fr": "table(s) ont perdu des lignes, sans explication", + "en": "table(s) lost rows, unexplained", + }, + "table(s) Odoo moved or retired": { + "fr": "table(s) qu'Odoo a déplacées ou retirées", + "en": "table(s) Odoo moved or retired", + }, + "retired from the database": { + "fr": "retirée de la base", + "en": "retired from the database", + }, + "but it gained nothing": { + "fr": "mais elle n'a rien reçu", + "en": "but it gained nothing", + }, + "there": { + "fr": "là-bas", + "en": "there", + }, "Clean the database before testing the pages?": { "fr": "Nettoyer la base avant de tester les pages ?", "en": "Clean the database before testing the pages?", diff --git a/test/test_check_migration_quality.py b/test/test_check_migration_quality.py index b783271..5f5522f 100644 --- a/test/test_check_migration_quality.py +++ b/test/test_check_migration_quality.py @@ -122,17 +122,19 @@ class TestWhatIsGainedAndLost(Base): def test_a_table_that_empties_is_reported(self): # LE signal : un module en moins se voit, une table qui passe de # cent lignes à zéro ne se voit nulle part. + # Une table NEUTRE : le mécanisme se teste sans la carte + # sémantique, qui a ses propres tests. diff = quality.compare( - snapshot(table={"account_invoice": 651}), - snapshot(table={"account_invoice": 0}), + snapshot(table={"ma_table": 651}), + snapshot(table={"ma_table": 0}), ) - self.assertEqual(diff["rows_lost"], [("account_invoice", 651, 0)]) + self.assertEqual(diff["rows_lost"], [("ma_table", 651, 0, None)]) def test_a_table_that_disappears_counts_as_emptied(self): diff = quality.compare( - snapshot(table={"account_invoice": 651}), snapshot(table={}) + snapshot(table={"ma_table": 651}), snapshot(table={}) ) - self.assertEqual(diff["rows_lost"], [("account_invoice", 651, 0)]) + self.assertEqual(diff["rows_lost"], [("ma_table", 651, 0, None)]) def test_an_empty_table_that_disappears_is_not_a_loss(self): # Rien à perdre : le signaler noierait les vraies pertes. @@ -207,7 +209,7 @@ class TestNotCryingWolfOnRenames(Base): self.assertEqual( diff["renamed"], [("muk_dms_directory", "dms_directory", 7)] ) - self.assertIn(("muk_dms_directory", 7, 0), diff["rows_lost"]) + self.assertIn(("muk_dms_directory", 7, 0, None), diff["rows_lost"]) def test_the_report_says_it_is_only_probable(self): diff = quality.compare( @@ -219,6 +221,142 @@ class TestNotCryingWolfOnRenames(Base): self.assertIn("probably renamed", texte) +class TestTheSemanticMap(Base): + """« 81 tables ont perdu des lignes » noyait les vraies questions. + + La plus grosse d'entre elles — `ir_translation`, 32 984 lignes — est + une refonte voulue par Odoo en 16. Mettre les refontes et les pertes + réelles sur le même plan est la façon la plus sûre de ne pas voir les + secondes. + """ + + def perte( + self, + table, + avant_n, + apres_n, + version="18.0", + cible=None, + cible_avant=0, + cible_apres=0, + ): + tbl_avant = {table: avant_n} + tbl_apres = {} if apres_n == 0 else {table: apres_n} + if cible: + tbl_avant[cible] = cible_avant + tbl_apres[cible] = cible_apres + return quality.compare( + snapshot(odoo="12.0", table=tbl_avant), + snapshot(odoo=version, table=tbl_apres), + ) + + def test_a_known_merge_is_explained(self): + diff = self.perte( + "account_invoice", + 651, + 0, + cible="account_move", + cible_avant=1371, + cible_apres=1812, + ) + connu = [x for x in diff["rows_lost"] if x[0] == "account_invoice"][0] + self.assertIsNotNone(connu[3]) + self.assertEqual(connu[3]["into"], "account_move") + self.assertEqual(connu[3]["gained"], 441) + + def test_a_retired_table_is_explained_without_a_target(self): + diff = self.perte("ir_translation", 32984, 0) + connu = [x for x in diff["rows_lost"] if x[0] == "ir_translation"][0] + self.assertIsNotNone(connu[3]) + self.assertIsNone(connu[3]["into"]) + + def test_the_map_does_not_apply_BEFORE_its_version(self): + """Une refonte de la 16 n'explique rien d'un palier 12 → 13. + + L'accepter ferait taire une vraie perte sous prétexte que la table + porte le nom d'une autre, refondue trois versions plus tard. + """ + diff = self.perte("ir_translation", 32984, 0, version="13.0") + connu = [x for x in diff["rows_lost"] if x[0] == "ir_translation"][0] + self.assertIsNone(connu[3]) + + def test_it_applies_AT_its_version(self): + diff = self.perte("ir_translation", 32984, 0, version="16.0") + connu = [x for x in diff["rows_lost"] if x[0] == "ir_translation"][0] + self.assertIsNotNone(connu[3]) + + def test_an_explained_loss_is_STILL_in_the_list(self): + # La règle qui vaut plus que tout : expliquer n'est pas cacher. + diff = self.perte("ir_translation", 32984, 0) + self.assertIn("ir_translation", [x[0] for x in diff["rows_lost"]]) + + def test_the_partition_loses_nothing(self): + diff = quality.compare( + snapshot( + odoo="12.0", table={"ir_translation": 100, "ma_table": 50} + ), + snapshot(odoo="18.0", table={}), + ) + perdues = diff["rows_lost"] + ouvertes = [x for x in perdues if not x[3]] + connues = [x for x in perdues if x[3]] + self.assertEqual(len(perdues), len(ouvertes) + len(connues)) + self.assertEqual(len(perdues), 2) + + def test_a_merge_whose_target_gained_NOTHING_is_flagged(self): + """Le cas qui compte : la carte dit où les données sont allées. + + Si elles n'y sont pas, l'explication ne tient pas — et la classer + « attendue » puis passer à autre chose serait exactement l'erreur + que la carte devait empêcher. + """ + diff = self.perte( + "account_invoice", + 651, + 0, + cible="account_move", + cible_avant=1371, + cible_apres=1371, + ) + connu = [x for x in diff["rows_lost"] if x[0] == "account_invoice"][0] + self.assertEqual(connu[3]["gained"], 0) + texte = "\n".join(quality.render_compare(diff, colour=False)) + self.assertIn("gained nothing", texte) + + def test_the_report_puts_the_unexplained_FIRST(self): + diff = quality.compare( + snapshot( + odoo="12.0", table={"ir_translation": 100, "ma_table": 50} + ), + snapshot(odoo="18.0", table={}), + ) + texte = "\n".join(quality.render_compare(diff, colour=False)) + self.assertLess(texte.index("unexplained"), texte.index("moved or")) + + def test_an_unknown_table_stays_unexplained(self): + diff = self.perte("ma_table_a_moi", 50, 0) + self.assertIsNone(diff["rows_lost"][0][3]) + + def test_every_entry_of_the_map_is_complete(self): + # Une entrée sans « why » expliquerait sans dire pourquoi. + for entree in quality.SEMANTIC_MAP: + for cle in ("since", "table", "into", "kind", "why"): + self.assertIn(cle, entree, entree) + self.assertTrue(entree["why"], entree) + self.assertIn(entree["kind"], ("merged", "renamed", "retired")) + + def test_the_column_counts_only_what_needs_an_answer(self): + # Afficher 81 quand 14 sont des refontes voulues ferait fuir le + # lecteur du seul chiffre qui demande une réponse. + lst = [ + snapshot( + odoo="12.0", table={"ir_translation": 100, "ma_table": 50} + ), + snapshot(odoo="18.0", table={}), + ] + self.assertEqual(qtui.rows(lst)[1]["detail"], "1") + + class TestTheOverallReport(Base): def test_it_compares_the_ENDS_not_the_sum_of_steps(self): """Un module retiré en 15 puis remis en 17 n'a rien perdu.