FIX: Plugin JS failing to load would break admin interface (#29139)

If a plugin's JS fails to load for some reason, most commonly
ad blockers, the entire admin interface would break. This is because
we are adding links to the admin routes for plugins that define
them in the sidebar.

We have a fix for this already in the plugin list which shows a warning
to the admin. This fix just prevents the broken link from rendering
in the sidebar if the route is not valid.

app/assets/javascripts/admin/addon/controllers/admin-plugins-index.js

@@ -1,6 +1,7 @@
 import Controller from "@ember/controller";
 import { action } from "@ember/object";
 import { service } from "@ember/service";
+import { adminRouteValid } from "discourse/lib/admin-utilities";
 import { popupAjaxError } from "discourse/lib/ajax-error";
 import SiteSetting from "admin/models/site-setting";
 
@@ -27,7 +28,9 @@ export default class AdminPluginsIndexController extends Controller {
   // NOTE: See also AdminPluginsController, there is some duplication here
   // while we convert plugins to use_new_show_route
   get adminRoutes() {
-    return this.allAdminRoutes.filter((route) => this.routeExists(route));
+    return this.allAdminRoutes.filter((route) =>
+      adminRouteValid(this.router, route)
+    );
   }
 
   get allAdminRoutes() {
@@ -37,17 +40,4 @@ export default class AdminPluginsIndexController extends Controller {
         return Object.assign(plugin.adminRoute, { plugin_id: plugin.id });
       });
   }
-
-  routeExists(route) {
-    try {
-      if (route.use_new_show_route) {
-        this.router.urlFor(route.full_location, route.location);
-      } else {
-        this.router.urlFor(route.full_location);
-      }
-      return true;
-    } catch (e) {
-      return false;
-    }
-  }
 }

app/assets/javascripts/admin/addon/controllers/admin-plugins.js

@@ -1,16 +1,21 @@
 import Controller from "@ember/controller";
 import { service } from "@ember/service";
+import { adminRouteValid } from "discourse/lib/admin-utilities";
 
 export default class AdminPluginsController extends Controller {
   @service adminPluginNavManager;
   @service router;
 
   get adminRoutes() {
-    return this.allAdminRoutes.filter((route) => this.routeExists(route));
+    return this.allAdminRoutes.filter((route) =>
+      adminRouteValid(this.router, route)
+    );
   }
 
   get brokenAdminRoutes() {
-    return this.allAdminRoutes.filter((route) => !this.routeExists(route));
+    return this.allAdminRoutes.filter(
+      (route) => !adminRouteValid(this.router, route)
+    );
   }
 
   // NOTE: See also AdminPluginsIndexController, there is some duplication here
@@ -30,17 +35,4 @@ export default class AdminPluginsController extends Controller {
         this.adminPluginNavManager.isSidebarMode)
     );
   }
-
-  routeExists(route) {
-    try {
-      if (route.use_new_show_route) {
-        this.router.urlFor(route.full_location, route.location);
-      } else {
-        this.router.urlFor(route.full_location);
-      }
-      return true;
-    } catch (e) {
-      return false;
-    }
-  }
 }

app/assets/javascripts/discourse/app/lib/admin-utilities.js

@@ -0,0 +1,12 @@
+export function adminRouteValid(router, adminRoute) {
+  try {
+    if (adminRoute.use_new_show_route) {
+      router.urlFor(adminRoute.full_location, adminRoute.location);
+    } else {
+      router.urlFor(adminRoute.full_location);
+    }
+    return true;
+  } catch (e) {
+    return false;
+  }
+}

app/assets/javascripts/discourse/app/lib/sidebar/admin-sidebar.js

@@ -1,5 +1,7 @@
 import { cached } from "@glimmer/tracking";
+import { warn } from "@ember/debug";
 import { htmlSafe } from "@ember/template";
