[16.0][IMP] sale_order_type: create invoices by grouping by type

[oihane]

No description provided.

[rousseldenis]

I'm wondering why? In fact, order types will help to flll in other sale fields that are, for some of them, already included in grouping keys.

So, IMHO, this is not necessary.

Could you explain your use case ?

[alexey-pelykh]

Thanks for the contribution @oihane. The idea of grouping invoices by sale order type is a valid functional concept. However, I found a critical bug that makes this code non-functional as written, plus a couple of style/consistency issues.

Critical: Wrong grouping key name

The _get_invoice_grouping_keys method returns field names that are used to look up values in the invoice vals dictionary (the dict returned by _prepare_invoice). Looking at the existing _prepare_invoice override in this same module:

def _prepare_invoice(self):
    res = super(SaleOrder, self)._prepare_invoice()
    if self.type_id.journal_id:
        res["journal_id"] = self.type_id.journal_id.id
    if self.type_id:
        res["sale_type_id"] = self.type_id.id
    return res

The key set in the invoice vals is sale_type_id, not type_id. In the core _create_invoices method, grouping uses x.get(grouping_key) on these invoice vals dicts. Since type_id is never set in the invoice vals, x.get("type_id") returns None for all orders, making the grouping silently ineffective -- all orders would collapse into one group regardless of their sale type.

Fix: Change "type_id" to "sale_type_id" in both the append and the not in check.

Style: Remove @api.model decorator

The upstream Odoo 16 _get_invoice_grouping_keys is a plain instance method with no decorator. Adding @api.model is inconsistent with the method being overridden. While not strictly breaking (since the method only returns static key names), it should be removed to match the upstream contract.

Style: Remove -> list return type annotation

Return type annotations are not used in the Odoo 16 codebase or in OCA modules. The overridden base method has no type annotation. Please remove it for consistency.

Tests

Please add a test that verifies invoice grouping by sale type actually works -- e.g., create two sale orders with different type_id for the same partner, generate invoices, and assert two separate invoices are created.

Summary

#	Finding	Severity
1	Wrong grouping key: type_id should be sale_type_id	Critical (bug -- feature silently broken)
2	@api.model decorator inconsistent with overridden method	Minor
3	-> list type annotation not used in Odoo/OCA codebase	Minor
4	No test coverage for the new grouping behavior	Moderate

[alexey-pelykh]

Bug: This should be "sale_type_id", not "type_id". The grouping keys are looked up on the invoice vals dictionary (from _prepare_invoice), which sets the key as sale_type_id (see line ~197 above: res["sale_type_id"] = self.type_id.id). Using type_id here means x.get("type_id") returns None for all orders, so the grouping by sale type never actually works.

Suggested change

	keys.append("type_id")
	if "sale_type_id" not in keys:
	keys.append("sale_type_id")

[alexey-pelykh]

Two issues with the method signature:

1. Remove @api.model -- the upstream _get_invoice_grouping_keys in Odoo 16 core has no decorator (it is a plain instance method).
2. Remove -> list return type annotation -- Odoo/OCA codebase does not use Python type hints.

Suggested change

	@api.model
	def _get_invoice_grouping_keys(self):