-
-
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
[17.0][MIG] sale_product_multi_add #3106
[17.0][MIG] sale_product_multi_add #3106
Conversation
b2a8cb5
to
48abdba
Compare
Currently translated at 50.0% (10 of 20 strings) Translation: sale-workflow-11.0/sale-workflow-11.0-sale_product_multi_add Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-11-0/sale-workflow-11-0-sale_product_multi_add/de/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: sale-workflow-12.0/sale-workflow-12.0-sale_product_multi_add Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-12-0/sale-workflow-12-0-sale_product_multi_add/
Currently translated at 100.0% (20 of 20 strings) Translation: sale-workflow-13.0/sale-workflow-13.0-sale_product_multi_add Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-13-0/sale-workflow-13-0-sale_product_multi_add/es/
Currently translated at 100.0% (20 of 20 strings) Translation: sale-workflow-13.0/sale-workflow-13.0-sale_product_multi_add Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-13-0/sale-workflow-13-0-sale_product_multi_add/ca/
Currently translated at 35.0% (7 of 20 strings) Translation: sale-workflow-14.0/sale-workflow-14.0-sale_product_multi_add Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-14-0/sale-workflow-14-0-sale_product_multi_add/zh_CN/
Currently translated at 100.0% (20 of 20 strings) Translation: sale-workflow-16.0/sale-workflow-16.0-sale_product_multi_add Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_product_multi_add/it/
create SO lines in batch
48abdba
to
fefb682
Compare
fefb682
to
b2ee807
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.
for the rest, functional and code review LGTM 👍🏿
b2ee807
to
1ab9438
Compare
/ocabot migration sale_product_multi_add |
@@ -33,7 +33,7 @@ | |||
<footer> | |||
<button | |||
name="select_products" | |||
string="Select" | |||
string="Confirm" |
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.
Why changing this ? Maybe a text help could be valuable in this wizard form. Something like 'Please select the products you want to add to the Sale Order, then click on 'Select' ? @jbaudoux
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.
Why changing this ? Maybe a text help could be valuable in this wizard form. Something like 'Please select the products you want to add to the Sale Order, then click on 'Select' ? @jbaudoux
I suggested to change to text from 'Select' to 'Confirm' as IMHO it makes more sense to be like so. As we are in the last step of dynamically adding various Sales Order Lines, we are not selecting anything, we are just confirming to add one/multiple order lines.
Then, a help text with the text you are suggesting would make sense on "wizard num 1", but not on the second one where you are just assigning the quantities for each product, and then confirming to add the products to the Sale Order. The text could be useful on "wizard num 1", nevertheless, I believe it is quite intuitive already that you need to add some products, but we can discuss on that of course.
What do you think?
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'm fine with this change.
It's better to put changes in a separate commit.
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 OK !!
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 OK!
/ocabot merge nobump |
On my way to merge this fine PR! |
Congratulations, your PR was merged at c9793cd. Thanks a lot for contributing to OCA. ❤️ |
Standard Migration
@ForgeFlow