DEV: Improve critical deprecation warnings (#34809)

- Enhance deprecation handling by tracking IDs in a set to prevent
duplicate notices for the same issue.
- Update deprecation message IDs to dynamically generate unique DOM
identifiers.
- Adjust the system specs to validate multiple deprecation messages and
ensure accurate coverage.

This ensures that multiple critical warning notices are displayed for
admins when necessary.
This commit is contained in:
Sérgio Saquetim
2025-09-16 16:16:45 -03:00
committed by GitHub
parent e07d976d0d
commit ca4e5654fa
2 changed files with 20 additions and 12 deletions
@@ -3,6 +3,7 @@ import { registerDeprecationHandler } from "@ember/debug";
import Service, { service } from "@ember/service";
import { addGlobalNotice } from "discourse/components/global-notice";
import DeprecationWorkflow from "discourse/deprecation-workflow";
import dasherize from "discourse/helpers/dasherize";
import { bind } from "discourse/lib/decorators";
import { registerDeprecationHandler as registerDiscourseDeprecationHandler } from "discourse/lib/deprecated";
import identifySource from "discourse/lib/source-identifier";
@@ -40,7 +41,7 @@ const REPLACEMENT_URLS = {};
if (DEBUG) {
// used in system specs
CRITICAL_DEPRECATIONS.push("fake-deprecation");
CRITICAL_DEPRECATIONS.push(/fake-deprecation.*/);
}
// Deprecation handling APIs don't have any way to unregister handlers, so we set up permanent
@@ -58,7 +59,7 @@ export default class DeprecationWarningHandler extends Service {
@service currentUser;
@service siteSettings;
#adminWarned = false;
#adminWarned = new Set();
constructor() {
super(...arguments);
@@ -84,10 +85,6 @@ export default class DeprecationWarningHandler extends Service {
}
maybeNotifyAdmin(opts, source) {
if (this.#adminWarned) {
return;
}
if (!this.currentUser?.admin) {
return;
}
@@ -110,12 +107,16 @@ export default class DeprecationWarningHandler extends Service {
}
notifyAdmin({ id, url }, source) {
if (this.#adminWarned.has(id)) {
return;
}
this.#adminWarned.add(id);
if (REPLACEMENT_URLS[id]) {
url = REPLACEMENT_URLS[id];
}
this.#adminWarned = true;
let sourceString;
if (source?.type === "theme") {
sourceString = i18n("critical_deprecation.theme_source", {
@@ -146,7 +147,7 @@ export default class DeprecationWarningHandler extends Service {
notice += " " + this.siteSettings.warn_critical_js_deprecations_message;
}
addGlobalNotice(notice, "critical-deprecation", {
addGlobalNotice(notice, `critical-deprecation--${dasherize(id)}`, {
dismissable: true,
dismissDuration: moment.duration(1, "day"),
level: "warn",
+10 -3
View File
@@ -39,11 +39,18 @@ describe "JS Deprecation Handling", type: :system do
page.execute_script <<~JS
const deprecated = require("discourse/lib/deprecated").default;
deprecated("Fake deprecation message", { id: "fake-deprecation" })
deprecated("Fake deprecation message", { id: "fake-deprecation1" })
deprecated("Other fake deprecation message", { id: "fake-deprecation2" })
JS
message = find("#global-notice-critical-deprecation")
message = find("#global-notice-critical-deprecation--fake-deprecation1")
expect(message).to have_text("One of your themes or plugins contains code which needs updating")
expect(message).to have_text("fake-deprecation1")
expect(message).to have_text(SiteSetting.warn_critical_js_deprecations_message)
message = find("#global-notice-critical-deprecation--fake-deprecation2")
expect(message).to have_text("One of your themes or plugins contains code which needs updating")
expect(message).to have_text("fake-deprecation2")
expect(message).to have_text(SiteSetting.warn_critical_js_deprecations_message)
end
@@ -70,6 +77,6 @@ describe "JS Deprecation Handling", type: :system do
visit "/latest"
expect(page).to have_css("#global-notice-critical-deprecation")
expect(page).to have_css("#global-notice-critical-deprecation--fake-deprecation")
end
end