Patching {"data": null} on To-One relationship fails

[tpansino]

Describe the bug
See https://jsonapi.org/format/#crud-updating-to-one-relationships

Specifically, this case:

PATCH /articles/1/relationships/author HTTP/1.1
Content-Type: application/vnd.api+json
Accept: application/vnd.api+json

{
  "data": null
}

Isn't working properly. The code seems to be expecting the data: object to have a type, so I think it needs to be updated to skip this check in the event we're patching a To-One relationship and the passed value is null. (The relationship ForeignKey or OneToOneField would also need to be nullable, but I have a feeling that's beyond the scope of what this package cares to check and is probably handled by Django at a lower level. Wouldn't hurt to test though to be sure...)

To Reproduce
Define two models, one with a ForeignKey linking it to the other, and two corresponding endpoints with a relationship endpoint between the resources. Attempt to PATCH {"data": null} on the To-One side of the relationship endpoint, observe an Exception is thrown.

Expected behavior
The response should be a 204 No Content code with the relationship successfully nullified.

[gargrave]

@tpansino and I already discussed this IRL, but I just wanted to leave a comment and mention that we have run into this issue in https://github.com/Vacasa/housekeeping-web, where I cannot clear an existing to-one relationship by sending { data: null } as describe in the spec.

Technically, we can work around this by making a DELETE request on the relationship from the opposite direction, but that is far from ideal, and is a bit confusing.

If it's possible to make any progress on this issue, it would be extremely helpful, because I imagine we will continue running into this as the API grows and we consume more endpoints.

[tpansino]

@gargrave For completeness - can you also try PATCHing { data: [] } on the opposite direction? Does that work? If not then I have 2 bugs to fix 😄

Edit: Whoops, I meant [], not null.

[gargrave]

@tpansino Hopefully I am understanding correctly--the other direction is a to-many, so rather than taking null to clear the relationship(s), it takes an empty array. That indeed does work.

If I completely missed your point, feel free to give me a shout. 👍

[tpansino]

Yep, that's correct, sorry for the confusion and thanks for checking.

[tpansino]

After some digging, I've determined that this is a regression that happened during #29, specifically this line: https://github.com/Vacasa/drf-jsonapi/pull/32/files#diff-9a2180fee48df47db205fc59be17fa76L380

By removing the resource.save(), any To-One relationship change never actually gets committed to the database. That applies both to null as well as any real resource identifier object.

A better place to put this line would be right after the RelationshipHandler sets the To-One relationship in it's set_related method. I'll open a PR for that.

[tpansino]

I just realized I'm conflating two unrelated issues here. The original reason I opened this Issue was because PATCHing {"data": null} on a To-One fails. That's still the case even if we fix the resource.save() line I mentioned in my previous comment.

So these are related issues, but have different fixes. I'll open another issue.

[tpansino]

Needs to also be fixed on master, reopening.