From 7ebc96f3b90e36f45c4530a8561a4bb1be570b6c Mon Sep 17 00:00:00 2001 From: Yassine Guedidi Date: Mon, 25 Dec 2023 21:42:08 +0100 Subject: [PATCH] Remove session-based redirection --- .../CoreBundle/Controller/ConfigController.php | 2 +- .../CoreBundle/Controller/EntryController.php | 14 +++++--------- .../CoreBundle/Controller/TagController.php | 8 ++++---- .../views/Entry/_card_actions.html.twig | 10 ++++++---- .../Resources/views/Entry/_card_list.html.twig | 11 +++++++---- .../Resources/views/Entry/_tags.html.twig | 3 ++- .../Resources/views/Entry/entries.html.twig | 7 ++++--- .../Resources/views/Entry/entry.html.twig | 18 ++++++++++-------- .../Resources/views/Tag/tags.html.twig | 6 ++++-- .../Controller/EntryControllerTest.php | 8 +++++--- .../Controller/TagControllerTest.php | 6 ++++-- 11 files changed, 52 insertions(+), 41 deletions(-) diff --git a/src/Wallabag/CoreBundle/Controller/ConfigController.php b/src/Wallabag/CoreBundle/Controller/ConfigController.php index 94336b82c..4709a4d2e 100644 --- a/src/Wallabag/CoreBundle/Controller/ConfigController.php +++ b/src/Wallabag/CoreBundle/Controller/ConfigController.php @@ -649,7 +649,7 @@ class ConfigController extends AbstractController $this->entityManager->persist($user); $this->entityManager->flush(); - $redirectUrl = $this->redirectHelper->to($request->getSession()->get('prevUrl')); + $redirectUrl = $this->redirectHelper->to($request->query->get('redirect')); return $this->redirect($redirectUrl); } diff --git a/src/Wallabag/CoreBundle/Controller/EntryController.php b/src/Wallabag/CoreBundle/Controller/EntryController.php index ca9ff56f3..9322860a6 100644 --- a/src/Wallabag/CoreBundle/Controller/EntryController.php +++ b/src/Wallabag/CoreBundle/Controller/EntryController.php @@ -15,7 +15,6 @@ use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Annotation\Route; -use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Contracts\Translation\TranslatorInterface; use Wallabag\CoreBundle\Entity\Entry; use Wallabag\CoreBundle\Entity\Tag; @@ -128,7 +127,7 @@ class EntryController extends AbstractController $this->entityManager->flush(); } - $redirectUrl = $this->redirectHelper->to($request->getSession()->get('prevUrl')); + $redirectUrl = $this->redirectHelper->to($request->query->get('redirect')); return $this->redirect($redirectUrl); } @@ -390,7 +389,6 @@ class EntryController extends AbstractController public function viewAction(Entry $entry) { $this->checkUserAction($entry); - $this->get('session')->set('prevUrl', $this->generateUrl('view', ['id' => $entry->getId()])); return $this->render( '@WallabagCore/Entry/entry.html.twig', @@ -452,7 +450,7 @@ class EntryController extends AbstractController $message ); - $redirectUrl = $this->redirectHelper->to($request->getSession()->get('prevUrl')); + $redirectUrl = $this->redirectHelper->to($request->query->get('redirect')); return $this->redirect($redirectUrl); } @@ -482,7 +480,7 @@ class EntryController extends AbstractController $message ); - $redirectUrl = $this->redirectHelper->to($request->getSession()->get('prevUrl')); + $redirectUrl = $this->redirectHelper->to($request->query->get('redirect')); return $this->redirect($redirectUrl); } @@ -502,8 +500,7 @@ class EntryController extends AbstractController // to avoid redirecting to the deleted entry. Ugh. $url = $this->generateUrl( 'view', - ['id' => $entry->getId()], - UrlGeneratorInterface::ABSOLUTE_PATH + ['id' => $entry->getId()] ); // entry deleted, dispatch event about it! @@ -518,7 +515,7 @@ class EntryController extends AbstractController ); // don't redirect user to the deleted entry (check that the referer doesn't end with the same url) - $prev = $request->getSession()->get('prevUrl'); + $prev = $request->query->get('redirect', ''); $to = (1 !== preg_match('#' . $url . '$#i', $prev) ? $prev : null); $redirectUrl = $this->redirectHelper->to($to); @@ -617,7 +614,6 @@ class EntryController extends AbstractController { $searchTerm = (isset($request->get('search_entry')['term']) ? $request->get('search_entry')['term'] : ''); $currentRoute = (null !== $request->query->get('currentRoute') ? $request->query->get('currentRoute') : ''); - $request->getSession()->set('prevUrl', $request->getRequestUri()); $formOptions = []; diff --git a/src/Wallabag/CoreBundle/Controller/TagController.php b/src/Wallabag/CoreBundle/Controller/TagController.php index 32db1ef0f..804512563 100644 --- a/src/Wallabag/CoreBundle/Controller/TagController.php +++ b/src/Wallabag/CoreBundle/Controller/TagController.php @@ -104,7 +104,7 @@ class TagController extends AbstractController $this->entityManager->flush(); } - $redirectUrl = $this->redirectHelper->to($request->getSession()->get('prevUrl'), true); + $redirectUrl = $this->redirectHelper->to($request->query->get('redirect'), true); return $this->redirect($redirectUrl); } @@ -185,7 +185,7 @@ class TagController extends AbstractController $form = $this->createForm(RenameTagType::class, new Tag()); $form->handleRequest($request); - $redirectUrl = $this->redirectHelper->to($request->getSession()->get('prevUrl'), true); + $redirectUrl = $this->redirectHelper->to($request->query->get('redirect'), true); if ($form->isSubmitted() && $form->isValid()) { $newTag = new Tag(); @@ -257,7 +257,7 @@ class TagController extends AbstractController $this->entityManager->flush(); } - return $this->redirect($this->redirectHelper->to($request->getSession()->get('prevUrl'), true)); + return $this->redirect($this->redirectHelper->to($request->query->get('redirect'), true)); } /** @@ -279,7 +279,7 @@ class TagController extends AbstractController $this->entityManager->remove($tag); $this->entityManager->flush(); } - $redirectUrl = $this->redirectHelper->to($request->getSession()->get('prevUrl'), true); + $redirectUrl = $this->redirectHelper->to($request->query->get('redirect'), true); return $this->redirect($redirectUrl); } diff --git a/src/Wallabag/CoreBundle/Resources/views/Entry/_card_actions.html.twig b/src/Wallabag/CoreBundle/Resources/views/Entry/_card_actions.html.twig index 26e78215b..d77a48ec0 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Entry/_card_actions.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Entry/_card_actions.html.twig @@ -7,18 +7,20 @@ + {% set current_path = path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')) %} + diff --git a/src/Wallabag/CoreBundle/Resources/views/Entry/_card_list.html.twig b/src/Wallabag/CoreBundle/Resources/views/Entry/_card_list.html.twig index 78e97ea37..4e112e774 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Entry/_card_list.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Entry/_card_list.html.twig @@ -9,12 +9,15 @@ {% endif %} {% include "@WallabagCore/Entry/Card/_content.html.twig" with {'entry': entry, 'withMetadata': true, 'subClass': 'metadata'} only %} + + {% set current_path = path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')) %} + diff --git a/src/Wallabag/CoreBundle/Resources/views/Entry/_tags.html.twig b/src/Wallabag/CoreBundle/Resources/views/Entry/_tags.html.twig index b6c84d959..2ab67e1c8 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Entry/_tags.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Entry/_tags.html.twig @@ -4,7 +4,8 @@
  • {{ tag.label }} {% if withRemove is defined and withRemove == true %} - + {% set current_path = path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')) %} + delete {% endif %} diff --git a/src/Wallabag/CoreBundle/Resources/views/Entry/entries.html.twig b/src/Wallabag/CoreBundle/Resources/views/Entry/entries.html.twig index 95023e714..e01d614ee 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Entry/entries.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Entry/entries.html.twig @@ -19,18 +19,19 @@ {% endblock %} {% block content %} + {% set current_path = path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')) %} {% set list_mode = app.user.config.listMode %} {% set entries_with_archived_class_routes = ['tag_entries', 'search', 'all'] %} {% set current_route = app.request.attributes.get('_route') %} {% if current_route == 'homepage' %} {% set current_route = 'unread' %} {% endif %} -
    +
    {{ 'entry.list.number_on_the_page'|trans({'%count%': entries.count}) }} {% if entries.count > 0 %} - {% if list_mode == 0 %}view_list{% else %}view_module{% endif %} + {% if list_mode == 0 %}view_list{% else %}view_module{% endif %} {% endif %} {% if entries.count > 0 %} @@ -39,7 +40,7 @@ {% include "@WallabagCore/Entry/_feed_link.html.twig" %} {% endif %}
    - {% if current_route == 'search' %}
    {{ 'entry.list.assign_search_tag'|trans }}
    {% endif %} + {% if current_route == 'search' %}
    {{ 'entry.list.assign_search_tag'|trans }}
    {% endif %} {% if entries.getNbPages > 1 %} {{ pagerfanta(entries, 'default_wallabag') }} {% endif %} diff --git a/src/Wallabag/CoreBundle/Resources/views/Entry/entry.html.twig b/src/Wallabag/CoreBundle/Resources/views/Entry/entry.html.twig index 7ae98bf94..9ce783bbf 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Entry/entry.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Entry/entry.html.twig @@ -4,6 +4,8 @@ {% block body_class %}entry{% endblock %} +{% set current_path = path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')) %} + {% block menu %}
    @@ -24,12 +26,12 @@
    diff --git a/src/Wallabag/CoreBundle/Resources/views/Tag/tags.html.twig b/src/Wallabag/CoreBundle/Resources/views/Tag/tags.html.twig index f951fcd65..bff22436e 100644 --- a/src/Wallabag/CoreBundle/Resources/views/Tag/tags.html.twig +++ b/src/Wallabag/CoreBundle/Resources/views/Tag/tags.html.twig @@ -3,6 +3,8 @@ {% block title %}{{ 'tag.page_title'|trans }}{% endblock %} {% block content %} + {% set current_path = path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')) %} +
    {{ 'tag.list.number_on_the_page'|trans({'%count%': tags|length}) }}
    @@ -18,7 +20,7 @@ {{ tag.label }} ({{ tag.nbEntries }}) {% if renameForms is defined and renameForms[tag.id] is defined %} - + {{ form_widget(renameForms[tag.id].label, {'attr': {'value': tag.label}}) }} {{ form_rest(renameForms[tag.id]) }}
    @@ -26,7 +28,7 @@ mode_edit {% endif %} - + delete {% if app.user.config.feedToken %} diff --git a/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php b/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php index a6e0f395b..db5555cda 100644 --- a/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php +++ b/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php @@ -1302,8 +1302,10 @@ class EntryControllerTest extends WallabagCoreTestCase $this->getEntityManager()->flush(); - $client->request('GET', '/view/' . $entry->getId()); - $client->request('GET', '/archive/' . $entry->getId()); + $crawler = $client->request('GET', '/view/' . $entry->getId()); + + $link = $crawler->filter('a[id="markAsRead"]')->link(); + $client->click($link); $this->assertSame(302, $client->getResponse()->getStatusCode()); $this->assertStringContainsString('/view/' . $entry->getId(), $client->getResponse()->headers->get('location')); @@ -1656,7 +1658,7 @@ class EntryControllerTest extends WallabagCoreTestCase // the deletion link of the first tag $link = $crawler->filter('body div#article div.tools ul.tags li.chip a')->extract(['href'])[1]; - $this->assertSame(sprintf('/remove-tag/%s/%s', $entry->getId(), $tag->getId()), $link); + $this->assertStringStartsWith(sprintf('/remove-tag/%s/%s', $entry->getId(), $tag->getId()), $link); } public function testRandom() diff --git a/tests/Wallabag/CoreBundle/Controller/TagControllerTest.php b/tests/Wallabag/CoreBundle/Controller/TagControllerTest.php index fe21693a2..9c5f61fd0 100644 --- a/tests/Wallabag/CoreBundle/Controller/TagControllerTest.php +++ b/tests/Wallabag/CoreBundle/Controller/TagControllerTest.php @@ -123,9 +123,11 @@ class TagControllerTest extends WallabagCoreTestCase $this->getEntityManager()->clear(); // We make a first request to set an history and test redirection after tag deletion - $client->request('GET', '/view/' . $entry->getId()); + $crawler = $client->request('GET', '/view/' . $entry->getId()); $entryUri = $client->getRequest()->getRequestUri(); - $client->request('GET', '/remove-tag/' . $entry->getId() . '/' . $tag->getId()); + + $link = $crawler->filter('a[href^="/remove-tag/' . $entry->getId() . '/' . $tag->getId() . '"]')->link(); + $client->click($link); $this->assertSame(302, $client->getResponse()->getStatusCode()); $this->assertSame($entryUri, $client->getResponse()->getTargetUrl());