[18.0][MIG] sale_order_carrier_auto_assign: Migration to 18.0 #3394
Conversation
Currently translated at 100.0% (1 of 1 strings) Translation: sale-workflow-13.0/sale-workflow-13.0-sale_order_carrier_auto_assign Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-13-0/sale-workflow-13-0-sale_order_carrier_auto_assign/es/
Currently translated at 100.0% (1 of 1 strings) Translation: sale-workflow-16.0/sale-workflow-16.0-sale_order_carrier_auto_assign Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_order_carrier_auto_assign/it/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: sale-workflow-16.0/sale-workflow-16.0-sale_order_carrier_auto_assign Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_order_carrier_auto_assign/
Currently translated at 100.0% (7 of 7 strings) Translation: sale-workflow-16.0/sale-workflow-16.0-sale_order_carrier_auto_assign Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_order_carrier_auto_assign/it/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: sale-workflow-17.0/sale-workflow-17.0-sale_order_carrier_auto_assign Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-17-0/sale-workflow-17-0-sale_order_carrier_auto_assign/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: sale-workflow-17.0/sale-workflow-17.0-sale_order_carrier_auto_assign Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-17-0/sale-workflow-17-0-sale_order_carrier_auto_assign/
Currently translated at 100.0% (6 of 6 strings) Translation: sale-workflow-17.0/sale-workflow-17.0-sale_order_carrier_auto_assign Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-17-0/sale-workflow-17-0-sale_order_carrier_auto_assign/es/
There was a problem hiding this comment.
hi @lef-adhoc, thanks for migration this module.
I tested the functional and it works fine.
btw, could you squash administrative commits.
| .with_context(**delivery_wiz_context) | ||
| .create({}) | ||
| ) | ||
| delivery_wiz._get_delivery_rate() |
There was a problem hiding this comment.
do we really need to call this function, I checked and found that it is mainly called in onchange functions and did not see it appear in any test in odoo codebase. I tried removing it and found that the module still works as expected and I guess that just calling button_confirm is enough, since in button_confirm there is a call to set_delivery_line function. I'm not sure if it's completely correct because I might miss something, do you think so?
There was a problem hiding this comment.
I deleted the line and I get an error in the tests and if I don't delete it I don't get an error. Can you check?
There was a problem hiding this comment.
thanks for taking the time, maybe it was worth appearing on that line.
|
|
||
| def _add_delivery_carrier_on_confirmation(self): | ||
| """Automatically add delivery.carrier on sale order confirmation""" | ||
| for order in self: |
There was a problem hiding this comment.
I see action_confirm has iterated over the records in sale.order and called _add_delivery_carrier_on_confirmation function for each single record. So why do we need to iterate over it again, you can consider it with function that directly handles that record and call ensure_one function will make the code clearer
There was a problem hiding this comment.
I did it another way, what do you think?
lef-adhoc
force-pushed
the
18.0-mig-sale_order_carrier_auto_assign
branch
3 times, most recently
from
7c292e2 to
362e2ef
Compare
|
This PR has the |
There was a problem hiding this comment.
You are missing this commit bfa00bb
and this change OCA/delivery-carrier@bdff1b3
and integrate those commits that were postponed for next version https://github.com/OCA/sale-workflow/pull/3081/commits
| for rec in self: | ||
| if not rec.company_id.carrier_auto_assign: | ||
| continue | ||
| rec._add_delivery_carrier_on_confirmation() | ||
| self.filtered( | ||
| lambda so: so.company_id.carrier_auto_assign | ||
| )._add_delivery_carrier_on_confirmation() |
There was a problem hiding this comment.
This change should not be part of the migration commit
There was a problem hiding this comment.
I have already changed it to another commit
|
@jbaudoux This is the migration to 18 of the module that is already in 17 and has the same commits that are in 17 https://github.com/OCA/sale-workflow/commits/17.0/sale_order_carrier_auto_assign shouldn't you do the PRs first to 17 before including them in 18? |
…irst Replaced the loop with `filtered()` to improve code clarity and avoid iterating over irrelevant records. The method `_add_delivery_carrier_on_confirmation` is now only called on applicable orders directly.
lef-adhoc
force-pushed
the
18.0-mig-sale_order_carrier_auto_assign
branch
from
362e2ef to
e082bdd
Compare
No description provided.