Comment 12 for bug 724131

Revision history for this message
Raphaƫl Valyi - http://www.akretion.com (rvalyi) wrote :

Just a remark about the fix:

of course a fix is certainly better than a bug, so thanks for the fix.

Now, something about the code style:
you create the invoice line here in the middle of the code with:
inv_line_id = self.pool.get('account.invoice.line').create(cr, uid, {...

the issue is that in many localizations (such as our in Brazil) or customization modules, custom fields will need to be added in that invoice line and propagated somehow from the purchase order line (fiscal codes in Brazil to be able to do the legal electronic invoicing for instance).

When you create the order line brutally here. Localization or customization overrides have no choice than browse the invoice line again (from the id) and write into it again.

So for instance if your purchase order has 300 lines, and you need to override that method twice (classical), you will re-browse and re-write into the the invoice lines 300 * 2 = 600 times without any good reason.

Worst: when writing to the invoice line, it will recompute the invoice amount_all stored field 600 times and this one will re-browse into all every single invoice line (eg much slower when hitting line #300).

In a word, this makes OpenERP extremely slow when used in the real world (eg with large documents and localizations). We had cases where confirming and invoice would take 10 minutes or worst. This kind of things makes a customer run away from OpenERP believe me (we lost one customer once because of this).

So yes, this patch is much better than a plain broken functionally, now please consider those aspects in the future.

In contrasts, objects that are highly likely to be localized or customized are better created in their own ovrridable methods
See for instance:
https://code.launchpad.net/~akretion-team/openobject-addons/addons-purchase-extensible-action-picking-create/+merge/79485
or
https://code.launchpad.net/~akretion-team/openobject-addons/addons-sale-extensible-action-ship-create/+merge/76609

Again, it's better to commit that than commit no patch, but please just try to consider my explanation if your have the opportunity or for next OpenERP release.