Skip to content
This repository has been archived by the owner on Jan 30, 2024. It is now read-only.

Make SecurityQuestion ActiveRecord class optional #131

Merged
merged 1 commit into from Mar 10, 2016

Conversation

monfresh
Copy link
Contributor

The SecurityQuestion class is empty, and it's only needed if the :security_questionable option is turned on. Therefore, I think it should be up to the user to create the model if they need it.

The problem with automatically creating and requiring the model is that it will override a class by the same name. I have my own implementation of security questions in my app, and was going crazy trying to figure out why the code I put in my SecurityQuestion class wasn't being recognized.

The SecurityQuestion class is empty, and it's only needed if the `:security_questionable` option is turned on. Therefore, I think it should be up to the user to create the model if they need it.

The problem with automatically creating and requiring the model is that it will override a class by the same name. I have my own implementation of security questions in my app, and was going crazy trying to figure out why the code I put in my `SecurityQuestion` class wasn't being recognized.
@evan-007
Copy link

👍 just ran into this exact issue with one of our apps.

@monfresh
Copy link
Contributor Author

monfresh commented Jan 4, 2016

Hello, and Happy New Year! Any thoughts on this PR? Is this project still being maintained? Thanks!

@monfresh
Copy link
Contributor Author

monfresh commented Mar 9, 2016

Hello. Thoughts on this PR?

@evan-007
Copy link

evan-007 commented Mar 9, 2016

I would love to see this merged as well. We've forked your branch and have been using it for this change since May of last year, thanks @monfresh.

@monfresh
Copy link
Contributor Author

monfresh commented Mar 9, 2016

It looks like this was actually fixed at some point. The model was renamed to SecurityQuestionable. The git history seems to be messed up so I can't tell exactly what commit changed this.

@monfresh monfresh closed this Mar 9, 2016
@monfresh
Copy link
Contributor Author

monfresh commented Mar 9, 2016

I was wrong. I was looking at the wrong thing. This issue still needs to be fixed. Re-opening my PR. @traxanos please consider merging this. Thanks!

@monfresh monfresh reopened this Mar 9, 2016
manno pushed a commit that referenced this pull request Mar 10, 2016
Make SecurityQuestion ActiveRecord class optional
@manno manno merged commit 8ecd175 into phatworx:master Mar 10, 2016
@monfresh
Copy link
Contributor Author

Yay! Thanks!

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

Successfully merging this pull request may close these issues.

None yet

3 participants