Multiple Error Messages per One Policy Action from README causes errors

[alaarab]

I am following the instructions here: https://github.com/varvet/pundit#multiple-error-messages-per-one-policy-action

and after putting the rescue_from block in my ApplicationController, I get the following error:

/app/controllers/application_controller.rb:5: syntax error, unexpected ',', expecting end
...or] = message, scope: "pundit", default: :default
...                              ^
/app/controllers/application_controller.rb:73: syntax error, unexpected end, expecting end-of-input

Is there something I'm missing?

[dgmstuart]

@holyketzer this is from the Readme section you wrote. Now that I look at this, I don't get what the , scope: "pundit", default: :default is - that looks wrong, since the flash[:error] = message is a complete statement.

Is this a typo, or am I missing something?

If there's a problem it's my fault for letting it through code review.

[dgmstuart]

@alaarab Try just removing everything on that line after the first comma - does it work then?

[holyketzer]

I see, just copypasted it from section above, then after some iteration changed it. Let me fix

[alaarab]

@dgmstuart I already tried that, it was the first thing I thought to try, but that also didn't work for me.

[alaarab]

Also, when trying the resolution from @holyketzer 's above commit in README, I still get an issue...

Top of application_controller:

  rescue_from Pundit::NotAuthorizedError do |e|
    message = 
      if e.reason 
        I18n.t("pundit.errors.#{e.reason}", scope: "pundit", default: :default)
      else 
        e.message
      end
    flash[:error] = message
    redirect_to(request.referrer || root_path)
  end

Job_Policy.rb (with some logic not relevant to this project):

  def new?
    unless Project.active.where(manager: user).present? || 
    Representative.where(representative: user).where("access_level LIKE (?)", "%" + record.class.name.underscore.pluralize + "%").present?
      raise Pundit::NotAuthorizedError, reason: 'job.new'
    end
  end

en.yml

en:
  pundit:
    errors:
      job:
        new: "Nope!"

The error is: not allowed to this NilClass'

A screenshot of error looks like this:

I suggest testing this in a live environment before posting a solution. Please let me know if there is anything I'm missing. Thank you.

[holyketzer]

@alaarab Ah, I see, you got default error message. I didn't test I18n last time.

change it to

rescue_from Pundit::NotAuthorizedError do |e|
    flash[:error] = I18n.t("errors.#{e.reason}", scope: "pundit", default: e.message)
    redirect_to(request.referrer || root_path)
  end

pundit. in I18 key isn't necessary if there is a scope pundit. That what I missed

[dgmstuart]

To be clear, these two things are the same:

I18n.t("b.c", scope: "a")
I18n.t("a.b.c")

I guess scope is there for specialised use-cases? Probably avoid it unless you have a specific need for it.

[dgmstuart]

@alaarab Sorry, I'm being dumb: this feature isn't in a released gem yet, so unless you're pointing at master it's not going to work. If you really need this feature now, I'd recommend creating your own error which inherits from Pundit::NotAuthorizedError and implementing reason: on that.

[caifara]

@dgmstuart this feature really helps us to simplify our codebase, thanks! Is a release somewhat nearby?

[dgmstuart]

@caifara unfortunately not, but see the comment above: this is more of a pattern than a feature, and I'm less and less sure that it should be built into Pundit. You should be able to implement this pattern in your application without this being released.

[caifara]

@dgmstuart I see, no problem. I do feel that something isn't right here. The fact that the update? method raises an error is not expected behaviour. It does not allow code like:

<% if policy(@post).update? %>
  <%= link_to "Edit post", edit_post_path(@post) %>
<% end %>

On the other hand, we want to communicate in clear terms exactly why a user can't do something.

What pattern can be used to handle such a situation? Or should pundit only use policies to check whether a particular person may do something. Not, for example, both whether the action is allowed for that person + whether the object is in a state allowing that change or something entirely different (like only allowed on tuesdays or the example given in the readme).

[dgmstuart]

@caifara I think you're onto something: the principle behind policies is (rightly or wrongly) that they're supposed to be plain Ruby objects which make sense on their own (so update? should return a boolean, according to convention).

But I don't think you can have it both ways: policies are focussed on saying "can the user do this?" by returning booleans, and the fact that they can provide a specific reason feels like a secondary bonus, rather than a primary feature.

If you were designing this from the ground up to support multiple reasons, then instead of returning booleans you might have policies return some kind of Result object which is either a success, or a failure with the reason. But that would mean a lot of boilerplate for the simple cases which only have one reason, so I guess it's a tradeoff.