Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Created a hash decoder for hashed ids within Rails params. #52

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dbastin
Copy link

@dbastin dbastin commented Apr 9, 2018

Hi Folks

I've created a class that decodes ids for inbound Rails param hashes. You use it like this...

class PostsController < ApplicationController
  ...
  def post_params
    p = params.require(:post).permit(...)
    Hashid::Rails::Decoder.decode_params(Post, p)
  end
end

So, in addition to having urls like https://www.example.com/posts/e45Frd behaving like https://www.example.com/posts/12345 you now also get the inbound hashids that are contained within form params converted to their real ids as well.

I'm a little surprised that something like this does not already exist within any id obfuscating gem I've tried, so a part of me thinks I'm perhaps missing something and there really is no need for any of this code? Or perhaps no-one is obfuscating ids within form params? No idea.

At any rate, this code is test driven, 100% covered, and complies with your Rubocop rules (except for one method Hashid::Rails::Decoder#decode that I tried splitting up but decided to leave for clarity).

I also fixed up a few existing minor Rubocop failures.

You'll see I've had to make the schema a little more complicated owing to the fact I needed to demonstrate different object association types. I think it remains pretty clear though.

P.S. I've made an earlier pull request but that failed a Travis build using Ruby 2.3.0, so I fixed it and force pushed a squashed single commit. This prevents the old pull request from being reopened. Apologies if that is an issue.

@krsyoung
Copy link

krsyoung commented Oct 3, 2018

@dbastin just seeing if I understand this right (and that your memory is sharp!), so would this help in the case that I am passing in a form with a hashid reference to another record?

i.e. { person: { name: 'bob', house_id: 'RyAHMR' } }

Then would I expect for the following to work?

# person params decodes the hashid for house_id
p = Person.new(person_params)
# house now correctly relates to the decoded house_id
p.house

@dbastin
Copy link
Author

dbastin commented Oct 3, 2018

@krsyoung — Exactly right. This pull-request enables hashed ids coming in via request parameters from a form for all sorts of associations, including the one mentioned in your example.

You'll need to explicitly decode the params like this though (seems from your code comment that you know this already)...

decoded_params = Hashid::Rails::Decoder.decode_params(Person, person_params)
p = Person.new(decoded_params)
p.house

@krsyoung
Copy link

krsyoung commented Oct 3, 2018

Amazing @dbastin! Will try it out and see if I can give it another vote / help for a merge! Very helpful (and nice looking code!)

@Tengoot
Copy link

Tengoot commented Oct 3, 2018

@dbastin I find this Pull request not compatible with idea of current hashid state. It bases on model layer and you make use of controller layer. I already tried decoding like that on model layer in work, it works fine. Soon I will send here example of how we can do it hashidway.

@dbastin
Copy link
Author

dbastin commented Oct 3, 2018

@Tengoot — Controller layer? I don't follow. The pull request is decoding a Hash that describes a Model. Nothing to do with Controllers.

In my mind, this process is just one part of the larger ORM process that is triggered on each request. Furthermore, ORM conceptually sits within the Model layer. I could be looking at it wrong.

Anyway, I'd love to see an example of how you perform decoding for nested hashed id params without recursively parsing the params hash like I have done. It would be great to see a better way to do this!

@dbastin
Copy link
Author

dbastin commented Oct 25, 2019

Repaired conflict. Merged.

@Tengoot - Still waiting to see the hashidway after a little while. I'm quite interested to see how others are handling the problem addressed by this pull request.

tl;dr - hashid-rails is great at handling hashed ids in URLs but does not handle hashed ids within form params. This pull request adds that functionality.

@xploshioOn
Copy link

xploshioOn commented Jan 12, 2020

@dbastin tested it on local and work in all cases I have, except with polymorphic associations that I got an error like this
ArgumentError (Polymorphic association does not support to compute class.):

I just took your code and used it on a service, added a change to make those on associations that are not polymorphic at the moment, as a side comment, if the value is empty, why not returning directly the value and not try to calculate it? I can be trying to create a new nested resource and it goes on all the process even if attr_id is empty

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants