[17.0][MIG] sale_order_amount_to_invoice

[dyngnijs]

No description provided.

[rousseldenis]

/ocabot migration sale_order_amount_to_invoice

[alexey-pelykh]

Review: sale_order_amount_to_invoice (17.0 migration)

This is a migration of the existing 16.0 module to 17.0. All CI checks pass, and the module has already received an APPROVED review from an OCA maintainer. Below is a technical analysis.

Summary

The migration is straightforward -- the only meaningful change from 16.0 to 17.0 is the conversion of attrs={'invisible': [...]} to the new 17.0 inline invisible="..." syntax in the XML views. The Python model and tests are unchanged from 16.0.

Positive Aspects

• Correct attrs to inline migration: All three view records properly convert attrs="{'invisible': [('state', '!=', 'sale')]}" to invisible="state != 'sale'", which is the required change for Odoo 17.0.
• Stored computed field with read_group: The untaxed_amount_to_invoice field uses read_group for efficient aggregation rather than iterating over order lines in Python. This is a good pattern for performance.
• Proper @api.depends: The compute depends on order_line.untaxed_amount_to_invoice which is the native sale.order.line field, ensuring recomputation triggers correctly.
• Test coverage: The test covers the key scenarios -- draft (0 to invoice), confirmed order with invoice_policy='order' (100), and delivery-based invoicing after delivery (250). Tests pass on both Odoo and OCB.
• Standard OCA scaffolding: pyproject.toml with whool, proper __manifest__.py, README generated by oca-gen-addon-readme, Italian translation, license headers.

Findings

1. [Minor] Product type='consu' in tests (line 20, 24)

The test creates products with "type": "consu". In Odoo 17.0 the type field on product.product still accepts 'consu' at the ORM level, so this works. However, since the UI has moved toward "Goods" terminology and later Odoo versions (17.4+) renamed the field to detailed_type, this is worth noting for forward-compatibility awareness. Not blocking since CI passes and the field is valid for 17.0.

2. [Minor] .pot file references Odoo Server 16.0 (line 7 of .pot)

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

This should ideally read Odoo Server 17.0 since this is a 17.0 module. Similarly, the Italian .po file header says 16.0. This is cosmetic and not blocking but could be cleaned up for consistency.

3. [Minor] Trailing whitespace in __manifest__.py summary

Line 14: "summary": "Show total amount to invoice in quotations/sales orders ", -- there is a trailing space before the closing quote. Pre-commit passed, so it may be within tolerance, but worth noting.

4. [Minor] Missing newline at end of several readme files

CONFIGURE.md, CONTEXT.md, CONTRIBUTORS.md, DESCRIPTION.md, and USAGE.md all lack a trailing newline (\ No newline at end of file in the diff). This is a minor style issue.

5. [Info] form.save() called outside with Form(...) context manager

In the test at line 34, sale_order = form.save() is called after the with block has exited. In Odoo 17.0 Form.__exit__ calls save() automatically, so the explicit form.save() is redundant -- the record is already saved. It does not cause errors (calling save twice is idempotent), but it is unnecessary.

6. [Info] Leading whitespace inconsistency in XML (line 30 of view XML)

The <record id="view_order_tree"... block has an extra leading space compared to the other <record> blocks. This was present in the 16.0 version as well, so it is inherited rather than introduced.

Verdict

The migration is clean and correct. The attrs to inline invisible conversion is the only migration-relevant change and it is done properly across all three view records. The module works, CI is green, and test coverage is adequate for the functionality.

No blocking issues found.