[16.0][IMP] sale_stock_cancel_restriction: Option to restrict if delivered

[mmequignon]

ping @pedrobaeza @jbaudoux

Addresses #3633

[pedrobaeza]

Please check the red CI

[simahawk]

It's unrelated #3638

[jbaudoux]

@mmequignon rename title sale_stock_cancel_reservation to sale_stock_cancel_restriction

[jbaudoux]

LG

[mt-software-de]

LG. Is there a backport for v14?

[pedrobaeza]

It's unrelated #3638

Yeah, but we can't merge this if not fixed.

[jbaudoux]

@mmequignon Need this on v14

[rousseldenis]

@mmequignon Could you rebase ?

[mt-software-de]

@mmequignon Could you rebase ?

ping @mmequignon

[rousseldenis]

@mmequignon

[alexey-pelykh]

Thanks for this improvement @mmequignon -- the option to restrict cancellation only when an outgoing delivery is done (vs. any transfer) is a useful enhancement for multi-step delivery setups.

A few findings from review:

Correctness

self.warehouse_id on a multi-record recordset

In action_cancel, self can be a recordset of multiple sale orders (e.g. batch cancellation). Accessing self.warehouse_id on a multi-record recordset with different warehouses will raise a ValueError in Odoo 16. The logic should iterate per order or handle this case. For example:

def action_cancel(self):
    for order in self:
        domain = [("state", "=", "done")]
        if order.warehouse_id.restrict_sale_cancel_after_delivery:
            domain += [("picking_type_id.code", "=", "outgoing")]
        order.picking_ids.filtered_domain(domain).action_cancel()
    return super().action_cancel()

Manifest / Versioning

• Missing version bump: The version in __manifest__.py is still 16.0.1.0.0. Since this adds a new feature (new field + configuration option), it should be bumped to at least 16.0.1.1.0.

Documentation

• Missing readme/DESCRIPTION.rst update: The new warehouse-level option should be documented so users know it exists.
• Missing readme/CONTRIBUTORS.rst update: Camptocamp / @mmequignon should be added.

Minor / Style

• default=False is redundant on the Boolean field in stock_warehouse.py -- False is already the default for Boolean fields in Odoo.
• XML indentation: views/stock_warehouse.xml uses 2-space indent for <record> children but 6-space for inner <field> arch content. OCA convention is typically 4-space consistent indentation.

Positive Notes

• Good use of filtered_domain instead of filtered with lambda (better performance).
• Test coverage is solid: covers default behavior, multi-step pick (internal done, should allow cancel), and multi-step ship (outgoing done, should block cancel).
• Clean separation of the new model extension.

The self.warehouse_id issue on multi-record recordsets is the main correctness concern. The rest are documentation/style items.

[alexey-pelykh]

Bug (multi-record recordset): self.warehouse_id will raise ValueError when self contains multiple sale orders with different warehouses (e.g. batch cancellation from list view). This should iterate per order:

for order in self:
    domain = [("state", "=", "done")]
    if order.warehouse_id.restrict_sale_cancel_after_delivery:
        domain += [("picking_type_id.code", "=", "outgoing")]
    order.picking_ids.filtered_domain(domain).action_cancel()
return super().action_cancel()

[alexey-pelykh]

Minor: default=False is redundant for Boolean fields in Odoo (False is already the default). Can be simplified to:

restrict_sale_cancel_after_delivery = fields.Boolean(help=HELP_RESTRICT)

[alexey-pelykh]

The version should be bumped (e.g. 16.0.1.1.0) since this PR adds a new feature (new field on stock.warehouse, new configurable behavior).