Here we go again: Custom error messages?
Sorry to beat a dead horse. People have been asking about custom error messages for over six years now (#66, #143, #212, #599). Clearly, there have been some improvements in the API since then (#114), and some great suggestions for how to implement this at the controller level (#503), but the approach currently recommended in the README still doesn't feel right to me.
Official Approach
The README suggests one of two options:
- Use i18n to generate error messages based on query, record, and policy.
- Raise errors inside your query methods.
What's wrong with this?
The first approach limits you to one error message per resource-query. Queries can fail for many reasons, and it's nice to be able to pass this information along in the exception message.
The second approach breaks the pattern of query methods returning a boolean. Pundit's own README recommends using them in views as follows:
<% if policy(@post).update? %>
<%= link_to "Edit post", edit_post_path(@post) %>
<% end %>Raising an error in #update? is not compatible with this approach.
What I'm not asking for
A lot of people have asked to specify custom exceptions as an argument to #authorize. I am 100% against this idea: if you're using policies in the first place, then that's the only class that should know about why authorization failed. If you spread that responsibility between the policy and the controller, then the policy just becomes a bucket for tossing helper methods into, which violates SRP and really defeats the purpose.
It appears that @jnicklas shares this sentiment.
A proposal
#503 came very close to what I'd like to see, but it accomplishes custom error messages in rescue_from, via a combination of policies, controllers, and the I18n gem. As a library, I think Pundit should make minimal assumptions about external dependencies, and so I'd suggest something that lives entirely within Pundit itself—i.e., depending only on policy classes and Pundit::NotAuthorizedError:
# lib/pundit.rb
class NotAuthorizedError < Error
def initialize(options = {})
...
message = options.[:message] || policy.try(:error_message) || "not allowed to #{query} this #{record.inspect}" }
...
end
endUsage
class ApplicationPolicy
attr_accessor :error_message
end
class PostPolicy < ApplicationPolicy
def update?
self.error_message = if record.author != user
'Keep your grubby hands off other people’s stuff!'
elsif record.archived?
'That’s old news, baby!'
end
error_message.nil?
end
endRisks
Any existing users who have defined an #error_message method (or getter) on their policies would be affected by this change.
EDIT: To address this, we could choose a method name that Pundit users are less likely to have already taken, like #denial. This has the added benefit of (conceptually) decoupling this attribute with the concept of errors/exceptions. After all, policies shouldn't "know about" errors in the first place—they just know about users, resources, and queries. (It's #authorize that handles raising errors.)
@Linuus & co., what do you think about this proposal? It's a small one, so I will prepare a PR anyway.
Source: varvet/pundit