Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to remove ToOne relationship? #74

Closed
bertramakers opened this issue Aug 11, 2023 · 6 comments
Closed

How to remove ToOne relationship? #74

bertramakers opened this issue Aug 11, 2023 · 6 comments

Comments

@bertramakers
Copy link
Contributor

bertramakers commented Aug 11, 2023

I'm trying to remove a ToOne relationship from a resource by sending a PATCH request to the resource with the ToOne relationship like this:

PATCH /api/v2/clusters/17

{
    "data": {
        "type": "clusters",
        "id": "17",
        "relationships": {
            "teamLead": {
                "data": null
            }
        }
    }
}

This results in a TypeError with the following message: obyz\JsonApiServer\Schema\Field\Relationship::findResourceForIdentifier(): Argument #1 ($identifier) must be of type array, null given, called in /var/www/vendor/tobyz/json-api-server/src/Schema/Field/ToOne.php on line 59

Is this a bug, or should I remove the ToOne relationship in a different way?

@bertramakers
Copy link
Contributor Author

Looking more closely at the source code of the ToOne relationship, I learned I can make it work by calling ->nullable() on the field.

However, this now also includes the relationship with its null value every time I fetch the resource, which is not what I necessarily wanted.

I think that setting the relationship to null to remove it, and actually including the null value in the response are two separate contexts.

In the case of removing the relationship, it would probably be better to check if it's required() or not vs nullable().

@tobyzerner
Copy link
Owner

tobyzerner commented Aug 16, 2023

The fact that it results in a TypeError is a bug – it should result in a nicer validation error. I'll look into a fix.

nullable() means "this field can have a null value", whereas required() means "this field MUST be present when creating a new resource". So if I'm understanding correctly, your relationship should indeed be marked as nullable.

By default, ToOne relationships have resource linkage enabled, meaning that the data member for the relationship will be present in the response (whether it is null or not). If you want to disable resource linkage, use withoutLinkage().

@bertramakers
Copy link
Contributor Author

bertramakers commented Aug 16, 2023

I'm not sure I completely understand resource linkage, but my issue with nullable is this:

  1. When I create a new resource of a type that has an optional ToOne relationship, and I don't specify a value for that relationship, the resource that is returned afterwards does not contain the ToOne relationship at all (not even with {"data": null})
  2. When I update the resource to set a value for the relationship, the returned resource then obviously contains a value for the ToOne relationship
  3. When I update the resource to remove the value of the relationship (only possible if nullable()), the returned resource now contains a literal null value for the relationship. While when it was initially created, it didn't have any value for this relationship, not even null

This is confusing for our frontend developers because in some cases when a resource does not have a value for the relationship, the relationship will not be present at all, while in others it will be included and set to null (depending on whether the relationship was set before but removed, or never set at all)

@tobyzerner
Copy link
Owner

The behaviour in your first point seems odd - as long as linkage is enabled on the relationship (as it is by default), it should be present in the response, null or not. Will have a look into it.

@bertramakers
Copy link
Contributor Author

I double-checked and it was a mistake on my end 🤦‍♂️ Because I was testing how to remove the relationship at the time, I accidentally tested the first point without the nullable() on the relationship, and then I added it to make it possible to remove the relationship and obviously it would become null in the third step. Since the behavior is now consistent (always null if it's not set), this is okay for me. Sorry for wasting your time on this one! 😅

@bertramakers
Copy link
Contributor Author

Ah I'll reopen the issue for the TypeError when you try to remove a relationship that is not nullable().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants