PATCH to RelationshipView can cause data to be inadvertently deleted

[tpilara]

What
Making a PATCH request to a relationship view should replace the entire relationship with the content of the PATCH. This behavior is provided by the package but in some instances can cause objects to be deleted that shouldn't be.

How
In the RelationshipView's patch method (

django-rest-framework-json-api/rest_framework_json_api/views.py

Line 81 in 7ad660b

def patch(self, request, *args, **kwargs):

), there are two code paths: if the relationship instance is a manager (ManyToMany or reverse ForeignKey) or not (a direct ForeignKey on the model). In the manager path, the intention is that all relationships are cleared and then re-created.

Currently, the patch method will validate the incoming data, delete all of the objects in the existing relationship (

django-rest-framework-json-api/rest_framework_json_api/views.py

Line 89 in 7ad660b

related_instance_or_manager.all().delete()

), and recreate the relationship with objects passed into the request.

I see 3 ways this code patch can be accessed: reverse ForeignKey with non-nullable ForeignKeys to this object, reverse ForeignKey with nullable ForeignKeys to this object, and a ManyToMany relationship from this object to other objects. Right now the code will actually delete the underlying objects in the relationship, which I think is only correct in one of those three cases (the reverse ForeignKey with non-nullable ForeignKeys to this object).

Solution
My proposed solution would replace the line that deletes the objects in the relationship with a function, called something like remove_relationship, which will check each of these cases and perform the correct action:

• ManyToMany relationship, the relationship should be cleared and then re-populated with the passed data
• Reverse ForeignKey with nullable ForeignKeys, the value of the ForeignKey on each object should be set to None
• Reverse ForeignKey with non-nullable ForeignKeys, those objects should be deleted as they are currently.

I don't have a patch for this right now which is why I'm just reporting it, but if no one else has time to tackle this I'll try to get something up that people can review.

[Alig1493]

Is it possible to make a contribution to this? If it is then some guidance would be helpful.

[sliverc]

@Alig1493 I guess best way to start is to write a in test_views.py to reproduce this issue and try to fix it from there.

[Alig1493]

I've been looking at this issue investigating the relationship between Author and Comments.
Comments has a nullable Author foreign key.

So I'm guessing when we define a new relationship between an existing Author object and a new Comment object using a patch request. the new comment should have the Author instance.

However after making the patch request the old comment gets deleted and a new relationship between the new comment and author gets created.

I wrote a test for this to investigate this issue and saw the list of comment objects before and after the patch request was made.

I wanted to make sure that this is the issue that's being discussed here before I try and see if I can find a way to solve the issue.

My test case output:
https://dpaste.de/5Jpr