ruby-grape / ruby-grape/grape-roar
Telling a representer to represent nil adds all of that class's methods to nil
Nobody has claimed this yet.
- Dominant language
- Ruby
- Stars
- 59
- Forks
- 14
- PR merge metrics
- No merged PRs in 30d
Description
The code causing this behavior is https://github.com/ruby-grape/grape-roar/blob/master/lib/grape/roar/representer.rb#L10
Example
user = User.find_by(email: 'i_dont_exist@gmail.com')
# user => nil
present user, with: User::Representer
# User::Representer.ancestors => [..., Grape::Roar::Representer, ...]
Obviously there's a bug in this application code, but the result is that, since nil is a singleton, now every time the application asks for nil, it gets it, but with all kinds of fun stuff mixed in, like a custom #to_hash that, for instance, mucks with the instantiation of ActionController::Parameters and causes completely unrelated pages in an app to blow up
Thoughts? I'd be happy to submit a PR, but i only have a cursory understanding of all the interplay between grape, roar, grape-roar, representable, etc. and just tossing in a nil check might not be the best approach
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with lib/grape/roar/representer.rb at line 10 and reproduce the example where presenting nil changes User::Representer. Read the surrounding representer behavior and the issue discussion before deciding how nil should be handled. Done means nil is not modified with representer methods and unrelated uses of nil no longer exhibit the reported side effects.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- ruby
- Domain
- api
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 28/100