Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add Custom Message option to can_definition #103

Closed
junaid opened this Issue · 15 comments

6 participants

junaid Ryan Bates Nathan Brazil Florent Monbillard Tony Miller bteitelb
junaid

Hi Ryan,

It would be great if we could change access denied message with each can definition like
can :edit, User, "You have to login to access this page"
or we could also do this like
can :edit, User, :message => "You cannot edit other user's profile do |user_to_edit|
user_to_edit.id = user.id
end

I have already sent you a pull request for this feature and you have also replied me here http://github.com/ryanb/cancan/issues/issue/40/#comment_315783.

Thanks & Regards
Junaid.

Ryan Bates
Owner

This would be really convenient. Currently if you want to customize the messages for each action you need to handle this when catching the exception which gets messy.

if exception.action == :edit && exception.subject.kind_of?(User)
  flash[:alert] = "You cannot edit other user's profile"
else
  flash[:alert] = exception.message
end

Here one also has to consider aliased actions (edit vs update) and passing in a User class vs a @user instance. That logic can get very complex, so why repeat it when it's already done for you in the Ability class.

So I agree on the ability to define the message in the Ability class. The problem is how the interface should work. I don't care for the idea of a the :message option because it conflicts with the conditions hash feature. Having the last argument be an optional string is a possibility, however the can method arguments are already quite complex and this will make it more-so.

What does everyone think, I'm open to hearing ideas.

Ryan Bates
Owner

Another possible interface is a message method which will effect every can call after it. For example.

message "You cannot edit other user's profile"
can :edit, User

Maybe unauthorized_message would be a better name, but kind of long.

The thing I like about this is that it's easy to define one message which is used for multiple can calls. One can use this to define a default message at the top which is used for everything.

The thing I don't like about this is that it shares state across can calls. It reminds me of the desc call in Rake tasks which I never cared for.

junaid

yeah 'unauthorized_message' is better name instead of 'message' and sharing 'unauthorized_message' among multiple can definitions is really good idea of you. I will also find other ways to share 'unauthorized_message'. Thanks Ryan for considering this as a feature.

Nathan Brazil

Actually, I was wondering if there can be a way to indicate which can or cannot rule tripped an unauthorized error.

Ryan Bates
Owner

You can use exception.action and exception.subject to help determine what rule was tripped. See the Exception Handling wiki page for details.

However it won't tell you exactly which rule was tripped. Do you want this info for debugging purposes or are there other reasons?

Florent Monbillard

Would be nice if the message was handled by I18n

Ryan Bates
Owner

@EppO, agreed. I haven't done much with I18n though. Anyone have suggestions on how best to implement that?

Florent Monbillard

Ryan, using i18n gem is really easy :

I18n.t "unauthorized.edit.profile"

and put the translated strings in files located in config/locales directory like config/locales/en.yml in haml format

en: 
  unauthorized:
    edit:
       profile: "You cannot edit other user's profile"
Ryan Bates
Owner

Thanks EppO. I wonder how the performance of this lookup is. I would like it to mimic the behavior of CanCan actions and follow aliases. If one is not authorized to edit a profile, it would first check edit.profile then update.profile then manage.profile then manage.all translations.

I also think it would be nice to support variables in the I18n string so one could do one translation to support multiple actions and subjects. I guess it's time to experiment. :)

Ryan Bates
Owner

Actually, the unauthorized message isn't something which gets triggered frequently, so I think it's safe to not worry much about the performance of the lookups. It seems like this is a great solution for customizing the messages. I'll work on adding this in 1.4.

Ryan Bates
Owner

use I18n for unauthorization messages - closed by a5f838a

Florent Monbillard

you rock ryan !
I don't think the lookup performance issue is a big deal
thanks a lot and keep up the good work

Ryan Bates
Owner

I just realized I didn't add the ability to insert variables into the messages. I'll make a separate issue for that.

Tony Miller

What if you have multiple conditions for a permission and you'd like a separate error message for each one? I can't add an error message to the unauthorized! method because I don't know which logic was triggered to deny access.

Would there be some way to name each condition and then add an entry to the locale file for each?

bteitelb

+1 to mcfiredrill's request.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.