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

Exclude devise modules #85

Merged
merged 2 commits into from
Dec 22, 2014

Conversation

jartek
Copy link
Contributor

@jartek jartek commented Dec 14, 2014

Add a configuration option to exclude devise modules from the defaults.

Fixes #78 #80 #60

@lynndylanhurley thoughts?

@lynndylanhurley
Copy link
Owner

@jartek - thank you so much for this PR! This is by far the most requested feature.

My only concern with this PR is that this seems to set the included modules globally, but they should be configurable on a per-model basis.

Consider this use-case:

  • There are two models, Member and Admin.
  • Admin requires email verification.
  • Member does not require email verification.

Can you revise this PR to address that use-case?

@jartek
Copy link
Contributor Author

jartek commented Dec 14, 2014

👍 Will do.

@jartek
Copy link
Contributor Author

jartek commented Dec 14, 2014

@lynndylanhurley wouldn't it be better if the code for choosing devise modules was moved out of the concern and into each class that requires concern?

Can't think of a simpler way to do this right now other than specifying it via global options

@lynndylanhurley
Copy link
Owner

@jartek - I think that's the only way to go about this.

@jartek
Copy link
Contributor Author

jartek commented Dec 15, 2014

@lynndylanhurley I've updated the PR.

@lynndylanhurley
Copy link
Owner

Awesome, thanks @jartek! I'll review ASAP!

@nicolas-besnard
Copy link
Contributor

I did the same think as you do, but I don't think to do a PR :( I'll look at it tomorrow morning, great job !

@nicolas-besnard
Copy link
Contributor

I just try hit, sounds good to me :)

@lynndylanhurley lynndylanhurley merged commit 2d398d0 into lynndylanhurley:master Dec 22, 2014
lynndylanhurley added a commit that referenced this pull request Dec 22, 2014
@lynndylanhurley
Copy link
Owner

Ok guys, I released these fixes.

Check out version 0.1.31.beta1 and let me know if everything works as expected.

Documentation is here.

@jartek jartek deleted the exclude_devise_modules branch December 22, 2014 06:12
@jartek
Copy link
Contributor Author

jartek commented Dec 22, 2014

thanks 👍

@ACPK
Copy link

ACPK commented Jan 6, 2015

Just what I needed! Thank you @lynndylanhurley.

caulfield added a commit to caulfield/devise_token_auth that referenced this pull request Jan 8, 2015
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.

Problem with skip_confirmation!
4 participants