Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[16.0][MIG] purchase_order_qty_change_no_recompute: Migration to 16.0 #2193

Conversation

victoralmau
Copy link
Member

@victoralmau victoralmau commented Feb 29, 2024

Superseed #1886

Changes done:

  • Make it work correctly taking into account that the price_unit, name and date_planned fields use the same compute method (and therefore the same depends).

@Tecnativa TT46595

@victoralmau victoralmau force-pushed the 16.0-mig-purchase_order_qty_change_no_recompute branch from 2027465 to 197bee4 Compare February 29, 2024 11:08
victoralmau and others added 7 commits March 1, 2024 09:01
[UPD] Update purchase_order_qty_change_no_recompute.pot

[UPD] README.rst
purchase_order_qty_change_no_recompute 15.0.1.0.1
Translated using Weblate (Spanish)

Currently translated at 100.0% (1 of 1 strings)

Translation: purchase-workflow-15.0/purchase-workflow-15.0-purchase_order_qty_change_no_recompute
Translate-URL: https://translation.odoo-community.org/projects/purchase-workflow-15-0/purchase-workflow-15-0-purchase_order_qty_change_no_recompute/es/

[UPD] README.rst
…ement

- Include context keys for avoiding mail operations overhead.

[BOT] post-merge updates
@victoralmau victoralmau force-pushed the 16.0-mig-purchase_order_qty_change_no_recompute branch from 197bee4 to 3f5d9f9 Compare March 1, 2024 08:05
@victoralmau
Copy link
Member Author

After changing the approach of purchase_blanket_order and purchase_only_by_packaging to make them all compatible, I am now running into purchase_triple_discount issues.

Problem: Previously (v15) there was a context that allowed (in the tests) to omit onchange the relative to purchase_order_qty_change_no_recompute, so it "seemed" to have the standard operation and therefore the tests worked correctly.

Now: Using monkeypatching of get_depends() we can not perform test.

How could we add a context (or similar) in the purchase_order_qty_change_no_recompute tests so that in that case the test DOES change the behavior but in the rest it doesn't? (I thought about creating an ir.config.parameter with a value to re-set it when running the module tests, but I think that wouldn't work either). If we got to do it (in one way or another) the rest of changes in other modules will NOT be necessary.

Ping @pedrobaeza

- Improve documentation
- Improve tests
- Put it as rebel module

TT46595
@pedrobaeza pedrobaeza force-pushed the 16.0-mig-purchase_order_qty_change_no_recompute branch from 02449f4 to b39b2c6 Compare March 1, 2024 20:31
@pedrobaeza
Copy link
Member

In this case, there's no other solution except put it as rebel module. I have push an update branch with the following changes:

  • No dependency on purchase_discount. This was added on the previous migration work and it's totally incorrect, and at first sight, unneeded. If something needs to be done, it should be done on the module itself or in a new glue module.
  • Improve documentation and test.
  • Proper fields in the monkeypatching.
  • Remove other modules commits, as it's not needed.

@pedrobaeza
Copy link
Member

/ocabot migration purchase_order_qty_change_no_recompute

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Mar 1, 2024
@OCA-git-bot OCA-git-bot mentioned this pull request Mar 1, 2024
53 tasks
@pedrobaeza pedrobaeza requested a review from chienandalu March 1, 2024 20:36
Comment on lines +29 to +30
# Monkey-patching of the method
Field.get_depends = get_depends
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you study the possibility to hook into the model _setup_fields method and use a similar estrategy to the one in _pop_field?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already did. It doesn't serve, as the depends is something dynamic that can be expanded through other modules.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, the main issue here is that this technique can be use only once? Am I wrong?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I'm afraid

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although maybe subsequent monkey patches will work over the previous replaced method...

@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-2193-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit d8ec6fc into OCA:16.0 Mar 4, 2024
7 of 9 checks passed
@OCA-git-bot
Copy link
Contributor

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

@pedrobaeza pedrobaeza deleted the 16.0-mig-purchase_order_qty_change_no_recompute branch March 4, 2024 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants