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

PATCH a relationship with {"data":null} #15

Closed
dimvic opened this issue Feb 22, 2016 · 5 comments
Closed

PATCH a relationship with {"data":null} #15

dimvic opened this issue Feb 22, 2016 · 5 comments
Assignees
Labels
Milestone

Comments

@dimvic
Copy link
Contributor

dimvic commented Feb 22, 2016

According to the specification issuing a PATCH request which includes a relationship with a {"data":null} body should clear the relationship, but in HydratorTrait->hydrateRelationships() this is found:

if (empty($data["relationships"])) {
    return $domainObject;
}

which results in the relationship hydrator never being called, is that intentional?

Of course one can override hydrateRelationships(), but shouldn't the defaults conform to the standards?

@kocsismate
Copy link
Member

That's really clever! I'll look at it tomorrow!

@kocsismate kocsismate added the bug label Feb 24, 2016
@kocsismate
Copy link
Member

kocsismate commented Feb 28, 2016

Sorry for being late, I had other works to do during the week.

So I investigated the problem and found that you mean the updating relationships part of the spec, and I had to realize that currently, hydrators only work well when you want to update or create resources.

As we talked that hydrators need some improvement anyway, I'll try to address this deficiency in v0.11. But now, I'll release the version with your other patches! Thank you for your help :)

@kocsismate kocsismate self-assigned this Feb 28, 2016
@kocsismate kocsismate added this to the 0.11 milestone Feb 28, 2016
@dimvic
Copy link
Contributor Author

dimvic commented Feb 28, 2016

No prob, my pleasure :) I don't have a use case this bug affects yet, I just thought it's quite an important point in the docs that should be supported, that's why I reported it...
Eagerly waiting for 0.10.2 for now!

@kocsismate
Copy link
Member

kocsismate commented Aug 8, 2016

I am closing this issue as the feature in question was implemented in #33 though it will need some small adjustments and additions. So v0.11 is not far away, I want to make it happen in 1-2 weeks!

@kocsismate
Copy link
Member

My latest commit brings full support for this feature: b11718c

Test it if you have some time :)

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

No branches or pull requests

2 participants