Feature/customable authorized users only error response #869
Conversation
| # current_admin # Current signed in admin | ||
| # user_session # Session data available only to the user scope | ||
| # admin_session # Session data available only to the admin scope | ||
| # render_authenticate_#{mapping}_error # Render error if |
There was a problem hiding this comment.
Perhaps this docs line needs to be 2 lines (one for each generated mapping).
And the comment at the end of the line seems unfinished.
There was a problem hiding this comment.
I mean: have two lines, with ending comments like these:
# Render error unless user is signed in# Render error unless admin is signed in
There was a problem hiding this comment.
@olleolleolle Thank you for your review.
You are right. I will modify the comment soom.
|
When will this PR be merged? 🤔 |
|
@abeyuya - There are conflicts and there is also an issue where the error response is not pluralized to match the prior string. Ie - "user" vs "users" (expected). |
| @@ -68,7 +68,7 @@ def current_#{group_name.to_s.pluralize} | |||
|
|
|||
| def render_authenticate_#{group_name}_error | |||
| return render json: { | |||
| errors: ["Authorized #{group_name} only."] | |||
| errors: [I18n.t('devise.failure.unauthenticated')] | |||
There was a problem hiding this comment.
I have resolved conflict about error message.
| end | ||
|
|
||
| it 'should define render_authenticate_member_error' do | ||
| assert @controller.methods.include?(:render_authenticate_member_error) | ||
| assert @controller.methods.include?(:render_authenticate_error) |
There was a problem hiding this comment.
These three tests now make less sense: their it descriptions talk about method names that are not checked for existence.
| @@ -66,14 +66,14 @@ def current_#{group_name.to_s.pluralize} | |||
| end.compact | |||
| end | |||
|
|
|||
| def render_authenticate_#{group_name}_error | |||
| def render_authenticate_error | |||
There was a problem hiding this comment.
I have changed the method name to render_authenticate_error .
Exists error-render methods are using simple method name like render_create_error.
They don't include #{group_name} and #{mapping} like render_create_error.
So new method name also don't include these.
https://github.com/lynndylanhurley/devise_token_auth#registrations-controller
| end | ||
|
|
||
| it 'should define render_authenticate_mang_error' do | ||
| assert @controller.methods.include?(:render_authenticate_mang_error) | ||
| assert @controller.methods.include?(:render_authenticate_error) |
There was a problem hiding this comment.
Also here: The same note about "tests' contents do not match their descriptions".
| end | ||
|
|
||
| it 'should define render_authenticate_mang_error' do | ||
| assert @controller.methods.include?(:render_authenticate_mang_error) | ||
| assert @controller.methods.include?(:render_authenticate_error) |
There was a problem hiding this comment.
(And here, too.)
Perhaps there's more investigation to be made?
There was a problem hiding this comment.
(Or, rather, renaming some tests?)
There was a problem hiding this comment.
Oh sorry 😱
I will modify bad tests.
|
@booleanbetrayal have a moment to finish review of this PR? All tests pass and no conflicts :) |
|
Going to merge it then! |
I have added new methods for customize error response when "Authorized users only".