FIX: Sort plugins by their setting category name (#25128)

Some plugins have names (e.g. discourse-x-yz) that
are totally different from what they are actually called,
and that causes issues when showing them in a sorted way
in the admin plugin list.

Now, we should use the setting category name from client.en.yml
if it exists, otherwise fall back to the name, for sorting.
This is what we do on the client to determine what text to
show for the plugin name as well.

lib/discourse.rb

@@ -378,8 +378,10 @@ module Discourse
   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)
+    if enabled_only
+      return visible_plugins.filter(&:enabled?).sort_by { |plugin| plugin.humanized_name.downcase }
+    end
+    visible_plugins.sort_by { |plugin| plugin.humanized_name.downcase }
   end
 
   def self.plugin_themes

lib/plugin/instance.rb

@@ -122,7 +122,11 @@ class Plugin::Instance
     @enabled_site_setting ? SiteSetting.get(@enabled_site_setting) : true
   end
 
-  delegate :name, :name_without_prefix, to: :metadata
+  delegate :name, to: :metadata
+
+  def humanized_name
+    (setting_category_name || name).delete_prefix("Discourse ").delete_prefix("discourse-")
+  end
 
   def add_to_serializer(
     serializer,
@@ -1369,6 +1373,16 @@ class Plugin::Instance
 
   private
 
+  def setting_category
+    return if @enabled_site_setting.blank?
+    SiteSetting.categories[enabled_site_setting]
+  end
+
+  def setting_category_name
+    return if setting_category.blank? || setting_category == "plugins"
+    I18n.t("admin_js.admin.site_settings.categories.#{setting_category}")
+  end
+
   def validate_directory_column_name(column_name)
     match = /\A[_a-z]+\z/.match(column_name)
     unless match

lib/plugin/metadata.rb

@@ -133,10 +133,6 @@ 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) }

plugins/chat/config/locales/client.en.yml

@@ -1,9 +1,12 @@
 en:
-  js:
+  admin_js:
     admin:
       site_settings:
         categories:
           chat: Chat
+  js:
+    admin:
+      site_settings:
         chat_separate_sidebar_mode:
           always: "Always"
           fullscreen: "When chat is in fullscreen"

spec/lib/discourse_spec.rb

@@ -67,6 +67,30 @@ RSpec.describe Discourse do
     end
   end
 
+  describe ".plugins_sorted_by_name" do
+    before do
+      Discourse.stubs(:visible_plugins).returns(
+        [
+          stub(enabled?: false, name: "discourse-doctor-sleep", humanized_name: "Doctor Sleep"),
+          stub(enabled?: true, name: "discourse-shining", humanized_name: "The Shining"),
+          stub(enabled?: true, name: "discourse-misery", humanized_name: "misery"),
+        ],
+      )
+    end
+
+    it "sorts enabled plugins by humanized name" do
+      expect(Discourse.plugins_sorted_by_name.map(&:name)).to eq(
+        %w[discourse-misery discourse-shining],
+      )
+    end
+
+    it "sorts both enabled and disabled plugins when that option is provided" do
+      expect(Discourse.plugins_sorted_by_name(enabled_only: false).map(&:name)).to eq(
+        %w[discourse-doctor-sleep discourse-misery discourse-shining],
+      )
+    end
+  end
+
   describe "plugins" do
     let(:plugin_class) do
       Class.new(Plugin::Instance) do

spec/lib/plugin/instance_spec.rb

@@ -1,10 +1,60 @@
 # frozen_string_literal: true
 
 RSpec.describe Plugin::Instance do
-  subject(:plugin_instance) { described_class.new }
+  subject(:plugin_instance) { described_class.new(metadata) }
+
+  let(:metadata) { Plugin::Metadata.parse <<TEXT }
+# name: discourse-sample-plugin
+# about: about: my plugin
+# version: 0.1
+# authors: Frank Zappa
+# contact emails: frankz@example.com
+# url: http://discourse.org
+# required version: 1.3.0beta6+48
+# meta_topic_id: 1234
+# label: experimental
+
+some_ruby
+TEXT
 
   after { DiscoursePluginRegistry.reset! }
 
+  # NOTE: sample_plugin_site_settings.yml is always loaded in tests in site_setting.rb
+
+  describe ".humanized_name" do
+    before do
+      TranslationOverride.upsert!(
+        "en",
+        "admin_js.admin.site_settings.categories.discourse_sample_plugin",
+        "Sample Plugin Category Name",
+      )
+    end
+
+    it "defaults to using the plugin name with the discourse- prefix removed" do
+      expect(plugin_instance.humanized_name).to eq("sample-plugin")
+    end
+
+    it "uses the plugin setting category name if it exists" do
+      plugin_instance.enabled_site_setting(:discourse_sample_plugin_enabled)
+      expect(plugin_instance.humanized_name).to eq("Sample Plugin Category Name")
+    end
+
+    it "the plugin name the plugin site settings are still under the generic plugins: category" do
+      plugin_instance.stubs(:setting_catgory).returns("plugins")
+      expect(plugin_instance.humanized_name).to eq("sample-plugin")
+    end
+
+    it "removes any Discourse prefix from the setting category name" do
+      TranslationOverride.upsert!(
+        "en",
+        "admin_js.admin.site_settings.categories.discourse_sample_plugin",
+        "Discourse Sample Plugin Category Name",
+      )
+      plugin_instance.enabled_site_setting(:discourse_sample_plugin_enabled)
+      expect(plugin_instance.humanized_name).to eq("Sample Plugin Category Name")
+    end
+  end
+
   describe "find_all" do
     it "can find plugins correctly" do
       plugins = Plugin::Instance.find_all("#{Rails.root}/spec/fixtures/plugins")

spec/support/sample_plugin_site_settings.yml

@@ -1,7 +1,9 @@
+discourse_sample_plugin:
+  discourse_sample_plugin_enabled: true
 site_settings:
   plugin_setting:
     default: "some value"
   test_some_topic_id:
     default: 0
   test_some_other_topic_id:
-    default: 0
+    default: 0