From c58cd697d2361b178b8a12593e0193e11496a733 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 30 Nov 2023 10:53:17 +1000 Subject: [PATCH] FIX: Further improvements for plugin list (#24622) Followup e37fb3042d6f56a27a01614e57bc7029f472b0c9 * 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. --- .../components/admin-plugins-list-item.gjs | 16 ++------ .../admin/addon/models/admin-plugin.js | 25 ++++++++---- .../stylesheets/common/admin/plugins.scss | 5 +-- app/serializers/admin_plugin_serializer.rb | 11 +++-- config/locales/client.en.yml | 2 +- lib/plugin/metadata.rb | 31 ++++++++++++-- spec/lib/plugin/metadata_spec.rb | 40 ++++++++++++++++++- 7 files changed, 98 insertions(+), 32 deletions(-) diff --git a/app/assets/javascripts/admin/addon/components/admin-plugins-list-item.gjs b/app/assets/javascripts/admin/addon/components/admin-plugins-list-item.gjs index cd69009998a..756f60224d9 100644 --- a/app/assets/javascripts/admin/addon/components/admin-plugins-list-item.gjs +++ b/app/assets/javascripts/admin/addon/components/admin-plugins-list-item.gjs @@ -47,23 +47,15 @@ export default class AdminPluginsListItem extends Component {
- {{#if @plugin.isExperimental}} - - {{i18n "admin.plugins.experimental_badge"}} + {{#if @plugin.label}} + + {{@plugin.label}} {{/if}}
{{@plugin.author}} - {{#if @plugin.isOfficial}} - {{icon - "fab-discourse" - }} - {{/if}}
{{@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"}} {{/if}}
diff --git a/app/assets/javascripts/admin/addon/models/admin-plugin.js b/app/assets/javascripts/admin/addon/models/admin-plugin.js index 648202252c4..eec9516bb65 100644 --- a/app/assets/javascripts/admin/addon/models/admin-plugin.js +++ b/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() { diff --git a/app/assets/stylesheets/common/admin/plugins.scss b/app/assets/stylesheets/common/admin/plugins.scss index 375034ffbcf..559d901077c 100644 --- a/app/assets/stylesheets/common/admin/plugins.scss +++ b/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); } } diff --git a/app/serializers/admin_plugin_serializer.rb b/app/serializers/admin_plugin_serializer.rb index 5747ac09d3c..9b0e488ea6e 100644 --- a/app/serializers/admin_plugin_serializer.rb +++ b/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 diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index ae3962e9509..b98d4fad470 100644 --- a/config/locales/client.en.yml +++ b/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" diff --git a/lib/plugin/metadata.rb b/lib/plugin/metadata.rb index 1592b3ee1e1..14e269fcbc2 100644 --- a/lib/plugin/metadata.rb +++ b/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 diff --git a/spec/lib/plugin/metadata_spec.rb b/spec/lib/plugin/metadata_spec.rb index 97467cc445e..859467edd1b 100644 --- a/spec/lib/plugin/metadata_spec.rb +++ b/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 <