Review: Disapprove
This bandages the peg leg, but does not fix the fundamental issue:
`selection.ids` is apparently `null` (or `undefined`) in a place where it is
not expected. Why?
`selection` is the result of calling `child.get_selection`. The `get_selection`
modified here (on the Group) always returns at least an empty Array for both
its `selection.records` and its `selection.ids` so that can't be the issue. So
why?
The other `get_selection` is in List (line 973). If the list is not marked as
selectable, it returns an empty array, then it creates an object with keys
`ids` and `records` mapping to empty arrays and fills those.
So the problem comes from the first 3 lines, which return an empty array. Why?
The documentation states (and the rest of) the method returns an object with
keys `ids` and `records` mapping to arrays. The array does not make sense, so
it might very well be an error. Plus if it was intentional, the default
returned value would be `{}` (an empty object, cheaper than an empty array).
And if we look at the `blame` for the first three lines versus the rest, the
first lines date back to revision 167.1.8 and *then* the object-return
(alteration of the documentation and setting of `results` to its current
initial state) date back to the more recent revision 365.1.2 with the commit
message:
> make get_selection call and selected ListView event provide both ids of
> selected records and the selected records themselves
indeed, looking at the diff a number of methods were changed from expecting
get_selection to return a straight array to expecting get_selection returning a
structure bearing two arrays.
So why?
Probably because, during the edition of revision 365.1.2, this section of the
method was missed and did not get edited properly.
The correct fix would be to either return `{ids: [], records: []}` instead of
`[]`, or (even better, avoids duplication) move the declaration of `result` to
the top of the method and return `result` (with its still-empty fields)
directly instead of an empty array.
--
https://code.launchpad.net/~openerp-dev/openerp-web/6.1-opw-572764-cpa/+merge/98996
Your team OpenERP R&D Team is subscribed to branch
lp:~openerp-dev/openerp-web/6.1-opw-572764-cpa.
_______________________________________________
Mailing list: https://launchpad.net/~openerp-dev-gtk
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openerp-dev-gtk
More help : https://help.launchpad.net/ListHelp
