From 503f9b6f02ac5c4918d41611848c886b8755e5a0 Mon Sep 17 00:00:00 2001
From: Ted Johansson <ted@discourse.org>
Date: Tue, 4 Feb 2025 14:57:28 +0800
Subject: [PATCH] DEV: Use default admin routes for plugins with settings
 (#30941)

This change adds a sidebar link for each plugin that fulfils the following criteria:

- Does not have an explicit admin route defined in the plugin.
- Has at least one site setting (not including enabled/disabled.)

That sidebar link leads to the automatically generated plugin show settings page.
---
 .../addon/components/admin-area-settings.gjs  |  7 +++++
 .../admin/addon/models/admin-plugin.js        | 26 ++-----------------
 .../app/lib/sidebar/admin-sidebar.js          |  5 +++-
 .../tests/acceptance/admin-plugins-test.js    |  3 ++-
 .../tests/unit/models/admin-plugin-test.js    | 19 --------------
 app/controllers/application_controller.rb     |  1 +
 app/serializers/admin_plugin_serializer.rb    |  5 ++++
 lib/plugin/instance.rb                        | 25 ++++++++++++++++--
 .../requests/application_controller_spec.rb   |  1 +
 plugins/footnote/config/locales/client.en.yml |  4 +++
 spec/lib/plugin/instance_spec.rb              |  4 +--
 spec/system/admin_sidebar_navigation_spec.rb  | 15 +++++++++++
 .../components/navigation_menu/sidebar.rb     |  4 +++
 13 files changed, 70 insertions(+), 49 deletions(-)
 delete mode 100644 app/assets/javascripts/discourse/tests/unit/models/admin-plugin-test.js
 create mode 100644 plugins/footnote/config/locales/client.en.yml

diff --git a/app/assets/javascripts/admin/addon/components/admin-area-settings.gjs b/app/assets/javascripts/admin/addon/components/admin-area-settings.gjs
index 91011c2ff46..e8f60b6ea03 100644
--- a/app/assets/javascripts/admin/addon/components/admin-area-settings.gjs
+++ b/app/assets/javascripts/admin/addon/components/admin-area-settings.gjs
@@ -1,6 +1,7 @@
 import Component from "@glimmer/component";
 import { tracked } from "@glimmer/tracking";
 import { action } from "@ember/object";
+import didUpdate from "@ember/render-modifiers/modifiers/did-update";
 import { service } from "@ember/service";
 import ConditionalLoadingSpinner from "discourse/components/conditional-loading-spinner";
 import DBreadcrumbsItem from "discourse/components/d-breadcrumbs-item";
@@ -28,6 +29,11 @@ export default class AdminAreaSettings extends Component {
     return !this.loading && this.settings.length > 0;
   }
 
+  @action
+  async reloadSettings() {
+    await this.#loadSettings();
+  }
+
   @bind
   async #loadSettings() {
     this.loading = true;
@@ -69,6 +75,7 @@ export default class AdminAreaSettings extends Component {
 
     <div
       class="content-body admin-config-area__settings admin-detail pull-left"
+      {{didUpdate this.reloadSettings @plugin}}
     >
       {{#if this.showSettings}}
         <AdminFilteredSiteSettings
diff --git a/app/assets/javascripts/admin/addon/models/admin-plugin.js b/app/assets/javascripts/admin/addon/models/admin-plugin.js
index 097e7ccc233..3b05739e038 100644
--- a/app/assets/javascripts/admin/addon/models/admin-plugin.js
+++ b/app/assets/javascripts/admin/addon/models/admin-plugin.js
@@ -1,5 +1,5 @@
 import { cached, tracked } from "@glimmer/tracking";
-import { capitalize, dasherize } from "@ember/string";
+import { dasherize } from "@ember/string";
 import { snakeCaseToCamelCase } from "discourse/lib/case-converter";
 import I18n, { i18n } from "discourse-i18n";
 
@@ -50,29 +50,7 @@ export default class AdminPlugin {
 
   @cached
   get nameTitleized() {
-    // The category name is better in a lot of cases, as it's a human-inputted
-    // translation, and we can handle things like SAML instead of showing them
-    // as Saml from discourse-saml. We can fall back to the programmatic version
-    // though if needed.
-    let name;
-    if (this.translatedCategoryName) {
-      name = this.translatedCategoryName;
-    } else {
-      name = this.name
-        .split(/[-_]/)
-        .map((word) => {
-          return capitalize(word);
-        })
-        .join(" ");
-    }
-
-    // Cuts down on repetition.
-    const discoursePrefix = "Discourse ";
-    if (name.startsWith(discoursePrefix)) {
-      name = name.slice(discoursePrefix.length);
-    }
-
-    return name;
+    return this.translatedCategoryName || this.humanizedName;
   }
 
   @cached
diff --git a/app/assets/javascripts/discourse/app/lib/sidebar/admin-sidebar.js b/app/assets/javascripts/discourse/app/lib/sidebar/admin-sidebar.js
index c39aa410ad4..5dd9c6a7832 100644
--- a/app/assets/javascripts/discourse/app/lib/sidebar/admin-sidebar.js
+++ b/app/assets/javascripts/discourse/app/lib/sidebar/admin-sidebar.js
@@ -60,7 +60,9 @@ class SidebarAdminSectionLink extends BaseCustomSidebarSectionLink {
 
   get text() {
     return this.adminSidebarNavLink.label
-      ? i18n(this.adminSidebarNavLink.label)
+      ? i18n(this.adminSidebarNavLink.label, {
+          translatedFallback: this.adminSidebarNavLink.text,
+        })
       : this.adminSidebarNavLink.text;
   }
 
@@ -316,6 +318,7 @@ function pluginAdminRouteLinks(router) {
           ? [plugin.admin_route.location]
           : [],
         label: plugin.admin_route.label,
+        text: plugin.humanized_name,
         icon: "gear",
       };
     });
diff --git a/app/assets/javascripts/discourse/tests/acceptance/admin-plugins-test.js b/app/assets/javascripts/discourse/tests/acceptance/admin-plugins-test.js
index cfda767bc9e..e7c7d326a92 100644
--- a/app/assets/javascripts/discourse/tests/acceptance/admin-plugins-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/admin-plugins-test.js
@@ -12,6 +12,7 @@ acceptance("Admin - Plugins", function (needs) {
           {
             id: "some-test-plugin",
             name: "some-test-plugin",
+            humanized_name: "Some test plugin",
             about: "Plugin description",
             version: "0.1",
             url: "https://example.com",
@@ -44,7 +45,7 @@ acceptance("Admin - Plugins", function (needs) {
       .dom(
         "table.admin-plugins-list .admin-plugins-list__row .admin-plugins-list__name-details .admin-plugins-list__name-with-badges .admin-plugins-list__name"
       )
-      .hasText("Some Test Plugin", "displays the plugin in the table");
+      .hasText("Some test plugin", "displays the plugin in the table");
 
     assert
       .dom(".admin-plugins .admin-config-page .alert-error")
diff --git a/app/assets/javascripts/discourse/tests/unit/models/admin-plugin-test.js b/app/assets/javascripts/discourse/tests/unit/models/admin-plugin-test.js
deleted file mode 100644
index a4368087e28..00000000000
--- a/app/assets/javascripts/discourse/tests/unit/models/admin-plugin-test.js
+++ /dev/null
@@ -1,19 +0,0 @@
-import { setupTest } from "ember-qunit";
-import { module, test } from "qunit";
-import AdminPlugin from "admin/models/admin-plugin";
-
-module("Unit | Model | admin plugin", function (hooks) {
-  setupTest(hooks);
-
-  test("nameTitleized", function (assert) {
-    const adminPlugin = AdminPlugin.create({
-      name: "docker_manager",
-    });
-
-    assert.strictEqual(
-      adminPlugin.nameTitleized,
-      "Docker Manager",
-      "it should return titleized name replacing underscores with spaces"
-    );
-  });
-});
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 631ea3ea65a..64810414534 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -659,6 +659,7 @@ class ApplicationController < ActionController::Base
             .map do |plugin|
               {
                 name: plugin.name.downcase,
+                humanized_name: plugin.humanized_name,
                 admin_route: plugin.full_admin_route,
                 enabled: plugin.enabled?,
               }
diff --git a/app/serializers/admin_plugin_serializer.rb b/app/serializers/admin_plugin_serializer.rb
index 66a7d968c4e..a462f9d16f7 100644
--- a/app/serializers/admin_plugin_serializer.rb
+++ b/app/serializers/admin_plugin_serializer.rb
@@ -11,6 +11,7 @@ class AdminPluginSerializer < ApplicationSerializer
              :enabled_setting,
              :has_settings,
              :has_only_enabled_setting,
+             :humanized_name,
              :is_official,
              :is_discourse_owned,
              :label,
@@ -27,6 +28,10 @@ class AdminPluginSerializer < ApplicationSerializer
     object.metadata.name
   end
 
+  def humanized_name
+    object.humanized_name
+  end
+
   def about
     object.metadata.about
   end
diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb
index 6b7ec2dd1d7..4c67fbb7b80 100644
--- a/lib/plugin/instance.rb
+++ b/lib/plugin/instance.rb
@@ -120,7 +120,12 @@ class Plugin::Instance
 
   def full_admin_route
     route = self.admin_route
-    return unless route
+
+    if route.blank?
+      return if !any_settings?
+
+      route = default_admin_route
+    end
 
     route
       .slice(:location, :label, :use_new_show_route)
@@ -130,6 +135,14 @@ class Plugin::Instance
       end
   end
 
+  def any_settings?
+    return false if !configurable?
+
+    SiteSetting
+      .all_settings(filter_plugin: self.name)
+      .any? { |s| s[:setting] != @enabled_site_setting }
+  end
+
   def configurable?
     true
   end
@@ -146,7 +159,11 @@ class Plugin::Instance
   delegate :name, to: :metadata
 
   def humanized_name
-    (setting_category_name || name).delete_prefix("Discourse ").delete_prefix("discourse-")
+    (setting_category_name || name)
+      .delete_prefix("Discourse ")
+      .delete_prefix("discourse-")
+      .gsub("-", " ")
+      .upcase_first
   end
 
   def add_to_serializer(
@@ -1476,4 +1493,8 @@ class Plugin::Instance
   def register_permitted_bulk_action_parameter(name)
     DiscoursePluginRegistry.register_permitted_bulk_action_parameter(name, self)
   end
+
+  def default_admin_route
+    { label: "#{name.underscore}.title", location: name, use_new_show_route: true }
+  end
 end
diff --git a/plugins/chat/spec/requests/application_controller_spec.rb b/plugins/chat/spec/requests/application_controller_spec.rb
index d628700ea2b..5e26782c38e 100644
--- a/plugins/chat/spec/requests/application_controller_spec.rb
+++ b/plugins/chat/spec/requests/application_controller_spec.rb
@@ -22,6 +22,7 @@ RSpec.describe ApplicationController do
       expect(JSON.parse(preloaded_json["visiblePlugins"])).to include(
         {
           "name" => "chat",
+          "humanized_name" => "Chat",
           "admin_route" => {
             "label" => "chat.admin.title",
             "location" => "chat",
diff --git a/plugins/footnote/config/locales/client.en.yml b/plugins/footnote/config/locales/client.en.yml
new file mode 100644
index 00000000000..3ec76b7191c
--- /dev/null
+++ b/plugins/footnote/config/locales/client.en.yml
@@ -0,0 +1,4 @@
+en:
+  js:
+    footnote:
+      title: "Footnotes"
diff --git a/spec/lib/plugin/instance_spec.rb b/spec/lib/plugin/instance_spec.rb
index 091b3aa448b..1691d279a21 100644
--- a/spec/lib/plugin/instance_spec.rb
+++ b/spec/lib/plugin/instance_spec.rb
@@ -33,7 +33,7 @@ TEXT
     end
 
     it "defaults to using the plugin name with the discourse- prefix removed" do
-      expect(plugin_instance.humanized_name).to eq("sample-plugin")
+      expect(plugin_instance.humanized_name).to eq("Sample plugin")
     end
 
     it "uses the plugin setting category name if it exists" do
@@ -43,7 +43,7 @@ TEXT
 
     it "the plugin name the plugin site settings are still under the generic plugins: category" do
       plugin_instance.stubs(:setting_category).returns("plugins")
-      expect(plugin_instance.humanized_name).to eq("sample-plugin")
+      expect(plugin_instance.humanized_name).to eq("Sample plugin")
     end
 
     it "removes any Discourse prefix from the setting category name" do
diff --git a/spec/system/admin_sidebar_navigation_spec.rb b/spec/system/admin_sidebar_navigation_spec.rb
index 68670ed11cc..e3638f62d96 100644
--- a/spec/system/admin_sidebar_navigation_spec.rb
+++ b/spec/system/admin_sidebar_navigation_spec.rb
@@ -337,4 +337,19 @@ describe "Admin | Sidebar Navigation", type: :system do
       ],
     )
   end
+
+  it "adds auto-generated plugin links for plugins with settings" do
+    skip if Discourse.plugins.empty?
+
+    SiteSetting.enable_markdown_footnotes = true
+
+    visit("/admin")
+
+    sidebar.toggle_section(:plugins)
+
+    expect(page).to have_css(
+      ".sidebar-section-link-content-text",
+      text: I18n.t("js.footnote.title"),
+    )
+  end
 end
diff --git a/spec/system/page_objects/components/navigation_menu/sidebar.rb b/spec/system/page_objects/components/navigation_menu/sidebar.rb
index 03e13b77066..a64e4609096 100644
--- a/spec/system/page_objects/components/navigation_menu/sidebar.rb
+++ b/spec/system/page_objects/components/navigation_menu/sidebar.rb
@@ -60,6 +60,10 @@ module PageObjects
         def toggle_all_sections
           find(".sidebar-toggle-all-sections").click
         end
+
+        def toggle_section(name)
+          find("[data-section-name='admin-#{name.to_s.downcase}']").click
+        end
       end
     end
   end