From 28078d78e2b5a5753bb084483c7afb4ab2daa756 Mon Sep 17 00:00:00 2001 From: Isaac Janzen <50783505+janzenisaac@users.noreply.github.com> Date: Thu, 12 Jan 2023 12:22:11 -0600 Subject: [PATCH] DEV: Make 'username' optional for bookmark notifications (#19851) Data Explorer queries have a `user_id` assigned to each query created. DE Reports can be bookmarked for later reference. When creating the bookmark notification there was the possibility of a notification error being thrown (that made the notification menu inaccessible) due to a DE Query not having a owner (associated user_id). This can happen in a couple ways: - having a query created by a user that was then later deleted leaving the query without ownership - having a TA create a query for a customer using a temporary account, that would then later be deleted leaving the query without ownership Since there is a case that `bookmark.user` is not valid the PR makes the `bookmark.user.username` optional for a bookmark notification. As [tested](https://github.com/discourse/discourse/pull/19851/files#diff-5b5154de37f96988d551feff6f1dfe5ba804fbcbc1c33b5478dde02a447a634f) in the case a username is not present, we will still render the `content` of the notification minus the username. This creates a safe fallback when looking up non-valid users. --- .../app/widgets/quick-access-bookmarks.js | 2 +- .../components/widgets/quick-access-item-test.js | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/discourse/app/widgets/quick-access-bookmarks.js b/app/assets/javascripts/discourse/app/widgets/quick-access-bookmarks.js index c7ec173d1e..e58b40f2f7 100644 --- a/app/assets/javascripts/discourse/app/widgets/quick-access-bookmarks.js +++ b/app/assets/javascripts/discourse/app/widgets/quick-access-bookmarks.js @@ -70,7 +70,7 @@ createWidgetFrom(QuickAccessPanel, "quick-access-bookmarks", { href, title: bookmark.name, content: bookmark.title, - username: bookmark.user.username, + username: bookmark.user?.username, }); }, diff --git a/app/assets/javascripts/discourse/tests/integration/components/widgets/quick-access-item-test.js b/app/assets/javascripts/discourse/tests/integration/components/widgets/quick-access-item-test.js index 9e3dbb2616..4b17281f38 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/widgets/quick-access-item-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/widgets/quick-access-item-test.js @@ -32,5 +32,21 @@ module( const contentDiv = query(CONTENT_DIV_SELECTOR); assert.strictEqual(contentDiv.innerText, '"quote"'); }); + + test("Renders the notification content with no username when username is not present", async function (assert) { + this.set("args", { + content: "content", + username: undefined, + }); + + await render( + hbs`` + ); + + const contentDiv = query(CONTENT_DIV_SELECTOR); + const usernameSpan = query("li a div span"); + assert.strictEqual(contentDiv.innerText, "content"); + assert.strictEqual(usernameSpan.innerText, ""); + }); } );