From 1ec37defc3172c16e9db16e22264331269c3d973 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Sat, 1 Aug 2026 00:54:44 -0400 Subject: [PATCH] [FIX] check_cow_views: detect on arch shape, not on mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two blind spots made the detector miss real breakages. 1. Comparing « mode » is not the right test. What decides the shape an arch must have is whether the target declares an inherit_id: with one, the arch must be inheritance specs (, , position=); without one, it must be a standalone template. A view moving from a root template to « inherit_id + primary="True" » keeps mode='primary' on BOTH sides, so the old test reported nothing while the copy still broke. The comparison is now (target declares inherit) vs (stored arch is spec-shaped), and each finding carries the reason. A mode change with a matching shape is still reported, as a lesser warning. arch_db is read as text up to 15.0 and as jsonb from 16.0; both are handled. 2. A module renamed upstream was reported as « module absent », hiding every view it owns. renamed_modules is now read from the target OpenUpgrade apriori.py (21 entries for 13.0, 56 for 14.0, 39 for 16.0, 20 for 18.0) and used before concluding the module is gone. Also correct the advice printed for a copy at risk: deactivating it is not enough, an inactive copy keeping the same key still shadows the module view. Renaming the key is what actually unpairs it. Co-Authored-By: Claude Opus 4.8 (1M context) --- script/odoo/migration/check_cow_views.py | 188 ++++++++++++++++++----- 1 file changed, 150 insertions(+), 38 deletions(-) diff --git a/script/odoo/migration/check_cow_views.py b/script/odoo/migration/check_cow_views.py index 6255482..07321a0 100755 --- a/script/odoo/migration/check_cow_views.py +++ b/script/odoo/migration/check_cow_views.py @@ -23,11 +23,19 @@ cannot find it in the parent, and the whole upgrade stops on:: So the rule is: - a COW view breaks when its module counterpart changes ``mode`` - between version N and version N+1. + a COW view breaks when the shape its arch must have changes between + version N and version N+1. -That is predictable *before* starting a multi-hour migration: the current mode -is in the database, and the target mode is declared in the target version +The discriminant is NOT ``mode``. What decides the required shape is whether +the target declares an ``inherit_id``: if it does, the arch must be inheritance +specs (````, ````, ``position=``); if it does not, the arch must be +a standalone template. Comparing ``mode`` alone misses a real case: a view +moving from a root template to ``inherit_id`` + ``primary="True"`` keeps +``mode='primary'`` on both sides yet still has to change shape, and the copy +still breaks. + +That is predictable *before* starting a multi-hour migration: the stored arch is +in the database, and the required shape is declared in the target version sources. This script compares the two and reports the views at risk. It only reads: no database write, no source modification. @@ -35,21 +43,29 @@ It only reads: no database write, no source modification. import argparse import glob +import json import os +import re import subprocess import sys import xml.etree.ElementTree as ET -# A view whose module counterpart cannot be found is reported separately: it is -# usually a view of a module that does not exist in the target version. +# A view whose module counterpart cannot be found at all. MODE_UNKNOWN = "unknown" +# arch_db is text up to 15.0 and jsonb from 16.0 ({"en_US": "..."}). +RE_XML_DECLARATION = re.compile(r"<\?xml.*?\?>", re.DOTALL) +RE_FIRST_TAG = re.compile(r"<\s*([A-Za-z_][\w.:-]*)") +# Tags that carry inheritance specs rather than a standalone template. +SPEC_ROOT_TAG = ("data", "xpath") + def query_cow_views(database): - """Return [(id, key, mode, website_id)] for every website COW view.""" + """Return [(id, key, mode, website_id, arch)] for every website COW view.""" sql = ( - "SELECT id, COALESCE(key, ''), mode, website_id FROM ir_ui_view" - " WHERE website_id IS NOT NULL ORDER BY id;" + "SELECT id, COALESCE(key, ''), mode, website_id," + " replace(left(COALESCE(arch_db::text, ''), 400), chr(10), ' ')" + " FROM ir_ui_view WHERE website_id IS NOT NULL ORDER BY id;" ) result = subprocess.run( ["psql", "-d", database, "-tAF", "|", "-c", sql], @@ -64,11 +80,61 @@ def query_cow_views(database): for line in result.stdout.splitlines(): if not line.strip(): continue - view_id, key, mode, website_id = line.split("|") - lst_view.append((int(view_id), key, mode, website_id)) + view_id, key, mode, website_id, arch = line.split("|", 4) + lst_view.append((int(view_id), key, mode, website_id, arch)) return lst_view +def arch_is_inheritance_spec(arch): + """True when the arch holds inheritance specs, not a standalone template. + + This is the real discriminant, not ``mode``. A view declared with an + ``inherit_id`` must hold specs (````, ````, or an element with + a ``position``); a root view holds a full template (````, + ``
``, ...). A copy that keeps the wrong form for what the target + version expects is exactly what raises « cannot be located in parent view ». + """ + if not arch: + return None + # From 16.0 arch_db is jsonb: take any translation, the structure is shared. + if arch.lstrip().startswith("{"): + try: + translations = json.loads(arch) + arch = next(iter(translations.values()), "") + except (ValueError, StopIteration): + # Truncated jsonb: fall through and look at the raw text. + pass + arch = RE_XML_DECLARATION.sub("", arch or "") + match = RE_FIRST_TAG.search(arch) + if not match: + return None + if match.group(1).lower() in SPEC_ROOT_TAG: + return True + # style specs. + return "position=" in arch[: match.end() + 200] + + +def load_renamed_modules(odoo_version): + """Return {old_module: new_module} from the target OpenUpgrade apriori.py. + + Without this a module renamed upstream looks absent, and every view of that + module is misreported as « module gone » instead of being checked. + """ + pattern = os.path.join(odoo_version, "**", "apriori.py") + for file_path in sorted(glob.glob(pattern, recursive=True)): + data_vars = {} + try: + with open(file_path, "r", encoding="utf-8") as f: + exec(f.read(), data_vars) # noqa: S102 - upstream data file + except Exception: + # A broken or exotic apriori.py must not stop the whole report. + continue + renamed = data_vars.get("renamed_modules") + if isinstance(renamed, dict) and renamed: + return renamed + return {} + + def find_module_dir(odoo_version, module_name): """Locate a module directory inside an odoo tree.""" lst_pattern = [ @@ -83,12 +149,14 @@ def find_module_dir(odoo_version, module_name): return None -def declared_mode(module_dir, template_id): - """Return 'primary', 'extension' or None for a view declared in sources. +def declared_view_shape(module_dir, template_id): + """Return (mode, inherits) for a view declared in the sources, else None. - Handles both declaration styles: