[IMP] don't delete the xmlid of an undeletable record

[hbrunn]

This is a demo to see if the strategy mentioned in #577 actually works

Marked as invalid because we should get something like this via a merge with upstream. Keep until either this is merged in upstream or we decide to use this anyways.

[StefanRijnhart]

It seems hard to tell from CI if this fixes the problem. Can you add a test to check that no view for analytic lines remain without a proper xml id?

[hbrunn]

good idea, let's see. Logging says it doesn't delete the xmlid in https://travis-ci.org/OCA/OpenUpgrade/builds/148643765#L6532, but trust is good, control is better.

[StefanRijnhart]

So it works! Can we merge this?

[hbrunn]

let's give Odoo SA a week or so to contemplate on the PR I made. If they don't merge until then,let's merge this. @jbeficent meanwhile can put this branch in hin buildout to test it

[JordiBForgeFlow]

It's not deleting the record. The old record still exists, without an external ID

[hbrunn]

by now it also keeps the xmlid as the test shows. Then it can be removed by usual database_cleanup operations.

[JordiBForgeFlow]

When I do the actual migration I can see the records there:

I am not sure why the test passes. Perhaps the test in module 'analytic' is run before the module 'account' is migrated?

See:
Test in module analytic is executed here:
https://travis-ci.org/OCA/OpenUpgrade/builds/148815247#L1848

Migration of account module starts here:
https://travis-ci.org/OCA/OpenUpgrade/builds/148815247#L2113

[pedrobaeza]

How is the state of this?

[pedrobaeza]

@hbrunn should this finally get merged?

[hbrunn]

I never came around to try this with @jbeficent's example, so I'm doubtful. But for some corner cases, I do think we need this (or something similar)

[pedrobaeza]

@jbeficent please approve / merge this if it's correct. I don't know enough the problem it tries to solve.

[mvaled]

@StefanRijnhart today I ran another migration with this branch merged (see https://github.com/merchise-autrement/openupgrade/tree/merchise-predevelop-9.0).

When running Odoo 9 (https://github.com/merchise-autrement/odoo/tree/merchise-develop-9.0) againts the DB I still get the following:

QWebException: Could not find /account_analytic_analysis/static/src/css/analytic.css defined in bundle 'web.assets_backend'

The same happens for edi and im_chat.

The xmlids of those modules were deleted:

$ psql -d mercurio -c "select name from ir_model_data where module='account.analytic.analysis'"
 name 
------
(0 rows)

Those were the removed xmlids I though #594 was about, but after reading more carefully, obsolete modules seem to be still a WIP: #577 (comment).

I see that odoo/odoo#12954 accepted the patch with odoo/odoo@14ac3a9 (a variation) for Odoo 10. So I'm OK to have this merged. But we still need to decide how to proceed with obsolote modules.

[StefanRijnhart]

On the upstream PR, there is talk of a bug (without making it clear what it is). And these commits are older than the one on the PR on which the bug is supposedly fixed. Does this change still contain the bug?

[StefanRijnhart]

The bug mentioned on the upstream PR is the one fixed here: odoo/odoo@8f6b5fc#diff-678bd201dfe1a0c69add630cf73e76ba. It does not apply to this change. So, +1 for having this merged.

[pedrobaeza]

But that one was done for 11.0, not 9.0. Does it matter?

[StefanRijnhart]

@pedrobaeza indeed, the bug was introduced in the refactoring to the new API, so it is not in 9.0 and it is not related to the change in this PR.

[pedrobaeza]

OK, merging then.

[MiquelRForgeFlow]

Merging this