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

Always prefix id params with resource #659

Closed
paulcsmith opened this issue Nov 28, 2018 · 3 comments
Closed

Always prefix id params with resource #659

paulcsmith opened this issue Nov 28, 2018 · 3 comments
Milestone

Comments

@paulcsmith
Copy link
Member

paulcsmith commented Nov 28, 2018

TL;DR /users/:user_id instead of /users/:id When generating routes with the route macro

I've been thinking about what the default path param should be for non-nest resources. In Rails/Phoenix/other frameworks it is typically just id. But when nested (for example /users/:user_id/projects/:id one path param gets prefixed with the resource name, and the other does not. I think this can lead to confusion which is why I like the idea in this issue: #497

I think we could go a step further though and always prefix the param with the resource, e.g. /users/:user_id instead of /users/:id. The advantage of this is that it is always super clear what kind of id you are working with, but also it means that you can use a module that works in nested and non-nested actions. For example: https://gist.github.com/paulcsmith/101fc224fd59c5d3d8c9b70bef3a38ee

module ProjectFinder
  def project
    # We would be able to use this method in nested and non-nested resources since 
    # the resource will be :project_id in either case
    ProjectQuery.new.user_id(current_user.id).find(project_id)
  end
end

It would also make it easier to change a controller from nested to non-nested and the param name would stay the same. Making it simpler to do.

@jwoertink
Copy link
Member

Is this only referring to when you use route? Like, if I do:

class User::Show < BrowserAction
  get "/users/:id" do
    # this gives me id still
  end
end
class User::Show < BrowserAction
  route do
     # this would give me user_id ?
   end
end

@paulcsmith
Copy link
Member Author

Yeah that’s correct. Good clarification 👍

@paulcsmith paulcsmith added this to the Priorities milestone Jan 4, 2019
bnjamin added a commit to bnjamin/lucky that referenced this issue Jan 30, 2019
bnjamin added a commit to bnjamin/lucky that referenced this issue Jan 30, 2019
bnjamin added a commit to bnjamin/lucky that referenced this issue Jan 30, 2019
bnjamin added a commit to bnjamin/lucky that referenced this issue Jan 30, 2019
paulcsmith pushed a commit that referenced this issue Jan 31, 2019
* Prefix id params with the resource name (#659)

* Update the templates used to generate the actions, to use the new resource id method.

* Added assertion to reflect the changes to the resource_id method name.
@paulcsmith
Copy link
Member Author

Closed by #680 Thanks @bnjamin!

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

No branches or pull requests

2 participants