openerp-dev-web team mailing list archive

["qdp \(OpenERP\)" <qdp-launchpad@xxxxxxxxxxx>]

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.