Skip to content

[19.0][FIX] upgrade_analysis: don't generate invalid noupdate xml for discarded field values#3535

Open
hbrunn wants to merge 1 commit intoOCA:19.0from
hbrunn:19.0-upgrade_analysis-falsy-value-for-discarded-value
Open

[19.0][FIX] upgrade_analysis: don't generate invalid noupdate xml for discarded field values#3535
hbrunn wants to merge 1 commit intoOCA:19.0from
hbrunn:19.0-upgrade_analysis-falsy-value-for-discarded-value

Conversation

@hbrunn
Copy link
Member

@hbrunn hbrunn commented Mar 2, 2026

in v18, there's currencies set on payment's payment_method_bank_transfer record: https://github.com/OCA/OCB/blob/18.0/addons/payment/data/payment_method_data.xml#L593

in v19, no currencies are set: https://github.com/OCA/OCB/blob/19.0/addons/payment/data/payment_method_data.xml#L565

which generates

<field name="supported_currency_ids"/>

in noupdate_changes.xml. But this amounts to writing '' on a many2many field, which fails as that's an invalid value for this field type.

So here I propose to write whatever the field's NULL value is instead, ensuring we generically empty that field in such a case.

@OCA-git-bot
Copy link
Contributor

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

@pedrobaeza pedrobaeza added this to the 19.0 milestone Mar 2, 2026
Copy link
Contributor

@MiquelRForgeFlow MiquelRForgeFlow left a comment

Choose a reason for hiding this comment

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

Code is not wrong probably, but could you explain/clarify my doubts?

Comment on lines +365 to +366
if record_remote_dict[key].tag == "field" and set(attribs) == {
"name"
Copy link
Contributor

Choose a reason for hiding this comment

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

why set(attribs) == {"name"} comparison? Could you explain a bit? I mean, why not put "name" in attribs instead or something else?

Copy link
Member Author

@hbrunn hbrunn Mar 2, 2026

Choose a reason for hiding this comment

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

this is to test that we're actually in a field node, and that there aren't other attributes set such as search. or to put it differently: This ensures we're only intervening when the resulting node would be <field name="fieldname" />

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants