Skip to content

[IMP] util.update_record_from_xml #289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[IMP] util.update_record_from_xml #289

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions src/util/records.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@
from psycopg2.extras import Json, execute_values

try:
from odoo import modules, release
from odoo import modules
from odoo.tools.convert import xml_import
from odoo.tools.misc import file_open
from odoo.tools.translate import xml_translate
except ImportError:
from openerp import modules, release
from openerp import modules
from openerp.tools.convert import xml_import
from openerp.tools.misc import file_open

Expand All @@ -34,7 +34,7 @@
from .inconsistencies import break_recursive_loops
from .indirect_references import indirect_references
from .inherit import direct_inherit_parents, for_each_inherit
from .misc import Sentinel, chunks, parse_version, version_gte
from .misc import Sentinel, chunks, version_between, version_gte
from .orm import env, flush
from .pg import (
PGRegexp,
Expand Down Expand Up @@ -1133,6 +1133,7 @@ def __update_record_from_xml(
template = False
found = False
extra_references = []
xml_filename = None

def add_ref(ref):
if "." not in ref:
Expand All @@ -1147,6 +1148,8 @@ def add_ref(ref):
doc = lxml.etree.parse(fp)
for node in doc.xpath(xpath):
found = True
if xml_filename is None:
Copy link
Contributor

@aj-fuentes aj-fuentes Jul 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we now need the found variable? We can keep the xml_filename as the "found" signal whenever it's set. Maybe named found_in_xml_file?

There is a slight subtlety. We do not break the search loop when found. Thus in practice we keep searching and adding nodes to our new root. This means that we will be lying if we keep the same filename when loading our new root. Maybe we should have a new_root per found_in_xml_file, a sort of map filename->new_root. We should double check the standard behavior if we really care about this.

EDIT: This is obviously a corner case since it's a mistake to have two records with same xmlid redefined in the same module in xml. But this has been actually seen before. Aligning with standard code would be "saner" even for the corner case.

xml_filename = "{}/{}".format(from_module, f)
parent = node.getparent()
if node.tag == "record" and fields is not None:
for fn in node.xpath("./field[@name]"):
Expand Down Expand Up @@ -1200,8 +1203,9 @@ def add_ref(ref):
)

cr_or_env = env(cr) if version_gte("saas~16.2") else cr
importer = xml_import(cr_or_env, from_module, idref={}, mode="update")
kw = {"mode": "update"} if parse_version("8.0") <= parse_version(release.series) <= parse_version("12.0") else {}
kw = {"xml_filename": xml_filename} if version_gte("8.saas~6") else {}
importer = xml_import(cr_or_env, from_module, idref={}, mode="update", **kw)
kw = {"mode": "update"} if version_between("8.0", "12.0") else {}
importer.parse(new_root, **kw)
if version_gte("13.0"):
flush(env(cr)["base"])
Expand Down