From a2cedfa64be8944d0da85f86c0771e241d431e90 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Wed, 12 Aug 2026 00:54:11 -0400 Subject: [PATCH] [ADD] analyse: diff each website copy against the view it shadows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The analysis counted 62 website copies and pointed at other tools to judge them. But the comparison that matters for a copy needs no registry at all: both arches are in the database, paired by key — the very pairing Odoo makes. So it works where the module-source comparison is refused, on a database whose version differs from the checkout, and on a backup zip. On the 12.0 database at hand, where the other comparison cannot run: 47 of the 62 copies have a twin, all 47 compared in under a second, 12 differ. The other 35 are byte-for-byte identical to their module view — they carry no customization at all, which is what decides whether neutralizing costs anything. The fields are named as the module comparison names them, so the full-screen browser and the text report work without knowing which comparison produced the data. A copy without a twin is left alone: it is a page made in the editor, with nothing to compare against. Checked on that database and on a real backup; 8 tests, including that re-indentation alone is not a difference and that a missing arch yields no verdict rather than « identical ». --- FR --- L'analyse comptait 62 copies de site web et renvoyait vers d'autres outils pour les juger. Or la comparaison qui compte pour une copie n'a besoin d'aucun registre : les deux arch sont dans la base, appariées par la clé — exactement l'appariement que fait Odoo. Elle marche donc là où celle avec la source du module est refusée : une base dont la version diffère du checkout, et une sauvegarde zip. Sur la base 12.0 en cours, où l'autre comparaison ne peut pas tourner : 47 des 62 copies ont une jumelle, les 47 comparées en moins d'une seconde, 12 diffèrent. Les 35 autres sont identiques octet pour octet à leur vue de module — elles ne portent aucune personnalisation, ce qui décide si les neutraliser coûte quelque chose. Les champs portent les noms de la comparaison avec le module, donc l'écran de navigation et le rapport texte marchent sans savoir laquelle des deux a produit la donnée. Une copie sans jumelle est laissée telle quelle : c'est une page faite dans l'éditeur, il n'y a rien à quoi la comparer. Vérifié sur cette base et sur une vraie sauvegarde ; 8 tests, dont qu'une ré-indentation seule n'est pas un écart et qu'une arch absente ne rend aucun verdict plutôt que « identique ». Assisted-by: Claude Opus 5 --- script/analyse/analyse_view_custom.py | 140 +++++++++++++++++++++++++- script/todo/todo.py | 39 ++++++- test/test_analyse_view_custom.py | 90 +++++++++++++++++ 3 files changed, 264 insertions(+), 5 deletions(-) diff --git a/script/analyse/analyse_view_custom.py b/script/analyse/analyse_view_custom.py index fe31310..7fdb22d 100755 --- a/script/analyse/analyse_view_custom.py +++ b/script/analyse/analyse_view_custom.py @@ -334,9 +334,114 @@ def add_reference_arch(database, lst_finding, config_path=None, timeout=600): return "orm", None +def attach_cow_twin_diff(lst_finding, dct_twin, dct_copy_arch): + """Comparer chaque copie COW à la vue de module qu'elle masque. + + C'est LA comparaison qui compte pour une copie, et elle n'a besoin d'aucun + registre : les deux côtés sont dans la base, appariés par leur clé — + exactement l'appariement que fait Odoo, et la raison pour laquelle + renommer une clé suffit à désapparier une copie. + + Elle marche donc là où la comparaison avec la source du module est + impossible : une base dont la version diffère du checkout, et une + sauvegarde .zip, qui portent l'une comme l'autre les deux arch. + + Une copie sans jumelle est laissée telle quelle : c'est une page faite + dans l'éditeur web, il n'y a rien à quoi la comparer. + + Fonction pure — les deux provenances lui passent leurs dictionnaires et + concluent donc la même chose des mêmes faits. Renvoie le nombre de copies + comparées. + """ + n_compared = 0 + for row in lst_finding: + if row["category"] != "website_cow_copy": + continue + twin = dct_twin.get(row.get("key")) + if not twin: + continue + twin_id, twin_arch = twin + copy_arch = dct_copy_arch.get(row["id"]) + if copy_arch is None: + continue + differs, comparable = arch_differs(twin_arch, copy_arch) + # Mêmes noms de champs que la comparaison avec la source du module : + # l'écran de navigation et le rendu texte marchent alors sans savoir + # laquelle des deux a produit la donnée. + row["arch_ref"] = twin_arch + row["arch_db_text"] = copy_arch + row["twin_id"] = twin_id + row["comparable"] = comparable + row["differs"] = differs + if comparable: + row["diff_stats"] = diff_stats(side_by_side(twin_arch, copy_arch)) + n_compared += 1 + return n_compared + + +def _cow_twin_arch(database, lst_key, **kwargs): + """{clé: (id, arch)} des vues de module masquées par ces copies.""" + if not lst_key: + return {} + values = ", ".join( + "'" + k.replace("'", "''") + "'" for k in sorted(set(lst_key)) + ) + rows = json_query( + database, + f""" + SELECT DISTINCT ON (v.key) + v.key AS key, + v.id AS id, + v.arch_db::text AS arch + FROM ir_ui_view v + WHERE v.key IN ({values}) AND v.website_id IS NULL + ORDER BY v.key, v.id + """, + **kwargs, + ) + return {r["key"]: (r["id"], normalise_arch(r["arch"])) for r in rows} + + +def _cow_copy_arch(database, lst_id, **kwargs): + """{id: arch} des seules copies retenues. + + Rapatrier l'arch de toutes les vues ferait une ligne de sortie de plusieurs + centaines de mégaoctets ; celle des seules copies COW en fait quelques-uns. + """ + if not lst_id: + return {} + ids = ", ".join(str(int(i)) for i in lst_id) + rows = json_query( + database, + "SELECT id AS id, arch_db::text AS arch" + f" FROM ir_ui_view WHERE id IN ({ids})", + **kwargs, + ) + return {r["id"]: normalise_arch(r["arch"]) for r in rows} + + +def add_cow_twin_diff(database, lst_finding, **kwargs): + """Comparer les copies COW d'une BASE à leur jumelle. Deux requêtes.""" + lst_copy = [ + row + for row in lst_finding + if row["category"] == "website_cow_copy" and row.get("has_module_twin") + ] + if not lst_copy: + return 0 + dct_twin = _cow_twin_arch( + database, [row["key"] for row in lst_copy if row.get("key")], **kwargs + ) + dct_copy = _cow_copy_arch( + database, [row["id"] for row in lst_copy], **kwargs + ) + return attach_cow_twin_diff(lst_finding, dct_twin, dct_copy) + + def collect( database, with_diff=False, + with_cow_diff=False, scope="flagged", config_path=None, timeout=120, @@ -370,6 +475,12 @@ def collect( if category in ACTIONABLE: lst_finding.append(row) + n_cow_compared = 0 + if with_cow_diff: + # Indépendant de la voie ORM : les deux arch sont en base, donc ceci + # marche même quand la version du checkout interdit l'autre. + n_cow_compared = add_cow_twin_diff(database, lst_finding, **kwargs) + arch_ref_source, arch_ref_error = "none", None checkout = checkout_odoo_version() if with_diff: @@ -455,6 +566,7 @@ def collect( "scope": scope, "arch_ref_error": arch_ref_error, "n_identical_after_canonical": n_identical, + "n_cow_compared": n_cow_compared, "n_views": len(lst_view), "counts": dct_count, "findings": lst_finding, @@ -528,11 +640,30 @@ def collect_from_backup(zip_path): if category in ACTIONABLE: lst_finding.append(row) + # Le dump porte les arch des deux côtés : la comparaison COW est donc + # possible depuis un zip, là où celle avec la source du module ne l'est + # pas faute de registre. + dct_twin = {} + for row in dct_rows["ir_ui_view"]: + if row.get("key") and row.get("website_id") in (None, ""): + dct_twin.setdefault( + row["key"], + (row.get("id"), normalise_arch(row.get("arch_db"))), + ) + dct_copy = { + (int(r.get("id")) if (r.get("id") or "").isdigit() else r.get("id")): ( + normalise_arch(r.get("arch_db")) + ) + for r in dct_rows["ir_ui_view"] + } + n_cow_compared = attach_cow_twin_diff(lst_finding, dct_twin, dct_copy) + return { "tool": "analyse_view_custom", "version": 1, "database": os.path.basename(zip_path), "source": "backup", + "n_cow_compared": n_cow_compared, "backup_path": zip_path, "odoo_version": backup_version(dct_rows, manifest), "checkout_version": checkout_odoo_version(), @@ -792,6 +923,12 @@ def main(argv=None): default="flagged", help=t("which views to compare (default: flagged)"), ) + parser.add_argument( + "--cow-diff", + dest="cow_diff", + action="store_true", + help=t("compare each website copy with the module view it shadows"), + ) parser.add_argument( "--strict", action="store_true", @@ -814,7 +951,8 @@ def main(argv=None): else: data = collect( config.database, - with_diff=config.diff or config.tui, + with_diff=config.diff, + with_cow_diff=config.cow_diff or config.tui, scope=config.scope, config_path=config.config, ) diff --git a/script/todo/todo.py b/script/todo/todo.py index 07ab2f4..3d19aa2 100755 --- a/script/todo/todo.py +++ b/script/todo/todo.py @@ -10374,6 +10374,8 @@ class TODO: def run(**kwargs): try: + # Une sauvegarde compare déjà ses copies COW à la lecture : + # les deux arch sont dans le dump, il n'y a rien à demander. state["data"] = ( analyse.collect_from_backup(database) if is_backup @@ -10430,6 +10432,19 @@ class TODO: print(analyse.render(data, verbose=True, hints=False)) lst_choice = [{"prompt_description": t("Show every view")}] + # La comparaison des copies COW n'a besoin d'aucun registre : les deux + # arch sont dans la base, appariées par leur clé. Elle est donc offerte + # partout, y compris sur une sauvegarde et sur une base dont la version + # diffère du checkout — là où l'autre comparaison est refusée. + has_cow = bool(state["data"]["counts"].get("website_cow_copy")) + if has_cow: + lst_choice.append( + { + "prompt_description": t( + "Compare the website copies with the view they shadow" + ) + } + ) if can_compare: lst_choice += [ { @@ -10446,19 +10461,35 @@ class TODO: ] lst_choice.append({"prompt_description": t("Export as JSON")}) + def compare_cow(): + if not run(with_cow_diff=True): + return + state["tried"] = True + data = state["data"] + print(analyse.render(data, hints=False)) + n = data.get("n_cow_compared") or 0 + n_diff = len([r for r in data["findings"] if r.get("differs")]) + print( + f" {n} {t('website copies compared with their module view,')}" + f" {n_diff} {t('differ.')}" + ) + def handler(rank): data = state["data"] + offset = 1 if has_cow else 0 if rank == 1: print(analyse.render(data, verbose=True, hints=False)) - elif not can_compare or rank == len(lst_choice): + elif has_cow and rank == 2: + compare_cow() + elif rank == len(lst_choice): self._analyse_export_json( data, os.path.basename(database), "view_custom" ) - elif rank == 2: + elif can_compare and rank == 2 + offset: compare("flagged") - elif rank == 3: + elif can_compare and rank == 3 + offset: compare("all") - elif rank == 4: + elif can_compare and rank == 4 + offset: browse() self._analyse_follow_up(lst_choice, handler) diff --git a/test/test_analyse_view_custom.py b/test/test_analyse_view_custom.py index 3bc8bed..a85a275 100644 --- a/test/test_analyse_view_custom.py +++ b/test/test_analyse_view_custom.py @@ -268,3 +268,93 @@ class TestRender(unittest.TestCase): if __name__ == "__main__": unittest.main() + + +class TestCowTwinDiff(unittest.TestCase): + """Comparer une copie de site web à la vue de module qu'elle masque. + + C'est LA comparaison qui compte pour une copie, et elle n'a besoin d'aucun + registre : les deux arch sont dans la base, appariées par la clé. Elle + marche donc là où la comparaison avec la source du module est refusée — + une base dont la version diffère du checkout, et une sauvegarde .zip. + """ + + def finding(self, **override): + row = view( + id=10, key="website.homepage", website_id=1, has_module_twin=True + ) + row["category"], row["reason"] = A.classify(row) + row.update(override) + return row + + def test_a_copy_that_differs_is_measured(self): + row = self.finding() + n = A.attach_cow_twin_diff( + [row], + {"website.homepage": (5, "
")}, + {10: "
"}, + ) + self.assertEqual(n, 1) + self.assertTrue(row["differs"]) + self.assertTrue(row["comparable"]) + self.assertEqual(row["twin_id"], 5) + self.assertEqual( + row["diff_stats"]["added"] + row["diff_stats"]["changed"], 1 + ) + + def test_a_copy_identical_to_its_twin(self): + # 35 des 62 copies d'une vraie base sont dans ce cas : elles ne + # portent aucune personnalisation, et le dire change la décision. + row = self.finding() + A.attach_cow_twin_diff( + [row], {"website.homepage": (5, "")}, {10: ""} + ) + self.assertFalse(row["differs"]) + self.assertTrue(row["comparable"]) + + def test_indentation_alone_is_not_a_difference(self): + row = self.finding() + A.attach_cow_twin_diff( + [row], + {"website.homepage": (5, "
")}, + {10: "\n
\n"}, + ) + self.assertFalse(row["differs"]) + + def test_a_copy_without_a_twin_is_left_alone(self): + # Une page faite dans l'éditeur web n'a rien à quoi se comparer. + row = self.finding(has_module_twin=False) + n = A.attach_cow_twin_diff([row], {}, {10: ""}) + self.assertEqual(n, 0) + self.assertNotIn("arch_ref", row) + self.assertNotIn("differs", row) + + def test_a_view_that_is_not_a_copy_is_left_alone(self): + row = view(id=11, key="sale.order_form", arch_fs="x.xml") + row["category"], row["reason"] = A.classify(row) + n = A.attach_cow_twin_diff( + [row], {"sale.order_form": (1, "
")}, {11: ""} + ) + self.assertEqual(n, 0) + self.assertNotIn("differs", row) + + def test_it_uses_the_same_field_names_as_the_module_comparison(self): + # Le nom des champs EST le contrat : l'écran de navigation et le rendu + # texte marchent alors sans savoir laquelle des deux comparaisons a + # produit la donnée. + row = self.finding() + A.attach_cow_twin_diff( + [row], {"website.homepage": (5, "")}, {10: ""} + ) + for field in ("arch_ref", "arch_db_text", "differs", "comparable"): + self.assertIn(field, row, field) + + def test_a_missing_arch_is_not_a_false_verdict(self): + # Sans l'arch de la copie, il n'y a pas eu de comparaison : ne rien + # conclure vaut mieux que conclure « identique ». + row = self.finding() + n = A.attach_cow_twin_diff( + [row], {"website.homepage": (5, "")}, {} + ) + self.assertEqual(n, 0) + self.assertNotIn("differs", row)