Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better handling of special system and quant pages #86

Closed
kepol opened this issue Dec 3, 2022 · 3 comments
Closed

Better handling of special system and quant pages #86

kepol opened this issue Dec 3, 2022 · 3 comments
Labels
Drupal 8 / 9 / 10 DX Developer experience enhancement New feature or request

Comments

@kepol
Copy link
Contributor

kepol commented Dec 3, 2022

The handling of special pages in 3 pages doesn't always take into consideration 1) if the site config has been set and 2) if the page is available. Need to update the logic to check these before adding to routes/etc.

Old description:

Right now, /_quant403 and /_quant404 are added when seeding even if the site configuration hasn't been created for the 403 and 404 pages. We should only add these into the $specialPages if they have been added to the site configuration.

    // Special case pages (403/404/Home)
    $specialPages = [
      '/' => $site_config->get('page.front'),
      '/_quant404' => $site_config->get('page.404'),
      '/_quant403' => $site_config->get('page.403'),
    ];
@kepol
Copy link
Contributor Author

kepol commented Dec 3, 2022

Actually, there is code in 3 places that deals with the home page and 404/403 pages so it's confusing. I was trying to test changing these but not sure this is the best thing. It would be good to understand why similar logic is in multiple places.

+++ b/quant.module
@@ -336,8 +336,10 @@ function quant_special_pages() {
     '/_quant403',
   ];
 
-  foreach ($system_pages as $route) {
-    $item = new RouteItem(['route' => $route]);
-    $item->send();
+  foreach ($system_pages as $page) {
+    if (\Drupal::service('path.validator')->getUrlIfValid($page)) {
+      $item = new RouteItem(['route' => $page]);
+      $item->send();
+    }
+++ b/src/Seed.php
@@ -206,12 +206,18 @@ class Seed {
       }
     }
 
-    // Special case pages (403/404/Home)
-    $specialPages = [
-      '/' => $site_config->get('page.front'),
-      '/_quant404' => $site_config->get('page.404'),
-      '/_quant403' => $site_config->get('page.403'),
+    // Special case pages (Home/404/403).
+    $specialPages = [];
+    $pageConfig = [
+      '/' => 'page.front',
+      '/_quant404' => 'page.404',
+      '/_quant403' => 'page.403',
     ];
+    foreach ($pageConfig as $page => $config) {
+      if (!empty($site_config->get($config))) {
+        $specialPages[$page] = $site_config->get($config);
+      }
+    }
+++ b/src/EventSubscriber/CollectionSubscriber.php
@@ -207,7 +207,9 @@ class CollectionSubscriber implements EventSubscriberInterface {
     $quant_pages = ['/', '/_quant404', '/_quant403'];
 
     foreach ($quant_pages as $page) {
-      $event->queueItem(['route' => $page]);
+      if (\Drupal::service('path.validator')->getUrlIfValid($page)) {
+        $event->queueItem(['route' => $page]);
+      }
     }

@kepol
Copy link
Contributor Author

kepol commented Dec 3, 2022

Tested those changes and they seem to fix the errors... just not sure that's the best approach.

@kepol kepol changed the title Only add _quant404 and _quant403 when seeding if site configuration has been done Better handling of special system and quant pages Dec 3, 2022
@kepol kepol added enhancement New feature or request Drupal 8 / 9 / 10 DX Developer experience labels Feb 4, 2023
@kepol
Copy link
Contributor Author

kepol commented Jan 4, 2024

Ported to d.o issue so closing:

https://www.drupal.org/project/quantcdn/issues/3412392

@kepol kepol closed this as completed Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Drupal 8 / 9 / 10 DX Developer experience enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant