Code review comment for lp:~openerp-dev/openobject-addons/trunk-osv_memory_crm_profiling-uco

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

I have reverted this merge, it was unacceptable.

1) Runbot tests were failing after the merge, so it was not tested properly

2) The code of the crm profiling wizard alters the _columns of the wizard object dynamically, which must never happen! This has global effects on the osv objects, which means that the behavior will be unpredictable when several people use the wizards, and it also makes memory leaks, etc.
When you need dynamic views for wizard you have 2 options usually:
 - generate dynamic "virtual" fields in field_view_get + fields_get (but do NOT add them in self._columns), and make custom create/read/write methods to handle these virtual fields appropriately
 - or when you simply need multiple lines, you can make a real "line" osv_memory object like we do in the new stock.partial.picking, and generate dynamic lines when the wizard is opened - in this case the extra fields are just part of the o2m lines

Please fix this and resubmit, and be careful about this kind of problem whenever you deal with dynamic views, from now on.

Thanks!

review: Needs Fixing

« Back to merge proposal