Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

UX: Disable plugin list settings button for some plugins (#27124) 

For plugins with only an "enabled" site setting, it doesn't
make sense to take them to the site settings page, since the
toggle switch in the list can be used to change enabled/disabled.

This will not be the case for plugins that have their own custom
config page (like Automation), but we will deal with this when
we actually overhaul this plugin to use the new show page.

Also adds another rspec fixture of a test plugin.
This commit is contained in:
Martin Brennan 2024-05-23 12:04:26 +10:00 committed by GitHub
parent 3137e60653
commit 312a930ac8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 117 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -42,6 +42,20 @@ export default class AdminPluginsListItem extends Component {
return this.currentUser.admin && this.args.plugin.hasSettings;
}
get disablePluginSettingsButton() {
return (
this.showPluginSettingsButton && this.args.plugin.hasOnlyEnabledSetting
);
}
get settingsButtonTitle() {
if (this.disablePluginSettingsButton) {
return i18n("admin.plugins.settings_disabled");
}
return "";
}
<template>
<tr
data-plugin-name={{@plugin.name}}
@ -111,6 +125,8 @@ export default class AdminPluginsListItem extends Component {
class="btn-default btn btn-icon-text"
@route="adminPlugins.show"
@model={{@plugin}}
@disabled={{this.disablePluginSettingsButton}}
title={{this.settingsButtonTitle}}
data-plugin-setting-button={{@plugin.name}}
>
{{icon "cog"}}
@ -122,6 +138,8 @@ export default class AdminPluginsListItem extends Component {
@route="adminSiteSettingsCategory"
@model={{@plugin.settingCategoryName}}
@query={{hash filter=(concat "plugin:" @plugin.name)}}
@disabled={{this.disablePluginSettingsButton}}
title={{this.settingsButtonTitle}}
data-plugin-setting-button={{@plugin.name}}
>
{{icon "cog"}}

1
app/assets/javascripts/admin/addon/models/admin-plugin.js
View File

@ -17,6 +17,7 @@ export default class AdminPlugin {
this.enabled = args.enabled;
this.enabledSetting = args.enabled_setting;
this.hasSettings = args.has_settings;
this.hasOnlyEnabledSetting = args.has_only_enabled_setting;
this.id = args.id;
this.isOfficial = args.is_official;
this.isDiscourseOwned = args.is_discourse_owned;

15
app/assets/javascripts/discourse/tests/integration/components/admin-plugins-list-item-test.js
View File

@ -18,6 +18,7 @@ module("Integration | Component | admin-plugins-list-item", function (hooks) {
full_location: "admin",
},
has_settings: true,
has_only_enabled_setting: false,
};
}
@ -56,4 +57,18 @@ module("Integration | Component | admin-plugins-list-item", function (hooks) {
await render(hbs`<AdminPluginsListItem @plugin={{this.plugin}} />`);
assert.dom(".admin-plugins-list__settings a").doesNotExist();
});
test("settings link disabled if only the enabled setting exists", async function (assert) {
this.currentUser.admin = true;
const store = getOwner(this).lookup("service:store");
this.plugin = store.createRecord("admin-plugin", pluginAttrs());
await render(hbs`<AdminPluginsListItem @plugin={{this.plugin}} />`);
assert.dom(".admin-plugins-list__settings a.disabled").doesNotExist();
this.plugin.hasOnlyEnabledSetting = true;
await render(hbs`<AdminPluginsListItem @plugin={{this.plugin}} />`);
assert.dom(".admin-plugins-list__settings a.disabled").exists();
});
});

11
app/serializers/admin_plugin_serializer.rb
View File

@ -10,6 +10,7 @@ class AdminPluginSerializer < ApplicationSerializer
:enabled,
:enabled_setting,
:has_settings,
:has_only_enabled_setting,
:is_official,
:is_discourse_owned,
:label,
@ -54,8 +55,16 @@ class AdminPluginSerializer < ApplicationSerializer
object.enabled_site_setting
end
def plugin_settings
@plugin_settings ||= SiteSetting.plugins.select { |_, v| v == id }
end
def has_settings
SiteSetting.plugins.values.include?(id)
plugin_settings.values.any?
end
def has_only_enabled_setting
plugin_settings.keys.length == 1 && plugin_settings.keys.first == enabled_setting
end
def include_url?

1
config/locales/client.en.yml
View File

@ -5413,6 +5413,7 @@ en:
is_enabled: "Y"
not_enabled: "N"
change_settings_short: "Settings"
settings_disabled: "This plugin can only be enabled or disabled, it has no additional settings."
howto: "How do I install plugins?"
official: "Official Discourse Plugin"
broken_route: "Unable to configure link to '%{name}'. Ensure ad-blockers are disabled and try reloading the page."

78
spec/serializers/admin_plugin_serializer_spec.rb
View File

@ -3,7 +3,74 @@
RSpec.describe AdminPluginSerializer do
subject(:serializer) { described_class.new(instance) }
let(:instance) { Plugin::Instance.new }
let(:all_test_plugins) { Plugin::Instance.find_all("#{Rails.root}/spec/fixtures/plugins") }
let(:instance) { all_test_plugins.find { |plugin| plugin.name == "color_definition" } }
describe "admin_route" do
it "returns the correct values when use_new_show_route is false" do
instance.expects(:admin_route).returns(
location: "admin",
label: "admin.test",
use_new_show_route: false,
)
expect(serializer.admin_route).to eq(
location: "admin",
label: "admin.test",
full_location: "adminPlugins.admin",
)
end
it "returns the correct values when use_new_show_route is true" do
instance.expects(:admin_route).returns(
location: "admin",
label: "admin.test",
use_new_show_route: true,
)
expect(serializer.admin_route).to eq(
location: "admin",
label: "admin.test",
full_location: "adminPlugins.show",
use_new_show_route: true,
)
end
end
describe "has_settings" do
it "is false for plugins with no settings" do
expect(described_class.new(instance).has_settings).to eq(false)
end
it "is true for plugins with settings" do
SiteSetting.expects(:plugins).returns(
{
"color_definition_enabled" => "color_definition",
"color_definition_api_key" => "color_definition",
},
)
expect(described_class.new(instance).has_settings).to eq(true)
end
end
describe "has_only_enabled_settings" do
it "is false for plugins with no settings" do
expect(described_class.new(instance).has_settings).to eq(false)
end
it "is true if only enabled_site_setting is present for the plugin" do
SiteSetting.expects(:plugins).returns({ "color_definition_enabled" => "color_definition" })
expect(described_class.new(instance).has_settings).to eq(true)
end
it "is false if there are other settings for the plugin" do
SiteSetting.expects(:plugins).returns(
{
"color_definition_enabled" => "color_definition",
"color_definition_api_key" => "color_definition",
},
)
expect(described_class.new(instance).has_only_enabled_setting).to eq(false)
end
end
describe "enabled_setting" do
it "should return the right value" do
@ -14,15 +81,14 @@ RSpec.describe AdminPluginSerializer do
describe "commit_hash" do
it "should return commit_hash and commit_url" do
instance = Plugin::Instance.find_all("#{Rails.root}/spec/fixtures/plugins")[0]
subject = described_class.new(instance)
git_repo = instance.git_repo
git_repo.stubs(:latest_local_commit).returns("123456")
git_repo.stubs(:url).returns("http://github.com/discourse/discourse-plugin")
expect(subject.commit_hash).to eq("123456")
expect(subject.commit_url).to eq("http://github.com/discourse/discourse-plugin/commit/123456")
expect(serializer.commit_hash).to eq("123456")
expect(serializer.commit_url).to eq(
"http://github.com/discourse/discourse-plugin/commit/123456",
)
end
end
end