-
-
Notifications
You must be signed in to change notification settings - Fork 551
[18.0][MIG] mrp_subcontracting_skip_no_negative: Migration to 18.0 #1674
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
base: 18.0
Are you sure you want to change the base?
[18.0][MIG] mrp_subcontracting_skip_no_negative: Migration to 18.0 #1674
Conversation
Currently translated at 50.0% (1 of 2 strings) Translation: manufacture-16.0/manufacture-16.0-mrp_subcontracting_skip_no_negative Translate-URL: https://translation.odoo-community.org/projects/manufacture-16-0/manufacture-16-0-mrp_subcontracting_skip_no_negative/it/
Currently translated at 100.0% (2 of 2 strings) Translation: manufacture-16.0/manufacture-16.0-mrp_subcontracting_skip_no_negative Translate-URL: https://translation.odoo-community.org/projects/manufacture-16-0/manufacture-16-0-mrp_subcontracting_skip_no_negative/it/
…of subcontracting component TT50668
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: manufacture-16.0/manufacture-16.0-mrp_subcontracting_skip_no_negative Translate-URL: https://translation.odoo-community.org/projects/manufacture-16-0/manufacture-16-0-mrp_subcontracting_skip_no_negative/
Currently translated at 100.0% (3 of 3 strings) Translation: manufacture-16.0/manufacture-16.0-mrp_subcontracting_skip_no_negative Translate-URL: https://translation.odoo-community.org/projects/manufacture-16-0/manufacture-16-0-mrp_subcontracting_skip_no_negative/it/
|
I found that the commit introduced during the 17.0 migration (#1454 (comment)) not only affects partial subcontracting receipts, but also skips the check from that PR (#1372) when the component quantity is insufficient. Therefore, in my latest commit, I decided to remove the code I originally added in my first PR (#1025), since it is not related to the no negative stock module and the scenario that motivated it no longer occurs in Odoo 18 (#1025 (comment)). |
kanda999
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functional review: It works as expected in my local env
yostashiro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AungKoKoLin1997 Would you consider adding a test where subcontracting picking receipt fails when component stock is insufficient?
|
In my last commit, I bring back the fix from odoo 16 that was not included in17.0 migration and improve tests. I believe my latest two commit should be back ported to 17.0. |
yostashiro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review. LGTM.
mrp_subcontracting_skip_no_negative/tests/test_mrp_subcontracting_skip_no_negative.py
Outdated
Show resolved
Hide resolved
|
This PR has the |
957bc71 to
0e3dabd
Compare
|
@victoralmau @fdesmottes @Raul-S73 |
Hi @AungKoKoLin1997 it's ok for me. |
victoralmau
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| from odoo.tests import Form, TransactionCase | ||
| from odoo.tools import mute_logger | ||
|
|
||
|
|
||
| class TestMrpSubcontractingSkipNoNegative(TransactionCase): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| from odoo.tests import Form, TransactionCase | |
| from odoo.tools import mute_logger | |
| class TestMrpSubcontractingSkipNoNegative(TransactionCase): | |
| from odoo.addons.base.tests.common import BaseCommon | |
| from odoo.tools import mute_logger | |
| class TestMrpSubcontractingSkipNoNegative(BaseCommon): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that BaseCommon is an internal helper built on top of TransactionCase, mainly used to standardize core tests (such as a mail-disabled context and common user/company setup).
Since our test doesn’t rely on those features, I prefer using TransactionCase to keep it simpler and more explicit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The suggestion is for speeding up tests. When you create a record, some chatter messages are added, some followers are auto-added, etc. Using this class, all that things are disabled by default. It's true there's a little overhead with the creation of some users, but it usually worths. Alternatively, you can do cls.env = cls.env(context=dict(cls.env.context, tracking_disable=True)).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pedrobaeza Thank you for the explanation.
I applied cls.env = cls.env(context=dict(cls.env.context, tracking_disable=True)) in the tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, it's not just those context values, it's more https://github.com/odoo/odoo/blob/18.0/odoo/addons/base/tests/common.py#L11. That's why we always suggest using that BaseCommon class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used BaseCommon class.
@victoralmau |
0e3dabd to
2fdfdb4
Compare
…r and improve tests
2fdfdb4 to
c5d2942
Compare

@qrtl QT6162