Review: sale_cancel_confirm migration to 18.0

Thanks for the migration work. I reviewed the diff, the base module (base_cancel_confirm in OCA/server-ux 18.0), the Odoo 18 native sale.order.action_cancel / _action_cancel flow, and the equivalent 14.0 source.

Overall the module is structurally sound for a migration port, but there are several issues ranging from correctness concerns to missing tests.

1. Functional overlap with Odoo 18 native cancel wizard

Odoo 18 sale.order.action_cancel() already shows a cancel wizard (sale.order.cancel) when cancelling non-draft orders via _show_cancel_wizard(). This module overrides _action_cancel() to intercept after Odoo's own wizard has already been shown (or skipped).

This means:

• For non-draft SOs, the user may see two consecutive wizards: first Odoo's native cancel wizard, then base_cancel_confirm's wizard.
• The cancel_reason captured here is stored separately from Odoo 18's native sale.order.cancel model, which also captures a reason.

This should be documented in the README, and ideally the module should either suppress the native wizard (via context disable_cancel_warning) or override action_cancel instead of _action_cancel to provide a single cancel experience.

2. test_enable guard pattern is fragile

def _action_cancel(self):
    test_condition = not config["test_enable"] or (
        config["test_enable"]
        and self.env.context.get("test_sale_order_cancel_confirm")
    )
    if not test_condition:
        return super()._action_cancel()

This pattern makes the cancel confirmation silently inactive during all test runs unless a specific context key is passed. This is inherited from the 14.0 approach, but:

• It means any other module's tests that call _action_cancel will bypass this module entirely, hiding potential conflicts.
• The purchase_request_cancel_confirm module (18.0) does not use this test_enable guard -- it intercepts unconditionally.
• A cleaner approach is to always intercept and have the module's own tests use the context key only where needed to drive through the wizard flow.

3. Missing tests (flagged in PR TODO)

The PR description already notes Test Script as TODO. This module has zero test coverage:

• No tests/ directory
• No __init__.py for tests
• codecov/patch and codecov/project both show failure

For OCA standards, tests are required. At minimum:

• Test that cancelling a confirmed SO triggers the wizard
• Test that action_draft clears cancel data
• Test the test_enable guard behavior

4. Translation files reference Odoo 14.0

The .pot and .po files contain:

"Project-Id-Version: Odoo Server 14.0\n"

These should be regenerated for 18.0. The .pot file and translation entries reference fields like __last_update which was removed in later Odoo versions. The translations should be refreshed.

5. model directory naming

The module uses model/ (singular) for its models directory. While this matches the 14.0 source and the base_cancel_confirm module, the OCA convention in 18.0 is generally models/ (plural). This is a minor style point -- consistency with the base module is also valid.

6. Missing security/ and views/ directories

The module adds fields (cancel_confirm, cancel_reason) to sale.order via the base.cancel.confirm mixin, and the view injection is handled by base_cancel_confirm's get_view override. No explicit security rules or views are needed if the base module handles everything. This is fine.

7. Agreement with rousseldenis feedback

The inline comment about preferring a res.company configuration to enable/disable the behavior is worth considering. The base_cancel_confirm module already supports an ir.config_parameter toggle ({model}.cancel_confirm_disable), but a company-level toggle would be more user-friendly. That said, this is an enhancement beyond the scope of a migration PR.

Summary

The blocking item is the missing test coverage. The double-wizard behavior (finding 1) should at minimum be documented, and ideally addressed by overriding action_cancel instead of _action_cancel, or by passing disable_cancel_warning in context.