-
-
Notifications
You must be signed in to change notification settings - Fork 694
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] project: Deal with forcecreate=False records #1063
Conversation
@mreficent this is what I was talking about on the other PR, but I decided to do it myself as I don't have it clear that everything is going to be correct. @hbrunn my fear with |
I tried this yesterday but |
I think we need to modify the core.. |
I'd rather not do anything at all with those records as far as xml files are concerned, and do conditional writes in python code where necessary. We're talking about a couple of records per release, right? |
Well the pity is to not have an standard way to do it. Maybe what we can do is to filter out the non existing records in openupgradelib method. |
You know: if you don't have it automated, for sure we will have problems in the future when we forget to do the specific thing. |
sure, standard ways are better. If you want to commit time to this, perfect. Otherwise, we'll better revert the commit loading this for the project migration, as I believe real world databases tend to have the record deleted |
@pedrobaeza this PR works with OCA/openupgradelib#80 |
@pedrobaeza as of OCA/openupgradelib#80, you'll have to pass |
Did you consider leaving out the noupdate entries from the project module in 10 that only disable the earlier forcecreate=False items from 9.0? If the migrated database is from Odoo 8.0 the entries won't exist (which causes the problem you are trying to solve here) and if the database was installed from version 9.0 the entries will have been cleaned up or reused(!) during the 9.0 lifecycle by the customer. |
As per my previous comment I am proposing #1124 |
After the merge of #1124, this is no longer needed I believe. |
Well, yes, but I still want to try the test. |
Uhm, strange error, but anyway tests are correct. I close this PR then. |
Let's see if we have all the cases covered. I have included a test for that.
cc @Tecnativa