Skip to content

[FIX] openupgrade_merge_records: safely merge jsonb and serialized fields#425

Open
xaviedoanhduy wants to merge 2 commits intoOCA:masterfrom
xaviedoanhduy:fix-openupgrade_merge_records
Open

[FIX] openupgrade_merge_records: safely merge jsonb and serialized fields#425
xaviedoanhduy wants to merge 2 commits intoOCA:masterfrom
xaviedoanhduy:fix-openupgrade_merge_records

Conversation

@xaviedoanhduy
Copy link
Copy Markdown
Contributor

when merging records using openupgrade_merge_records.merge_records(),
the helper apply_operations_by_field_type attempted to apply the |= operator directly on string values. however jsonb fields may be wrapped types (e.g. psycopg2.extras.Json), causing:
TypeError: unsupported operand type(s) for |=: 'dict' and 'str'

maybe not as expected the result returned like method="sql" (ref), will return dict value like {"en_US": "Awaiting"}. but unfortunately in method="orm" even though name field in database is jsonb but in ORM level it is still Char type and return a string like "Awaiting" so the error occurred.

this proposed fix will convert string value to dict based on current language (mostly en_US). In case field type is Serialized, the old logic remains the same because they are mostly dicts. And in python versions like 3.8 it seems that the |= operator is not supported yet so I replaced it with the .update() function

on this occasion also corrected the warning sush as: UserWarning: unsupported operand type(s) for "==": 'res.company()' == '1' in the second commit 96be2e9

@StefanRijnhart
Copy link
Copy Markdown
Member

@MiquelRForgeFlow Why are you asking me for a review without reviewing yourself?

Copy link
Copy Markdown
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.

I forgot 😅

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.

3 participants