[Routing] Fix resource miss #24642
[Routing] Fix resource miss #24642
Conversation
1e00afb to
1dd4f8f
Compare
| @@ -311,6 +311,9 @@ public function build() | |||
| $subCollection->addPrefix($this->prefix); | |||
|
|
|||
| $routeCollection->addCollection($subCollection); | |||
| foreach ($route->resources as $resource) { | |||
There was a problem hiding this comment.
shouldn't this be handled by $routeCollection->addCollection instead ?
There was a problem hiding this comment.
This doesn't work because $route->build() (https://github.com/symfony/symfony/pull/24642/files#diff-61a7c46324774b76b342efb3f3e87b7fL310) doesn't add sub-resources to $subCollection. This is exactly what this patch fix.
There was a problem hiding this comment.
it is adding them just a few lines below in this code.
There was a problem hiding this comment.
$route->resources vs $this->resources
There was a problem hiding this comment.
While you are on this file... it looks like nothing calls the private method addResource, so maybe you can remove the resources property of that class, as well as the loop "below".
There was a problem hiding this comment.
sure. But the loop below happens inside ->build() and so the subcollection will have the resources in it if it has any.
There was a problem hiding this comment.
But it’s not the case (the new test fail without my patch). I’ll Investigate this afternoon.
There was a problem hiding this comment.
I double checked and pushed a better fix:
$route can have resources but no routes. So the loop over $this->resources must be outside the loop on $this->routes.
Can you check if it's ok @stof?
|
Thank you @dunglas. |
This PR was squashed before being merged into the 2.8 branch (closes #24642). Discussion ---------- [Routing] Fix resource miss | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no <!-- don't forget to update src/**/CHANGELOG.md files --> | BC breaks? | no | Deprecations? | no <!-- don't forget to update UPGRADE-*.md files --> | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a Some routing resources are not watched. To reproduce: 1. Install Symfony 4 2. Change something in `config/routes.yaml` The change is not taken into account. This PR fixes this bug. Commits ------- 6610c25 [Routing] Fix resource miss
Some routing resources are not watched. To reproduce:
config/routes.yamlThe change is not taken into account.
This PR fixes this bug.