[hbrunn]

depends on:

• [20.0] initialize branch #6045

[hbrunn]

pre-commit failure fixed in OCA/pylint-odoo#590

[FerDeNetlab]

Tested on a 19.0 db with crm, sale_management, purchase, stock, account + l10n_mx, project, hr, mrp and point_of_sale (131 modules after the upgrade). These are the framework issues we hit, one per comment so you can take what makes sense.

Drafted with AI assistance (Claude Opus 5.5), reviewed by me; each suggestion was checked against the current code of this PR and our test migration.

[FerDeNetlab]

button_install skips auto_install modules whose country_ids don't match any company, this loop doesn't, so e.g. account_peppol gets installed on databases without a Peppol country.

Suggested change

	new_auto_install_modules = self.browse([])
	for module in self.env["ir.module.module"].search(
	[
	("auto_install", "=", True),
	("state", "=", "uninstalled"),
	("name", "not like", ("l10n_%")),
	]
	):
	new_auto_install_modules = self.browse([])
	company_countries = self.env["res.company"].sudo().search([]).country_id
	for module in self.env["ir.module.module"].search(
	[
	("auto_install", "=", True),
	("state", "=", "uninstalled"),
	("name", "not like", ("l10n_%")),
	]
	):
	# same rule as button_install
	if module.country_ids and not module.country_ids & company_countries:
	continue

[hbrunn]

thanks, I pushed the condition into the domain though and took the opportunity to rewrite this to Domain syntax

[FerDeNetlab]

Two things here:

• _validate_module_views and create() call _check_xml on several views, so self.xml_id in the handler raises Expected singleton and aborts the upgrade instead of logging.
• As _raise_view_error doesn't raise anymore, validation goes on and fails later with AttributeError/KeyError/NameError/ValueError (_check_field_paths, _validate_domain_identifiers).

Suggested change

	try:
	return IrUiView._check_xml._original_method(self)
	except ValidationError as e:
	_logger.warning(
	"Can't render custom view %s for model %s. "
	"Assuming you are migrating between major versions of Odoo. "
	"Please review the view contents manually after the migration.\n"
	"Error: %s",
	self.xml_id,
	self.model,
	e,
	)
	for view in self:
	try:
	IrUiView._check_xml._original_method(view)
	# with _raise_view_error not raising, validation can fail later on
	except (
	ValidationError,
	ValueError,
	AttributeError,
	KeyError,
	NameError,
	) as e:
	_logger.warning(
	"Can't render custom view %s for model %s. "
	"Assuming you are migrating between major versions of Odoo. "
	"Please review the view contents manually after the migration.\n"
	"Error: %s",
	view.xml_id,
	view.model,
	e,
	)

[hbrunn]

here I've disabled _check_xml completely, as it doesn't add anything interesting to the log. we get all the info we need from _raise_view_error. Then we also don't need to patch away a plethora of followup issues like in _check_field_paths

[oca-cla-bot]

The following commit authors need to sign the Contributor License Agreement:

• roy.le@viindoo.com
• daiduongnguyen2709@gmail.com

[etobella]

@daiduongnguyen-odoo @kafai-lam @royle-vietnam I see that your CLA is not updated properly. can you sign it again 🙏 ?

[carlosdauden]

Tested by merging it into our 20.0 integration branch: the 19.0 -> 20.0 migration of the OCA test database with ~420 module scripts on top passes, with 130 migration tests (run with the same test-migration workflow, Python 3.12 / PostgreSQL 16). Thanks for taking FerDeNetlab's suggestions (attachments of removed models, country filter, savepoint in unlink): I checked all three against origin/20.0 and they are right.

Changes I would ask for:

1. Custom view diagnostics are gone. With _check_xml as a no-op, nothing reports broken views any more: our run logs 0 "Can't render custom view" lines, against 1813 with the previous version of this PR (19.0 also logged them). Most of those 1813 were noise from standard views checked before their module was updated, so I agree with not validating while the modules load. But core's STEP 6 (odoo/modules/loading.py: View._validate_custom_views(model) inside try/except Exception) validates exactly the views a migrator has to fix by hand (no xmlid, __export__, studio), once, at the end, and never aborts. Now that goes through the no-op too. Suggestion, keeping your approach for everything else (tested on our branch: unchanged result on the OCA test database, 0 warnings, and with one custom view adding a field removed in 20.0, the migration still ends fine and STEP 6 reports that view and the other broken elements of the same inheritance tree, right before "Modules loaded"):

