Code review comment for lp:~openerp-dev/openerp-web/7.0-bug-1047911-fme

Revision history for this message
Fabien Meghazi (OpenERP) (fme) wrote :

> > state["active_ids"] = this.inner_action.context.active_ids.toString();
>
> The toString() is a bit iffy, I've got to say I'd prefer an explicit join here, or some other type of serialization

I was relying on the ecma spec but I admit it's biffy. Sorry, I meant iffy.

> > add_context.active_ids = state.active_ids.toString().split(',').map
>
> Not sure what's toString() supposed to be here, isn't active_ids already a string?

No because jQuery BBQ does some parsing in some case. I've documented it.

> Also the parsing might be a tad risky: what if we have "string" or "virtual" ids in that? They're going to be parseInt'd to NaN.

Fixed that

« Back to merge proposal