← Back to team overview

openerp-dev-web team mailing list archive

Re: lp:~openerp-dev/openobject-client-web/trunk-bug-727046-vda into lp:openobject-client-web

 

Review: Needs Fixing
* From my reading, auto_search is not optional on act_windows, so there is no point in using .get here, it's always there and either True or False

* Likewise for limit, you can't have no limit (unless it's set to "0" maybe, so you could have the original `limit` call as `action['limit'] or data.get('limit'), this way if the action's limit is 0 it uses the one provided)

* Since we already have limits in list views (as far as I know), I don't understand why you add `auto_limit` everywhere in the JS code, why not leave it as `limit`?

* I'm not too fond of the "auto_search" token internally, in our own code: it's either search or don't search, there isn't much in the way of auto there.

* And in listgrid, the second test on auto_search should not be necessary. Tell me if I'm wrong, but by default ids=[], if auto_search (or whatever it'll be called) is False then ids will remain [], therefore the second test (ids and len(ids) > 0, which is redundant by the way as an empty container is falsy) will be False and it won't perform a read() in any case.
-- 
https://code.launchpad.net/~openerp-dev/openobject-client-web/trunk-bug-727046-vda/+merge/53574
Your team OpenERP R&D Team is subscribed to branch lp:~openerp-dev/openobject-client-web/trunk-bug-727046-vda.



Follow ups

References