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

Flexible routing scopes #1922

Merged
merged 6 commits into from Jun 15, 2012
Merged

Flexible routing scopes #1922

merged 6 commits into from Jun 15, 2012

Conversation

excid3
Copy link
Contributor

@excid3 excid3 commented Jun 15, 2012

One of the things that I think could make Devise really handy, and understandable to newcomers is making scopes more flexible for constraints. Take this for example:

  devise_scope :user, lambda { |user| user.role == "admin"  } do
    root to: "admin#dashboard"
  end

or

  authenticated :user, lambda { |user| user.role == "admin"  } do
    root to: "admin#dashboard"
  end

Each of these is something pretty easily understood, and would provide a bit of clarity to the routes, and clean up any before_filters that you would use instead.

I've already got a start on an implementation of this, but wanted to know if this makes sense, or maybe there is already a way to do this with Devise as it stands.

@josevalim
Copy link
Contributor

Yes, this looks great and makes sense for authenticate(d).

@excid3
Copy link
Contributor Author

excid3 commented Jun 15, 2012

Is there a test for authenticate(d) I could use as a basis for testing these? And how does the implementation look? Anything I should clean up? Thanks!

@josevalim
Copy link
Contributor

The implementation looks good, but could you please use (&& and ||) instead of and and or?

About the tests, you will need to define new routes here:

https://github.com/plataformatec/devise/blob/master/test/rails_app/config/routes.rb

And then add new tests for these routes inside this test case (where we keep tests for authenticated, unauthenticated, and so on):

https://github.com/plataformatec/devise/blob/master/test/integration/authenticatable_test.rb#L137

@excid3
Copy link
Contributor Author

excid3 commented Jun 15, 2012

Thanks José! I added some tests and changed to the boolean logic like you mentioned. Anything else?

josevalim pushed a commit that referenced this pull request Jun 15, 2012
Flexible routing scopes
@josevalim josevalim merged commit 16b688e into heartcombo:master Jun 15, 2012
@josevalim
Copy link
Contributor

Meeeeeeerged, thanks!

@excid3
Copy link
Contributor Author

excid3 commented Jun 15, 2012

👍

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

Successfully merging this pull request may close these issues.

None yet

2 participants