-
Notifications
You must be signed in to change notification settings - Fork 623
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
Generated ApplicationPolicy #index? #17
Comments
You're right. That's probably not optimal. We should remove that or replace it with something more sensible. |
bemurphy
added a commit
to bemurphy/pundit
that referenced
this issue
Mar 9, 2013
See varvetgh-17 for initial discussion. I think that the default provided before this commit is confusing, because an empty result is not a good indicator of authorzation failure. Returning false better communicates "override this is you want it".
bemurphy
added a commit
to bemurphy/pundit
that referenced
this issue
Mar 9, 2013
See varvetgh-17 for initial discussion. I think that the default provided before this commit is confusing, because an empty result is not a good indicator of authorization failure. Returning false better communicates "override this is you want it".
Merged the pull request. Closing. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I'm confused as to why the default generated index policy looks like this:
In the example scope
where(:published => true)
that would throw an exception on the empty set, which in this case doesn't really mean failed authorization, it might really be empty. Perhaps this is why theverify_authorization
is suggesting skipping index, because you just need to rely on the scope finder all the time instead?The text was updated successfully, but these errors were encountered: