Review: Needs Fixing
few comments, just by reading the diff:
line 21-22 of the diff are useless as context isn't used later in the code of
the function. Can be removed for better readability.
line 24: why this ids.reverse()? is it to treat the case where we have records
A,B,C respectively in timeline, then change the temporal_date_from of C and set
something between A and B (to get A,C,B)? in that case, i would have seen a
search with order by temporal_date_from.. I need information here.
line 29: why this break?
in get_next_id_in_timeline
line35: unused
line37: the params of the search seem wrong. I would have seen something like:
reference_field = record.temporal_parent_id and record.temporal_parent_id.id or
record.id
['|',('temporal_parent_id', '=', reference_field), ('id', '=',
reference_field),('temporal_date_from', '=', record.temporal_date_to)]
line 38: you return always the last value of search_id, it means that if you
pass a list of ids like [1,2,3,4] or [4] it will have exactly the same behavior.
code of write() can be improved
line 77-78: can be removed i think
if context.get('temporal_mode', True) and not vals.get('temporal_date_from'):
line 69: i don't see the point to do "ids.pop()", it will simply not work doing
that. Just a continue would be enough
code of browse: didn't checked yet, i had set this point as not needed in the
pad because in theory it should already work currently as the browse is only
using search and read... But keep a patch for that in case of... ;-) (but
remove it from this merge prop please
thanks,
Quentin
--
https://code.launchpad.net/~openerp-dev/openobject-server/trunk-temporal-db-read-search-unlink-ksa/+merge/60367
Your team OpenERP R&D Team is subscribed to branch
lp:~openerp-dev/openobject-server/trunk-temporal-db.
_______________________________________________
Mailing list: https://launchpad.net/~openerp-dev-web
Post to : openerp-dev-web@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openerp-dev-web
More help : https://help.launchpad.net/ListHelp
