From b381d6681c96f847919b66b49ea44569746b0b21 Mon Sep 17 00:00:00 2001 From: asdfzdfj Date: Mon, 8 Apr 2024 16:28:20 +0700 Subject: [PATCH] WIP: front controller path/routing adjustment 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 note that this patch main focus is just the path routing, there's probably more to adjust/polish in front controller and related areas but those would likely be a separate follow up patch --- config/kbin_routes/front.yaml | 63 ++++--- src/Controller/Entry/EntryFrontController.php | 157 ++++++++++++------ src/Repository/Criteria.php | 23 +-- src/Twig/Extension/FrontExtension.php | 19 +++ src/Twig/Runtime/FrontExtensionRuntime.php | 64 +++++++ templates/entry/_options.html.twig | 50 +++--- templates/layout/_header_nav.html.twig | 4 + templates/post/_options.html.twig | 50 +++--- 8 files changed, 284 insertions(+), 146 deletions(-) create mode 100644 src/Twig/Extension/FrontExtension.php create mode 100644 src/Twig/Runtime/FrontExtensionRuntime.php diff --git a/config/kbin_routes/front.yaml b/config/kbin_routes/front.yaml index ef14c5b5f4..90f0776863 100644 --- a/config/kbin_routes/front.yaml +++ b/config/kbin_routes/front.yaml @@ -1,9 +1,9 @@ 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: '∞', type: all, federation: all } + path: /{subscription}/{content}/{sortBy}/{time}/{type}/{federation} methods: [GET] - requirements: + requirements: &front_requirement subscription: "%default_subscription_options%" sortBy: "%default_sort_options%" time: "%default_time_options%" @@ -11,34 +11,45 @@ front: 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}/{type}/{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}/{type}/{federation} + methods: [GET] + requirements: *front_requirement + +front_short: + controller: App\Controller\Entry\EntryFrontController::front + defaults: *front_defaults + path: /{sortBy}/{time}/{type}/{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: '∞', type: all, federation: all } + path: /m/{name}/{content}/{sortBy}/{time}/{type}/{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}/{type}/{federation} + methods: [GET] + requirements: *front_requirement # Microblog compatibility stuff, redirects from the old routes' URLs posts_front: - controller: App\Controller\Entry\EntryFrontController::front_redirect + controller: App\Controller\Entry\EntryFrontController::frontRedirect defaults: { sortBy: hot, time: '∞', type: all, federation: all, content: microblog } path: /microblog/{sortBy}/{time} methods: [ GET ] @@ -47,7 +58,7 @@ posts_front: time: "%default_time_options%" posts_subscribed: - controller: App\Controller\Entry\EntryFrontController::front_redirect + controller: App\Controller\Entry\EntryFrontController::frontRedirect defaults: { sortBy: hot, time: '∞', type: all, federation: all, content: microblog, subscription: 'sub' } path: /sub/microblog/{sortBy}/{time} methods: [ GET ] @@ -56,7 +67,7 @@ posts_subscribed: time: "%default_time_options%" posts_moderated: - controller: App\Controller\Entry\EntryFrontController::front_redirect + controller: App\Controller\Entry\EntryFrontController::frontRedirect defaults: { sortBy: hot, time: '∞', type: all, federation: all, content: microblog, subscription: 'mod' } path: /mod/microblog/{sortBy}/{time} methods: [ GET ] @@ -65,7 +76,7 @@ posts_moderated: time: "%default_time_options%" posts_favourite: - controller: App\Controller\Entry\EntryFrontController::front_redirect + controller: App\Controller\Entry\EntryFrontController::frontRedirect defaults: { sortBy: hot, time: '∞', type: all, federation: all, content: microblog, subscription: 'fav' } path: /fav/microblog/{sortBy}/{time} methods: [ GET ] @@ -74,7 +85,7 @@ posts_favourite: time: "%default_time_options%" magazine_posts: - controller: App\Controller\Entry\EntryFrontController::front_redirect + controller: App\Controller\Entry\EntryFrontController::magazineRedirect defaults: { sortBy: hot, time: '∞', type: all, federation: all, content: microblog } path: /m/{name}/microblog/{sortBy}/{time}/{type}/{federation} methods: [ GET ] diff --git a/src/Controller/Entry/EntryFrontController.php b/src/Controller/Entry/EntryFrontController.php index cad2d09944..15587a3472 100644 --- a/src/Controller/Entry/EntryFrontController.php +++ b/src/Controller/Entry/EntryFrontController.php @@ -22,12 +22,21 @@ 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 $type, + string $federation, + Request $request + ): Response { $user = $this->getUser(); $criteria = $this->createCriteria($content, $request); @@ -39,57 +48,61 @@ 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); + return $this->renderResponse( + $request, + $content, + $criteria, + [$dataKey => $entities], + $templatePath + ); } - // $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 $type, + string $federation, + 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, Request $request ): Response { $user = $this->getUser(); @@ -106,21 +119,57 @@ 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}'"); + } - $templatePath = ('threads' === $content) ? 'entry/' : 'post/'; - $dataKey = ('threads' === $content) ? 'entries' : 'posts'; + return $this->renderResponse( + $request, + $content, + $criteria, + [$dataKey => $entities, 'magazine' => $magazine], + $templatePath + ); + } - return $this->renderResponse($request, $content, $criteria, [$dataKey => $entities, 'magazine' => $magazine], $templatePath); + /** + * @param string $name magazine name + */ + public function magazineRedirect( + string $name, + string $content, + ?string $sortBy, + ?string $time, + ?string $type, + string $federation, + Request $request + ): Response { + $user = $this->getUser(); // Fetch the user + $subscription = $this->subscriptionFor($user); // Determine the subscription filter based on the 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 +185,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 +212,8 @@ private function setUserPreferences(?User $user, &$criteria) private function renderResponse(Request $request, $content, $criteria, $data, $templatePath) { - $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 c9edd8a944..747eda64cb 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 0000000000..04554e3f99 --- /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/templates/entry/_options.html.twig b/templates/entry/_options.html.twig index 86aef80947..ac657f99a0 100644 --- a/templates/entry/_options.html.twig +++ b/templates/entry/_options.html.twig @@ -11,31 +11,31 @@