Skip to content
This repository was archived by the owner on Apr 14, 2026. It is now read-only.

Make redirect url fallback configurable #190

Open
AsfalothDE wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Make redirect url fallback configurable #190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
2bfbe41
Make redirect url fallback configurable
Jun 7, 2018
e961202
Fix unit tests
Jun 13, 2018
92ba4e6
Use / as fallback when there is no route given
Jun 13, 2018
3b49d2c
Fix missing $this…
Jun 13, 2018
ef72908
Make statement CS compliant
Jun 13, 2018
741ca10
Remove redundant code
Jun 13, 2018
ab6eb14
Set default value of redirect_fallback to null
Jun 13, 2018
be1e60d
Draft for routing test
Jun 15, 2018
7537a58
fixed tests by removing the Request mock
lsmith77 Jun 16, 2018
08aef0e
Merge pull request #1 from liip/AsfalothDE-master
AsfalothDE Jun 18, 2018
068dbb0
Add documentation for redirect_fallback
Jun 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions Controller/ThemeController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Symfony\Component\Routing\RouterInterface;

/**
* Theme controller.
Expand All @@ -40,18 +41,32 @@ class ThemeController
*/
protected $cookieOptions;

/**
* @var RouterInterface
*/
protected $router;

/**
* @var string|null
*/
protected $defaultRoute;

/**
* Theme controller construct.
*
* @param ActiveTheme $activeTheme active theme instance
* @param array $themes Available themes
* @param array|null $cookieOptions The options of the cookie we look for the theme to set
* @param RouterInterface $router
* @param string|null $defaultRoute
*/
public function __construct(ActiveTheme $activeTheme, array $themes, array $cookieOptions = null)
public function __construct(ActiveTheme $activeTheme, array $themes, array $cookieOptions = null, RouterInterface $router, $defaultRoute = null)
Copy link
Copy Markdown
Contributor

@lsmith77 lsmith77 Jun 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also make the $router parameter nullable to maintain BC?

{
$this->activeTheme = $activeTheme;
$this->themes = $themes;
$this->cookieOptions = $cookieOptions;
$this->router = $router;
$this->defaultRoute = $defaultRoute;
}

/**
Expand All @@ -73,7 +88,15 @@ public function switchAction(Request $request)

$this->activeTheme->setName($theme);

$url = $request->headers->get('Referer', '/');

if ($this->defaultRoute) {
Copy link
Copy Markdown
Contributor

@lsmith77 lsmith77 Jun 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

once the router property is nullable, you need to check if its set here as well

$redirect = $this->router->generate($this->defaultRoute);
} else {
$redirect = '/';
}

$url = $request->headers->get('Referer', $redirect);

$response = new RedirectResponse($url);

if (!empty($this->cookieOptions)) {
Expand Down
1 change: 1 addition & 0 deletions DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public function getConfigTreeBuilder()
->prototype('scalar')->end()
->end()
->scalarNode('active_theme')->defaultNull()->end()
->scalarNode('redirect_fallback')->defaultNull()->end()
->arrayNode('path_patterns')
->addDefaultsIfNotSet()
->children()
Expand Down
2 changes: 1 addition & 1 deletion DependencyInjection/LiipThemeExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public function load(array $configs, ContainerBuilder $container)
{
$config = $this->processConfiguration(new Configuration(), $configs);

foreach (array('themes', 'active_theme', 'path_patterns', 'cache_warming') as $key) {
foreach (array('themes', 'active_theme', 'path_patterns', 'cache_warming', 'redirect_fallback') as $key) {
$container->setParameter($this->getAlias().'.'.$key, $config[$key]);
}

Expand Down
11 changes: 11 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,17 @@ liip_theme:
device_detection: my_devcice_detection
````

### Fallback redirect

Per default the bundle redirects to the referer URL after theme switching. When there is no referer the bundle redirects to "/" as fallback. It is possible to configure a route to use as a redirect fallback instead of "/". This is useful if your Symfony installation is running in a subfolder of your webroot or if you do not want to redirect to the homepage after theme switching. **Attention:** redirecting to a route is only possible when there is no referer set by the browser.

```yaml
# app/config/config.yml
liip_theme:
# ...
redirect_fallback: any_route
```

## Contribution

Active contribution and patches are very welcome. To keep things in shape we
Expand Down
2 changes: 2 additions & 0 deletions Resources/config/controller.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
<argument type="service" id="liip_theme.active_theme" />
<argument>%liip_theme.themes%</argument>
<argument>%liip_theme.cookie%</argument>
<argument type="service" id="router"/>
<argument>%liip_theme.redirect_fallback%</argument>
</service>
</services>
</container>
68 changes: 33 additions & 35 deletions Tests/UseCaseTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,39 +68,22 @@ protected function getEventMock($request, $response, $type = 'Symfony\Component\
return $event;
}

protected function getMockRequest($theme, $cookieReturnValue = 'cookie', $userAgent = 'autodetect')
/**
* @return \Symfony\Bundle\FrameworkBundle\Routing\Router
*/
protected function getRouterMock()
{
$request = $this->getMockBuilder('Symfony\Component\HttpFoundation\Request')
->disableOriginalConstructor()
->getMock();
$router = $this->getMockBuilder('Symfony\Component\Routing\Router')
->disableOriginalConstructor()
->setMethods(['generate', 'supports', 'exists'])
->getMock();

$request->expects($this->any())
->method('get')
->will($this->returnValue($theme));
$request->cookies = $this->getMockBuilder('Symfony\Component\HttpFoundation\ParameterBag')
->disableOriginalConstructor()
->getMock();
$request->cookies->expects($this->any())
->method('get')
->will($this->returnValue($cookieReturnValue));
$request->headers = $this->getMockBuilder('Symfony\Component\HttpFoundation\ParameterBag')
->disableOriginalConstructor()
->getMock();
$request->headers->expects($this->any())
->method('get')
->will($this->returnValue('/'));
$request->headers->expects($this->any())
->method('get')
->will($this->returnValue($cookieReturnValue));
$router->expects($this->any())
->method('generate')
->with('test_route')
->will($this->returnValue('/test_route'));

$request->server = $this->getMockBuilder('Symfony\Component\HttpFoundation\ParameterBag')
->disableOriginalConstructor()
->getMock();
$request->server->expects($this->any())
->method('get')
->will($this->returnValue($userAgent));

return $request;
return $router;
}

