[19.0][MIG] sale_order_carrier_auto_assign: Migration to 19.0

[matiasperalta1]

No description provided.

[rousseldenis]

/ocabot migration sale_order_carrier_auto_assign

[rousseldenis]

@matiasperalta1 Could you fix tests ?

[rousseldenis]

@matiasperalta1 You should not try using demo data. Please create data in test instead

[matiasperalta1]

@rousseldenis Hi! I've already fixed tests

[JordiMForgeFlow]

@matiasperalta1 which is the rationale of not using the delivery wizard anymore?

[alexey-pelykh]

Review: Migration to 19.0

Thanks for the migration work. The module structure is clean and CI is green. I have several observations after comparing with the 18.0 version.

Critical / Blocking

1. _set_delivery_carrier significantly simplified -- potential behavioral regression

The 18.0 version used action_open_delivery_wizard() + choose.delivery.carrier wizard flow to set the carrier and compute rates. The 19.0 version replaces this with direct field assignment + carrier.rate_shipment() + order._create_delivery_line().

While this simplification might be intentional due to Odoo 19.0 API changes, the result.get("carrier_price", result.get("price")) fallback pattern on line 67 of sale_order.py looks ad-hoc. In standard Odoo, rate_shipment returns price (not carrier_price) as the key. If carrier_price was introduced by the wizard in previous versions, this key may no longer exist in 19.0, making the fallback always hit price. Please verify which key rate_shipment() actually returns in 19.0 and simplify accordingly.

2. write() override and _set_carrier_on_create() removed -- feature regression

The 18.0 version overrode write() so that when order lines are added to an existing draft sale order, the carrier is automatically set. The 19.0 version removes this entirely (no write override, no _set_carrier_on_create method). This means:

• Creating a sale order in two steps (first create empty order, then add lines) will NOT auto-assign a carrier in 19.0
• The 18.0 tests test_sale_order_carrier_auto_assign_create_2steps_from_order, test_sale_order_carrier_auto_assign_create_2steps_from_line, and test_sale_order_carrier_auto_assign_create_3steps_from_line are all dropped

If this is intentional (simplification for 19.0), it should be documented. If not, this is a behavioral regression that affects users who build orders incrementally.

3. sale_order_views.xml removed from manifest but not from repo?

The 18.0 __manifest__.py includes "views/sale_order_views.xml" which added carrier_id as an invisible field in the form view (needed for onchange writes). The 19.0 manifest only lists "views/res_config_settings_views.xml". If carrier_id is no longer needed in the view for the onchange to work (likely due to Odoo 19.0 changes), that is fine. But please confirm this was verified -- otherwise the @api.onchange method _add_delivery_carrier_on_partner_change may fail to write carrier_id on the order.

Moderate

4. delivery_set check inconsistency

In _set_delivery_carrier, line 48: if order.delivery_set: continue -- this checks delivery_set (a computed field that checks if a delivery line exists). But in the 18.0 version, the check was if preserve_order_carrier and order.delivery_set: continue -- meaning the delivery_set check was only applied when preserving the carrier. The 19.0 version always skips if delivery_set is truthy, regardless of the preserve_order_carrier flag. This means that when called from _add_delivery_carrier_on_partner_change (which passes preserve_order_carrier=False), orders that already have a delivery line will be skipped even though the intent might be to update the carrier.

5. Tests: test_sale_order_carrier_auto_assign_carrier_already_set uses self-created carrier instead of demo ref

Good that this was addressed per @rousseldenis's earlier review comment about not using demo data. The alternative carrier is now created in setUpClass. However, delivery_local_delivery.fixed_price = 10 is set both in the create() dict ("fixed_price": 10.0) and then overridden again on the next line. This is harmless but redundant -- consider removing the duplicate assignment.

6. Missing weight on product_storable in 18.0 tests added in 19.0

The 19.0 version adds "weight": 1.0 to product_storable. This is a good addition for shipping rate calculations, but it means the test behavior differs from 18.0. Worth a note in the PR description.

7. test_sale_order_carrier_onchange_no_order_line test dropped

The 18.0 version had this test ensuring no error occurs when changing partner on an empty sale order. It was removed in 19.0 without explanation.

8. _create_sale_order in OnConfirm -- removed extra _create_sale_order() call

In 18.0 TestSaleOrderCarrierAutoAssignOnConfirm.setUpClass, there was an extra cls._create_sale_order() call before the Form-based creation. This was removed in 19.0. If it was needed for setup (e.g., to trigger some initialization), its removal might cause subtle test differences.

Suggestions

• The PR body is empty. For a migration PR with significant logic changes (wizard flow removed, write override removed, simplified rate calculation), documenting what changed and why helps reviewers and future maintainers.
• Consider whether the context key carrier_on_create guard that was present in 18.0 (to prevent infinite recursion during create/write) is still needed. The 19.0 create method calls _set_delivery_carrier directly without the guard, which might cause issues if other modules call create in contexts where carrier assignment should be suppressed.
• The .pot file header says Project-Id-Version: Odoo Server 18.0 -- should be updated to 19.0.

Summary

The migration compiles and passes CI, but there are meaningful behavioral changes from 18.0 that should be explicitly acknowledged: the removal of the write() override (two-step order creation), the simplified carrier assignment flow (no wizard), and several dropped tests. If these are intentional simplifications for the 19.0 API, please document them in the PR description so reviewers can confirm.

[rousseldenis]

LGTM