Support for setting the redirect content as a content document #163
Conversation
|
|
||
| $adapter->createRedirectRoute($this->route->reveal(), $newRoute->reveal()); | ||
| } | ||
|
|
dantleech
force-pushed
the
redirect_target_type
branch
from
7bd390f to
cdb6e3a
Compare
| $referringAutoRoute->setRedirectTarget($newRoute); | ||
| switch ($this->redirectTarget) { | ||
| case self::REDIRECT_CONTENT: | ||
| $target = $newRoute->getContent(); |
There was a problem hiding this comment.
imho, we should make redirect to content the default, but fall back to the route if there is no content (which is a valid situation). i don't think we need the configuration options with constants, if there is a content of a route and it implements RouteReferrersReadInterface, we should take that over the route. but only in that case. the content could be not route aware, or there can be routes with no content object.
There was a problem hiding this comment.
The main thing that concerns me is the change of behavior - the option is there to preserve the existing behavior, if we now set the redirect target as the content, and the user is handling the redirect as if it were always a Route object, then it's a BC break ?
|
The PR on the component is nowmerged, @dantleech can you please finish this PR? |
|
@dantleech is this still relevant or should we drop it? |
Allow configuring the redirect target to be set as the content document instead of the route.
See: symfony-cmf/routing-auto#43
Depends: symfony-cmf/routing-auto#56
Discussion: Redirecting to the content is certainly better -- otherwise there is just an ever growing chain of auto routes which must be resolved (e.g. rename something 50 times, then there is a maximum of 50 routes which must be resolved).
So I would say that in 2.0 the content document should be the redirect target by default. But do we want to make this the ONLY choice? To what extent does it make sense to redirect to another route?
TODO: Need to add configuration for this.