private function getCookieValueFromResponse($response)
Expand Down Expand Up @@ -128,14 +111,17 @@ public function testThemeAction($config, $assertion, $hasAlreadyACookie = true)
}
$response = new \Symfony\Component\HttpFoundation\Response();
$request = new \Symfony\Component\HttpFoundation\Request();

if ($hasAlreadyACookie) {
$request = $this->getMockRequest('cookie');
$request->query->set('theme', 'cookie');
$request->cookies->set('cookieName', 'cookie');
$request->server->set('HTTP_USER_AGENT', 'autodetect');
}

$router = $this->getRouterMock();

$controller = false;
if ($config['load_controllers']) {
$controller = new ThemeController($activeTheme, $config['themes'], $config['cookie']);
$controller = new ThemeController($activeTheme, $config['themes'], $config['cookie'], $router, $config['redirect_fallback']);
}

$listener = new ThemeRequestListener($activeTheme, $config['cookie'], $device);
Expand All @@ -145,10 +131,13 @@ public function testThemeAction($config, $assertion, $hasAlreadyACookie = true)
$this->assertEquals($activeTheme->getName(), $assertion['themeAfterKernelRequest']);

if ($controller) {
$request->query->set('theme', $assertion['themeAfterController']);

$response = $controller->switchAction(
$this->getMockRequest($assertion['themeAfterController'])
$request
);
$this->assertCookieValue($response, $assertion['themeAfterController']);
$this->assertEquals($response->getTargetUrl(), $assertion['redirect']);
}

// onResponse will not set the cookie if the controller modified it
Expand Down Expand Up @@ -178,6 +167,7 @@ public function dataProvider()
// all-in Controller wins over Cookie and Autodetection
array(
'themes' => array('default', 'controller', 'cookie', 'autodetect'),
'redirect_fallback' => 'test_route',
'active_theme' => 'default',
'autodetect_theme' => true,
'load_controllers' => true,
Expand All @@ -187,13 +177,15 @@ public function dataProvider()
'themeAfterKernelRequest' => 'cookie',
'themeAfterController' => 'controller',
'themeAfterKernelResponse' => 'controller',
'redirect' => '/test_route',
),
true,
),
// autodetect if no cookie exists, but at the end controller wins
array(
array(
'themes' => array('default', 'controller', 'cookie', 'autodetect'),
'redirect_fallback' => 'test_route',
'active_theme' => 'default',
'autodetect_theme' => true,
'load_controllers' => true,
Expand All @@ -203,13 +195,15 @@ public function dataProvider()
'themeAfterKernelRequest' => 'autodetect',
'themeAfterController' => 'controller',
'themeAfterKernelResponse' => 'controller',
'redirect' => '/test_route',
),
false,
),
// no cookie pre-esist, so set autodect value into cookie if the controller is not called
array(
array(
'themes' => array('default', 'controller', 'cookie', 'autodetect'),
'redirect_fallback' => 'test_route',
'active_theme' => 'default',
'autodetect_theme' => true,
'load_controllers' => false,
Expand All @@ -219,13 +213,15 @@ public function dataProvider()
'themeAfterKernelRequest' => 'autodetect',
'themeAfterController' => 'autodetect',
'themeAfterKernelResponse' => 'autodetect',
'redirect' => '/test_route',
),
false,
),
// a cookie don't pre-esist, autodetection is disabled, controller is not called, set the cookie
array(
array(
'themes' => array('default', 'controller', 'cookie', 'autodetect'),
'redirect_fallback' => 'test_route',
'active_theme' => 'default',
'autodetect_theme' => false,
'load_controllers' => true,
Expand All @@ -235,13 +231,15 @@ public function dataProvider()
'themeAfterKernelRequest' => 'default',
'themeAfterController' => 'controller',
'themeAfterKernelResponse' => 'controller',
'redirect' => '/test_route',
),
false,
),
// a cookie pre-esist, autodetection is disabled, controller is not called, set the cookie
array(
array(
'themes' => array('default', 'controller', 'cookie', 'autodetect'),
'redirect_fallback' => 'test_route',
'active_theme' => 'default',
'autodetect_theme' => false,
'load_controllers' => true,
Expand All @@ -251,10 +249,10 @@ public function dataProvider()
'themeAfterKernelRequest' => 'cookie',
'themeAfterController' => 'controller',
'themeAfterKernelResponse' => 'controller',
'redirect' => '/test_route',
),
true,
),

);
}
}