From 97f094e5ee709018fa81dd0d3d3b996f8caf2725 Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Fri, 26 Aug 2022 00:44:06 +0300 Subject: [PATCH] DEV: Apply model transformer API on more models (#18087) This commit extends the plugin API introduced in https://github.com/discourse/discourse/commit/40fd82e2d196db955334582cae6040ba5c1fa6fd to the `Bookmark` and `Notification` models. It also refactors the code that's responsible for loading items in the experimental menu to use `async`...`await` instead of `Promise`s. --- .../components/user-menu/bookmarks-list.js | 50 +++++++++++-------- .../app/components/user-menu/items-list.js | 27 ++++++---- .../app/components/user-menu/messages-list.js | 43 ++++++++-------- .../user-menu/notifications-list.js | 25 +++++----- .../discourse/app/lib/plugin-api.js | 2 +- .../discourse/app/models/bookmark.js | 5 ++ .../discourse/app/models/notification.js | 5 ++ .../tests/acceptance/user-menu-test.js | 49 ++++++++++++++++++ .../tests/fixtures/notification-fixtures.js | 2 + 9 files changed, 143 insertions(+), 65 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/user-menu/bookmarks-list.js b/app/assets/javascripts/discourse/app/components/user-menu/bookmarks-list.js index add6540322..b9f3716215 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/bookmarks-list.js +++ b/app/assets/javascripts/discourse/app/components/user-menu/bookmarks-list.js @@ -5,6 +5,7 @@ import showModal from "discourse/lib/show-modal"; import I18n from "I18n"; import UserMenuNotificationItem from "discourse/lib/user-menu/notification-item"; import UserMenuBookmarkItem from "discourse/lib/user-menu/bookmark-item"; +import Bookmark from "discourse/models/bookmark"; export default class UserMenuBookmarksList extends UserMenuNotificationsList { get dismissTypes() { @@ -44,29 +45,34 @@ export default class UserMenuBookmarksList extends UserMenuNotificationsList { return this.currentUser.get(key) || 0; } - fetchItems() { - return ajax(`/u/${this.currentUser.username}/user-menu-bookmarks`).then( - (data) => { - const content = []; - data.notifications.forEach((rawNotification) => { - const notification = Notification.create(rawNotification); - content.push( - new UserMenuNotificationItem({ - notification, - currentUser: this.currentUser, - siteSettings: this.siteSettings, - site: this.site, - }) - ); - }); - content.push( - ...data.bookmarks.map((bookmark) => { - return new UserMenuBookmarkItem({ bookmark }); - }) - ); - return content; - } + async fetchItems() { + const data = await ajax( + `/u/${this.currentUser.username}/user-menu-bookmarks` ); + const content = []; + + const notifications = data.notifications.map((n) => Notification.create(n)); + await Notification.applyTransformations(notifications); + notifications.forEach((notification) => { + content.push( + new UserMenuNotificationItem({ + notification, + currentUser: this.currentUser, + siteSettings: this.siteSettings, + site: this.site, + }) + ); + }); + + const bookmarks = data.bookmarks.map((b) => Bookmark.create(b)); + await Bookmark.applyTransformations(bookmarks); + content.push( + ...bookmarks.map((bookmark) => { + return new UserMenuBookmarkItem({ bookmark }); + }) + ); + + return content; } dismissWarningModal() { diff --git a/app/assets/javascripts/discourse/app/components/user-menu/items-list.js b/app/assets/javascripts/discourse/app/components/user-menu/items-list.js index 7ab92d8fab..0c5681b6d5 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/items-list.js +++ b/app/assets/javascripts/discourse/app/components/user-menu/items-list.js @@ -28,33 +28,40 @@ export default class UserMenuItemsList extends Component { return "user-menu/items-list-empty-state"; } - fetchItems() { + async fetchItems() { throw new Error( `the fetchItems method must be implemented in ${this.constructor.name}` ); } - refreshList() { - this.#load(); + async refreshList() { + await this.#load(); } dismissWarningModal() { return null; } - #load() { + async #load() { const cached = this.#getCachedItems(); if (cached?.length) { this.items = cached; } else { this.loading = true; } - this.fetchItems() - .then((items) => { - this.#setCachedItems(items); - this.items = items; - }) - .finally(() => (this.loading = false)); + try { + const items = await this.fetchItems(); + this.#setCachedItems(items); + this.items = items; + } catch (err) { + // eslint-disable-next-line no-console + console.error( + `an error occurred when loading items for ${this.constructor.name}`, + err + ); + } finally { + this.loading = false; + } } #getCachedItems() { diff --git a/app/assets/javascripts/discourse/app/components/user-menu/messages-list.js b/app/assets/javascripts/discourse/app/components/user-menu/messages-list.js index 241cc94eb1..3fe2800da7 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/messages-list.js +++ b/app/assets/javascripts/discourse/app/components/user-menu/messages-list.js @@ -45,31 +45,34 @@ export default class UserMenuMessagesList extends UserMenuNotificationsList { return this.currentUser.get(key) || 0; } - fetchItems() { - return ajax( + async fetchItems() { + const data = await ajax( `/u/${this.currentUser.username}/user-menu-private-messages` - ).then(async (data) => { - const content = []; - data.notifications.forEach((rawNotification) => { - const notification = Notification.create(rawNotification); - content.push( - new UserMenuNotificationItem({ - notification, - currentUser: this.currentUser, - siteSettings: this.siteSettings, - site: this.site, - }) - ); - }); - const topics = data.topics.map((t) => Topic.create(t)); - await Topic.applyTransformations(topics); + ); + const content = []; + + const notifications = data.notifications.map((n) => Notification.create(n)); + await Notification.applyTransformations(notifications); + notifications.forEach((notification) => { content.push( - ...topics.map((topic) => { - return new UserMenuMessageItem({ message: topic }); + new UserMenuNotificationItem({ + notification, + currentUser: this.currentUser, + siteSettings: this.siteSettings, + site: this.site, }) ); - return content; }); + + const topics = data.topics.map((t) => Topic.create(t)); + await Topic.applyTransformations(topics); + content.push( + ...topics.map((topic) => { + return new UserMenuMessageItem({ message: topic }); + }) + ); + + return content; } dismissWarningModal() { diff --git a/app/assets/javascripts/discourse/app/components/user-menu/notifications-list.js b/app/assets/javascripts/discourse/app/components/user-menu/notifications-list.js index 78fe44213a..2c304d4ec9 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/notifications-list.js +++ b/app/assets/javascripts/discourse/app/components/user-menu/notifications-list.js @@ -6,6 +6,7 @@ import { postRNWebviewMessage } from "discourse/lib/utilities"; import showModal from "discourse/lib/show-modal"; import { inject as service } from "@ember/service"; import UserMenuNotificationItem from "discourse/lib/user-menu/notification-item"; +import Notification from "discourse/models/notification"; export default class UserMenuNotificationsList extends UserMenuItemsList { @service currentUser; @@ -54,7 +55,7 @@ export default class UserMenuNotificationsList extends UserMenuItemsList { } } - fetchItems() { + async fetchItems() { const params = { limit: 30, recent: true, @@ -67,19 +68,19 @@ export default class UserMenuNotificationsList extends UserMenuItemsList { params.filter_by_types = types.join(","); params.silent = true; } - return this.store + const collection = await this.store .findStale("notification", params) - .refresh() - .then((c) => { - return c.content.map((notification) => { - return new UserMenuNotificationItem({ - notification, - currentUser: this.currentUser, - siteSettings: this.siteSettings, - site: this.site, - }); - }); + .refresh(); + const notifications = collection.content; + await Notification.applyTransformations(notifications); + return notifications.map((notification) => { + return new UserMenuNotificationItem({ + notification, + currentUser: this.currentUser, + siteSettings: this.siteSettings, + site: this.site, }); + }); } dismissWarningModal() { diff --git a/app/assets/javascripts/discourse/app/lib/plugin-api.js b/app/assets/javascripts/discourse/app/lib/plugin-api.js index 666d5624cc..8a72a19b5e 100644 --- a/app/assets/javascripts/discourse/app/lib/plugin-api.js +++ b/app/assets/javascripts/discourse/app/lib/plugin-api.js @@ -1952,7 +1952,7 @@ class PluginApi { * @callback registerModelTransformerCallback * @param {Object[]} A list of model instances * - * @param {string} modelName - Model type on which transformation should be applied. Currently the only valid type is "topic". + * @param {string} modelName - Model type on which transformation should be applied. Currently valid types are "topic", "notification" and "bookmark". * @param {registerModelTransformerCallback} transformer - Callback function that receives a list of model objects of the specified type and applies transformation on them. */ registerModelTransformer(modelName, transformer) { diff --git a/app/assets/javascripts/discourse/app/models/bookmark.js b/app/assets/javascripts/discourse/app/models/bookmark.js index ff8dda8a4e..f6c059713e 100644 --- a/app/assets/javascripts/discourse/app/models/bookmark.js +++ b/app/assets/javascripts/discourse/app/models/bookmark.js @@ -12,6 +12,7 @@ import getURL from "discourse-common/lib/get-url"; import { longDate } from "discourse/lib/formatter"; import { none } from "@ember/object/computed"; import { capitalize } from "@ember/string"; +import { applyModelTransformations } from "discourse/lib/model-transformers"; export const AUTO_DELETE_PREFERENCES = { NEVER: 0, @@ -168,6 +169,10 @@ Bookmark.reopenClass({ args.user = User.create(args.user); return this._super(args); }, + + async applyTransformations(bookmarks) { + await applyModelTransformations("bookmark", bookmarks); + }, }); export default Bookmark; diff --git a/app/assets/javascripts/discourse/app/models/notification.js b/app/assets/javascripts/discourse/app/models/notification.js index 6e917b9182..000aa0c10a 100644 --- a/app/assets/javascripts/discourse/app/models/notification.js +++ b/app/assets/javascripts/discourse/app/models/notification.js @@ -1,6 +1,11 @@ import RestModel from "discourse/models/rest"; import { tracked } from "@glimmer/tracking"; +import { applyModelTransformations } from "discourse/lib/model-transformers"; export default class Notification extends RestModel { + static async applyTransformations(notifications) { + await applyModelTransformations("notification", notifications); + } + @tracked read; } diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js index 72cbbc7311..3b8514cadf 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js @@ -189,6 +189,55 @@ acceptance("User menu", function (needs) { ); }); + test("notifications tab applies model transformations registered by plugins", async function (assert) { + withPluginApi("0.1", (api) => { + api.registerModelTransformer("notification", (notifications) => { + notifications.forEach((notification, index) => { + if (notification.fancy_title) { + notification.fancy_title = `pluginNotificationTransformer ${index} ${notification.fancy_title}`; + } + }); + }); + }); + + await visit("/"); + await click(".d-header-icons .current-user"); + + const notifications = queryAll( + "#quick-access-all-notifications ul li.notification" + ); + assert.strictEqual( + notifications[0].textContent.replace(/\s+/g, " ").trim(), + "velesin pluginNotificationTransformer 0 edited topic 443" + ); + assert.strictEqual( + notifications[1].textContent.replace(/\s+/g, " ").trim(), + "velesin pluginNotificationTransformer 1 some title" + ); + }); + + test("bookmarks tab applies model transformations registered by plugins", async function (assert) { + withPluginApi("0.1", (api) => { + api.registerModelTransformer("bookmark", (bookmarks) => { + bookmarks.forEach((bookmark) => { + if (bookmark.title) { + bookmark.title = `pluginBookmarkTransformer ${bookmark.title}`; + } + }); + }); + }); + + await visit("/"); + await click(".d-header-icons .current-user"); + await click("#user-menu-button-bookmarks"); + + const bookmarks = queryAll("#quick-access-bookmarks ul li.bookmark"); + assert.strictEqual( + bookmarks[0].textContent.replace(/\s+/g, " ").trim(), + "osama pluginBookmarkTransformer Test poll topic hello world" + ); + }); + test("messages tab applies model transformations registered by plugins", async function (assert) { withPluginApi("0.1", (api) => { api.registerModelTransformer("topic", (topics) => { diff --git a/app/assets/javascripts/discourse/tests/fixtures/notification-fixtures.js b/app/assets/javascripts/discourse/tests/fixtures/notification-fixtures.js index a3ff25c5d8..4097c2de19 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/notification-fixtures.js +++ b/app/assets/javascripts/discourse/tests/fixtures/notification-fixtures.js @@ -10,6 +10,7 @@ export default { post_number: 1, topic_id: 130, slug: "lorem-ipsum-dolor-sit-amet", + fancy_title: "edited topic 443", data: { topic_title: "edited topic 443", display_username: "velesin", @@ -26,6 +27,7 @@ export default { post_number: 1, topic_id: 1234, slug: "a-slug", + fancy_title: "some title", data: { topic_title: "some title", display_username: "velesin" }, }, {