Fix route name resolving with subresources#1110
Conversation
| * @return string | ||
| */ | ||
| public function getRouteName(string $resourceClass, $operationType): string; | ||
| public function getRouteName(string $resourceClass, $operationType, array $context = []): string; |
There was a problem hiding this comment.
I think that this is a breaking change. Should I avoid changing this interface? I need more informations when retrieving the route name for a subresource because multiple routes may find a wrong match on the same resource class. It has to take into consideration the subresource tree. See isSameSubresource in RouteNameResolver.
There was a problem hiding this comment.
Yes, this is a BC break.
| * @ApiResource | ||
| * @ORM\Entity | ||
| */ | ||
| class DummyTour |
There was a problem hiding this comment.
I suggest renaming this to DummyProduct (Tour is from my own project), since the offers property is defined by https://schema.org/offers
e962113 to
df0c44d
Compare
|
I've done some dirty fix to avoid breaking, let me know your thoughts or if you have another idea on how to do this. |
|
Hehe... We can ignore the PHPstan error for now. 😄 |
460e2b7 to
59afc11
Compare
|
ping @dunglas is also good to merge if you agree with the routename patch! |
| } | ||
|
|
||
| $routeName = $this->decorated->getRouteName($resourceClass, $operationType); | ||
| $routeName = $this->decorated->getRouteName($resourceClass, $operationType, @func_get_arg(2)); |
There was a problem hiding this comment.
Can you do it like that instead https://github.com/symfony/symfony/blob/3.4/src/Symfony/Component/Serializer/Serializer.php#L182-L190
| public function getRouteName(string $resourceClass, $operationType): string | ||
| public function getRouteName(string $resourceClass, $operationType /**, array $context = [] **/): string | ||
| { | ||
| $context = @func_get_arg(2); |
Also resolves conflicts with existing subresources on a property having the same name
|
Thanks @soyuka |
Fix route name resolving with subresources
3 participants
Also resolves conflicts with existing subresources on a property having
the same name
Unit tests don't pass for now, waiting for an answer to the below comment.