from contextvars import ContextVar

# _validate_custom_views replaces the context of the views it checks
# (with_context({'load_all_views': True})), so a context key would not get through
_validating_custom_views = ContextVar(
    "openupgrade_validating_custom_views", default=False
)


def _check_xml(self):
    """Don't validate views while the modules are loading; only validate
    custom views, one by one, at the end of the upgrade (core STEP 6)"""
    if not _validating_custom_views.get():
        return
    for view in self:
        try:
            _check_xml._original_method(view)
        except Exception as e:  # pylint: disable=broad-except
            _logger.warning(
                "Custom view %s (%s) of model %s is invalid after the migration, "
                "review it manually: %s",
                view.id, view.xml_id or view.name, view.model, e,
            )


def _validate_custom_views(self, model):
    token = _validating_custom_views.set(True)
    try:
        return _validate_custom_views._original_method(self, model)
    finally:
        _validating_custom_views.reset(token)

Most problems are still logged by your _raise_view_error patch; the loop catches the rest (for example the attrs/states ValidationError, or a follow-up AttributeError after _raise_view_error returned), without hiding the other views of the model. A test could create a custom view with an unknown field (creation passes, _check_xml is off) and check that _validate_custom_views("res.partner") logs it.

2. test_03_update_list does not test the new country filter: auto_install_country_specific has no country_ids, and its l10n_ name is already excluded by the name filter. Please add a non-l10n module with country_ids of a country no company uses (must stay uninstalled) and one with the main company's country (must be installed).

3. pre-commit: the pylint-odoo crash on odoo_patch/odoo/modules/module_graph.py is fixed in OCA/pylint-odoo#590, which is released in v10.0.12. .pre-commit-config.yaml still pins v10.0.11; bumping it makes the check green.

Not blocking, can be follow-ups:

4. The l10n_% exclusion in update_list is now redundant and costs features. Every auto_install l10n_* module in 20.0 either declares countries (handled by the new filter, the same rule as button_install) or depends on another l10n_* module (I checked all 189: none has neither). Meanwhile it skips bridges a fresh 20.0 database gets: new in 20.0 are l10n_fr_payment, l10n_kr_sale, l10n_eg_edi_pos, l10n_bh_pos, l10n_fr_pdp_penalty_rate. Dropping the name filter would give migrated databases the same modules as new ones.
5. update_list also re-installs auto_install modules the user uninstalled on purpose in the old version (any uninstalled auto_install module whose dependencies are installed qualifies, not only modules new in this version). That has been so since 18.0, so not for this PR, but consider limiting it to modules whose ir.module.module row did not exist before update_list() (take the names before calling the original), and log the list of modules it installs either way.
6. update_list checks all dependencies_id, while core only looks at the auto_install_required ones. No module new in 20.0 has a partial auto_install list, so this changes nothing today; a comment would help the next port.
7. Nit: the check_external_dependencies patch looks dead in 20.0. It returns early through Manifest.for_addon() when the module is not available, and an unknown dependency raises UserError in _state_update before. I could not find a path that raises AttributeError any more.
8. DEVELOP.md: worth a line saying that, since the framework force-runs the scripts of modules installed during the upgrade, those scripts get an empty version, so @openupgrade.migrate() skips them unless they use no_version=True. We hit this twice in 20.0 scripts (fleet_maintenance, account_payment_custom).
9. Outside this PR, but needed before #6068: base's load_data fails on 20.0 until OCA/openupgradelib#468 is merged (it is approved by two maintainers) and installed by the workflow. Otherwise a small shim in the framework (odoo_patch/odoo/tools/__init__.py setting convert_xml_import/convert_csv_import from odoo.tools.convert when missing) unblocks it.
10. The CLA check fails because of the old history commits of two Viindoo authors (v17 port, _raise_view_error changes); etobella already pinged them.