+import { adminRouteValid } from "discourse/lib/admin-utilities";
 import PreloadStore from "discourse/lib/preload-store";
 import { ADMIN_NAV_MAP } from "discourse/lib/sidebar/admin-nav-map";
 import BaseCustomSidebarPanel from "discourse/lib/sidebar/base-custom-sidebar-panel";
@@ -269,9 +271,26 @@ export function addAdminSidebarSectionLink(sectionName, link) {
   additionalAdminSidebarSectionLinks[sectionName].push(link);
 }
 
-function pluginAdminRouteLinks() {
+function pluginAdminRouteLinks(router) {
   return (PreloadStore.get("visiblePlugins") || [])
-    .filter((plugin) => plugin.admin_route && plugin.enabled)
+    .filter((plugin) => {
+      if (!plugin.admin_route || !plugin.enabled) {
+        return false;
+      }
+
+      // Check if the admin route is valid, if it is not the whole admin
+      // interface can break because of this. This can be the case for things
+      // like ad blockers stopping plugin JS from loading.
+      if (adminRouteValid(router, plugin.admin_route)) {
+        return true;
+      } else {
+        warn(
+          `[AdminSidebar] Could not find admin route for ${plugin.name}, route was ${plugin.admin_route.full_location}. This could be caused by an ad blocker.`,
+          { id: "discourse.admin-sidebar:plugin-admin-route-links" }
+        );
+        return false;
+      }
+    })
     .map((plugin) => {
       return {
         name: `admin_plugin_${plugin.admin_route.location}`,
@@ -324,7 +343,7 @@ export default class AdminSidebarPanel extends BaseCustomSidebarPanel {
       const pluginLinks = navMap.find(
         (section) => section.name === "plugins"
       ).links;
-      pluginAdminRouteLinks().forEach((pluginLink) => {
+      pluginAdminRouteLinks(router).forEach((pluginLink) => {
         if (!pluginLinks.mapBy("name").includes(pluginLink.name)) {
           pluginLinks.push(pluginLink);
         }

app/controllers/application_controller.rb

@@ -659,7 +659,7 @@ class ApplicationController < ActionController::Base
             .map do |plugin|
               {
                 name: plugin.name.downcase,
-                admin_route: plugin.admin_route,
+                admin_route: plugin.full_admin_route,
                 enabled: plugin.enabled?,
               }
             end,

app/serializers/admin_plugin_serializer.rb

@@ -72,17 +72,7 @@ class AdminPluginSerializer < ApplicationSerializer
   end
 
   def admin_route
-    route = object.admin_route
-    return unless route
-
-    ret = route.slice(:location, :label)
-    if route[:use_new_show_route]
-      ret[:full_location] = "adminPlugins.show"
-      ret[:use_new_show_route] = true
-    else
-      ret[:full_location] = "adminPlugins.#{ret[:location]}"
-    end
-    ret
+    object.full_admin_route
   end
 
   def include_admin_route?

lib/plugin/instance.rb

@@ -117,6 +117,18 @@ class Plugin::Instance
     }
   end
 
+  def full_admin_route
+    route = self.admin_route
+    return unless route
+
+    route
+      .slice(:location, :label, :use_new_show_route)
+      .tap do |admin_route|
+        path = admin_route[:use_new_show_route] ? "show" : admin_route[:location]
+        admin_route[:full_location] = "adminPlugins.#{path}"
+      end
+  end
+
   def configurable?
     true
   end

plugins/chat/spec/requests/application_controller_spec.rb

@@ -25,6 +25,7 @@ RSpec.describe ApplicationController do
           "admin_route" => {
             "label" => "chat.admin.title",
             "location" => "chat",
+            "full_location" => "adminPlugins.show",
             "use_new_show_route" => true,
           },
           "enabled" => true,

spec/serializers/admin_plugin_serializer_spec.rb

@@ -17,6 +17,7 @@ RSpec.describe AdminPluginSerializer do
         location: "admin",
         label: "admin.test",
         full_location: "adminPlugins.admin",
+        use_new_show_route: false,
       )
     end