From ec29781b7a64d085f7f5bbe6bd4c9b789d57b4c5 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Mon, 17 Aug 2026 04:49:32 -0400 Subject: [PATCH] [FIX] migration: reset the copy that is actually stale, and say when a key misses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Answering « all » reset one copy of two and reported success. Two defects behind it, both mine. A requested key matching no finding did nothing, silently: the tool returned early on « nothing drifted » and never looked. Detection is differential — it only sees a copy whose CHILD breaks — so a stale copy without children escapes it. A key can now be reset outside detection, an unknown one is named, and the exit code is 2. And the culprit is not always the parent. On /contactus the parent was identical to its module view; the CHILD held the stale arch. Both are proposed now, parent first — it is the more common case. Measured on the migration: 33 of 33 public URLs answer. --- FR --- [FIX] migration : réinitialiser la copie vraiment périmée, et signaler une clé sans objet Répondre « toutes » réinitialisait une copie sur deux et annonçait un succès. Deux défauts derrière, tous deux à moi. Une clé demandée ne correspondant à aucun constat ne faisait rien, en silence : l'outil sortait sur « rien n'a dérivé » sans jamais chercher. La détection est différentielle — elle ne voit qu'une copie dont un ENFANT casse — donc une copie périmée sans enfant lui échappe. Une clé peut désormais être réinitialisée hors détection, une clé inconnue est nommée, et le code de sortie vaut 2. Et le coupable n'est pas toujours le parent. Sur /contactus, le parent était identique à sa vue module ; c'est l'ENFANT qui portait l'arch périmée. Les deux sont proposés, le parent d'abord — le cas le plus fréquent. Mesuré sur la migration : 33 URL publiques sur 33 répondent. Assisted-by: Claude Opus 5 --- .../odoo/migration/reset_stale_cow_views.py | 86 ++++++++++++++++--- script/odoo/migration/smoke_public_url.py | 11 ++- script/todo/todo_i18n.py | 16 ++++ test/test_smoke_public_url.py | 23 ++++- 4 files changed, 120 insertions(+), 16 deletions(-) diff --git a/script/odoo/migration/reset_stale_cow_views.py b/script/odoo/migration/reset_stale_cow_views.py index 01021ed..f4cc956 100755 --- a/script/odoo/migration/reset_stale_cow_views.py +++ b/script/odoo/migration/reset_stale_cow_views.py @@ -227,6 +227,27 @@ def analyse(database): return findings +def find_copy_by_key(views, key): + """(copie COW, jumelle module) pour cette clé, ou (None, None). + + La détection différentielle ne voit qu'une copie dont un ENFANT casse. + Une copie périmée sans enfant lui échappe — mesuré : la copie de + `website_crm.contactus_form` était plus petite d'un tiers que sa + jumelle, et c'est elle qui rendait /contactus en 500. Demander sa + réinitialisation par clé doit donc marcher même hors détection. + """ + copy = twin = None + for row in views.values(): + if row.get("key") != key: + continue + if row.get("website_id"): + if copy is None or row["id"] < copy["id"]: + copy = row + elif twin is None or row["id"] < twin["id"]: + twin = row + return copy, twin + + def show_diff(module_view, cow_view): """Module arch vs copy: what the copy would gain and lose on a reset.""" diff = difflib.unified_diff( @@ -317,18 +338,26 @@ def main(): print(cow_view["key"]) return 1 if findings else 0 - if not findings: + # « Rien détecté » ne veut pas dire « rien à faire » quand une clé est + # demandée : la détection différentielle ne voit qu'une copie dont un + # ENFANT casse, et une copie périmée sans enfant lui échappe. Sortir ici + # faisait taire la demande — on croyait la copie réinitialisée. + if not findings and not (set(config.reset) - {"all"}): print(f"✅ {t('No COW copy has drifted from its module view.')}") return 0 + if not findings: + print(f"ℹ {t('No COW copy has drifted from its module view.')}") - print( - f"⚠️ {len(findings)} {t('COW copy(ies) drifted from their module')}" - f" {t('view in')} {config.database}" - ) - print( - f" {t('Odoo surfaces this when the module view is rewritten (a')}" - f" {t('version bump) or when the page is rendered.')}\n" - ) + if findings: + print( + f"⚠️ {len(findings)}" + f" {t('COW copy(ies) drifted from their module')}" + f" {t('view in')} {config.database}" + ) + print( + f" {t('Odoo surfaces this when the module view is rewritten')}" + f" {t('(a version bump) or when the page is rendered.')}\n" + ) for cow_view, module_view, broken in findings: twin = ( f"module id={module_view['id']}" @@ -357,9 +386,15 @@ def main(): "private", "odoo", "migration", config.database, "cow_reset" ) done = 0 + honoured = set() + missed = [] + # Les vues brutes servent aux clés que la détection ne voit pas ; on ne + # les lit que si l'on va s'en servir. + views = fetch_views(config.database) if wanted - {"all"} else {} for cow_view, module_view, _broken in findings: if "all" not in wanted and cow_view["key"] not in wanted: continue + honoured.add(cow_view["key"]) if not module_view: print( f"⏭ {cow_view['key']} :" @@ -377,13 +412,44 @@ def main(): done += 1 print(f"✅ {t('reset')} id={cow_view['id']} ({cow_view['key']})") print(f" {t('previous arch saved to')} {path}") + # Une clé demandée qui ne correspond à rien N'EST PAS un succès. Elle + # l'était : la commande tournait, ne faisait rien, et se taisait. On a + # donc cru une copie réinitialisée alors que /contactus rendait encore + # 500 — mesuré sur une vraie migration. + for key in sorted(wanted - honoured - {"all"}): + copy, twin = find_copy_by_key(views, key) + if copy is None: + print(f"⚠️ {t('No COW copy carries this key')} : {key}") + missed.append(key) + continue + if twin is None: + print(f"⚠️ {key} : {t('no module view to reset onto, skipped.')}") + missed.append(key) + continue + if copy["arch"] == twin["arch"]: + print(f"ℹ {key} : {t('already identical to the module view.')}") + continue + if not config.apply: + print( + f"[{t('dry-run')}] {t('would reset')} id={copy['id']}" + f" ({key}) {t('onto')} id={twin['id']}" + ) + continue + path = backup(config.database, copy, directory) + reset(config.database, copy, twin) + done += 1 + print(f"✅ {t('reset')} id={copy['id']} ({key})") + print(f" {t('previous arch saved to')} {path}") + if config.apply and done: print( f"\n{done} {t('copy(ies) reset. Re-apply any real customisation')}" f" {t('as an INHERITING view, not a copy, so it cannot go stale')}" f" {t('again.')}" ) - return 0 + # Une demande non honorée doit se voir jusque dans le code de sortie : + # l'appelant qui enchaîne ne lit pas le texte. + return 2 if missed else 0 if __name__ == "__main__": diff --git a/script/odoo/migration/smoke_public_url.py b/script/odoo/migration/smoke_public_url.py index 527a420..1eb914d 100755 --- a/script/odoo/migration/smoke_public_url.py +++ b/script/odoo/migration/smoke_public_url.py @@ -215,9 +215,14 @@ def attach_missing_parents(lst_failure, lst_log): known = {pid for _u, _s, lst in lst_failure for pid in lst} extra = [] for line in lst_log: - for _view_id, parent_id in RE_CONTEXT.findall(line): - if parent_id not in known and parent_id not in extra: - extra.append(parent_id) + # LES DEUX : le parent ET l'enfant. Mesuré sur /contactus — le + # parent était identique à sa vue module, et c'est l'ENFANT qui + # portait l'arch périmée. Ne proposer que le parent envoyait + # réinitialiser une copie qui allait déjà bien. + for view_id, parent_id in RE_CONTEXT.findall(line): + for candidate in (parent_id, view_id): + if candidate not in known and candidate not in extra: + extra.append(candidate) if not extra: return lst_failure rebuilt = [] diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index de705a9..af9668c 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -5066,6 +5066,22 @@ TRANSLATIONS = { "fr": "Entrée =", "en": "Enter =", }, + "No COW copy carries this key": { + "fr": "Aucune copie COW ne porte cette clé", + "en": "No COW copy carries this key", + }, + "already identical to the module view.": { + "fr": "déjà identique à la vue module.", + "en": "already identical to the module view.", + }, + "Odoo surfaces this when the module view is rewritten": { + "fr": "Odoo le fait apparaître quand la vue module est réécrite", + "en": "Odoo surfaces this when the module view is rewritten", + }, + "(a version bump) or when the page is rendered.": { + "fr": "(un palier de version) ou quand la page est rendue.", + "en": "(a version bump) or when the page is rendered.", + }, "Nothing to decide yet": { "fr": "Rien à décider pour l'instant", "en": "Nothing to decide yet", diff --git a/test/test_smoke_public_url.py b/test/test_smoke_public_url.py index 3ebad84..7e51d15 100755 --- a/test/test_smoke_public_url.py +++ b/test/test_smoke_public_url.py @@ -214,13 +214,30 @@ class TestTheCulpritViewsAreNamed(unittest.TestCase): lst_failure = [("http://h/a", 500, [])] log = ["[view_id: 3288, model: n/a, parent_id: 2841]"] rebuilt = smoke.attach_missing_parents(lst_failure, log) - self.assertEqual(rebuilt[0][2], ["2841"]) + self.assertIn("2841", rebuilt[0][2]) - def test_an_already_attributed_parent_is_not_duplicated(self): + def test_both_the_parent_and_the_child_are_proposed(self): + # LE défaut mesuré sur /contactus : le parent (3281) était IDENTIQUE + # à sa vue module, et c'est l'enfant (3282) qui portait l'arch + # périmée. Ne nommer que le parent envoyait réinitialiser une copie + # qui allait déjà bien, et la page restait en 500. + lst_failure = [("http://h/contactus", 500, [])] + log = ["[view_id: 3282, model: n/a, parent_id: 3281]"] + rebuilt = smoke.attach_missing_parents(lst_failure, log) + self.assertEqual(rebuilt[0][2], ["3281", "3282"]) + + def test_the_parent_comes_first(self): + # C'est le cas le plus fréquent — le blogue — donc en tête de liste. + lst_failure = [("http://h/a", 500, [])] + log = ["[view_id: 3288, model: n/a, parent_id: 2841]"] + rebuilt = smoke.attach_missing_parents(lst_failure, log) + self.assertEqual(rebuilt[0][2][0], "2841") + + def test_an_already_attributed_id_is_not_duplicated(self): lst_failure = [("http://h/a", 500, ["2841"])] log = ["[view_id: 3288, model: n/a, parent_id: 2841]"] rebuilt = smoke.attach_missing_parents(lst_failure, log) - self.assertEqual(rebuilt[0][2], ["2841"]) + self.assertEqual(rebuilt[0][2].count("2841"), 1) class TestTheServerIsKilledForReal(unittest.TestCase):