[IMP] util.update_record_from_xml

[KangOl]

Set the xml_filename argument of the xml_import records.

[robodoo]

[aj-fuentes]

Why do we check module vs from_module? Don't we need to set the xml_filename to be the actual file always?

[KangOl]

It's to avoid setting the filename when we load an overridden record (initially declared in another module).
However, the models that use this xml_filename (views, mail templates) are not usually changed by other modules (and even less updated via update_record_from_xml), so I can remove this condition.

[aj-fuentes]

I think we should do what standard code does.

[aj-fuentes]

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.

[KangOl]

upgradeci retry with always only hr

[KangOl]

After some tests, it doesn't work as expected in all cases.

• ir.ui.view: works correctly due to the arch field being computed with an inverse function
• theme.ir.ui.view: just read in the default function, so not updated during write
• template.reset.mixin ({mail,sms}.template): set in the create overwrite only.

Maybe all those usages should be standardized in a common mixin which also handle the update.

[aj-fuentes]

After some tests, it doesn't work as expected in all cases.

* `ir.ui.view`: works correctly due to the `arch` field being computed with an `inverse` function

* `theme.ir.ui.view`: just read in the `default` function, so not updated during `write`

* `template.reset.mixin` (`{mail,sms}.template`): set in the `create` overwrite only.

• theme.ir.ui.view: can't we just add @api.depends('arch') here https://github.com/odoo/odoo/blob/07626050bd0104fecd8799b56d30245d00da3f27/addons/website/models/theme_models.py#L56 ?
• template.reset.mixin: we can do in write the same as in create, https://github.com/odoo/odoo/blob/07626050bd0104fecd8799b56d30245d00da3f27/addons/mail/models/template_reset_mixin.py#L30-L36 (in master it could be extracted to a local private method...)

Maybe all those usages should be standardized in a common mixin which also handle the update.

I think this could be an option if this becomes more widely use. For current cases it looks very localized and (if I'm not mistaked) with a simple solution. The mixin may add extra complexity that we don't need for these few cases.

[KangOl]

• theme.ir.ui.view: can't we just add @api.depends('arch') here https://github.com/odoo/odoo/blob/07626050bd0104fecd8799b56d30245d00da3f27/addons/website/models/theme_models.py#L56 ?

No, it's not enough. It should be set in create and write, like for other models.

[aj-fuentes]

• theme.ir.ui.view: can't we just add @api.depends('arch') here https://github.com/odoo/odoo/blob/07626050bd0104fecd8799b56d30245d00da3f27/addons/website/models/theme_models.py#L56 ?

No, it's not enough. It should be set in create and write, like for other models.

What I had in mind was that the field should be a computed stored field, with "depends". I missed that it was just default=...