[19.0][OU-IMP] hr_expense: hr_expense_sheet compatibility

[victoralmau]

hr_expense_sheet compatibility

Related to OCA/hr-expense#378

@Tecnativa TT58631

[MiquelRForgeFlow]

@victoralmau please, rebase. I made an error when force pushing to 19.0 (it's already amended)

[hbrunn]

why does any of this happen in hr_expense's migration, and not in the pre-init script of hr_expense_sheet?

[hbrunn]

what has this column to do with the above?

[victoralmau]

I am referring to everything that comes after that: hr.expense.sheet

[hbrunn]

so put it after what you're referring to

[victoralmau]

why does any of this happen in hr_expense's migration, and not in the pre-init script of hr_expense_sheet?

Why can't it be in OpenUpgrade?

There are several reasons why I believe it SHOULD be here:

• It is “difficult” for someone who has used Odoo and hr.expense.sheet before 19.0 to stop using that functionality; reporting ‘grouped’ expenses (and reviewing them) without that option is “very difficult.”
• Modifying data in tables that will no longer be used in Odoo core has no side effects; in my opinion, it’s unnecessary to add logic to hr_expense_sheet to perform that renaming when OpenUpgrade already does it in hr_expense.

[hbrunn]

I think it just doesn't belong to this migration script because people won't necessary use hr_expense_sheet. And technically, it's not difficult to detect on installation of that module if you're coming from an ex-v18 version, so I don't see a reason to have unrelated code in the migration script of hr_expense

[victoralmau]

@pedrobaeza and @MiquelRForgeFlow What do you think about this?

[pedrobaeza]

It's a fact that both OpenUpgrade and OCA hr_expense_sheet initialization are depending each other, as if you migrate with enterprise, you won't find the expected columns (it removes them), but it's true that copying some columns, processing them or renaming them without knowing if you are going to install hr_expense_sheet is useless and time/space consuming (although we plan to install it in most installations), so let's move it to pre_init_hook as stated by Holger, and you can detect if former_sheet_id exists and then trigger in that case the rest of the code. In the module, you should also include a migration script without @openupgrade.migrate decorator importing and calling pre_init_hook.

[victoralmau]

Moved to OCA/hr-expense#378