Use the of operator to detect for to_param and id in objects

[dark-panda]

Using a path with an id or a to_param field with a value of 0 or false will result in a path that has has the output of object.toString() as the id, often resulting in "[object Object]" being set as the id. This patch will use the of operator to check for the existence of to_param and id fields rather than picking the first non-falsy value we can find.

[le0pard]

Thanks, but your changes broken the tests. And I think this is not a problem with tests - it is problem with changes (more info https://travis-ci.org/railsware/js-routes/builds/15062361)

[dark-panda]

Ah sorry. I'll investigate tomorrow. As for the core concept, is this a viable patch? I have a record with an id of 0 which is how I came across this issue. I know that in the rails world ids of 0 are uncommon, but still...

On Dec 6, 2013, at 6:41 PM, Alexey Vasiliev notifications@github.com wrote:

Thanks, but your changes broken the tests. And I think this is not a problem with tests - it is problem with changes (more info https://travis-ci.org/railsware/js-routes/builds/15062361)

�
Reply to this email directly or view it on GitHub.

[le0pard]

@dark-panda Just added test with id equal zero. It pass. Also as I can see by code - this should work. https://github.com/le0pard/js-routes/blob/3e0eee172af56fc0689ba0828ea8bf19989db75b/spec/js_routes/options_spec.rb#L89

What is your example of route in rails for this problem?

[dark-panda]

This occurs when you use an options hash using either to_param or id keys with 0 as the value. For instance, consider the evaluation of the following:

property = object.to_param || object.id || object;

If to_param is 0, property is set to object, which causes the evaluation of property.toString() to be a string that contains [object Object]. If you use arguments with just an integer like 0, you won't see this problem, as property will be set to 0 and the subsequent property.toString call will correctly output 0.

I have pushed an updated version of the patch which includes additional tests and all current tests should be working correctly. I've also used the of operator instead of in, which should work better.

[bogdan]

Nice catch, thanks.