Use distinct policy method for clearing out has-many association?

[valscion]

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:

1. Add a new method to DefaultPunditAuthorizer for this case. Name the method remove_to_many_relationship and create a similar implementation to remove_to_one_relationship.

2. Create tests for the new authorizer method. You can mirror the #replace_to_one_relationship method specs.

3. Add new logic to AuthorizingProcessor#authorize_replace_to_many_relationships method to check for the possible emptiness of params[:data] array, and then bail out by calling the new remove_to_one_relationship method on the authorizer.

4. 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' do block

5. Use distinct policy method for clearing out has-many association? #73 (comment)

[valscion]

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.

[valscion]

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?