From 5431dc34bc515a460d787d628ffd6edacec6dc79 Mon Sep 17 00:00:00 2001 From: asdfzdfj Date: Wed, 15 May 2024 11:35:03 +0700 Subject: [PATCH] front controller path/routing adjustment (#687) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the new filter ui changes also introduces a couple more filtering parameters in the form of controller path params, for threads/entries listing it looks fine and relatively staightforward, but for microblog posts listing, what used to be a simple `/microblog` is now something like `/all/hot/∞/all/all/microblog`, which hinders the user experience on url readability and memorability compared to the previous scheme, for those who still care this changes tries to adjust the path/routing of these front controller especially for the microblog part, to make it more simpler and could omit some default filter parameters if they aren't set, similar to the previous path scheme Co-authored-by: e-five <146029455+e-five256@users.noreply.github.com> --- config/kbin_routes/domain.yaml | 5 +- config/kbin_routes/front.yaml | 77 ++++---- .../Domain/DomainFrontController.php | 11 +- src/Controller/Entry/EntryFrontController.php | 166 ++++++++++++------ src/Repository/Criteria.php | 23 +-- src/Twig/Extension/FrontExtension.php | 19 ++ src/Twig/Runtime/FrontExtensionRuntime.php | 64 +++++++ src/Twig/Runtime/UrlExtensionRuntime.php | 2 +- templates/domain/_options.html.twig | 2 +- templates/entry/_options.html.twig | 50 +++--- templates/layout/_header_nav.html.twig | 8 +- templates/post/_options.html.twig | 50 +++--- 12 files changed, 313 insertions(+), 164 deletions(-) create mode 100644 src/Twig/Extension/FrontExtension.php create mode 100644 src/Twig/Runtime/FrontExtensionRuntime.php diff --git a/config/kbin_routes/domain.yaml b/config/kbin_routes/domain.yaml index d209af86b..6c0631049 100644 --- a/config/kbin_routes/domain.yaml +++ b/config/kbin_routes/domain.yaml @@ -1,12 +1,11 @@ domain_entries: controller: App\Controller\Domain\DomainFrontController - defaults: { sortBy: hot, time: '∞', type: ~ } - path: /d/{name}/{sortBy}/{time}/{type} + defaults: { sortBy: hot, time: '∞'} + path: /d/{name}/{sortBy}/{time} methods: [ GET ] requirements: sortBy: "%default_sort_options%" time: "%default_time_options%" - type: "%default_type_options%" domain_comments: controller: App\Controller\Domain\DomainCommentFrontController diff --git a/config/kbin_routes/front.yaml b/config/kbin_routes/front.yaml index ef14c5b5f..01a28a07d 100644 --- a/config/kbin_routes/front.yaml +++ b/config/kbin_routes/front.yaml @@ -1,45 +1,55 @@ front: controller: App\Controller\Entry\EntryFrontController::front - defaults: { subscription: home, sortBy: hot, time: '∞', type: all, federation: all, content: threads } - path: /{subscription}/{sortBy}/{time}/{type}/{federation}/{content} + defaults: &front_defaults { subscription: home, content: threads, sortBy: hot, time: '∞', federation: all } + path: /{subscription}/{content}/{sortBy}/{time}/{federation} methods: [GET] - requirements: + requirements: &front_requirement subscription: "%default_subscription_options%" sortBy: "%default_sort_options%" time: "%default_time_options%" - type: "%default_type_options%" federation: "%default_federation_options%" content: "%default_content_options%" -front_redirect: - controller: App\Controller\Entry\EntryFrontController::front_redirect - defaults: { sortBy: hot, time: '∞', type: all, federation: all, content: threads } - path: /{sortBy}/{time}/{type}/{federation}/{content} +front_sub: + controller: App\Controller\Entry\EntryFrontController::front + defaults: *front_defaults + path: /{subscription}/{sortBy}/{time}/{federation} methods: [GET] - requirements: - sortBy: "%default_sort_options%" - time: "%default_time_options%" - type: "%default_type_options%" - federation: "%default_federation_options%" - content: "%default_content_options%" + requirements: *front_requirement + +front_content: + controller: App\Controller\Entry\EntryFrontController::front + defaults: *front_defaults + path: /{content}/{sortBy}/{time}/{federation} + methods: [GET] + requirements: *front_requirement + +front_short: + controller: App\Controller\Entry\EntryFrontController::front + defaults: *front_defaults + path: /{sortBy}/{time}/{federation} + methods: [GET] + requirements: *front_requirement front_magazine: controller: App\Controller\Entry\EntryFrontController::magazine - defaults: { sortBy: hot, time: '∞', type: all, federation: all, content: threads } - path: /m/{name}/{sortBy}/{time}/{type}/{federation}/{content} + defaults: &front_magazine_defaults { content: threads, sortBy: hot, time: '∞', federation: all } + path: /m/{name}/{content}/{sortBy}/{time}/{federation} methods: [GET] - requirements: - sortBy: "%default_sort_options%" - time: "%default_time_options%" - type: "%default_type_options%" - federation: "%default_federation_options%" - content: "%default_content_options%" + requirements: *front_requirement + +front_magazine_short: + controller: App\Controller\Entry\EntryFrontController::magazine + defaults: &front_magazine_defaults + path: /m/{name}/{sortBy}/{time}/{federation} + methods: [GET] + requirements: *front_requirement # Microblog compatibility stuff, redirects from the old routes' URLs posts_front: - controller: App\Controller\Entry\EntryFrontController::front_redirect - defaults: { sortBy: hot, time: '∞', type: all, federation: all, content: microblog } + controller: App\Controller\Entry\EntryFrontController::frontRedirect + defaults: { sortBy: hot, time: '∞', federation: all, content: microblog } path: /microblog/{sortBy}/{time} methods: [ GET ] requirements: @@ -47,8 +57,8 @@ posts_front: time: "%default_time_options%" posts_subscribed: - controller: App\Controller\Entry\EntryFrontController::front_redirect - defaults: { sortBy: hot, time: '∞', type: all, federation: all, content: microblog, subscription: 'sub' } + controller: App\Controller\Entry\EntryFrontController::frontRedirect + defaults: { sortBy: hot, time: '∞', federation: all, content: microblog, subscription: 'sub' } path: /sub/microblog/{sortBy}/{time} methods: [ GET ] requirements: @@ -56,8 +66,8 @@ posts_subscribed: time: "%default_time_options%" posts_moderated: - controller: App\Controller\Entry\EntryFrontController::front_redirect - defaults: { sortBy: hot, time: '∞', type: all, federation: all, content: microblog, subscription: 'mod' } + controller: App\Controller\Entry\EntryFrontController::frontRedirect + defaults: { sortBy: hot, time: '∞', federation: all, content: microblog, subscription: 'mod' } path: /mod/microblog/{sortBy}/{time} methods: [ GET ] requirements: @@ -65,8 +75,8 @@ posts_moderated: time: "%default_time_options%" posts_favourite: - controller: App\Controller\Entry\EntryFrontController::front_redirect - defaults: { sortBy: hot, time: '∞', type: all, federation: all, content: microblog, subscription: 'fav' } + controller: App\Controller\Entry\EntryFrontController::frontRedirect + defaults: { sortBy: hot, time: '∞', federation: all, content: microblog, subscription: 'fav' } path: /fav/microblog/{sortBy}/{time} methods: [ GET ] requirements: @@ -74,12 +84,11 @@ posts_favourite: time: "%default_time_options%" magazine_posts: - controller: App\Controller\Entry\EntryFrontController::front_redirect - defaults: { sortBy: hot, time: '∞', type: all, federation: all, content: microblog } - path: /m/{name}/microblog/{sortBy}/{time}/{type}/{federation} + controller: App\Controller\Entry\EntryFrontController::magazineRedirect + defaults: { sortBy: hot, time: '∞', federation: all, content: microblog } + path: /m/{name}/microblog/{sortBy}/{time}/{federation} methods: [ GET ] requirements: sortBy: "%default_sort_options%" time: "%default_time_options%" - type: "%default_type_options%" federation: "%default_federation_options%" diff --git a/src/Controller/Domain/DomainFrontController.php b/src/Controller/Domain/DomainFrontController.php index 54d2b5714..167f336eb 100644 --- a/src/Controller/Domain/DomainFrontController.php +++ b/src/Controller/Domain/DomainFrontController.php @@ -13,6 +13,7 @@ use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\Attribute\MapQueryParameter; class DomainFrontController extends AbstractController { @@ -22,8 +23,14 @@ public function __construct( ) { } - public function __invoke(?string $name, ?string $sortBy, ?string $time, ?string $type, Request $request): Response - { + public function __invoke( + ?string $name, + ?string $sortBy, + ?string $time, + #[MapQueryParameter] + ?string $type, + Request $request + ): Response { if (!$domain = $this->domainRepository->findOneBy(['name' => $name])) { throw $this->createNotFoundException(); } diff --git a/src/Controller/Entry/EntryFrontController.php b/src/Controller/Entry/EntryFrontController.php index 6e290e612..d9cf2112a 100644 --- a/src/Controller/Entry/EntryFrontController.php +++ b/src/Controller/Entry/EntryFrontController.php @@ -19,15 +19,26 @@ use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\Attribute\MapQueryParameter; class EntryFrontController extends AbstractController { - public function __construct(private readonly EntryRepository $entryRepository, private readonly PostRepository $postRepository) - { + public function __construct( + private readonly EntryRepository $entryRepository, + private readonly PostRepository $postRepository + ) { } - public function front(?string $sortBy, ?string $time, ?string $type, string $subscription, string $federation, string $content, Request $request): Response - { + public function front( + string $subscription, + string $content, + ?string $sortBy, + ?string $time, + string $federation, + #[MapQueryParameter] + ?string $type, + Request $request + ): Response { $user = $this->getUser(); $criteria = $this->createCriteria($content, $request); @@ -39,57 +50,64 @@ public function front(?string $sortBy, ?string $time, ?string $type, string $sub if ('home' === $subscription) { $subscription = $this->subscriptionFor($user); } - $this->handleSubscription($subscription, $user, $criteria); + $this->handleSubscription($subscription, $criteria); $this->setUserPreferences($user, $criteria); - $entities = ('threads' === $content) ? $this->entryRepository->findByCriteria($criteria) : $this->postRepository->findByCriteria($criteria); if ('threads' === $content) { + $entities = $this->entryRepository->findByCriteria($criteria); $entities = $this->handleCrossposts($entities); + $templatePath = 'entry/'; + $dataKey = 'entries'; + } elseif ('microblog' === $content) { + $entities = $this->postRepository->findByCriteria($criteria); + $templatePath = 'post/'; + $dataKey = 'posts'; + } else { + throw new \LogicException("Invalid content filter '{$content}'"); } - $templatePath = ('threads' === $content) ? 'entry/' : 'post/'; - $dataKey = ('threads' === $content) ? 'entries' : 'posts'; - - return $this->renderResponse($request, $content, $criteria, [$dataKey => $entities], $templatePath, $user); + return $this->renderResponse( + $request, + $content, + $criteria, + [$dataKey => $entities], + $templatePath, + $user + ); } - // $name is magazine name, for compatibility - public function front_redirect(?string $sortBy, ?string $time, ?string $type, string $federation, string $content, ?string $name, Request $request): Response - { - $user = $this->getUser(); // Fetch the user - $subscription = $this->subscriptionFor($user); // Determine the subscription filter based on the user - - if ($name) { - return $this->redirectToRoute('front_magazine', [ - 'name' => $name, - 'subscription' => $subscription, - 'sortBy' => $sortBy, - 'time' => $time, - 'type' => $type, - 'federation' => $federation, - 'content' => $content, - ]); - } else { - return $this->redirectToRoute('front', [ - 'subscription' => $subscription, - 'sortBy' => $sortBy, - 'time' => $time, - 'type' => $type, - 'federation' => $federation, - 'content' => $content, - ]); - } + public function frontRedirect( + string $content, + ?string $sortBy, + ?string $time, + string $federation, + #[MapQueryParameter] + ?string $type, + Request $request + ): Response { + $user = $this->getUser(); + $subscription = $this->subscriptionFor($user); + + return $this->redirectToRoute('front', [ + 'subscription' => $subscription, + 'sortBy' => $sortBy, + 'time' => $time, + 'type' => $type, + 'federation' => $federation, + 'content' => $content, + ]); } public function magazine( #[MapEntity(expr: 'repository.findOneByName(name)')] Magazine $magazine, + string $content, ?string $sortBy, ?string $time, - ?string $type, string $federation, - string $content, + #[MapQueryParameter] + ?string $type, Request $request ): Response { $user = $this->getUser(); @@ -106,21 +124,59 @@ public function magazine( $criteria->magazine = $magazine; $criteria->stickiesFirst = true; - $subscription = $request->query->get('subscription'); - if (!$subscription) { - $subscription = 'all'; - } - $this->handleSubscription($subscription, $user, $criteria); + $subscription = $request->query->get('subscription') ?: 'all'; + $this->handleSubscription($subscription, $criteria); $this->setUserPreferences($user, $criteria); - $entities = ('threads' === $content) ? $this->entryRepository->findByCriteria($criteria) : $this->postRepository->findByCriteria($criteria); - // Note no crosspost handling + if ('threads' === $content) { + $entities = $this->entryRepository->findByCriteria($criteria); + // Note no crosspost handling + $templatePath = 'entry/'; + $dataKey = 'entries'; + } elseif ('microblog' === $content) { + $entities = $this->postRepository->findByCriteria($criteria); + $templatePath = 'post/'; + $dataKey = 'posts'; + } else { + throw new \LogicException("Invalid content filter '{$content}'"); + } + + return $this->renderResponse( + $request, + $content, + $criteria, + [$dataKey => $entities, 'magazine' => $magazine], + $templatePath, + $user + ); + } - $templatePath = ('threads' === $content) ? 'entry/' : 'post/'; - $dataKey = ('threads' === $content) ? 'entries' : 'posts'; + /** + * @param string $name magazine name + */ + public function magazineRedirect( + string $name, + string $content, + ?string $sortBy, + ?string $time, + string $federation, + #[MapQueryParameter] + ?string $type, + Request $request + ): Response { + $user = $this->getUser(); // Fetch the user + $subscription = $this->subscriptionFor($user); // Determine the subscription filter based on the user - return $this->renderResponse($request, $content, $criteria, [$dataKey => $entities, 'magazine' => $magazine], $templatePath, $user); + return $this->redirectToRoute('front_magazine', [ + 'name' => $name, + 'subscription' => $subscription, + 'sortBy' => $sortBy, + 'time' => $time, + 'type' => $type, + 'federation' => $federation, + 'content' => $content, + ]); } private function createCriteria(string $content, Request $request) @@ -136,19 +192,18 @@ private function createCriteria(string $content, Request $request) return $criteria->setContent($content); } - private function handleSubscription(string $subscription, $user, &$criteria) + private function handleSubscription(string $subscription, &$criteria) { - if ('sub' === $subscription) { + if (\in_array($subscription, ['sub', 'mod', 'fav'])) { $this->denyAccessUnlessGranted('ROLE_USER'); $this->getUserOrThrow(); + } + + if ('sub' === $subscription) { $criteria->subscribed = true; } elseif ('mod' === $subscription) { - $this->denyAccessUnlessGranted('ROLE_USER'); - $this->getUserOrThrow(); $criteria->moderated = true; } elseif ('fav' === $subscription) { - $this->denyAccessUnlessGranted('ROLE_USER'); - $this->getUserOrThrow(); $criteria->favourite = true; } elseif ($subscription && 'all' !== $subscription) { throw new \LogicException('Invalid subscription filter '.$subscription); @@ -164,7 +219,8 @@ private function setUserPreferences(?User $user, &$criteria) private function renderResponse(Request $request, $content, $criteria, $data, $templatePath, ?User $user) { - $baseData = ['criteria' => $criteria] + $data; + $baseData = array_merge(['criteria' => $criteria], $data); + if ('microblog' === $content) { $dto = new PostDto(); if (isset($data['magazine'])) { diff --git a/src/Repository/Criteria.php b/src/Repository/Criteria.php index c9edd8a94..747eda64c 100644 --- a/src/Repository/Criteria.php +++ b/src/Repository/Criteria.php @@ -230,22 +230,13 @@ public function resolveTime(?string $value, bool $reverse = false): ?string public function resolveType(?string $value): ?string { - // @todo - $routes = [ - 'all' => 'all', - 'article' => Entry::ENTRY_TYPE_ARTICLE, - 'articles' => Entry::ENTRY_TYPE_ARTICLE, - 'link' => Entry::ENTRY_TYPE_LINK, - 'links' => Entry::ENTRY_TYPE_LINK, - 'video' => Entry::ENTRY_TYPE_VIDEO, - 'videos' => Entry::ENTRY_TYPE_VIDEO, - 'photo' => Entry::ENTRY_TYPE_IMAGE, - 'photos' => Entry::ENTRY_TYPE_IMAGE, - 'image' => Entry::ENTRY_TYPE_IMAGE, - 'images' => Entry::ENTRY_TYPE_IMAGE, - ]; - - return $routes[$value] ?? 'all'; + return match ($value) { + 'article', 'articles' => Entry::ENTRY_TYPE_ARTICLE, + 'link', 'links' => Entry::ENTRY_TYPE_LINK, + 'video', 'videos' => Entry::ENTRY_TYPE_VIDEO, + 'photo', 'photos', 'image', 'images' => Entry::ENTRY_TYPE_IMAGE, + default => 'all' + }; } public function translateType(): string diff --git a/src/Twig/Extension/FrontExtension.php b/src/Twig/Extension/FrontExtension.php new file mode 100644 index 000000000..04554e3f9 --- /dev/null +++ b/src/Twig/Extension/FrontExtension.php @@ -0,0 +1,19 @@ +requestStack->getCurrentRequest(); + $attrs = $request->attributes; + $route = $routeName ?? $attrs->get('_route'); + + $params = array_merge($attrs->get('_route_params', []), $request->query->all()); + $params = array_replace($params, $additionalParams); + $params = array_filter($params, fn ($v) => null !== $v); + + $params[$name] = $value; + + if (str_starts_with($route, 'front') && !str_contains($route, '_magazine')) { + $route = $this->getFrontRoute($route, $params); + } + + return $this->urlGenerator->generate($route, $params); + } + + /** + * Upgrades shorter `front_*` routes to a front route that can fit all specified params. + */ + private function getFrontRoute(string $currentRoute, array $params): string + { + $content = $params['content'] ?? null; + $subscription = $params['subscription'] ?? null; + + if (\in_array($currentRoute, ['front_sub', 'front_content']) && $content && $subscription) { + return 'front'; + } elseif ('front_short' === $currentRoute) { + return match (true) { + !empty($content) => 'front_content', + !empty($subscription) => 'front_sub', + default => 'front', + }; + } + + return 'front'; + } +} diff --git a/src/Twig/Runtime/UrlExtensionRuntime.php b/src/Twig/Runtime/UrlExtensionRuntime.php index 54a13661c..46d19859b 100644 --- a/src/Twig/Runtime/UrlExtensionRuntime.php +++ b/src/Twig/Runtime/UrlExtensionRuntime.php @@ -265,7 +265,7 @@ public function postCommentDeleteUrl(PostComment $comment): string // $additionalParams indicates extra parameters to set in addition to [$name] = $value // Set $value to null to indicate deleting a parameter // TODO: It'd be better to have just a single $params which is an associative array - public function optionsUrl(string $name, string $value, string $routeName = null, array $additionalParams = []): string + public function optionsUrl(string $name, ?string $value, string $routeName = null, array $additionalParams = []): string { $route = $routeName ?? $this->requestStack->getCurrentRequest()->attributes->get('_route'); $params = $this->requestStack->getCurrentRequest()->attributes->get('_route_params', []); diff --git a/templates/domain/_options.html.twig b/templates/domain/_options.html.twig index 65bd7c990..c50c3326c 100644 --- a/templates/domain/_options.html.twig +++ b/templates/domain/_options.html.twig @@ -124,7 +124,7 @@