From af497a641c2a46c99bbc67215e041a46c91695bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20L=C5=93uillet?= Date: Fri, 15 Apr 2016 07:58:01 +0200 Subject: Redirect to homepage if referer is null Fix #1924 --- .../CoreBundle/Controller/EntryController.php | 14 +++++-- .../CoreBundle/Controller/TagController.php | 4 +- src/Wallabag/CoreBundle/Helper/Redirect.php | 36 ++++++++++++++++ .../CoreBundle/Resources/config/services.yml | 5 +++ .../CoreBundle/Tests/Helper/RedirectTest.php | 49 ++++++++++++++++++++++ 5 files changed, 104 insertions(+), 4 deletions(-) create mode 100644 src/Wallabag/CoreBundle/Helper/Redirect.php create mode 100644 src/Wallabag/CoreBundle/Tests/Helper/RedirectTest.php (limited to 'src') diff --git a/src/Wallabag/CoreBundle/Controller/EntryController.php b/src/Wallabag/CoreBundle/Controller/EntryController.php index 17b72bd1..9443ae82 100644 --- a/src/Wallabag/CoreBundle/Controller/EntryController.php +++ b/src/Wallabag/CoreBundle/Controller/EntryController.php @@ -344,7 +344,9 @@ class EntryController extends Controller $message ); - return $this->redirect($request->headers->get('referer')); + $redirectUrl = $this->get('wallabag_core.helper.redirect')->to($request->headers->get('referer')); + + return $this->redirect($redirectUrl); } /** @@ -374,7 +376,9 @@ class EntryController extends Controller $message ); - return $this->redirect($request->headers->get('referer')); + $redirectUrl = $this->get('wallabag_core.helper.redirect')->to($request->headers->get('referer')); + + return $this->redirect($redirectUrl); } /** @@ -408,7 +412,11 @@ class EntryController extends Controller ); // don't redirect user to the deleted entry - return $this->redirect($url !== $request->headers->get('referer') ? $request->headers->get('referer') : $this->generateUrl('homepage')); + $to = ($url !== $request->headers->get('referer') ? $request->headers->get('referer') : $this->generateUrl('homepage')); + + $redirectUrl = $this->get('wallabag_core.helper.redirect')->to($to); + + return $this->redirect($redirectUrl); } /** diff --git a/src/Wallabag/CoreBundle/Controller/TagController.php b/src/Wallabag/CoreBundle/Controller/TagController.php index e8e9ecbe..16d14d79 100644 --- a/src/Wallabag/CoreBundle/Controller/TagController.php +++ b/src/Wallabag/CoreBundle/Controller/TagController.php @@ -65,7 +65,9 @@ class TagController extends Controller } $em->flush(); - return $this->redirect($request->headers->get('referer')); + $redirectUrl = $this->get('wallabag_core.helper.redirect')->to($request->headers->get('referer')); + + return $this->redirect($redirectUrl); } /** diff --git a/src/Wallabag/CoreBundle/Helper/Redirect.php b/src/Wallabag/CoreBundle/Helper/Redirect.php new file mode 100644 index 00000000..0921c3f9 --- /dev/null +++ b/src/Wallabag/CoreBundle/Helper/Redirect.php @@ -0,0 +1,36 @@ +router = $router; + } + + /** + * @param string $url URL to redirect + * @param string $fallback Fallback URL if $url is null + * + * @return string + */ + public function to($url, $fallback = '') + { + $returnUrl = $url; + + if (null === $url) { + if ('' !== $fallback) { + $returnUrl = $fallback; + } else { + $returnUrl = $this->router->generate('homepage'); + } + } + + return $returnUrl; + } +} diff --git a/src/Wallabag/CoreBundle/Resources/config/services.yml b/src/Wallabag/CoreBundle/Resources/config/services.yml index 6dc1f1d7..f8835198 100644 --- a/src/Wallabag/CoreBundle/Resources/config/services.yml +++ b/src/Wallabag/CoreBundle/Resources/config/services.yml @@ -114,3 +114,8 @@ services: class: Wallabag\CoreBundle\Operator\Doctrine\Matches tags: - { name: rulerz.operator, executor: rulerz.executor.doctrine, operator: matches, inline: true } + + wallabag_core.helper.redirect: + class: Wallabag\CoreBundle\Helper\Redirect + arguments: + - "@router" diff --git a/src/Wallabag/CoreBundle/Tests/Helper/RedirectTest.php b/src/Wallabag/CoreBundle/Tests/Helper/RedirectTest.php new file mode 100644 index 00000000..da19cf58 --- /dev/null +++ b/src/Wallabag/CoreBundle/Tests/Helper/RedirectTest.php @@ -0,0 +1,49 @@ +routerMock = $this->getRouterMock(); + $this->redirect = new Redirect($this->routerMock); + } + + public function testRedirectToNullWithFallback() + { + $redirectUrl = $this->redirect->to(null, 'fallback'); + + $this->assertEquals('fallback', $redirectUrl); + } + + public function testRedirectToNullWithoutFallback() + { + $redirectUrl = $this->redirect->to(null); + + $this->assertEquals($this->routerMock->generate('homepage'), $redirectUrl); + } + + public function testRedirectToValidUrl() + { + $redirectUrl = $this->redirect->to('/unread/list'); + + $this->assertEquals('/unread/list', $redirectUrl); + } + + private function getRouterMock() + { + return $this->getMockBuilder('Symfony\Component\Routing\Router') + ->setMethods(['generate']) + ->disableOriginalConstructor() + ->getMock(); + } +} -- cgit v1.2.3 From 4086e0782e4545bd3572f929b4476b200588f6c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20L=C5=93uillet?= Date: Fri, 15 Apr 2016 09:58:29 +0200 Subject: Fix tests --- src/Wallabag/CoreBundle/Helper/Redirect.php | 17 +++++++++-------- src/Wallabag/CoreBundle/Tests/Helper/RedirectTest.php | 12 +++++++++--- 2 files changed, 18 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/Wallabag/CoreBundle/Helper/Redirect.php b/src/Wallabag/CoreBundle/Helper/Redirect.php index 0921c3f9..c14c79d1 100644 --- a/src/Wallabag/CoreBundle/Helper/Redirect.php +++ b/src/Wallabag/CoreBundle/Helper/Redirect.php @@ -4,6 +4,9 @@ namespace Wallabag\CoreBundle\Helper; use Symfony\Component\Routing\Router; +/** + * Manage redirections to avoid redirecting to empty routes. + */ class Redirect { private $router; @@ -21,16 +24,14 @@ class Redirect */ public function to($url, $fallback = '') { - $returnUrl = $url; + if (null !== $url) { + return $url; + } - if (null === $url) { - if ('' !== $fallback) { - $returnUrl = $fallback; - } else { - $returnUrl = $this->router->generate('homepage'); - } + if ('' === $fallback) { + return $this->router->generate('homepage'); } - return $returnUrl; + return $fallback; } } diff --git a/src/Wallabag/CoreBundle/Tests/Helper/RedirectTest.php b/src/Wallabag/CoreBundle/Tests/Helper/RedirectTest.php index da19cf58..f4aecc80 100644 --- a/src/Wallabag/CoreBundle/Tests/Helper/RedirectTest.php +++ b/src/Wallabag/CoreBundle/Tests/Helper/RedirectTest.php @@ -6,7 +6,7 @@ use Wallabag\CoreBundle\Helper\Redirect; class RedirectTest extends \PHPUnit_Framework_TestCase { - /** @var \Symfony\Component\Routing\Router */ + /** @var \PHPUnit_Framework_MockObject_MockObject */ private $routerMock; /** @var Redirect */ @@ -41,9 +41,15 @@ class RedirectTest extends \PHPUnit_Framework_TestCase private function getRouterMock() { - return $this->getMockBuilder('Symfony\Component\Routing\Router') - ->setMethods(['generate']) + $mock = $this->getMockBuilder('Symfony\Component\Routing\Router') ->disableOriginalConstructor() ->getMock(); + + $mock->expects($this->any()) + ->method('generate') + ->with('homepage') + ->willReturn('homepage'); + + return $mock; } } -- cgit v1.2.3 From 345d74268b2d3d232b2de02f30e950d032a2e7b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20L=C5=93uillet?= Date: Fri, 15 Apr 2016 15:39:00 +0200 Subject: Fix redirect when delete entry --- src/Wallabag/CoreBundle/Controller/EntryController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/Wallabag/CoreBundle/Controller/EntryController.php b/src/Wallabag/CoreBundle/Controller/EntryController.php index 9443ae82..69dfd4b1 100644 --- a/src/Wallabag/CoreBundle/Controller/EntryController.php +++ b/src/Wallabag/CoreBundle/Controller/EntryController.php @@ -412,7 +412,7 @@ class EntryController extends Controller ); // don't redirect user to the deleted entry - $to = ($url !== $request->headers->get('referer') ? $request->headers->get('referer') : $this->generateUrl('homepage')); + $to = ($url !== $request->headers->get('referer') ? $request->headers->get('referer') : null); $redirectUrl = $this->get('wallabag_core.helper.redirect')->to($to); -- cgit v1.2.3