Skip to content

[19.0][FIX] upgrade_analysis: reset discarded fields to their default, not falsy value#3673

Merged
OCA-git-bot merged 1 commit into
OCA:19.0from
ForgeFlow:19.0-fix-upgrade_analysis-default-falsy
Jul 15, 2026
Merged

[19.0][FIX] upgrade_analysis: reset discarded fields to their default, not falsy value#3673
OCA-git-bot merged 1 commit into
OCA:19.0from
ForgeFlow:19.0-fix-upgrade_analysis-default-falsy

Conversation

@MiquelRForgeFlow

@MiquelRForgeFlow MiquelRForgeFlow commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

When the previous version set a field explicitly and the new version omits it, _get_xml_diff generated eval="False" using field.falsy_value. This is wrong for fields whose default is truthy (e.g. ir.rule.perm_read/write/create/unlink or active): a fresh install applies the field's default, not its falsy value. The generated noupdate_changes.xml then mismatched a clean install and could even violate model constraints (e.g. ir.rule's _no_access_rights CHECK), breaking the migration.

Use model.default_get() to reset omitted fields to the value a fresh install would give them, falling back to falsy_value when there is no default or it is not a scalar.

Fixes #3535.

Bug discovered doing OCA/OpenUpgrade#5820.

…falsy value

When the previous version set a field explicitly and the new version omits it, _get_xml_diff generated eval="False" using field.falsy_value. This is wrong for fields whose default is truthy (e.g. ir.rule.perm_read/write/create/unlink or active): a fresh install applies the field's default, not its falsy value. The generated noupdate_changes.xml then mismatched a clean install and could even violate model constraints (e.g. ir.rule's _no_access_rights CHECK), breaking the migration.

Use model.default_get() to reset omitted fields to the value a fresh install would give them, falling back to falsy_value when there is no default or it is not a scalar.
@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hi @StefanRijnhart, @legalsylvain,
some modules you are maintaining are being modified, check this out!

@MiquelRForgeFlow

Copy link
Copy Markdown
Contributor Author

@pedrobaeza @hbrunn

@pedrobaeza pedrobaeza added this to the 19.0 milestone Jul 15, 2026

@pedrobaeza pedrobaeza left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, indeed this is something to fix.

@pedrobaeza
pedrobaeza requested a review from hbrunn July 15, 2026 10:48
@pedrobaeza

Copy link
Copy Markdown
Member

/ocabot merge patch

@OCA-git-bot

Copy link
Copy Markdown
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 19.0-ocabot-merge-pr-3673-by-pedrobaeza-bump-patch, awaiting test results.

@OCA-git-bot
OCA-git-bot merged commit 256af2b into OCA:19.0 Jul 15, 2026
5 of 7 checks passed
@OCA-git-bot

Copy link
Copy Markdown
Contributor

Congratulations, your PR was merged at f7ada69. Thanks a lot for contributing to OCA. ❤️

@MiquelRForgeFlow
MiquelRForgeFlow deleted the 19.0-fix-upgrade_analysis-default-falsy branch July 15, 2026 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants