[IMP] util.update_record_from_xml

[Pirols]

Sometimes records get a field removed from their XML declaration. This creates a
divergence between new dbs and upgraded ones. The former will be initialised
with NULL/default values, whereas the latter will retain their current one.
This is usually handled in dedicated upgrade scripts with simple queries
un/setting the necessary columns. However, in some cases, these XML changes are
backported/noticed too late and need to be addressed in multiple versions.
That may create the pressure to target future versions, which creates a hidden
coupling between the xml declaration and the query, prone to turn into a bug.
It is one such example¹ that inspired this PR.

Here update_record_from_xml is adapted to unset fields missing from the xml
declaration, if passed explicitely via the fields kwarg. This enables us to
leverage the foreward compatibility of update_record_from_xml in the scenario
described above, preventing the associated potential bugs.

Footnotes

1. https://github.com/odoo/upgrade/pull/9512 ↩

[robodoo]

[aj-fuentes]

I think current version is correct. My suggestion is because I find a bit simpler to read "override each field not found in the xml" than keeping a list of fields to find.

That being said what I think could be problematic here are field with defaults. We would always write NULL but maybe that's fine for the use cases we have in mind. Another option may be to pass a dict of default values per field... but that may be too much for current cases... @KangOl ?

[Pirols]

That being said what I think could be problematic here are field with defaults. We would always write NULL but maybe that's fine for the use cases we have in mind. Another option may be to pass a dict of default values per field... but that may be too much for current cases

I have the same concern. Another option would be to only inject the Falsey xml field for non default fields.

[KangOl]

Simple solution. Great.

[KangOl]

Another option may be to pass a dict of default values per field... but that may be too much for current cases...

We could get the default value from the ORM directly.

[KangOl]

Even without the get_defauts part, there are 2 bugs in the current patch, both due to the fact that we can find multiple <record> nodes:

• when we filter out the fields, we can end up with an empty <record> node. We should then not add it to new_root (can be done with a continue statement)
• we should only add the missing fields if not define in any <record> node.