jsonapi-authorization
jsonapi-authorization copied to clipboard
Use distinct policy method for clearing out has-many association?
This PR is up for grabs!
If you want to implement this, add a comment to this issue saying so! ☺️
Setup:
user_1 = User.create(id: 'user-1')
comment_1 = Comment.create(id: 'comment-1')
comment_2 = Comment.create(id: 'comment-2')
article_1 =
Article.create(
id: 'article-1',
comments: [comment_1, comment_2],
author: user_1
)
HTTP call:
PATCH /articles/article-1/relationships/comments{ "data": [] }
Current situation:
We will call
ArticlePolicy.new(current_user, article_1).replace_comments?([])
Suggestion:
Call a specific method when removing all comments:
ArticlePolicy.new(current_user, article_1).remove_comments?
Reason for suggestion:
This mirrors the way a removal of has-one relationship is done:
PATCH /articles/article-1/relationships/author{ "data": null }
...which ends up calling
ArticlePolicy.new(current_user, article_1).remove_author?
because of the check done in JSONAPI:: Authorization:: AuthorizingProcessor
def authorize_replace_to_one_relationship
return authorize_remove_to_one_relationship if params[:key_value].nil?
# ...
end
Implementation suggestion:
-
Add a new method to
DefaultPunditAuthorizerfor this case. Name the methodremove_to_many_relationshipand create a similar implementation toremove_to_one_relationship. -
Create tests for the new authorizer method. You can mirror the
#replace_to_one_relationshipmethod specs. -
Add new logic to
AuthorizingProcessor#authorize_replace_to_many_relationshipsmethod to check for the possible emptiness ofparams[:data]array, and then bail out by calling the newremove_to_one_relationshipmethod on the authorizer. -
Add new case to relationship operations request spec to cover this new logic. You can mirror the existing specs of "when nullifying the author" on has-one relationship PATCH. The new spec should be under the
describe 'PATCH /articles/:id/relationships/comments' doblock -
https://github.com/venuu/jsonapi-authorization/issues/73#issuecomment-312819108
I discovered this while writing docs in #71 and this inconsistency struck out a bit. I added a TODO comment in the upcoming docs for now.
Hmm I guess that if we change this, we should also change how clearing out the has-many relationship with a PATCH on the resource itself is handled:
PATCH /articles/article-1{ "type": "articles", "id": "article-1", "relationships": { "comments": { "data": [] } } }
This should also call:
ArticlePolicy.new(current_user, article_1).remove_comments?
This is because it would mirror how we do it with a PATCH:
PATCH /articles/article-1{ "type": "articles", "id": "article-1", "relationships": { "author": { "data": null } } }
...which ends up calling:
ArticlePolicy.new(current_user, article_1).remove_author?