Code review comment for lp:~sebastien.beau/openerp-connector/openerp-connector-fix-useless-fire-sparse-fields

Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Hi,

Thanks for your MP.

Can you add the test as well please (for a write in a serialized field too)?

Could I propose to rename 'keys' to 'fields'?

I think we can have a more clever fix for not much work. I'm maybe wrong without investigating further, but I'm sure you'll stop me in such case ;-)

If we write directly in a serialized field, we know the fields we are writing on.
I would propose to:
Fire vals.keys() + serialized.keys(), unless if '__do_not_fire_serialized' is present in the context. '__do_not_fire_serialized' has to be put in the context given to the `write_original` method.

Thus,
* if we write directly to sparse fields -> ok because the many writes on the serialized will be skipped due to the presence of the sentinel '__do_not_fire_serialized'
* If we write directly to a serialized field -> ok because the fields keys will be present in the first fire

review: Needs Fixing (code review, no test)

« Back to merge proposal