Review: Needs Fixing Hi Guewen,
You approach is globally good, but I have a few minor remarks. Of course, checking for possible foreign keys reference when you delete a record would be too expensive for virtual fields, as you said yourself, and it's not really necessary anyway. So we can simply handle it at the time of reading, like we do it for similar cases, such as property fields. However I'd rather not modify the record immediately when such a "dead link" is detected, because that can have unpredictable and undesired effects. A read() operation should never alter the record being read. And because all accesses to a sparse field value should pass through read(), it does not matter, as the dead link will be ignored in all circumstances. And when the record is later changed the new value will be saved and cleanup the dead link. I've now implemented a similar (but simpler: just a spot filtering via model.exists()) solution in the branch where I'm merging Sebastien's work, so I think we can close this merge proposal. Otherwise your patch was good, well done! -- https://code.launchpad.net/~c2c/openobject-server/trunk-sparse-field-akretion-integrity/+merge/86044 Your team OpenERP R&D Team is requested to review the proposed merge of lp:~c2c/openobject-server/trunk-sparse-field-akretion-integrity into lp:~openerp-dev/openobject-server/trunk-sparse-field-akretion. _______________________________________________ Mailing list: https://launchpad.net/~openerp-dev-gtk Post to : [email protected] Unsubscribe : https://launchpad.net/~openerp-dev-gtk More help : https://help.launchpad.net/ListHelp
