jeremyevans / rodauth

Ruby's Most Advanced Authentication Framework
http://rodauth.jeremyevans.net
MIT License
1.69k stars 95 forks source link

lib/rodauth/features/base.rb: add `throw_status` method that throws :rodauth_error without needing to set_field_error #418

Closed jf closed 5 months ago

jf commented 5 months ago

as per https://github.com/jeremyevans/rodauth/discussions/412

jeremyevans commented 5 months ago

The method I said I would accept in https://github.com/jeremyevans/rodauth/discussions/412#discussioncomment-9182587 would just call throw :rodauth_error. Something like:

def throw_error
  throw :rodauth_error
end

You could then write your own throw_status method that calls this method. I would not accept a throw_status method, because there is nothing internal to Rodauth that uses it, as I mentioned in https://github.com/jeremyevans/rodauth/discussions/412#discussioncomment-9110491.

jf commented 5 months ago

I see. Thanks for the clarification. I'll update the PR

jf commented 5 months ago

hm, just realized that there is a throw_error method already in lib/rodauth/features/base.rb that has 2 arguments (field, and error). I could call the new method throw_rodauth_error... but I'm not sure I like this name / approach (throw_rodauth_error as a name means you are sort of made aware of the specific error being thrown; but throw_error does not). What do you think of making field and error optional for throw_error like so?

def throw_error(field=nil, error=nil)
  set_field_error(field, error) unless field.nil? and error.nil?
  throw :rodauth_error
end

This way you can simply call throw_error to just throw :rodauth_error.. but if you want to also set_field_error, you can do that as well, with throw_error(field, error)

jeremyevans commented 5 months ago

I already made it clear in https://github.com/jeremyevans/rodauth/discussions/412#discussioncomment-9182587 that I don't want to make the throw_error arguments optional. I would be OK adding throw_rodauth_error as a separate method.

jf commented 5 months ago

My apologies. In my mind we were talking about modifying throw_error_status (https://github.com/jeremyevans/rodauth/discussions/412#discussioncomment-9172187); I thought things might be different for throw_error.

I've updated the PR to add throw_rodauth_error.

jeremyevans commented 5 months ago

Squash merged at b44b441