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
[13.0][MIG] sale_order_type #1025
Conversation
Hey @kongrattapong, thank you for your Pull Request. It looks like some users haven't signed our Contributor License Agreement, yet.
Appreciation of efforts, |
1325133
to
2c1bb1b
Compare
2c1bb1b
to
e41962b
Compare
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.
Hi there,
I made a functional review. I could create a new sale order type with a different sequence, payment term, shipping policy, and so on.
Quotations were created according to the chosen type. So, LGTM. :)
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
e41962b
to
a3ef053
Compare
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
This PR has the |
This module contains more than one module (sale_order_line_description) |
/ocabot merge |
What a great day to merge this nice PR. Let's do it! |
Aborting because there are changes of other modules. |
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.
Remove the changes in sale_order_line_description module.
@dreispt please always check the full thread, not only the approvals. Some people thinks that putting request changes is very hard and only comment in the thread, but this is not correct to be merged as is. |
b90b67b
to
09c93d1
Compare
Thank for review I Removed. |
[UPD] Update sale_order_type.pot
The other one is a fake one that is dangling
If a contact doesn't have sale type, we use the commercial partner one.
Currently translated at 100.0% (31 of 31 strings) Translation: sale-workflow-11.0/sale-workflow-11.0-sale_order_type Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-11-0/sale-workflow-11-0-sale_order_type/es/
Currently translated at 100.0% (31 of 31 strings) Translation: sale-workflow-12.0/sale-workflow-12.0-sale_order_type Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-12-0/sale-workflow-12-0-sale_order_type/zh_CN/
Currently translated at 83.9% (26 of 31 strings) Translation: sale-workflow-12.0/sale-workflow-12.0-sale_order_type Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-12-0/sale-workflow-12-0-sale_order_type/it/
In the original onchange method by odoo, this return an dict with values(warnings and domians), but when inherit this method in this module they not return this dict. Because of this the alert setting in the partner was no working.
* Switch onchange on the main field to computed writable fields * Record rule is not up to v13 * Added migration script for record rule * Proper payment term field name in invoice * Realign tests
811b6fc
to
8e40947
Compare
/ocabot merge |
What a great day to merge this nice PR. Let's do it! |
@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 13.0-ocabot-merge-pr-1025-by-pedrobaeza-bump-no. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
Let's merge it manually as it's not possible to use bot. |
Ready to review