From 7fcef5f2f95307b694e599f24ff5e26c87c93556 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 21 Dec 2023 11:37:20 +1000 Subject: [PATCH] FIX: Show admin plugin route sub-links in sidebar (#24982) This changes the Plugins link in the admin sidebar to be a section instead, which then shows all enabled plugin admin routes (which are custom routes some plugins e.g. chat define). This is done via adding some special preloaded data for all controllers based on AdminController, and also specifically on Admin::PluginsController, to have the routes loaded without additional requests on page load. We just use a cog for all the route icons for now...we don't have anything better. --- .../instance-initializers/admin-sidebar.js | 23 +++++++++++-------- .../app/lib/sidebar/admin-nav-map.js | 13 +++++++++++ app/controllers/admin/admin_controller.rb | 9 ++++++++ app/controllers/admin/plugins_controller.rb | 11 ++++++++- app/controllers/application_controller.rb | 6 +++++ lib/discourse.rb | 5 ++++ lib/plugin/instance.rb | 2 +- lib/plugin/metadata.rb | 4 ++++ .../system/admin_sidebar_navigation_spec.rb | 17 ++++++++++++++ .../admin_revamp_sidebar_navigation_spec.rb | 15 +++++++----- 10 files changed, 87 insertions(+), 18 deletions(-) create mode 100644 plugins/chat/spec/system/admin_sidebar_navigation_spec.rb diff --git a/app/assets/javascripts/discourse/app/instance-initializers/admin-sidebar.js b/app/assets/javascripts/discourse/app/instance-initializers/admin-sidebar.js index 46151e2d0c7..4d2fb70cbf2 100644 --- a/app/assets/javascripts/discourse/app/instance-initializers/admin-sidebar.js +++ b/app/assets/javascripts/discourse/app/instance-initializers/admin-sidebar.js @@ -1,3 +1,4 @@ +import PreloadStore from "discourse/lib/preload-store"; import { ADMIN_NAV_MAP } from "discourse/lib/sidebar/admin-nav-map"; import { addSidebarPanel, @@ -137,13 +138,6 @@ export function useAdminNavConfig(navMap) { label: "admin.dashboard.reports_tab", icon: "chart-pie", }, - { - name: "admin_plugins", - route: "adminPlugins", - label: "admin.plugins.title", - icon: "puzzle-piece", - }, - { name: "admin_badges", route: "adminBadges", @@ -206,6 +200,17 @@ export default { const savedConfig = this.adminSidebarExperimentStateManager.navConfig; const navMap = savedConfig || ADMIN_NAV_MAP; + const enabledPluginAdminRoutes = + PreloadStore.get("enabledPluginAdminRoutes") || []; + enabledPluginAdminRoutes.forEach((pluginAdminRoute) => { + navMap.findBy("name", "admin_plugins").links.push({ + name: `admin_plugin_${pluginAdminRoute.location}`, + route: `adminPlugins.${pluginAdminRoute.location}`, + label: pluginAdminRoute.label, + icon: "cog", + }); + }); + if (this.siteSettings.experimental_form_templates) { navMap.findBy("name", "customize").links.push({ name: "admin_customize_form_templates", @@ -215,8 +220,6 @@ export default { }); } - const navConfig = useAdminNavConfig(navMap); - - buildAdminSidebar(navConfig, adminSectionLinkClass); + buildAdminSidebar(useAdminNavConfig(navMap), adminSectionLinkClass); }, }; diff --git a/app/assets/javascripts/discourse/app/lib/sidebar/admin-nav-map.js b/app/assets/javascripts/discourse/app/lib/sidebar/admin-nav-map.js index 318956053d9..213d28966cb 100644 --- a/app/assets/javascripts/discourse/app/lib/sidebar/admin-nav-map.js +++ b/app/assets/javascripts/discourse/app/lib/sidebar/admin-nav-map.js @@ -1,4 +1,17 @@ export const ADMIN_NAV_MAP = [ + { + name: "admin_plugins", + route: "adminPlugins.index", + label: "admin.plugins.title", + links: [ + { + name: "admin_installed_plugins", + route: "adminPlugins", + label: "admin.plugins.installed", + icon: "puzzle-piece", + }, + ], + }, { name: "email", text: "Email", diff --git a/app/controllers/admin/admin_controller.rb b/app/controllers/admin/admin_controller.rb index 2220a511e77..c7bd195359b 100644 --- a/app/controllers/admin/admin_controller.rb +++ b/app/controllers/admin/admin_controller.rb @@ -7,4 +7,13 @@ class Admin::AdminController < ApplicationController def index render body: nil end + + private + + def preload_additional_json + store_preloaded( + "enabledPluginAdminRoutes", + MultiJson.dump(Discourse.plugins_sorted_by_name.filter_map(&:admin_route)), + ) + end end diff --git a/app/controllers/admin/plugins_controller.rb b/app/controllers/admin/plugins_controller.rb index 2a1ce6b051e..c4e63a69949 100644 --- a/app/controllers/admin/plugins_controller.rb +++ b/app/controllers/admin/plugins_controller.rb @@ -3,9 +3,18 @@ class Admin::PluginsController < Admin::StaffController def index render_serialized( - Discourse.visible_plugins.sort_by { |p| p.name.downcase.delete_prefix("discourse-") }, + Discourse.plugins_sorted_by_name(enabled_only: false), AdminPluginSerializer, root: "plugins", ) end + + private + + def preload_additional_json + store_preloaded( + "enabledPluginAdminRoutes", + MultiJson.dump(Discourse.plugins_sorted_by_name.filter_map(&:admin_route)), + ) + end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 7081feb81e9..f8d10cea2e7 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -446,6 +446,8 @@ class ApplicationController < ActionController::Base current_user.sync_notification_channel_position preload_current_user_data end + + preload_additional_json end def set_mobile_view @@ -671,6 +673,10 @@ class ApplicationController < ActionController::Base store_preloaded("fontMap", MultiJson.dump(load_font_map)) if current_user.admin? end + def preload_additional_json + # noop, should be defined by subcontrollers + end + def custom_html_json target = view_context.mobile_view? ? :mobile : :desktop diff --git a/lib/discourse.rb b/lib/discourse.rb index c6280786eb2..ccf650b9ed8 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -377,6 +377,11 @@ module Discourse plugins.filter(&:visible?) end + def self.plugins_sorted_by_name(enabled_only: true) + return visible_plugins.filter(&:enabled?).sort_by(&:name_without_prefix) if enabled_only + visible_plugins.sort_by(&:name_without_prefix) + end + def self.plugin_themes @plugin_themes ||= plugins.map(&:themes).flatten end diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index 3f16cf026a3..63ba5527934 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -122,7 +122,7 @@ class Plugin::Instance @enabled_site_setting ? SiteSetting.get(@enabled_site_setting) : true end - delegate :name, to: :metadata + delegate :name, :name_without_prefix, to: :metadata def add_to_serializer( serializer, diff --git a/lib/plugin/metadata.rb b/lib/plugin/metadata.rb index 14e269fcbc2..561c36924c2 100644 --- a/lib/plugin/metadata.rb +++ b/lib/plugin/metadata.rb @@ -133,6 +133,10 @@ class Plugin::Metadata end end + def name_without_prefix + name.downcase.delete_prefix("discourse-") + end + def self.parse(text) metadata = self.new text.each_line { |line| break unless metadata.parse_line(line) } diff --git a/plugins/chat/spec/system/admin_sidebar_navigation_spec.rb b/plugins/chat/spec/system/admin_sidebar_navigation_spec.rb new file mode 100644 index 00000000000..55ee42cbe81 --- /dev/null +++ b/plugins/chat/spec/system/admin_sidebar_navigation_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +describe "Admin Revamp | Sidebar Navigation | Plugin Links", type: :system do + fab!(:admin) + let(:sidebar) { PageObjects::Components::NavigationMenu::Sidebar.new } + + before do + chat_system_bootstrap + SiteSetting.enable_admin_sidebar_navigation = true + sign_in(admin) + end + + it "shows links to enabled plugin admin routes" do + visit("/admin") + expect(sidebar).to have_section_link("Chat", href: "/admin/plugins/chat") + end +end diff --git a/spec/system/admin_revamp_sidebar_navigation_spec.rb b/spec/system/admin_revamp_sidebar_navigation_spec.rb index ff8a898e6d6..6a235c3c673 100644 --- a/spec/system/admin_revamp_sidebar_navigation_spec.rb +++ b/spec/system/admin_revamp_sidebar_navigation_spec.rb @@ -20,11 +20,14 @@ describe "Admin Revamp | Sidebar Navigation", type: :system do expect(sidebar).to have_no_section("admin-nav-section-root") end - it "does not show the admin sidebar if the setting is disabled" do - SiteSetting.enable_admin_sidebar_navigation = false - visit("/latest") - sidebar.click_link_in_section("community", "admin") - expect(page).to have_current_path("/admin") - expect(sidebar).to have_no_section("admin-nav-section-root") + context "when the setting is disabled" do + before { SiteSetting.enable_admin_sidebar_navigation = false } + + it "does not show the admin sidebar" do + visit("/latest") + sidebar.click_link_in_section("community", "admin") + expect(page).to have_current_path("/admin") + expect(sidebar).to have_no_section("admin-nav-section-root") + end end end