grape-entity icon indicating copy to clipboard operation
grape-entity copied to clipboard

Grape::Entity::Delegator::HashObject fails when extracting string keys

Open tom-mayer opened this issue 9 years ago • 4 comments

Hi,

I was trying to present a hash of values with a grape entity. The keys in the hash were strings and I got null values all over the place as a response.

It boils down to Grape::Entity::Delegator::HashObject, when it tries to render the attribute it only tries to extract the attribute with a symbol as a key and not a string.

screen shot 2016-05-30 at 13 50 29

I think a simple object[attribute] || object[attribute.to_s] would fix the issue, although I'm not sure whether you need to make the destinction between symbols and strings there for other reasons. For now I simple convert all my keys to symbols before handing them to grape entity. But in any case would be more convenient the other way around.

Cheers Tom

tom-mayer avatar May 30 '16 12:05 tom-mayer

Treating hash keys indifferently is probably a good idea, but I wouldn't just change it. Start by writing some specs around this and then see whether this change has an impact on other existing tests?

dblock avatar May 30 '16 13:05 dblock

I'll check later if it breaks any of the current specs. If I got some free time on the weekend I can write a few specs for the different key cases.

tom-mayer avatar May 30 '16 13:05 tom-mayer

Hi @dblock. I faced the same problem and I don't want to do symbolize_keys for each entity.

@tom-mayer is right - changing Delegator::HashObject to process attributes like object[attribute] || object[attribute.to_s] will fix the issue. I wrote a small spec - you can find it here spec/grape_entity/entity_spec.rb#L631. Old (current) specs are green.

What do you think about it?

nbulaj avatar Jun 22 '16 14:06 nbulaj

@nbulaj I think you should PR it and we can take a look.

dblock avatar Jun 23 '16 13:06 dblock