[15.0][OU-ADD] commission: Migration scripts

[chienandalu]

cc @Tecnativa TT43411

[OCA-git-bot]

Hi @pedrobaeza,
some modules you are maintaining are being modified, check this out!

[pedrobaeza]

This should be tagged as [OU-ADD], not [IMP].

[pedrobaeza]

The rest is OK and no more changes detected by my side.

[chienandalu]

The rest is OK and no more changes detected by my side.

What about the change of agent_line into settlement_line_ids?: https://github.com/OCA/commission/pull/387/files/b9bc5b033c3d29a9c03b303917ae4945754e9a7b..359c1c13e294456ebf1ebb90c2b668f843d0c038#diff-c9164979c9473b61249db43c11ccb7ec045fee4e548c0f327dc636a8d161a15dR164

[pedrobaeza]

It's a one2many, so no schema changed. You can call though to rename_fields for benefiting of the future feature of changing domains in filters and exports.

[chienandalu]

It's a one2many, so no schema changed. You can call though to rename_fields for benefiting of the future feature of changing domains in filters and exports.

But it was a m2m before

[pedrobaeza]

OK, then you must convert the other part of the inverse name. Or is it already there in previous version?

[hildickethan]

OK, then you must convert the other part of the inverse name. Or is it already there in previous version?

It isn't there in the previous version, but it seems a bit complicated logistically to not either lose data or break it.
If we add extra settlement lines so all the agent lines keep their link, it adds incorrect data to the settlement. I think we just take the first agent line per settlement line to migrate it

env.cr.execute("""
SELECT DISTINCT ON (agent_line_id), settlement_id
FROM settlement_agent_line_rel
ORDER BY agent_line_id
""")
for agent_line_id, settlement_id in env.cr.fetchall():
    env["commission.settlement.line"].browse(settlement_id).invoice_agent_line_id = agent_line_id

This should work, but I don't have a db to migrate to test it atm

[chienandalu]

hildickethan-S73

Thanks. Take a look on post-migration for a performant version of your proposal :)

[hildickethan]

Looks good! Looking to try it out soon but I was thinking, wouldn't these scripts need to be in hooks rather than migration scripts?

For instance a new database that is migrating from 14 to 15 I imagine gets to the point where it updates sale_commission, sees that theres a new commission dependency, which is when we need to adapt the data before it creates all the tables and models messing up the rename methods

I think it should be a hooks with a table_exists before it, since I'm not sure how the method acts when it doesn't exist but I assume it errors

[pedrobaeza]

OpenUpgrade runs the migration scripts also for newly installed modules. Hooks are designed for other purposes and can't have things like version discrimination.

[hildickethan]

OpenUpgrade runs the migration scripts also for newly installed modules. Hooks are designed for other purposes and can't have things like version discrimination.

Ah I didn't know OU did some magic there, I was thinking of normal upgrading. No problem then
Our real migration test will be some time next week, but it LGTM!

[chienandalu]

I just run it in a customer test migrations and it looks good :)

[pedrobaeza]

I have added a missing piece for the computed stored commission_id field in commission.settlement.line.

[hildickethan]

Worked for us too