[FIX][database_cleanup] don't destroy values when there are empty default properties

[hbrunn]

when you have a default property with a falsy value, the cleanup will go very wrong because it will delete all your properties.

The right thing to do in such a case is to remove the empty default value.

This one we should cherry pick back and forth through the versions.

I also slipped in a cosmetic fix for display of redundant properties, this was always showing False. Last thing I fixed while being at this PR is that the action to purge just a selection of lines was migrated wrongly to 10 (and the following)

[pedrobaeza]

Please check Travis

[hbrunn]

looks like a general failure, 10.0 also fails there: https://travis-ci.org/OCA/server-tools/builds/486443599

[pedrobaeza]

OK, I see... it's a problem in auth_brute_force since the merge of odoo/odoo#30768

[hbrunn]

aha, thanks for the link. I still believe odoo/odoo#20033 would be a better patch...

[pedrobaeza]

Are they complementary or replaceable? I'd say for pushing on reviewing your PR.

[christophlsa]

OK, I see... it's a problem in auth_brute_force since the merge of odoo/odoo#30768

@pedrobaeza @hbrunn Is there already a ticket for the issue with the failing unit tests? We were also seeing the failing tests but I couldn't fix it. I could just find out that the changes by the function "_auth_attempt_update" were not recognized or were not in effect at the point where the unit test is failing. I printed all three records in res.authentication.attempt but all three have still the result False instead of 'failed'.

[pedrobaeza]

@christophlsa we have been investigating locally, but also without success for now. At the end, we were using a hack, and the hack is no more possible due to the changes. One possible workaround for now is to disable that tests for having a green branch and re-enable them when fixed.

[hbrunn]

let's don't work around like that. I put this on my list to look at (no promise on delivery time)

[hbrunn]

I spent a while on this, we really need stackable context manager cursors for this to ever work properly. We can patch away a few rollbacks at some places in the tests and then it doesn't fail, but then we don't really test what we should test

[pedrobaeza]

Mocks aren't an option?

[hbrunn]

not really. When you rewrite most of the module from using context managers to committing/rolling back the transactions manually, it should be possible to make this work

[StefanRijnhart]

@hbrunn you are right, your nestable TestCursor seems to fix fixes the tests. I am applying it here: https://github.com/OCA/server-tools/pull/1526/files

[pedrobaeza]

Please squash last 2 commits and take the occasion to rebase as now main branch is green

[hbrunn]

@pedrobaeza done
@StefanRijnhart cool! any complaints that I yank this into https://github.com/OCA/oca-decorators if ever need arises in some other repo?

[StefanRijnhart]

@hbrunn no, go ahead! And thanks for the original work!