Improve route error handling in admin/plugins (#18911)

Previously if a specific plugin route was not available (e.g. there was an error loading the plugin's JS due to an ad blocker), the entire page would fail to load. This commit updates the behavior to catch this kind of issue and display a user-friendly message at the top of the screen.
This commit is contained in:
David Taylor 2022-11-07 16:39:27 +00:00 committed by GitHub
parent fd207f8730
commit 782f43cc55
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 85 additions and 7 deletions

View File

@ -1,17 +1,27 @@
import { action } from "@ember/object";
import Controller from "@ember/controller";
import discourseComputed from "discourse-common/utils/decorators";
import { inject as service } from "@ember/service";
export default Controller.extend({
@discourseComputed
adminRoutes() {
router: service(),
get adminRoutes() {
return this.allAdminRoutes.filter((r) => this.routeExists(r.full_location));
},
get brokenAdminRoutes() {
return this.allAdminRoutes.filter(
(r) => !this.routeExists(r.full_location)
);
},
get allAdminRoutes() {
return this.model
.filter((p) => p?.enabled)
.map((p) => {
if (p.get("enabled")) {
return p.admin_route;
}
return p.admin_route;
})
.compact();
.filter(Boolean);
},
@action
@ -21,4 +31,13 @@ export default Controller.extend({
adminDetail.classList.toggle(state);
});
},
routeExists(routeName) {
try {
this.router.urlFor(routeName);
return true;
} catch (e) {
return false;
}
},
});

View File

@ -19,5 +19,11 @@
</div>
<div class="admin-detail pull-left mobile-closed">
{{#each this.brokenAdminRoutes as |route|}}
<div class="alert alert-error">
{{i18n "admin.plugins.broken_route" name=(i18n route.label)}}
</div>
{{/each}}
{{outlet}}
</div>

View File

@ -0,0 +1,51 @@
import {
acceptance,
exists,
query,
} from "discourse/tests/helpers/qunit-helpers";
import { visit } from "@ember/test-helpers";
import { test } from "qunit";
acceptance("Admin - Plugins", function (needs) {
needs.user();
needs.pretender((server, helper) => {
server.get("/admin/plugins", () =>
helper.response({
plugins: [
{
id: "some-test-plugin",
name: "some-test-plugin",
about: "Plugin description",
version: "0.1",
url: "https://example.com",
admin_route: {
location: "testlocation",
label: "test.plugin.label",
full_location: "adminPlugins.testlocation",
},
enabled: true,
enabled_setting: "testplugin_enabled",
has_settings: true,
is_official: true,
},
],
})
);
});
test("shows plugin list", async function (assert) {
await visit("/admin/plugins");
const table = query("table.admin-plugins");
assert.strictEqual(
table.querySelector("tr .plugin-name .name").innerText,
"some-test-plugin",
"displays the plugin in the table"
);
assert.true(
exists(".admin-plugins .admin-detail .alert-error"),
"displays an error for unknown routes"
);
});
});

View File

@ -19,6 +19,7 @@ $love: #fa6c8d !default;
@import "common/foundation/mixins";
@import "desktop";
@import "color_definitions";
@import "admin";
#ember-testing-container {
box-sizing: border-box;

View File

@ -4639,6 +4639,7 @@ en:
change_settings_short: "Settings"
howto: "How do I install plugins?"
official: "Official Plugin"
broken_route: "Unable to configure link to '%{name}'. Ensure ad-blockers are disabled and try reloading the page."
backups:
title: "Backups"