Review: Needs Fixing
Tech review from reading code (I'll try it out a bit later):
* In the JS, there are a few instances of `[id={some_value}]`, those should be
ID selectors `#{some_value}`
* in addons/openerp/controllers/listgrid.py @3080, you added `from openerp
import widgets as tw` in the `multiple_groupby` function,why not just add
`listgroup` to the existing `from openerp.widgets import listgrid` in the
toplevel imports?
* In the group_by js method (addons/openerp/static/javascript/listgrid.js),
code would probably be simplified by creating an intermediate variable for
`jQuery('[records="'+record+'"]')` instead of re-fetching it every time.
* Also, as far as I can tell in the second contional
(`if(jQuery('[records="'+record+'"]').attr('parent'))`) both branches are
exactly the same, am I wrong? Is this a mistake in the branches or is it just
that the test isn't actually useful?
* In addons/openerp/static/javascript/search.js, you commended a few lines, I
think they should just be removed.
* In addons/openerp/widgets/listgroup.py method parse, after the `for hidden in
hiddens:` you check hidden[0] and hidden[1]. Is hidden a pair (2-tuple)? If it
is, why not unpack it directly?
* Also a few lines below, there is a `if padding is None` test, but both
branches seem to do the same thing. Same in the next group of iterations
* In parse_groups,
if len(group_by) > 1:
child = False
else:
child = True
could be replaced by
child = len(group_by) > 1
and I'm not quite sure about the name, what does the child flag mean?
* At the end of the method, is `rec` a `dict` or something else (e.g.
`TinyDict`)? If `dict`, why not use `.update` method to set all three keys at
the same time?
* And big loop above could probably be a list comprehension, something along
the lines of:
if child:
ch_ids = [d for id in proxy.search(dom, offset, limit, 0, context)
for d in data
if d.get('id') == id]
else:
ch_ids = []
no?
--
https://code.launchpad.net/~openerp-dev-web/openobject-client-web/group_by/+merge/25140
Your team OpenERP SA's Web Client R&D is subscribed to branch
lp:~openerp-dev/openobject-client-web/trunk-dev-web.
_______________________________________________
Mailing list: https://launchpad.net/~openerp-dev-web
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openerp-dev-web
More help : https://help.launchpad.net/ListHelp
