[15.0][OU-FIX] account: should not merge website_legal_page module - #4536
Conversation
|
Traceback of exception raised when running migration of a database with both account and website_legal_page installed : |
|
Hi @CarlosRoca13 @chienandalu Best Regards, |
|
The merging is correct, as now Odoo includes the term page accessible in "frontend" in About the CacheMiss you are talking about, I think you are executing things without loading |
|
Hi @pedrobaeza thank you for your feedback. I see that website_legal_page is still available in v15 and v16 and already gets a PR for v17, and I understand why : because from my point of view, the account terms page from v15+ does not necessarily covers the same scope as that module, and merging it during openupgrade migration to v15 causes uninstallation of that module. Also, I am pretty sure openupgrade_framework is properly loaded since when it is not I get different kind of errors, and I just checked adding a print in openupgrade_framework init.py which is correctly displayed when starting... |
|
I also reproduced the issue this morning before raising PR with a fresh v14 install with only account and website_legal_page installed (with their dependencies). |
|
Yes, they continue for that extra needs, and you can just install them again, but the general behavior is to merge it. We have done this kind of merging in a lot of occasions. Try to put it on pre-migration in your test to see, but as said, we haven't experienced any problem as is. |
|
I have the same issue having the merge in pre-migration |
christian-ramos-tecnativa
left a comment
There was a problem hiding this comment.
Hello @pedrobaeza we are having the same issue as @remi-filament and this change LGTM
|
As said, you can just install it again after running the migration, or is there any other problem? |
|
I think what is missing is adding |
edescalona
left a comment
There was a problem hiding this comment.
Excellent contribution @remi-filament tested and functional. LGTM
|
@pedrobaeza It triggers an error that block the process (with the framework also) @MiquelRForgeFlow even assuming that |
|
Which error? |
|
This one #4536 (comment) |
pedrobaeza
left a comment
There was a problem hiding this comment.
Well, OK, after talking with Christian by chat, and giving the fact that I don't have customers in prior versions with this module, let's not block your reasoning.
|
Wait, is not better to apply my suggestion? |
|
Miquel, I'm not sure the dependency graph and loading mechanism is properly handle doing the update list inside a migration script, so as I'm the only one wanting to remove |
No issues. Done in #5306. |
Issue raised while migrating database with both account and website_legal_page installed.
website_legal_page is merged into account model with post-migration.py script since PR #3728
The problem is that merging module in post-migration does not reload the list of modules to be updated, so when you run the command with -u all, it then tries to update website_legal_page which does not exist anymore, leading to CacheMiss exception. (example of error in the first comment on this PR to not overload the description here).
After giving it some thoughts, I do not think merging website_legal_page into account is a good solution, because in many cases legal page from that module are not the same as the one we want to link to from account move. (for instance in France, legal page is mandatory on every website with few information to identify owner, but the terms you will need on account move are related to the sale you performed).
Also, website_legal_page adds a link to legal page in website footer which is then lost with this merging (since account is not doing that).
Still it can make sense to initialize new invoice_terms_html with legal page content as long as the new mode is not forced (already fixed/reverted with PR #4110 )
Therefore, I propose here to not merge website_legal_page into account but keeping the initialization of the company invoice_terms_html field.
One would then need to uninstall website_legal_page manually if it is not needed anymore.