[FrameworkBundle] Improve DX of RedirectController #25259
Conversation
98eb91c to
349cd92
Compare
73672a2 to
4f48b5a
Compare
|
Status: needs review |
|
isnt adding a single test to cover the method forwarding simpler? |
|
Well doing that allowed me to find some bugs in my forwarding logic. So I would prefer to keep al the tests. |
|
Maybe that's not that useful if we go with this PR that allows redirection to be set directly in the routing configuration? |
|
@sroze IMO we must have an invoke method for all builtin controllers. |
|
|
||
| public function __invoke(Request $request): Response | ||
| { | ||
| if (null !== $route = $request->attributes->get('route')) { |
There was a problem hiding this comment.
That looks like alot of magic. It might be cleaner to just create a new controller with invoke for one action. And possibly deprecate it in RedirectController so there is no ambiguity.
This PR was merged into the 4.1-dev branch. Discussion ---------- [DI] Allow for invokable event listeners | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #... <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!--highly recommended for new features--> Inspired by #24637 / #25259. This adds invokable support for event listeners :) ```yaml Some\Foo: tags: [{ name: kernel.event_listener, event: kernel.request }] ``` ```php class Foo { public function __invoke(GetResponseEvent $event) { } } ``` Commits ------- fa5b7eb [DI] Allow for invokable event listeners
|
I'm not convinced this provides anything. Actually, I'm much more convinced by the aliasing discussed in #25145 (comment) 👎 |
|
I don't see why we must provide an |
Similar to #24637 but for redirect controller.
TODO: