-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix incorrect docs for subresource operations #779
Conversation
We have just upgraded a big app from `2.3.6` to `2.4.2` and I think documentation is incorrect for subResources. Without these changes, our tests were failing 1. First of all, I think this is a BC break, because id *did* work before, but doesn't work after upgrading 2. What we did is just replaced `collectionOperations` with `subresourceOperations`, pleasee see the simplified diff ```diff # This is a subResource config - collectionOperations: + subresourceOperations: api_services_service_products_get_subresource: normalization_context: groups: [...] ``` Why doesn't it work in `2.4.2` anymore? Because the following code expects sub resources' configuration to be under `subresourceOperations` key: https://github.com/api-platform/core/blob/ef76e6bc20ca0658a28c5ccccc1b406f47dbbec3/src/Metadata/Resource/ResourceMetadata.php#L179-L182
That does not seem correct.
There may well be a bug, but this is not it. |
Then this is definitely a bug in the code. Please look at the screenshot:
As you can see, when the request is a request to get sub resources, the |
@borNfreee Could you share a stack trace please? Or add a Behat test. Thanks. 😄 |
This change looks correct to me. |
XML config needs fixing. We need to change this part too:
|
Also, @soyuka if I understand correctly, the operation names for subresource operations are weird... Those are route names, aren't they? |
yes |
Replace collectionOperations with subresourceOperations
done |
Many thanks @borNfreee and sorry I forgot to update the docs when we fixed that bug! |
@soyuka So the operation names are wrong, aren't they? Shouldn't we fix that? It's weird that the user has to use the route name... |
Other than being weird, it also is a portability issue as we don't want to be tied to the Symfony Routing component. |
@@ -376,7 +376,7 @@ If you put the subresource on a relation that is to-many, you will retrieve a co | |||
|
|||
Last but not least, subresources can be nested, such that `/questions/42/answer/comments` will get the collection of comments for the answer to question 42. | |||
|
|||
You may want custom groups on subresources. Because a subresource is nothing more than a collection operation, you can set `normalization_context` or `denormalization_context` on that operation. To do so, you need to override `collectionOperations`. Based on the above operation, because we retrieve an answer, we need to alter its configuration: | |||
You may want custom groups on subresources. Because a subresource is nothing more than a collection operation, you can set `normalization_context` or `denormalization_context` on that operation. To do so, you need to override `subresourceOperations`. Based on the above operation, because we retrieve an answer, we need to alter its configuration: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because a subresource is nothing more than a collection operation
This part needs to be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I think this whole paragraph needs to be rewritten. I'll take care of it, if no one gets to it before me.
We have just upgraded a big app from
2.3.6
to2.4.2
and I think documentation is incorrect for subResources.Without these changes, our tests were failing
2.4.2
is a BC break, because it did work before, but doesn't work after upgradingcollectionOperations
withsubresourceOperations
, pleasee see the simplified diffWhy doesn't it work in
2.4.2
anymore?Because the following code expects sub resources' configuration to be under
subresourceOperations
key:https://github.com/api-platform/core/blob/ef76e6bc20ca0658a28c5ccccc1b406f47dbbec3/src/Metadata/Resource/ResourceMetadata.php#L179-L182