[FIX] delete_record_translations: adapt to Odoo v16

[MiquelRForgeFlow]

Needed.

[ndd-odoo]

Nice @MiquelRForgeFlow

[pedrobaeza]

As commented in OU, I'm seeing that this method is removing all translations for all fields, but there's a lot of times where this is not needed, so we should add a granularity for passing (as keyword argument), a field list for resetting translations.

[MiquelRForgeFlow]

@pedrobaeza fields_list optional parameter added.

[pedrobaeza]

Last question: what happens in a DB with en_US disabled and another single language enabled? Are translations still stored with en_US JSON value?

[MiquelRForgeFlow]

Are translations still stored with en_US JSON value?

Yes. I just tested it.

[pedrobaeza]

I know it's a bit picky, but fields_list is a bad variable name, as the s is going to be typoed most of the time. Please call field_list as normalized way of naming variables.

[pedrobaeza]

I'm also wondering if we have done it correctly all this time, doing the delete_record_translations on post, instead of pre. That way, we require another update for loading the translations. What do you think about this?

[MiquelRForgeFlow]

I'm also wondering if we have done it correctly all this time, doing the delete_record_translations on post, instead of pre. That way, we require another update for loading the translations. What do you think about this?

But the update of the noupdate changed fields is done in post, so if you delete the translations in pre, during the load they will be wrongly set, as they don't have the noupdate changes yet. I mean, to do it in pre, you would also need to apply the noudate changes in pre, which is wrong.

[pedrobaeza]

I don't think so, as the search is not done by the source term, but the comment. And now on v16 the rules change. It's not very important, as we all perform a second update after the migration itself for the cleanup, but it we let the system the best on the first round, it would be great.

[MiquelRForgeFlow]

I delete now all translations using NULL. Let's check in a simple migration if this works or if it messes up.

[pedrobaeza]

Well, it's replacing the load_data loaded data. And I'm also wondering now how this new JSON fields behaves with noupdate=1 data and translations.

[MiquelRForgeFlow]

@pedrobaeza I fully tested it. It's ready and safe.

[pedrobaeza]

I see that you are still doing a select to fetch the column values, and that's exactly what I want to avoid on my concern: only do the UPDATE query, handling it through JSON operators in SQL.

[MiquelRForgeFlow]

handling it through JSON operators in SQL

Done!

[pedrobaeza]

OK, this seems good enough for now. Thanks for the efforts.

[legalsylvain]

Hi @ALL thanks for this work.
The recent merge is breaking openupgrade 16.0 CI.
See : https://github.com/OCA/OpenUpgrade/actions/runs/5460474242/jobs/9937497789?pr=4042#step:15:5315

an existing script (in auth_signup) is now failing. Could you take a look ? Thanks !

   File "/home/runner/work/OpenUpgrade/OpenUpgrade/openupgrade/openupgrade_scripts/scripts/auth_signup/16.0.1.0/post-migration.py", line 17, in migrate
    openupgrade.delete_record_translations(
  File "/opt/hostedtoolcache/Python/3.10.12/x64/lib/python3.10/site-packages/openupgradelib/openupgrade.py", line 2696, in delete_record_translations
    logged_query(cr, query, (record_id,))
  File "/opt/hostedtoolcache/Python/3.10.12/x64/lib/python3.10/site-packages/openupgradelib/openupgrade.py", line 1681, in logged_query
    cr.execute(query, args)
  File "/home/runner/work/OpenUpgrade/OpenUpgrade/odoo/odoo/sql_db.py", line 321, in execute
    res = self._obj.execute(query, params)
psycopg2.errors.InvalidTextRepresentation: invalid input syntax for type json
LINE 3: ...   SET (name, body_html, subject, description) = ('{"en_US":...
                                                             ^
DETAIL:  Token "name" is invalid.
CONTEXT:  JSON data, line 1: {"en_US": name...

[MiquelRForgeFlow]

@legalsylvain sorry, that breaking line was a last untested change. It is being fixed in #335.