Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMP] rename_models: add condition to check version and existence of ir_property table #395

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xaviedoanhduy
Copy link

@xaviedoanhduy xaviedoanhduy commented Jan 13, 2025

model ir.property was removed in commit odoo/odoo@de302c2 in version 18.0 so if new databases is initialized from 18.0 onwards will not have ir_property table then there will be an error exception that relation ir_property does not exist when using the rename_models function

Copy link

@jeroenev jeroenev left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

You need to handle the renaming for 18+, changing model inside the JSON field.

(old,),
)
field_ids = [x[0] for x in cr.fetchall()]
logged_query(
Copy link
Member

Choose a reason for hiding this comment

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

This should go outside the if.

),
)
if field_ids:
if version_info[0] < 18 and table_exists(cr, "ir_property"):
Copy link
Member

Choose a reason for hiding this comment

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

What would be the reason to check if ir_property exists as a table?

Copy link
Author

Choose a reason for hiding this comment

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

I'm just denying the case if the ir_property table exists and the version is >= 18.0. because in fact if you use a database from 17.0 or earlier to migrate data to version >= 18.0, the ir_property table still exists and is just no longer used.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed that even in version 18 no operations should be performed in the table even if it exists! I would say checking the version is enough because we can rely on the table existing in earlier versions.

Copy link
Author

Choose a reason for hiding this comment

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

thanks, maybe it's redundant. i will update it in next patch

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