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

[FIX] Preserve all translated fields on v16 migration #131

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

Conversation

yajo
Copy link
Contributor

@yajo yajo commented Aug 28, 2024

Currently, the upgrade process preserves translations only for Odoo CE+EE fields. However, databases usually have more modules (or even custom fields) and those translations get lost when upgrading to Odoo 16.

With this script, all translated fields will inherit their translations in all languages. Since it is executed in the pre phase of base, it should be able to still find the ir_translation table.

@moduon MT-7120

@robodoo
Copy link
Contributor

robodoo commented Aug 28, 2024

Pull request status dashboard

@yajo yajo force-pushed the 16.0-fix-translations2jsonb branch from 4e4a801 to 28449d1 Compare August 28, 2024 08:34
@aj-fuentes
Copy link
Contributor

However, databases usually have more modules (or even custom fields) and those translations get lost when upgrading to Odoo 16.

The table is renamed to _ir_translation and kept always. What translations are you trying to keep? Only entries with type='code' are cleaned before renaming the table.

@NomadsComputerllc
Copy link

Merge to upgrade

those Errors are killing me

Copy link

@NomadsComputerllc NomadsComputerllc left a comment

Choose a reason for hiding this comment

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

Thanks for the updates

Copy link

@NomadsComputerllc NomadsComputerllc left a comment

Choose a reason for hiding this comment

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

Thanks

@yajo
Copy link
Contributor Author

yajo commented Aug 30, 2024

After a migration, you can see an upgrade report like this:

image

It specifies that migrations may be wrong. However, in reality, all those are wrong because only the base language (usually en_US) is migrated.

With this script, all langs are always migrated in all fields (declared by Odoo or not). Still, something may be wrong; but at least, most problems get auto-fixed now.

@aj-fuentes
Copy link
Contributor

Yes, you need to use _get_translation_upgrade_queries on your side (via an upgrade script) when updating the custom modules (with custom code available) on your upgraded DB.

That's the same mechanism we use in the standard upgrade script. We cannot do this for custom fields because we don't know if a field has a special translate= method. This option comes via code and we do not have the custom code. There are two standard options xml_translate and html_translate, but there could be even a custom translate.

What you could do is something like (beware: not tested!)

    for model in your_custom_models:      
        if model._auto and not model._abstract:
            for field in model._fields.values():
                if field.store and field.translate:
                    migrate, cleanup = _get_translation_upgrade_queries(cr, field)
                    # run the queries in migrate, and cleanup

Side node: when _get_translation_upgrade_queries was introduced this repo didn't exist thus the way to share the queries with the community was via the main odoo repo.

@yajo yajo force-pushed the 16.0-fix-translations2jsonb branch 2 times, most recently from 11feadb to d8a5585 Compare August 30, 2024 10:18
@yajo
Copy link
Contributor Author

yajo commented Aug 30, 2024

IIUC there shouldn't be an incompatibility between this PR and your suggestion, right? 🤔

This PR only improves the default value for those fields. If there's more magic involved, then you can still do that and get to the appropriate fix.

@aj-fuentes
Copy link
Contributor

@yajo

IIUC there shouldn't be an incompatibility between this PR and your suggestion, right? 🤔

This change is risky because it will interfere with all the battle tested machinery of the translations in 15->16 upgrade. Including PO loading, noupdate flags, etc. Also for specific scripts of some clients.

This PR only improves the default value for those fields. If there's more magic involved, then you can still do that and get to the appropriate fix.

If you want to contribute an improvement I think it would be better to add something that could be used directly in end- scripts for custom code (in specific.py). That way the community could have a simpler alternative than figuring out how to use _get_translation_upgrade_queries. We could even add a reference to it in the upgrade report (like in the example picture you sent before).

@KangOl could you give some insight here please?

@yajo yajo force-pushed the 16.0-fix-translations2jsonb branch from d8a5585 to f5871e7 Compare September 18, 2024 07:41
@yajo
Copy link
Contributor Author

yajo commented Sep 18, 2024

I changed it according to your suggestion. Could you please review again?

Copy link
Contributor

@aj-fuentes aj-fuentes left a comment

Choose a reason for hiding this comment

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

The idea looks correct, thanks!

I left some comments. In summary, IMO the important part of the suggestions is to allow to narrow down what needs to be updated as parameter(s) to the function.

src/util/specific.py Outdated Show resolved Hide resolved
src/util/specific.py Outdated Show resolved Hide resolved
src/util/specific.py Outdated Show resolved Hide resolved
src/util/specific.py Outdated Show resolved Hide resolved
@KangOl
Copy link
Contributor

KangOl commented Sep 18, 2024

upgradeci retry with always only base

@yajo yajo force-pushed the 16.0-fix-translations2jsonb branch 2 times, most recently from ae05ea4 to 182fe32 Compare October 22, 2024 09:19
Copy link
Contributor

@aj-fuentes aj-fuentes left a comment

Choose a reason for hiding this comment

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

Some nitpicks

src/util/specific.py Outdated Show resolved Hide resolved
src/util/specific.py Outdated Show resolved Hide resolved
src/util/specific.py Show resolved Hide resolved
src/util/specific.py Show resolved Hide resolved
@yajo yajo force-pushed the 16.0-fix-translations2jsonb branch from 3a035df to 7127443 Compare October 30, 2024 10:39
@yajo
Copy link
Contributor Author

yajo commented Oct 30, 2024

All attended.

@aj-fuentes
Copy link
Contributor

Could you please rebase your fork's branch in the latest master of this repo? The CIs are all red due to this.

@aj-fuentes
Copy link
Contributor

upgradeci retry

Currently, the upgrade process preserves translations only for Odoo CE+EE fields. However, databases usually have more modules (or even custom fields) and those translations get lost when upgrading to Odoo 16.

Running this script in a migrated database, all translated fields will inherit their translations in all languages.

@moduon MT-7120

Co-authored-by: Alvaro Fuentes <[email protected]>
@yajo yajo force-pushed the 16.0-fix-translations2jsonb branch from 7127443 to 4475084 Compare November 6, 2024 11:03
@yajo
Copy link
Contributor Author

yajo commented Nov 6, 2024

Rebased

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.

5 participants