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

Document a way to implement error messages. #116

Merged
merged 1 commit into from Mar 7, 2014

Conversation

Projects
None yet
3 participants
@ucarion
Contributor

ucarion commented Feb 26, 2014

If #114 gets merged, then this PR documents a way a user could implement error messaging with Pundit.

You can see what the new README would look like here: https://github.com/ulyssecarion/pundit/tree/custom_error_messages#rescuing-a-denied-authorization-in-rails

Edit: [ci-skip]

@thomasklemm

This comment has been minimized.

Show comment
Hide comment
@thomasklemm

thomasklemm Feb 27, 2014

Collaborator

@ulyssecarion Awesome. Thanks for splitting this PR up, good move. Lets see what happens on #114 and merge this one afterwards.

Collaborator

thomasklemm commented Feb 27, 2014

@ulyssecarion Awesome. Thanks for splitting this PR up, good move. Lets see what happens on #114 and merge this one afterwards.

@ucarion

This comment has been minimized.

Show comment
Hide comment
@ucarion

ucarion Mar 5, 2014

Contributor

@thomasklemm I've updated this PR to reflect the addition of the policy property of the errors raised. Before I was doing something like this:

record_name = if error.record.is_a?(Class)
  error.record
else
  error.record.class
end.to_s.downcase

Which would be used with pundit.post.create?. This isn't quite as simple as using policies and activesupport's #underscore method:

policy_name = error.policy.class.to_s.underscore

Which will work with pundit.post_policy.create?. I'm not a super big fan of having _policy at the end, but I think the README's example should be super easy to understand, since people reading the README are trying to find out if Pundit is for them and don't want to deal with subtleties.

Contributor

ucarion commented Mar 5, 2014

@thomasklemm I've updated this PR to reflect the addition of the policy property of the errors raised. Before I was doing something like this:

record_name = if error.record.is_a?(Class)
  error.record
else
  error.record.class
end.to_s.downcase

Which would be used with pundit.post.create?. This isn't quite as simple as using policies and activesupport's #underscore method:

policy_name = error.policy.class.to_s.underscore

Which will work with pundit.post_policy.create?. I'm not a super big fan of having _policy at the end, but I think the README's example should be super easy to understand, since people reading the README are trying to find out if Pundit is for them and don't want to deal with subtleties.

@thomasklemm

This comment has been minimized.

Show comment
Hide comment
@thomasklemm

thomasklemm Mar 5, 2014

Collaborator

I'm wondering whether redirect_to request.headers['Referer'] || root_path can be done more easily. #118 suggests redirect_to request.referrer || root_path should also work and is shorter.

Collaborator

thomasklemm commented Mar 5, 2014

I'm wondering whether redirect_to request.headers['Referer'] || root_path can be done more easily. #118 suggests redirect_to request.referrer || root_path should also work and is shorter.

@ucarion

This comment has been minimized.

Show comment
Hide comment
@ucarion

ucarion Mar 7, 2014

Contributor

@thomasklemm Nice, I had no idea about request.referrer. I'll update this PR.

Contributor

ucarion commented Mar 7, 2014

@thomasklemm Nice, I had no idea about request.referrer. I'll update this PR.

@ucarion

This comment has been minimized.

Show comment
Hide comment
@ucarion

ucarion Mar 7, 2014

Contributor

Sweet, I love GitHub's rendered diff view! You can use it to see the changes in the README as rendered markdown. 💖

Contributor

ucarion commented Mar 7, 2014

Sweet, I love GitHub's rendered diff view! You can use it to see the changes in the README as rendered markdown. 💖

@jnicklas

This comment has been minimized.

Show comment
Hide comment
@jnicklas

jnicklas Mar 7, 2014

Collaborator

@ulyssecarion oohh, that is nice.

Collaborator

jnicklas commented Mar 7, 2014

@ulyssecarion oohh, that is nice.

jnicklas added a commit that referenced this pull request Mar 7, 2014

Merge pull request #116 from ulyssecarion/custom_error_messages
Document a way to implement error messages.

@jnicklas jnicklas merged commit 5fae42f into varvet:master Mar 7, 2014

1 check passed

default The Travis CI build passed
Details

@ucarion ucarion deleted the ucarion:custom_error_messages branch Mar 7, 2014

@ucarion

This comment has been minimized.

Show comment
Hide comment
@ucarion

ucarion Mar 7, 2014

Contributor

Awesome. I look forward to adding some error messages to my Rails apps.

Thanks @jnicklas and @thomasklemm !

Contributor

ucarion commented Mar 7, 2014

Awesome. I look forward to adding some error messages to my Rails apps.

Thanks @jnicklas and @thomasklemm !

@thomasklemm

This comment has been minimized.

Show comment
Hide comment
@thomasklemm

thomasklemm Mar 7, 2014

Collaborator

@ulyssecarion Thanks for all your efforts and the two great PRs!

Collaborator

thomasklemm commented Mar 7, 2014

@ulyssecarion Thanks for all your efforts and the two great PRs!

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