From fa58eea64e6dcd7d3b5c94b517b8c24b5c271ef1 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Mon, 12 Sep 2022 14:05:21 +0200 Subject: [PATCH] DEV: Minor code cleanup (#18225) Various small changes made while debugging MessageBus-related tests. --- .../app/components/basic-topic-list.js | 6 +--- .../app/components/topic-list-item.js | 9 ++--- .../discourse/app/initializers/banner.js | 10 +++--- .../discourse/app/initializers/logout.js | 32 +++++++++-------- .../discourse/app/initializers/read-only.js | 2 +- .../subscribe-user-notifications.js | 10 +++--- .../app/initializers/welcome-topic-banner.js | 2 +- .../app/lib/desktop-notifications.js | 30 ++++++++-------- .../javascripts/discourse/app/models/user.js | 36 ++++++++++--------- .../app/services/pm-topic-tracking-state.js | 7 ++-- .../discourse/app/services/presence.js | 6 ++-- .../initializers/new-user-narrative.js | 10 +++--- .../initializers/extend-for-poll.js | 8 ++--- 13 files changed, 82 insertions(+), 86 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/basic-topic-list.js b/app/assets/javascripts/discourse/app/components/basic-topic-list.js index 14aa4d5f340..ca8fae33180 100644 --- a/app/assets/javascripts/discourse/app/components/basic-topic-list.js +++ b/app/assets/javascripts/discourse/app/components/basic-topic-list.js @@ -50,11 +50,7 @@ export default Component.extend({ `.indicator-topic-${data.topic_id}` ).classList; - if (data.show_indicator) { - nodeClassList.remove("read"); - } else { - nodeClassList.add("read"); - } + nodeClassList.toggle("read", !data.show_indicator); }); } }); diff --git a/app/assets/javascripts/discourse/app/components/topic-list-item.js b/app/assets/javascripts/discourse/app/components/topic-list-item.js index 391f79789f4..972e8241732 100644 --- a/app/assets/javascripts/discourse/app/components/topic-list-item.js +++ b/app/assets/javascripts/discourse/app/components/topic-list-item.js @@ -82,11 +82,7 @@ export default Component.extend({ `.indicator-topic-${data.topic_id}` ).classList; - if (data.show_indicator) { - nodeClassList.remove("read"); - } else { - nodeClassList.add("read"); - } + nodeClassList.toggle("read", !data.show_indicator); }); } @@ -95,8 +91,7 @@ export default Component.extend({ const rawTopicLink = this.element.querySelector(".raw-topic-link"); rawTopicLink && - topicTitleDecorators && - topicTitleDecorators.forEach((cb) => + topicTitleDecorators?.forEach((cb) => cb(this.topic, rawTopicLink, "topic-list-item-title") ); } diff --git a/app/assets/javascripts/discourse/app/initializers/banner.js b/app/assets/javascripts/discourse/app/initializers/banner.js index c1c2b97532c..f766003b4b1 100644 --- a/app/assets/javascripts/discourse/app/initializers/banner.js +++ b/app/assets/javascripts/discourse/app/initializers/banner.js @@ -6,14 +6,14 @@ export default { after: "message-bus", initialize(container) { - const banner = EmberObject.create(PreloadStore.get("banner") || {}), - site = container.lookup("service:site"); + const site = container.lookup("service:site"); + const banner = EmberObject.create(PreloadStore.get("banner") || {}); + const messageBus = container.lookup("service:message-bus"); site.set("banner", banner); - const messageBus = container.lookup("service:message-bus"); - messageBus.subscribe("/site/banner", function (ban) { - site.set("banner", EmberObject.create(ban || {})); + messageBus.subscribe("/site/banner", (data) => { + site.set("banner", EmberObject.create(data || {})); }); }, }; diff --git a/app/assets/javascripts/discourse/app/initializers/logout.js b/app/assets/javascripts/discourse/app/initializers/logout.js index a382893f96d..2bc7a3822fc 100644 --- a/app/assets/javascripts/discourse/app/initializers/logout.js +++ b/app/assets/javascripts/discourse/app/initializers/logout.js @@ -11,22 +11,24 @@ export default { initialize(container) { const messageBus = container.lookup("service:message-bus"); - messageBus.subscribe("/logout", function () { - if (!_showingLogout) { - _showingLogout = true; - - bootbox.dialog( - I18n.t("logout"), - { - label: I18n.t("home"), - callback: logout, - }, - { - onEscape: logout, - backdrop: "static", - } - ); + messageBus.subscribe("/logout", () => { + if (_showingLogout) { + return; } + + _showingLogout = true; + + bootbox.dialog( + I18n.t("logout"), + { + label: I18n.t("home"), + callback: logout, + }, + { + onEscape: logout, + backdrop: "static", + } + ); }); }, }; diff --git a/app/assets/javascripts/discourse/app/initializers/read-only.js b/app/assets/javascripts/discourse/app/initializers/read-only.js index 6932aac78b6..7ab4ddb9a3f 100644 --- a/app/assets/javascripts/discourse/app/initializers/read-only.js +++ b/app/assets/javascripts/discourse/app/initializers/read-only.js @@ -6,7 +6,7 @@ export default { initialize(container) { const messageBus = container.lookup("service:message-bus"); const site = container.lookup("service:site"); - messageBus.subscribe("/site/read-only", function (enabled) { + messageBus.subscribe("/site/read-only", (enabled) => { site.set("isReadOnly", enabled); }); }, diff --git a/app/assets/javascripts/discourse/app/initializers/subscribe-user-notifications.js b/app/assets/javascripts/discourse/app/initializers/subscribe-user-notifications.js index 1e17c8eb16a..6b4d15997e0 100644 --- a/app/assets/javascripts/discourse/app/initializers/subscribe-user-notifications.js +++ b/app/assets/javascripts/discourse/app/initializers/subscribe-user-notifications.js @@ -38,7 +38,7 @@ export default { }); bus.subscribe( - `/notification/${user.get("id")}`, + `/notification/${user.id}`, (data) => { const store = container.lookup("service:store"); const oldUnread = user.unread_notifications; @@ -76,10 +76,9 @@ export default { {}, { cacheKey: "recent-notifications" } ); - const lastNotification = - data.last_notification && data.last_notification.notification; + const lastNotification = data.last_notification?.notification; - if (stale && stale.hasResults && lastNotification) { + if (stale?.hasResults && lastNotification) { const oldNotifications = stale.results.get("content"); const staleIndex = oldNotifications.findIndex( (n) => n.id === lastNotification.id @@ -115,6 +114,7 @@ export default { } }) .filter(Boolean); + stale.results.set("content", newNotifications); } }, @@ -153,6 +153,7 @@ export default { } return site.updateCategory(c); }); + (data.deleted_categories || []).forEach((id) => site.removeCategory(id) ); @@ -166,6 +167,7 @@ export default { bus.subscribe(alertChannel(user), (data) => onNotification(data, siteSettings, user) ); + initDesktopNotifications(bus, appEvents); if (isPushNotificationsEnabled(user)) { diff --git a/app/assets/javascripts/discourse/app/initializers/welcome-topic-banner.js b/app/assets/javascripts/discourse/app/initializers/welcome-topic-banner.js index cbf9cfb8fdb..e161df53d38 100644 --- a/app/assets/javascripts/discourse/app/initializers/welcome-topic-banner.js +++ b/app/assets/javascripts/discourse/app/initializers/welcome-topic-banner.js @@ -7,7 +7,7 @@ export default { if (site.show_welcome_topic_banner) { const messageBus = container.lookup("service:message-bus"); - messageBus.subscribe("/site/welcome-topic-banner", function (disabled) { + messageBus.subscribe("/site/welcome-topic-banner", (disabled) => { site.set("show_welcome_topic_banner", disabled); }); } diff --git a/app/assets/javascripts/discourse/app/lib/desktop-notifications.js b/app/assets/javascripts/discourse/app/lib/desktop-notifications.js index 4df01460ddd..2dcf4ea7559 100644 --- a/app/assets/javascripts/discourse/app/lib/desktop-notifications.js +++ b/app/assets/javascripts/discourse/app/lib/desktop-notifications.js @@ -144,7 +144,7 @@ function isIdle() { } // Call-in point from message bus -function onNotification(data, siteSettings, user) { +async function onNotification(data, siteSettings, user) { if (!liveEnabled) { return; } @@ -177,22 +177,22 @@ function onNotification(data, siteSettings, user) { const notificationTag = "discourse-notification-" + siteSettings.title + "-" + data.topic_id; - requestPermission().then(function () { - // This shows the notification! - const notification = new Notification(notificationTitle, { - body: notificationBody, - icon: notificationIcon, - tag: notificationTag, - }); - notification.onclick = () => { - DiscourseURL.routeTo(data.post_url); - notification.close(); - }; + await requestPermission(); - desktopNotificationHandlers.forEach((handler) => - handler(data, siteSettings, user) - ); + // This shows the notification! + const notification = new Notification(notificationTitle, { + body: notificationBody, + icon: notificationIcon, + tag: notificationTag, }); + notification.onclick = () => { + DiscourseURL.routeTo(data.post_url); + notification.close(); + }; + + desktopNotificationHandlers.forEach((handler) => + handler(data, siteSettings, user) + ); } // Utility function diff --git a/app/assets/javascripts/discourse/app/models/user.js b/app/assets/javascripts/discourse/app/models/user.js index 64b1077a98b..de55bd393ca 100644 --- a/app/assets/javascripts/discourse/app/models/user.js +++ b/app/assets/javascripts/discourse/app/models/user.js @@ -581,25 +581,27 @@ const User = RestModel.extend({ }); }, - loadUserAction(id) { - const stream = this.stream; - return ajax(`/user_actions/${id}.json`).then((result) => { - if (result && result.user_action) { - const ua = result.user_action; + async loadUserAction(id) { + const result = await ajax(`/user_actions/${id}.json`); - if ((this.get("stream.filter") || ua.action_type) !== ua.action_type) { - return; - } - if (!this.get("stream.filter") && !this.inAllStream(ua)) { - return; - } + if (!result?.user_action) { + return; + } - ua.title = emojiUnescape(escapeExpression(ua.title)); - const action = UserAction.collapseStream([UserAction.create(ua)]); - stream.set("itemsLoaded", stream.get("itemsLoaded") + 1); - stream.get("content").insertAt(0, action[0]); - } - }); + const ua = result.user_action; + + if ((this.get("stream.filter") || ua.action_type) !== ua.action_type) { + return; + } + + if (!this.get("stream.filter") && !this.inAllStream(ua)) { + return; + } + + ua.title = emojiUnescape(escapeExpression(ua.title)); + const action = UserAction.collapseStream([UserAction.create(ua)]); + this.stream.set("itemsLoaded", this.stream.get("itemsLoaded") + 1); + this.stream.get("content").insertAt(0, action[0]); }, inAllStream(ua) { diff --git a/app/assets/javascripts/discourse/app/services/pm-topic-tracking-state.js b/app/assets/javascripts/discourse/app/services/pm-topic-tracking-state.js index 66165b6a295..1afefabbff1 100644 --- a/app/assets/javascripts/discourse/app/services/pm-topic-tracking-state.js +++ b/app/assets/javascripts/discourse/app/services/pm-topic-tracking-state.js @@ -51,15 +51,12 @@ const PrivateMessageTopicTrackingState = Service.extend({ }, _establishChannels() { - this.messageBus.subscribe( - this.userChannel(), - this._processMessage.bind(this) - ); + this.messageBus.subscribe(this.userChannel(), this._processMessage); this.currentUser.groupsWithMessages?.forEach((group) => { this.messageBus.subscribe( this.groupChannel(group.id), - this._processMessage.bind(this) + this._processMessage ); }); }, diff --git a/app/assets/javascripts/discourse/app/services/presence.js b/app/assets/javascripts/discourse/app/services/presence.js index d3742eb0176..aef41ea1bad 100644 --- a/app/assets/javascripts/discourse/app/services/presence.js +++ b/app/assets/javascripts/discourse/app/services/presence.js @@ -213,10 +213,10 @@ class PresenceChannelState extends EmberObject.extend(Evented) { await this._resubscribe(); return; - } else { - this.lastSeenId = message_id; } + this.lastSeenId = message_id; + if (this.countOnly && data.count_delta !== undefined) { this.set("count", this.count + data.count_delta); this.trigger("change"); @@ -228,11 +228,13 @@ class PresenceChannelState extends EmberObject.extend(Evented) { const users = data.entering_users.map((u) => User.create(u)); this.users.addObjects(users); } + if (data.leaving_user_ids) { const leavingIds = new Set(data.leaving_user_ids); const toRemove = this.users.filter((u) => leavingIds.has(u.id)); this.users.removeObjects(toRemove); } + this.set("count", this.users.length); this.trigger("change"); } else { diff --git a/plugins/discourse-narrative-bot/assets/javascripts/initializers/new-user-narrative.js b/plugins/discourse-narrative-bot/assets/javascripts/initializers/new-user-narrative.js index 2df37a4124b..267ed5e44a1 100644 --- a/plugins/discourse-narrative-bot/assets/javascripts/initializers/new-user-narrative.js +++ b/plugins/discourse-narrative-bot/assets/javascripts/initializers/new-user-narrative.js @@ -46,21 +46,21 @@ function initialize(api) { subscribe() { this._super(...arguments); - this.messageBus.subscribe(`/topic/${this.get("model.id")}`, (data) => { + this.messageBus.subscribe(`/topic/${this.model.id}`, (data) => { const topic = this.model; // scroll only for discobot (-2 is discobot id) if ( - topic.get("isPrivateMessage") && + topic.isPrivateMessage && this.currentUser && - this.currentUser.get("id") !== data.user_id && + this.currentUser.id !== data.user_id && data.user_id === -2 && data.type === "created" ) { const postNumber = data.post_number; const notInPostStream = topic.get("highest_post_number") <= postNumber; - const postNumberDifference = postNumber - topic.get("currentPost"); + const postNumberDifference = postNumber - topic.currentPost; if ( notInPostStream && @@ -116,7 +116,7 @@ function initialize(api) { } export default { - name: "new-user-narratve", + name: "new-user-narrative", initialize(container) { const siteSettings = container.lookup("service:site-settings"); diff --git a/plugins/poll/assets/javascripts/initializers/extend-for-poll.js b/plugins/poll/assets/javascripts/initializers/extend-for-poll.js index 22dba118c4e..cd04356b1a0 100644 --- a/plugins/poll/assets/javascripts/initializers/extend-for-poll.js +++ b/plugins/poll/assets/javascripts/initializers/extend-for-poll.js @@ -31,15 +31,15 @@ function initializePolls(api) { api.modifyClass("controller:topic", { pluginId: PLUGIN_ID, + subscribe() { this._super(...arguments); - this.messageBus.subscribe("/polls/" + this.get("model.id"), (msg) => { + this.messageBus.subscribe(`/polls/${this.model.id}`, (msg) => { const post = this.get("model.postStream").findLoadedPost(msg.post_id); - if (post) { - post.set("polls", msg.polls); - } + post?.set("polls", msg.polls); }); }, + unsubscribe() { this.messageBus.unsubscribe("/polls/*"); this._super(...arguments);