[15.0][MIG] contract: Migration to version 15.0

[jcdrubay]

Fixes deprecation warnings

• onchange method ContractAbstractContractLine._onchange_product_id returned a domain, this is deprecated
• SavepointCase is deprecated: https://github.com/odoo/odoo/blob/15.0/odoo/tests/common.py#L742
• assertDictContainsSubset: According to: https://stackoverflow.com/questions/20050913/python-unittests-assertdictcontainssubset-recommended-alternative

includes migration scripts to upgrade from v14:

To test

update ir_module_module set latest_version='15.0.0' where name='contract';

Output

2021-10-29 05:53:06,279 46091 INFO v15 OpenUpgrade: contract: post-migration script called with version 15.0.0
2021-10-29 05:53:06,279 46091 INFO v15 OpenUpgrade: contract: loading migrations/15.0.1.0.0/noupdate_changes.xml
2021-10-29 05:53:06,309 46091 DEBUG v15 OpenUpgrade: 0 rows affected after 0:00:00.000359 running
            DELETE FROM ir_translation
            WHERE module = 'contract' AND name LIKE 'mail.template,%' AND res_id = 10;

2021-10-29 05:53:06,309 46091 DEBUG v15 OpenUpgrade: 0 rows affected after 0:00:00.000221 running
            DELETE FROM ir_translation
            WHERE module = 'contract' AND name LIKE 'ir.ui.view,%' AND res_id = 752;

2021-10-29 05:53:06,309 46091 DEBUG v15 OpenUpgrade: 0 rows affected after 0:00:00.000232 running
            DELETE FROM ir_translation
            WHERE module = 'contract' AND name LIKE 'mail.template,%' AND res_id = 11;

[dreispt]

I find this dangerous... a subsequent module upgrade will wipe manual changes to the template.

[jcdrubay]

Thanks, it did this during dev to test the mail template changes more easily and forgot to put it back.

[dreispt]

Well. This should be a migration. I'm not sure about the best way do do this.

[jcdrubay]

Are you worried about the upgrade from v14 to v15? Then, indeed, it's more complex than I first thought.

Anyway, I think we should keep the noupdate=1 because that it is the expected behavior.

Then we could have a migration script to force the update, because anyway any old templates from an older major version of Odoo wouldn't work. But is such a migration script from one major version to another meant to be here, in this repo? It has been discussed few months back (without conclusion so far I think).

OCA/OpenUpgrade#2661 (comment)

[pedrobaeza]

Yes, the usual flow is to let them noupdate=1 always, and add a migration script that loads the changes in this kind of templates and delete translations. Example:

https://github.com/OCA/OpenUpgrade/blob/e6ea2d420aca964e374ba671380b8948ce65c707/addons/account/migrations/13.0.1.1/post-migration.py#L1067-L1074

https://github.com/OCA/OpenUpgrade/blob/e6ea2d420aca964e374ba671380b8948ce65c707/addons/account/migrations/13.0.1.1/noupdate_changes.xml#L49

[jcdrubay]

Thanks for sharing the examples @pedrobaeza .

[dreispt]

Why this change?

[jcdrubay]

SavepointCase is deprecated:
https://github.com/odoo/odoo/blob/15.0/odoo/tests/common.py#L742

"Deprecated class SavepointCase has been merged into TransactionCase",

[dreispt]

Note that pre-commit is red.

[jcdrubay]

Thanks @dreispt . I will submit a new version tomorrow. Hopefully this will be merged during the OCA days :)

[jcdrubay]

@dreispt @pedrobaeza I finalized the changes. Hopefully, next time you only need to ask me to rebase ;)

[dreispt]

LGTM, but would like confirmation from Pedro that the template migration looks good

[pedrobaeza]

Thanks for the migration. Please try to put a good PR title, or on contrary, it won't be found when searching, and this may lead to duplicated migration PR and so on. I have already changed the title for this one.

Other formal aspects before starting my review:

• Please squash administrative commits with the main one as per the migration guide, as the number of commits is very high if not. For example, you can perform this squashing:
  
• It seems you have several pre-commit + black commits. You should squash them with the commit that originates them.
• I would also squash all the migration stuff, explaining in the migration commit message the actions done.

[jcdrubay]

@pedrobaeza If you want to review, the rebase is complete. (it's not the place to discuss this, but I find it a bit "over-demanding" to ask contributor of migration to squash historical commits from the previous versions). I am fine with it, but I guess it is something to consider to facilitate the on-boarding of new contributors.

Runbot still fails but I think it is with know issues:

• [0mThe command '/bin/sh -c /bin/bash -c "source ${REPO_REQUIREMENTS}/virtualenv/python3.8/bin/activate && source ${REPO_REQUIREMENTS}/virtualenv/nodejs/bin/activate && source /rvm_env.sh && /install"' returned a non-zero code: 1
• docker: Error response from daemon: pull access denied for oca-contract, repository does not exist or may require 'docker login': denied: requested access to the resource is denied.

[ghost]

[Not a maintainer] [Figured how to Review now :)] Tested on clean VM all seems to work only thing that flagged in my logs was:

2021-11-18 09:24:40,145 2940 INFO odoo odoo.addons.base.models.ir_module: python external dependency on 'dateutil' does not appear to be a valid PyPI package. Using a PyPI package name is recommended.
Which I believe to not be a migration issue but a dependency issue in general. Putting it on my production instance until the merge goes through. Thanks @jcdrubay

[pedrobaeza]

/ocabot merge nobump

[OCA-git-bot]

On my way to merge this fine PR!
Prepared branch 15.0-ocabot-merge-pr-738-by-pedrobaeza-bump-nobump, awaiting test results.

[OCA-git-bot]

Congratulations, your PR was merged at 06e41e5. Thanks a lot for contributing to OCA. ❤️