FIX: Further improvements for plugin list (#24622)

Followup e37fb3042d

* Automatically remove the prefix `Discourse ` from all the plugin titles to avoid repetition
* Remove the :discourse_dev: icon from the author. Consider a "By Discourse" with no labels as official
* We add a `label` metadata to plugin.rb
  * Only plugins made by us in `discourse` and `discourse-org` GitHub organizations will show these in the list
* Make the plugin author font size a little smaller
* Make the commit sha look like a link so it's more obvious it goes to the code

Also I added some validation and truncation for plugin metadata
parsing since currently you can put absolutely anything in there
and it will show on the plugin list.

app/assets/javascripts/admin/addon/components/admin-plugins-list-item.gjs

@@ -47,23 +47,15 @@ export default class AdminPluginsListItem extends Component {
           </div>
 
           <div class="badges">
-            {{#if @plugin.isExperimental}}
-              <span
-                class="admin-plugins-list__badge -experimental"
-                title={{i18n "admin.plugins.experimental"}}
-              >
-                {{i18n "admin.plugins.experimental_badge"}}
+            {{#if @plugin.label}}
+              <span class="admin-plugins-list__badge">
+                {{@plugin.label}}
               </span>
             {{/if}}
           </div>
         </div>
         <div class="admin-plugins-list__author">
           {{@plugin.author}}
-          {{#if @plugin.isOfficial}}
-            <span title={{i18n "admin.plugins.official"}}>{{icon
-                "fab-discourse"
-              }}</span>
-          {{/if}}
         </div>
         <div class="admin-plugins-list__about">
           {{@plugin.about}}
@@ -73,7 +65,7 @@ export default class AdminPluginsListItem extends Component {
               rel="noopener noreferrer"
               target="_blank"
             >
-              {{i18n "learn_more"}}
+              {{i18n "admin.plugins.learn_more"}}
             </a>
           {{/if}}
         </div>

app/assets/javascripts/admin/addon/models/admin-plugin.js

@@ -20,7 +20,7 @@ export default class AdminPlugin {
     this.id = args.id;
     this.isOfficial = args.is_official;
     this.isDiscourseOwned = args.is_discourse_owned;
-    this.isExperimental = args.is_experimental;
+    this.label = args.label;
     this.name = args.name;
     this.url = args.url;
     this.version = args.version;
@@ -57,16 +57,25 @@ export default class AdminPlugin {
     // translation, and we can handle things like SAML instead of showing them
     // as Saml from discourse-saml. We can fall back to the programattic version
     // though if needed.
+    let name;
     if (this.translatedCategoryName) {
-      return this.translatedCategoryName;
+      name = this.translatedCategoryName;
+    } else {
+      name = this.name
+        .split("-")
+        .map((word) => {
+          return capitalize(word);
+        })
+        .join(" ");
     }
 
-    return 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;
   }
 
   get author() {

app/assets/stylesheets/common/admin/plugins.scss

@@ -42,8 +42,8 @@
     }
 
     &__author {
-      font-size: var(--font-down-1);
-      padding: 0.25em 0;
+      font-size: var(--font-down-2);
+      padding: 0 0 0.25em 0;
     }
 
     &__name-with-badges {
@@ -73,7 +73,6 @@
 
     &__version {
       .commit-hash {
-        color: var(--primary-low-mid);
         font-size: var(--font-down-1);
       }
     }

app/serializers/admin_plugin_serializer.rb

@@ -11,8 +11,8 @@ class AdminPluginSerializer < ApplicationSerializer
              :enabled_setting,
              :has_settings,
              :is_official,
-             :is_experimental,
              :is_discourse_owned,
+             :label,
              :commit_hash,
              :commit_url,
              :meta_url,
@@ -79,8 +79,13 @@ class AdminPluginSerializer < ApplicationSerializer
     Plugin::Metadata::OFFICIAL_PLUGINS.include?(object.name)
   end
 
-  def is_experimental
-    object.metadata.experimental
+  def include_label?
+    is_discourse_owned
+  end
+
+  def label
+    return if !is_discourse_owned
+    object.metadata.label
   end
 
   def is_discourse_owned

config/locales/client.en.yml

@@ -5078,10 +5078,10 @@ en:
         change_settings_short: "Settings"
         howto: "How do I install plugins?"
         official: "Official Discourse Plugin"
-        experimental: "Experimental Plugin"
         broken_route: "Unable to configure link to '%{name}'. Ensure ad-blockers are disabled and try reloading the page."
         author: "By %{author}"
         experimental_badge: "experimental"
+        learn_more: "Learn more"
 
       navigation_menu:
         sidebar: "Sidebar"

lib/plugin/metadata.rb

@@ -111,10 +111,28 @@ class Plugin::Metadata
     required_version
     transpile_js
     meta_topic_id
-    experimental
+    label
   ]
   attr_accessor(*FIELDS)
 
+  MAX_FIELD_LENGTHS ||= {
+    name: 75,
+    about: 350,
+    authors: 200,
+    contact_emails: 200,
+    url: 500,
+    label: 20,
+  }
+
+  def meta_topic_id=(value)
+    @meta_topic_id =
+      begin
+        Integer(value)
+      rescue StandardError
+        nil
+      end
+  end
+
   def self.parse(text)
     metadata = self.new
     text.each_line { |line| break unless metadata.parse_line(line) }
@@ -130,12 +148,17 @@ class Plugin::Metadata
 
     unless line.empty?
       return false unless line[0] == "#"
-      attribute, *description = line[1..-1].split(":")
+      attribute, *value = line[1..-1].split(":")
 
-      description = description.join(":")
+      value = value.join(":")
       attribute = attribute.strip.gsub(/ /, "_").to_sym
 
-      self.public_send("#{attribute}=", description.strip) if FIELDS.include?(attribute)
+      if FIELDS.include?(attribute)
+        self.public_send(
+          "#{attribute}=",
+          value.strip.truncate(MAX_FIELD_LENGTHS[attribute] || 1000),
+        )
+      end
     end
 
     true

spec/lib/plugin/metadata_spec.rb

@@ -12,6 +12,7 @@ RSpec.describe Plugin::Metadata do
 # url: http://discourse.org
 # required version: 1.3.0beta6+48
 # meta_topic_id: 1234
+# label: experimental
 
 some_ruby
 TEXT
@@ -23,7 +24,8 @@ TEXT
       expect(metadata.contact_emails).to eq("frankz@example.com")
       expect(metadata.url).to eq("http://discourse.org")
       expect(metadata.required_version).to eq("1.3.0beta6+48")
-      expect(metadata.meta_topic_id).to eq("1234")
+      expect(metadata.meta_topic_id).to eq(1234)
+      expect(metadata.label).to eq("experimental")
     end
   end
 
@@ -50,4 +52,40 @@ TEXT
     official("discourse-data-explorer")
     unofficial("babble")
   end
+
+  it "does not support anything but integer for meta_topic_id" do
+    metadata = Plugin::Metadata.parse <<TEXT
+# meta_topic_id: 1234
+TEXT
+
+    expect(metadata.meta_topic_id).to eq(1234)
+
+    metadata = Plugin::Metadata.parse <<TEXT
+# meta_topic_id: t/1234 blah
+TEXT
+
+    expect(metadata.meta_topic_id).to eq(nil)
+  end
+
+  it "truncates long field lengths" do
+    metadata = Plugin::Metadata.parse <<TEXT
+# name: #{"a" * 100}
+# about: #{"a" * 400}
+# authors: #{"a" * 300}
+# contact_emails: #{"a" * 300}
+# url: #{"a" * 600}
+# label: #{"a" * 100}
+# required_version: #{"a" * 1500}
+TEXT
+
+    expect(metadata.name.length).to eq(Plugin::Metadata::MAX_FIELD_LENGTHS[:name])
+    expect(metadata.about.length).to eq(Plugin::Metadata::MAX_FIELD_LENGTHS[:about])
+    expect(metadata.authors.length).to eq(Plugin::Metadata::MAX_FIELD_LENGTHS[:authors])
+    expect(metadata.contact_emails.length).to eq(
+      Plugin::Metadata::MAX_FIELD_LENGTHS[:contact_emails],
+    )
+    expect(metadata.url.length).to eq(Plugin::Metadata::MAX_FIELD_LENGTHS[:url])
+    expect(metadata.label.length).to eq(Plugin::Metadata::MAX_FIELD_LENGTHS[:label])
+    expect(metadata.required_version.length).to eq(1000)
+  end
 end