[17.0][OU-FIX] hr: set private address data from private partners

[hbrunn]

I don't think the hr migration should touch partner data at all: If a partner was marked as private in v16, its data is masked in the migration of the base module. If it's not private, it hasn't been protected before and we shouldn't overwrite it (think address_id == address_home_id == user_id.partner_id).

Also for handling private partners, we need to take the data from ou_res_partner_private if it exists, as otherwise we copy garbage into the employee fields.

[legalsylvain]

Makes sense !

[pedrobaeza]

The UPDATE is needed, as we don't want to leak private information, now that there's no record rule protection on private addresses. You can restrict the update to type="private", but not to remove it.

[hbrunn]

please read the link to what the base migration does and reconsider

[pedrobaeza]

OK, we can remove it then, but with that code, you may have "NOT NULL" constraints over that fields if they have been added by third-party modules, so it's better to add "***" as I did, don't you think?

[hbrunn]

the hr.employee#private_* fields are new in v17, so any constraints added by other modules won't be in effect yet. And whatever constraints exist on partner fields, will have been satisfied or violated before, so not really an issue here.

If you mean a v16 module adding ie a column private_email with a not null constraint on hr.employee, well, that's just tough luck I think, but no reason to insert bogus data for everyone. Users with such a setting might want to contribute detecting such constraints then and add the asterisks if they exist. (that case is covered now by adding he.$fieldname to the fallback chain)

[pedrobaeza]

No, I mean that you have for example a not null constraint on res.partner~state_id, or country_id.

[hbrunn]

then all partners have this field set (or not and then the constraint is not effect) - I don't see the relevance of this for either this PR or the code from base?

[pedrobaeza]

I mean the UPDATE sentence in base migration: https://github.com/OCA/OpenUpgrade/blob/17.0/openupgrade_scripts/scripts/base/17.0.1.3/pre-migration.py#L167 that is setting to NULL.

[hbrunn]

ah, I was confused because that's your own code and you talked about street before. well that could be a problem, but still not relevant for this PR, right?

[pedrobaeza]

Yes, but it's just to take that into account. It's true that I mention street because I didn't see that field was correctly masked.