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

Allow type to be a model #597

Open
venkatd opened this issue Mar 18, 2014 · 8 comments
Open

Allow type to be a model #597

venkatd opened this issue Mar 18, 2014 · 8 comments

Comments

@venkatd
Copy link

venkatd commented Mar 18, 2014

I don't have an idea for the best syntax but I find myself loading models from the params hash a lot. What if you were able to do something like this?

params do
    requires :user_id, identifies: User
end
get 'users/:user_id' do
   params[:user] # params calls User.find internally and raises the appropriate error if nothing is found
end

Thoughts?

@dblock
Copy link
Member

dblock commented Mar 18, 2014

I like it. We do the same, just a bit more explicitly with an error!(404, 'Not Found') type-thing.

@dblock
Copy link
Member

dblock commented Mar 18, 2014

I think the DSL should allow something more evolved, maybe

evolves :user_id, into: User, with: ActiveRecordEvolverManagerTransformer

@mbleigh
Copy link
Contributor

mbleigh commented Mar 18, 2014

This is definitely a common pattern and probably worth abstracting. It seems like we'd actually want to go the other way to really be describing what's happening. Something like:

requires :user, type: User, source: :user_id

I've had a convention for a long time of creating a class-level .from_param(param) method on classes that can load from URL parameters. We could look for that by default but also support a load option:

# if a symbol is passed to load, it does options[:type].send(options[:load], params[:source])
require :user, type: User, source: :user_id, load: :find_by_id
# if a lambda is passed, it passes the param to the lambda
# note that this pattern would look for anything that responds to `.call` so
# could be a class, whatever
requires :user, type: User, source: :user_id, load: ->(param){ User.find_by_id(param) }

Something more like that maybe?

@dm1try
Copy link
Member

dm1try commented Mar 18, 2014

👍

@dblock
Copy link
Member

dblock commented Mar 18, 2014

@mbleigh Very nice. It makes total sense to require the loaded object vs. the source parameter. We might want to rename source to param and load to maybe just with or is it too overlapping with other places where we use similar keywords?

require :user, type: User, param: :user_id, with: :find_by_id

Furthermore, this should support a group of params or multiple params. In general the inside of that load block should have access to all params, and possibly to the entire API endpoint, I think.

@dblock
Copy link
Member

dblock commented Mar 18, 2014

This could also just be a block:

requires :user do
   User.find(params[:user_id]
end

@dblock
Copy link
Member

dblock commented Nov 29, 2014

Note that if this is implemented it might conflict with #829, so we need to find a DSL that works for both.

@rnubel
Copy link
Contributor

rnubel commented Jul 8, 2015

#1039 mostly implemented this, provided you don't mind adding a ::parse method to your models:

class User < ActiveRecord::Base
  # ...
  def self.parse(param)
    find(param.to_i)
  end
end

# ...

params do
  requires :user_id, type: User
end
get do
  params[:user_id] # is a User already
end

However, it would be nice to alias params[:user] to params[:user_id] so it's less clunky in the code, but still meaningful at the API level. Maybe supporting aliases for params wouldn't be a bad idea.

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

No branches or pull requests

5 participants