-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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] sale_order_line_description #3070
[16.0][MIG] sale_order_line_description #3070
Conversation
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: sale-workflow-11.0/sale-workflow-11.0-sale_order_line_description Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-11-0/sale-workflow-11-0-sale_order_line_description/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: sale-workflow-12.0/sale-workflow-12.0-sale_order_line_description Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-12-0/sale-workflow-12-0-sale_order_line_description/
Currently translated at 100.0% (7 of 7 strings) Translation: sale-workflow-12.0/sale-workflow-12.0-sale_order_line_description Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-12-0/sale-workflow-12-0-sale_order_line_description/zh_CN/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: sale-workflow-14.0/sale-workflow-14.0-sale_order_line_description Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-14-0/sale-workflow-14-0-sale_order_line_description/
Currently translated at 100.0% (7 of 7 strings) Translation: sale-workflow-14.0/sale-workflow-14.0-sale_order_line_description Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-14-0/sale-workflow-14-0-sale_order_line_description/de/
Currently translated at 60.0% (6 of 10 strings) Translation: sale-workflow-14.0/sale-workflow-14.0-sale_order_line_description Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-14-0/sale-workflow-14-0-sale_order_line_description/fr_FR/
Currently translated at 14.2% (1 of 7 strings) Translation: sale-workflow-15.0/sale-workflow-15.0-sale_order_line_description Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-15-0/sale-workflow-15-0-sale_order_line_description/it/
Currently translated at 100.0% (7 of 7 strings) Translation: sale-workflow-15.0/sale-workflow-15.0-sale_order_line_description Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-15-0/sale-workflow-15-0-sale_order_line_description/es/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: sale-workflow-15.0/sale-workflow-15.0-sale_order_line_description Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-15-0/sale-workflow-15-0-sale_order_line_description/
/ocabot migration sale_order_line_description |
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.
@mariobinhex Thanks for this.
Usually, it's fair to base your PR on other one to integrate the previous work (from #2450).
Moreover, his code was better than this one.
Could you change ? Thanks
@@ -8,8 +8,8 @@ class SaleOrderLine(models.Model): | |||
_inherit = "sale.order.line" | |||
|
|||
@api.onchange("product_id") | |||
def product_id_change(self): | |||
res = super(SaleOrderLine, self).product_id_change() | |||
def _onchange_product_id_warning(self): |
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.
Please use _compute_name() function instead.
@@ -57,6 +58,7 @@ def test_check_sale_order_line_description(self): | |||
sale_order_line = self.sale_order_line_model.with_user(self.user_1).create( | |||
line_values.copy() | |||
) | |||
sale_order_line._onchange_product_id_warning() |
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.
After having changed to _compute_name() override, you don't need this anymore
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.
@rousseldenis Hi and thanks for the review
Pls check my changes and let me know if any further changes are needed.
Greetings!
class SaleOrderLine(models.Model): | ||
_inherit = "sale.order.line" | ||
|
||
@api.onchange("product_id") |
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.
It should be @api.depends
.
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 Thanks for the review
Done!
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.
LGTM! Functional review on runboat.
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.
Hello @MarioLM-23, thanks a lot for your work!
Could you please squash the following commits to only one?
According to https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-16.0 at the end we're supposed to have:
- [IMP] sale_order_line_description: black, isort, prettier
- [MIG] sale_order_line_description: Migration to 16.0
Thank you in advance.
I close this in favor of #3523 where I keep @MarioLM-23's changes. |
Last PR of migration module in open stage: #2450