From 473695ee4dac69e7e59a10f8698346ee6812f2bc Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Wed, 10 Aug 2022 08:25:39 +0300 Subject: [PATCH] DEV: Add messages tab to the new user menu (#17850) Some of the changes in this PR are extracted from https://github.com/discourse/discourse/pull/17379. Similar to the bookmarks tab in the new user menu, the messages tab also displays a mix of notifications and messages. When there are unread message notifications, the tab displays all of these notifications at the top and fills the remaining space in the menu with a list of the user's messages. The bubble/badge count on the messages tab indicates how many unread message notifications there are. --- .../app/components/user-menu/menu.js | 24 +++ .../app/components/user-menu/message-item.js | 42 ++++++ .../user-menu/message-notification-item.hbs | 1 + .../user-menu/message-notification-item.js | 12 ++ .../user-menu/messages-list-empty-state.hbs | 10 ++ .../app/components/user-menu/messages-list.js | 72 +++++++++ .../user-menu/notifications-list.js | 22 +++ .../tests/acceptance/user-menu-test.js | 85 +++++++++-- .../discourse/tests/fixtures/user-menu.js | 122 +++++++++++++++ .../user-menu/bookmark-item-test.js | 79 +++++----- .../components/user-menu/menu-test.js | 68 ++++++--- .../components/user-menu/message-item-test.js | 38 +++++ .../user-menu/messages-list-test.js | 87 +++++++++++ app/controllers/users_controller.rb | 83 +++++++++-- app/models/notification.rb | 3 + config/locales/client.en.yml | 5 + config/routes.rb | 1 + lib/topic_query/private_message_lists.rb | 4 +- spec/fabricators/post_fabricator.rb | 3 +- spec/requests/users_controller_spec.rb | 140 +++++++++++++++++- 20 files changed, 811 insertions(+), 90 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/components/user-menu/message-item.js create mode 100644 app/assets/javascripts/discourse/app/components/user-menu/message-notification-item.hbs create mode 100644 app/assets/javascripts/discourse/app/components/user-menu/message-notification-item.js create mode 100644 app/assets/javascripts/discourse/app/components/user-menu/messages-list-empty-state.hbs create mode 100644 app/assets/javascripts/discourse/app/components/user-menu/messages-list.js create mode 100644 app/assets/javascripts/discourse/tests/integration/components/user-menu/message-item-test.js create mode 100644 app/assets/javascripts/discourse/tests/integration/components/user-menu/messages-list-test.js diff --git a/app/assets/javascripts/discourse/app/components/user-menu/menu.js b/app/assets/javascripts/discourse/app/components/user-menu/menu.js index e75a48d77e8..55b184ce3f2 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/menu.js +++ b/app/assets/javascripts/discourse/app/components/user-menu/menu.js @@ -70,6 +70,30 @@ const CORE_TOP_TABS = [ } }, + class extends UserMenuTab { + get id() { + return "messages"; + } + + get icon() { + return "notification.private_message"; + } + + get panelComponent() { + return "user-menu/messages-list"; + } + + get count() { + return this.getUnreadCountForType("private_message"); + } + + get shouldDisplay() { + return ( + this.siteSettings.enable_personal_messages || this.currentUser.staff + ); + } + }, + class extends UserMenuTab { get id() { return "bookmarks"; diff --git a/app/assets/javascripts/discourse/app/components/user-menu/message-item.js b/app/assets/javascripts/discourse/app/components/user-menu/message-item.js new file mode 100644 index 00000000000..81cc8151c07 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/user-menu/message-item.js @@ -0,0 +1,42 @@ +import UserMenuItem from "discourse/components/user-menu/menu-item"; +import { postUrl } from "discourse/lib/utilities"; +import { htmlSafe } from "@ember/template"; +import I18n from "I18n"; + +export default class UserMenuMessageItem extends UserMenuItem { + get className() { + return "message"; + } + + get linkHref() { + const nextUnreadPostNumber = Math.min( + (this.message.last_read_post_number || 0) + 1, + this.message.highest_post_number + ); + return postUrl(this.message.slug, this.message.id, nextUnreadPostNumber); + } + + get linkTitle() { + return I18n.t("user.private_message"); + } + + get icon() { + return "notification.private_message"; + } + + get label() { + return this.message.last_poster_username; + } + + get description() { + return htmlSafe(this.message.fancy_title); + } + + get topicId() { + return this.message.id; + } + + get message() { + return this.args.item; + } +} diff --git a/app/assets/javascripts/discourse/app/components/user-menu/message-notification-item.hbs b/app/assets/javascripts/discourse/app/components/user-menu/message-notification-item.hbs new file mode 100644 index 00000000000..e0829c1c076 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/user-menu/message-notification-item.hbs @@ -0,0 +1 @@ +{{component this.component item=@item}} diff --git a/app/assets/javascripts/discourse/app/components/user-menu/message-notification-item.js b/app/assets/javascripts/discourse/app/components/user-menu/message-notification-item.js new file mode 100644 index 00000000000..c5eb793b150 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/user-menu/message-notification-item.js @@ -0,0 +1,12 @@ +import GlimmerComponent from "discourse/components/glimmer"; +import Notification from "discourse/models/notification"; + +export default class UserMenuMessageNotificationItem extends GlimmerComponent { + get component() { + if (this.args.item.constructor === Notification) { + return "user-menu/notification-item"; + } else { + return "user-menu/message-item"; + } + } +} diff --git a/app/assets/javascripts/discourse/app/components/user-menu/messages-list-empty-state.hbs b/app/assets/javascripts/discourse/app/components/user-menu/messages-list-empty-state.hbs new file mode 100644 index 00000000000..273dcc19e0c --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/user-menu/messages-list-empty-state.hbs @@ -0,0 +1,10 @@ +
+ + {{i18n "user.no_messages_title"}} + +
+

+ {{html-safe (i18n "user.no_messages_body" icon=(d-icon "envelope") aboutUrl=(get-url "/about"))}} +

+
+
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 new file mode 100644 index 00000000000..cfc7a83e986 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/user-menu/messages-list.js @@ -0,0 +1,72 @@ +import UserMenuNotificationsList from "discourse/components/user-menu/notifications-list"; +import { ajax } from "discourse/lib/ajax"; +import Notification from "discourse/models/notification"; +import showModal from "discourse/lib/show-modal"; +import I18n from "I18n"; + +export default class UserMenuMessagesList extends UserMenuNotificationsList { + get dismissTypes() { + return ["private_message"]; + } + + get showAllHref() { + return `${this.currentUser.path}/messages`; + } + + get showAllTitle() { + return I18n.t("user_menu.view_all_messages"); + } + + get showDismiss() { + return this.#unreadMessaagesNotifications > 0; + } + + get dismissTitle() { + return I18n.t("user.dismiss_messages_tooltip"); + } + + get itemsCacheKey() { + return "user-menu-messages-tab"; + } + + get itemComponent() { + return "user-menu/message-notification-item"; + } + + get emptyStateComponent() { + return "user-menu/messages-list-empty-state"; + } + + get #unreadMessaagesNotifications() { + const key = `grouped_unread_high_priority_notifications.${this.site.notification_types.private_message}`; + // we're retrieving the value with get() so that Ember tracks the property + // and re-renders the UI when it changes. + // we can stop using `get()` when the User model is refactored into native + // class with @tracked properties. + return this.currentUser.get(key) || 0; + } + + fetchItems() { + return ajax( + `/u/${this.currentUser.username}/user-menu-private-messages` + ).then((data) => { + const content = []; + data.notifications.forEach((notification) => { + content.push(Notification.create(notification)); + }); + content.push(...data.topics); + return content; + }); + } + + dismissWarningModal() { + const modalController = showModal("dismiss-notification-confirmation"); + modalController.set( + "confirmationMessage", + I18n.t("notifications.dismiss_confirmation.body.messages", { + count: this.#unreadMessaagesNotifications, + }) + ); + return modalController; + } +} 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 f9200599824..2303f6fbbc8 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 @@ -93,6 +93,28 @@ export default class UserMenuNotificationsList extends UserMenuItemsList { const modalController = this.dismissWarningModal(); const modalCallback = () => { ajax("/notifications/mark-read", opts).then(() => { + if (dismissTypes) { + const unreadNotificationCountsHash = { + ...this.currentUser.grouped_unread_high_priority_notifications, + }; + dismissTypes.forEach((type) => { + const typeId = this.site.notification_types[type]; + if (typeId) { + delete unreadNotificationCountsHash[typeId]; + } + }); + this.currentUser.set( + "grouped_unread_high_priority_notifications", + unreadNotificationCountsHash + ); + } else { + this.currentUser.set("all_unread_notifications_count", 0); + this.currentUser.set("unread_high_priority_notifications", 0); + this.currentUser.set( + "grouped_unread_high_priority_notifications", + {} + ); + } this.refreshList(); postRNWebviewMessage("markRead", "1"); }); 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 8633d9c9865..84be04227e4 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js @@ -2,8 +2,6 @@ import { click, visit } from "@ember/test-helpers"; import { acceptance, exists, - loggedInUser, - publishToMessageBus, query, queryAll, } from "discourse/tests/helpers/qunit-helpers"; @@ -184,6 +182,7 @@ acceptance("User menu - Dismiss button", function (needs) { unread_high_priority_notifications: 10, grouped_unread_high_priority_notifications: { [NOTIFICATION_TYPES.bookmark_reminder]: 103, + [NOTIFICATION_TYPES.private_message]: 89, }, }); @@ -210,6 +209,20 @@ acceptance("User menu - Dismiss button", function (needs) { ); } }); + + server.get("/u/eviltrout/user-menu-private-messages", () => { + if (markRead) { + const copy = cloneJSON( + UserMenuFixtures["/u/:username/user-menu-private-messages"] + ); + copy.notifications = []; + return helper.response(copy); + } else { + return helper.response( + UserMenuFixtures["/u/:username/user-menu-private-messages"] + ); + } + }); }); needs.hooks.afterEach(() => { @@ -227,18 +240,13 @@ acceptance("User menu - Dismiss button", function (needs) { I18n.t("notifications.dismiss_confirmation.body.default", { count: 10 }), "confirmation modal is shown when there are unread high pri notifications" ); - assert.notOk(markRead, "mark-read request isn't sent"); await click(".modal-footer .btn-default"); // click cancel on the dismiss modal + assert.notOk(markRead, "mark-read request isn't sent"); - await publishToMessageBus(`/notification/${loggedInUser().id}`, { - unread_high_priority_notifications: 0, - }); await click(".user-menu .notifications-dismiss"); - assert.ok( - markRead, - "mark-read request is sent without a confirmation modal when there are no unread high pri notifications" - ); + await click(".modal-footer .btn-primary"); // click confirm on the dismiss modal + assert.ok(markRead, "mark-read request is sent"); }); test("shows confirmation modal for the bookmarks list", async function (assert) { @@ -273,9 +281,6 @@ acceptance("User menu - Dismiss button", function (needs) { assert.notOk(markRead, "mark-read request isn't sent"); await click(".modal-footer .btn-primary"); // confirm dismiss on the dismiss modal - await publishToMessageBus(`/notification/${loggedInUser().id}`, { - grouped_unread_high_priority_notifications: {}, - }); assert.notOk( exists("#quick-access-bookmarks ul li.notification"), @@ -298,6 +303,60 @@ acceptance("User menu - Dismiss button", function (needs) { assert.notOk(exists(".user-menu .notifications-dismiss")); }); + test("shows confirmation modal for the messages list", async function (assert) { + await visit("/"); + await click(".d-header-icons .current-user"); + + assert.strictEqual( + query("#user-menu-button-messages .badge-notification").textContent, + "89", + "messages tab has bubble with count" + ); + + await click("#user-menu-button-messages"); + assert.ok( + exists("#quick-access-messages ul li.notification"), + "messages notifications are visible" + ); + assert.ok( + exists("#quick-access-messages ul li.message"), + "messages are visible" + ); + + await click(".user-menu .notifications-dismiss"); + + assert.strictEqual( + query(".dismiss-notification-confirmation").textContent.trim(), + I18n.t("notifications.dismiss_confirmation.body.messages", { + count: 89, + }), + "confirmation modal is shown when there are unread messages notifications" + ); + assert.notOk(markRead, "mark-read request isn't sent"); + + await click(".modal-footer .btn-primary"); // confirm dismiss on the dismiss modal + + assert.notOk( + exists("#quick-access-messages ul li.notification"), + "messages notifications are gone" + ); + assert.ok( + exists("#quick-access-messages ul li.message"), + "messages are still visible" + ); + assert.notOk( + exists("#user-menu-button-messages .badge-notification"), + "messages tab no longer has bubble" + ); + assert.ok(markRead, "mark-read request is sent"); + assert.strictEqual( + markReadRequestBody, + "dismiss_types=private_message", + "mark-read request specifies private_message types" + ); + assert.notOk(exists(".user-menu .notifications-dismiss")); + }); + test("doesn't show confirmation modal for the likes notifications list", async function (assert) { await visit("/"); await click(".d-header-icons .current-user"); diff --git a/app/assets/javascripts/discourse/tests/fixtures/user-menu.js b/app/assets/javascripts/discourse/tests/fixtures/user-menu.js index a886c18e827..a96893ea661 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/user-menu.js +++ b/app/assets/javascripts/discourse/tests/fixtures/user-menu.js @@ -62,5 +62,127 @@ export default { }, }, ], + }, + "/u/:username/user-menu-private-messages": { + notifications: [ + { + id: 8315, + user_id: 1, + notification_type: 6, + read: false, + high_priority: true, + created_at: "2022-08-05T17:27:24.873Z", + post_number: 1, + topic_id: 249, + fancy_title: "Very secret message!", + slug: "very-secret-message", + data: { + topic_title: "very secret message!", + original_post_id: 1043, + original_post_type: 1, + original_username: "osama", + revision_number: null, + display_username: "osama" + }, + }, + ], + topics: [ + { + id: 8092, + title: "BUG: Can not render emoji properly :/", + fancy_title: "BUG: Can not render emoji properly :confused:", + slug: "bug-can-not-render-emoji-properly", + posts_count: 1, + reply_count: 0, + highest_post_number: 2, + image_url: null, + created_at: "2019-07-26T01:29:24.008Z", + last_posted_at: "2019-07-26T01:29:24.177Z", + bumped: true, + bumped_at: "2019-07-26T01:29:24.177Z", + unseen: false, + last_read_post_number: 2, + unread_posts: 0, + pinned: false, + unpinned: null, + visible: true, + closed: false, + archived: false, + notification_level: 3, + bookmarked: false, + bookmarks: [], + liked: false, + views: 5, + like_count: 0, + has_summary: false, + archetype: "private_message", + last_poster_username: "mixtape", + category_id: null, + pinned_globally: false, + featured_link: null, + posters: [ + { + extras: "latest single", + description: "Original Poster, Most Recent Poster", + user_id: 13, + primary_group_id: null, + }, + ], + participants: [ + { + extras: "latest", + description: null, + user_id: 13, + primary_group_id: null, + }, + ], + fancy_title: "BUG: Can not render emoji properly :confused:", + slug: "bug-can-not-render-emoji-properly", + posts_count: 1, + reply_count: 0, + highest_post_number: 2, + image_url: null, + created_at: "2019-07-26T01:29:24.008Z", + last_posted_at: "2019-07-26T01:29:24.177Z", + bumped: true, + bumped_at: "2019-07-26T01:29:24.177Z", + unseen: false, + last_read_post_number: 2, + unread_posts: 0, + pinned: false, + unpinned: null, + visible: true, + closed: false, + archived: false, + notification_level: 3, + bookmarked: false, + bookmarks: [], + liked: false, + views: 5, + like_count: 0, + has_summary: false, + archetype: "private_message", + last_poster_username: "mixtape", + category_id: null, + pinned_globally: false, + featured_link: null, + posters: [ + { + extras: "latest single", + description: "Original Poster, Most Recent Poster", + user_id: 13, + primary_group_id: null, + }, + ], + participants: [ + { + extras: "latest", + description: null, + user_id: 13, + primary_group_id: null, + }, + ], + } + ], } } diff --git a/app/assets/javascripts/discourse/tests/integration/components/user-menu/bookmark-item-test.js b/app/assets/javascripts/discourse/tests/integration/components/user-menu/bookmark-item-test.js index 956802dbc0d..8a6d98c2dfb 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/user-menu/bookmark-item-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/user-menu/bookmark-item-test.js @@ -3,51 +3,48 @@ import { setupRenderingTest } from "discourse/tests/helpers/component-test"; import { query } from "discourse/tests/helpers/qunit-helpers"; import { render } from "@ember/test-helpers"; import { deepMerge } from "discourse-common/lib/object"; -import Notification from "discourse/models/notification"; import { hbs } from "ember-cli-htmlbars"; function getBookmark(overrides = {}) { - return Notification.create( - deepMerge( - { - id: 6, - created_at: "2022-08-05T06:09:39.559Z", - updated_at: "2022-08-05T06:11:27.246Z", - name: "", - reminder_at: "2022-08-05T06:10:42.223Z", - reminder_at_ics_start: "20220805T061042Z", - reminder_at_ics_end: "20220805T071042Z", - pinned: false, - title: "Test poll topic hello world", - fancy_title: "Test poll topic hello world", - excerpt: "poll", - bookmarkable_id: 1009, - bookmarkable_type: "Post", - bookmarkable_url: "http://localhost:4200/t/this-bookmarkable-url/227/1", - tags: [], - tags_descriptions: {}, - truncated: true, - topic_id: 227, - linked_post_number: 1, - deleted: false, - hidden: false, - category_id: 1, - closed: false, - archived: false, - archetype: "regular", - highest_post_number: 45, - last_read_post_number: 31, - bumped_at: "2022-04-21T15:14:37.359Z", - slug: "test-poll-topic-hello-world", - user: { - id: 1, - username: "somebody", - name: "Mr. Somebody", - avatar_template: "/letter_avatar_proxy/v4/letter/o/f05b48/{size}.png", - }, + return deepMerge( + { + id: 6, + created_at: "2022-08-05T06:09:39.559Z", + updated_at: "2022-08-05T06:11:27.246Z", + name: "", + reminder_at: "2022-08-05T06:10:42.223Z", + reminder_at_ics_start: "20220805T061042Z", + reminder_at_ics_end: "20220805T071042Z", + pinned: false, + title: "Test poll topic hello world", + fancy_title: "Test poll topic hello world", + excerpt: "poll", + bookmarkable_id: 1009, + bookmarkable_type: "Post", + bookmarkable_url: "http://localhost:4200/t/this-bookmarkable-url/227/1", + tags: [], + tags_descriptions: {}, + truncated: true, + topic_id: 227, + linked_post_number: 1, + deleted: false, + hidden: false, + category_id: 1, + closed: false, + archived: false, + archetype: "regular", + highest_post_number: 45, + last_read_post_number: 31, + bumped_at: "2022-04-21T15:14:37.359Z", + slug: "test-poll-topic-hello-world", + user: { + id: 1, + username: "somebody", + name: "Mr. Somebody", + avatar_template: "/letter_avatar_proxy/v4/letter/o/f05b48/{size}.png", }, - overrides - ) + }, + overrides ); } diff --git a/app/assets/javascripts/discourse/tests/integration/components/user-menu/menu-test.js b/app/assets/javascripts/discourse/tests/integration/components/user-menu/menu-test.js index 73677f2c556..a42004fbbce 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/user-menu/menu-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/user-menu/menu-test.js @@ -48,17 +48,22 @@ module("Integration | Component | user-menu", function (hooks) { test("the menu has a group of tabs at the top", async function (assert) { await render(template); const tabs = queryAll(".top-tabs.tabs-list .btn"); - assert.strictEqual(tabs.length, 5); - ["all-notifications", "replies", "mentions", "likes"].forEach( - (tab, index) => { - assert.strictEqual(tabs[index].id, `user-menu-button-${tab}`); - assert.strictEqual(tabs[index].dataset.tabNumber, index.toString()); - assert.strictEqual( - tabs[index].getAttribute("aria-controls"), - `quick-access-${tab}` - ); - } - ); + assert.strictEqual(tabs.length, 6); + [ + "all-notifications", + "replies", + "mentions", + "likes", + "messages", + "bookmarks", + ].forEach((tab, index) => { + assert.strictEqual(tabs[index].id, `user-menu-button-${tab}`); + assert.strictEqual(tabs[index].dataset.tabNumber, index.toString()); + assert.strictEqual( + tabs[index].getAttribute("aria-controls"), + `quick-access-${tab}` + ); + }); }); test("the menu has a group of tabs at the bottom", async function (assert) { @@ -67,7 +72,7 @@ module("Integration | Component | user-menu", function (hooks) { assert.strictEqual(tabs.length, 1); const preferencesTab = tabs[0]; assert.ok(preferencesTab.href.endsWith("/u/eviltrout/preferences")); - assert.strictEqual(preferencesTab.dataset.tabNumber, "5"); + assert.strictEqual(preferencesTab.dataset.tabNumber, "6"); assert.strictEqual(preferencesTab.getAttribute("tabindex"), "-1"); }); @@ -77,11 +82,11 @@ module("Integration | Component | user-menu", function (hooks) { assert.ok(!exists("#user-menu-button-likes")); const tabs = Array.from(queryAll(".tabs-list .btn")); // top and bottom tabs - assert.strictEqual(tabs.length, 5); + assert.strictEqual(tabs.length, 6); assert.deepEqual( tabs.map((t) => t.dataset.tabNumber), - ["0", "1", "2", "3", "4"], + ["0", "1", "2", "3", "4", "5"], "data-tab-number of the tabs has no gaps when the likes tab is hidden" ); }); @@ -90,18 +95,47 @@ module("Integration | Component | user-menu", function (hooks) { this.currentUser.set("can_review", true); await render(template); const tab = query("#user-menu-button-review-queue"); - assert.strictEqual(tab.dataset.tabNumber, "5"); + assert.strictEqual(tab.dataset.tabNumber, "6"); const tabs = Array.from(queryAll(".tabs-list .btn")); // top and bottom tabs - assert.strictEqual(tabs.length, 7); + assert.strictEqual(tabs.length, 8); assert.deepEqual( tabs.map((t) => t.dataset.tabNumber), - ["0", "1", "2", "3", "4", "5", "6"], + ["0", "1", "2", "3", "4", "5", "6", "7"], "data-tab-number of the tabs has no gaps when the reviewables tab is show" ); }); + test("messages tab isn't shown if current user isn't staff and enable_personal_messages setting is disabled", async function (assert) { + this.currentUser.set("moderator", false); + this.currentUser.set("admin", false); + this.siteSettings.enable_personal_messages = false; + + await render(template); + + assert.ok(!exists("#user-menu-button-messages")); + + const tabs = Array.from(queryAll(".tabs-list .btn")); // top and bottom tabs + assert.strictEqual(tabs.length, 6); + + assert.deepEqual( + tabs.map((t) => t.dataset.tabNumber), + ["0", "1", "2", "3", "4", "5"], + "data-tab-number of the tabs has no gaps when the messages tab is hidden" + ); + }); + + test("messages tab is shown if current user is staff even if enable_personal_messages setting is disabled", async function (assert) { + this.currentUser.set("moderator", true); + this.currentUser.set("admin", false); + this.siteSettings.enable_personal_messages = false; + + await render(template); + + assert.ok(exists("#user-menu-button-messages")); + }); + test("reviewables count is shown on the reviewables tab", async function (assert) { this.currentUser.set("can_review", true); this.currentUser.set("reviewable_count", 4); diff --git a/app/assets/javascripts/discourse/tests/integration/components/user-menu/message-item-test.js b/app/assets/javascripts/discourse/tests/integration/components/user-menu/message-item-test.js new file mode 100644 index 00000000000..3e649bc8bb9 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/integration/components/user-menu/message-item-test.js @@ -0,0 +1,38 @@ +import { module, test } from "qunit"; +import { setupRenderingTest } from "discourse/tests/helpers/component-test"; +import { query } from "discourse/tests/helpers/qunit-helpers"; +import { render } from "@ember/test-helpers"; +import { cloneJSON, deepMerge } from "discourse-common/lib/object"; +import { hbs } from "ember-cli-htmlbars"; +import PrivateMessagesFixture from "discourse/tests/fixtures/private-messages-fixtures"; + +function getMessage(overrides = {}) { + const data = cloneJSON( + PrivateMessagesFixture["/topics/private-messages/eviltrout.json"].topic_list + .topics[0] + ); + return deepMerge(data, overrides); +} + +module("Integration | Component | user-menu | message-item", function (hooks) { + setupRenderingTest(hooks); + + const template = hbs``; + + test("item description is the fancy title of the message", async function (assert) { + this.set( + "message", + getMessage({ fancy_title: "This is a safe title!" }) + ); + await render(template); + assert.strictEqual( + query("li.message .item-description").textContent.trim(), + "This is a safe title!" + ); + assert.strictEqual( + query("li.message .item-description b").textContent.trim(), + "safe", + "fancy title is not escaped" + ); + }); +}); diff --git a/app/assets/javascripts/discourse/tests/integration/components/user-menu/messages-list-test.js b/app/assets/javascripts/discourse/tests/integration/components/user-menu/messages-list-test.js new file mode 100644 index 00000000000..0b1e0491be7 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/integration/components/user-menu/messages-list-test.js @@ -0,0 +1,87 @@ +import { module, test } from "qunit"; +import { setupRenderingTest } from "discourse/tests/helpers/component-test"; +import { exists, query, queryAll } from "discourse/tests/helpers/qunit-helpers"; +import { render, settled } from "@ember/test-helpers"; +import { NOTIFICATION_TYPES } from "discourse/tests/fixtures/concerns/notification-types"; +import { hbs } from "ember-cli-htmlbars"; +import pretender, { response } from "discourse/tests/helpers/create-pretender"; +import I18n from "I18n"; + +module("Integration | Component | user-menu | messages-list", function (hooks) { + setupRenderingTest(hooks); + + const template = hbs``; + + test("renders notifications on top and messages on bottom", async function (assert) { + await render(template); + const items = queryAll("ul li"); + + assert.strictEqual(items.length, 2); + + assert.ok(items[0].classList.contains("notification")); + assert.ok(items[0].classList.contains("unread")); + assert.ok(items[0].classList.contains("private-message")); + + assert.ok(items[1].classList.contains("message")); + }); + + test("show all link", async function (assert) { + await render(template); + const link = query(".panel-body-bottom .show-all"); + assert.ok( + link.href.endsWith("/u/eviltrout/messages"), + "links to the user's messages page" + ); + assert.strictEqual( + link.title, + I18n.t("user_menu.view_all_messages"), + "has a title" + ); + }); + + test("dismiss button", async function (assert) { + this.currentUser.set("grouped_unread_high_priority_notifications", { + [NOTIFICATION_TYPES.private_message]: 72, + }); + await render(template); + const dismiss = query(".panel-body-bottom .notifications-dismiss"); + assert.ok( + dismiss, + "dismiss button is shown if the user has unread private_message notifications" + ); + assert.strictEqual( + dismiss.title, + I18n.t("user.dismiss_messages_tooltip"), + "dismiss button has a title" + ); + + this.currentUser.set("grouped_unread_high_priority_notifications", {}); + await settled(); + + assert.notOk( + exists(".panel-body-bottom .notifications-dismiss"), + "dismiss button is not shown if the user no unread private_message notifications" + ); + }); + + test("empty state (aka blank page syndrome)", async function (assert) { + pretender.get("/u/eviltrout/user-menu-private-messages", () => { + return response({ notifications: [], topics: [] }); + }); + await render(template); + assert.strictEqual( + query(".empty-state-title").textContent.trim(), + I18n.t("user.no_messages_title"), + "empty state title is shown" + ); + assert.ok( + exists(".empty-state-body svg.d-icon-envelope"), + "icon is correctly rendered in the empty state body" + ); + const emptyStateBodyLink = query(".empty-state-body a"); + assert.ok( + emptyStateBodyLink.href.endsWith("/about"), + "link inside empty state body is rendered" + ); + }); +}); diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index e35ff0e34f8..8d83520d61e 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -12,14 +12,14 @@ class UsersController < ApplicationController :notification_level, :revoke_auth_token, :register_second_factor_security_key, :create_second_factor_security_key, :feature_topic, :clear_featured_topic, :bookmarks, :invited, :check_sso_email, :check_sso_payload, - :recent_searches, :reset_recent_searches, :user_menu_bookmarks + :recent_searches, :reset_recent_searches, :user_menu_bookmarks, :user_menu_messages ] skip_before_action :check_xhr, only: [ :show, :badges, :password_reset_show, :password_reset_update, :update, :account_created, :activate_account, :perform_account_activation, :avatar, :my_redirect, :toggle_anon, :admin_login, :confirm_admin, :email_login, :summary, - :feature_topic, :clear_featured_topic, :bookmarks, :user_menu_bookmarks + :feature_topic, :clear_featured_topic, :bookmarks, :user_menu_bookmarks, :user_menu_messages ] before_action :second_factor_check_confirmed_password, only: [ @@ -1736,21 +1736,19 @@ class UsersController < ApplicationController end end - USER_MENU_BOOKMARKS_LIST_LIMIT = 20 + USER_MENU_LIST_LIMIT = 20 def user_menu_bookmarks if !current_user.username_equals_to?(params[:username]) raise Discourse::InvalidAccess.new("username doesn't match current_user's username") end - reminder_notifications = current_user - .notifications - .visible - .includes(:topic) - .where(read: false, notification_type: Notification.types[:bookmark_reminder]) - .limit(USER_MENU_BOOKMARKS_LIST_LIMIT) - .to_a + reminder_notifications = Notification.unread_type( + current_user, + Notification.types[:bookmark_reminder], + USER_MENU_LIST_LIMIT + ) - if reminder_notifications.size < USER_MENU_BOOKMARKS_LIST_LIMIT + if reminder_notifications.size < USER_MENU_LIST_LIMIT exclude_bookmark_ids = reminder_notifications .filter_map { |notification| notification.data_hash[:bookmark_id] } @@ -1758,7 +1756,7 @@ class UsersController < ApplicationController user: current_user, guardian: guardian, params: { - per_page: USER_MENU_BOOKMARKS_LIST_LIMIT - reminder_notifications.size + per_page: USER_MENU_LIST_LIMIT - reminder_notifications.size } ) bookmark_list.load do |query| @@ -1792,6 +1790,58 @@ class UsersController < ApplicationController } end + def user_menu_messages + if !current_user.username_equals_to?(params[:username]) + raise Discourse::InvalidAccess.new("username doesn't match current_user's username") + end + + if !current_user.staff? && !SiteSetting.enable_personal_messages + raise Discourse::InvalidAccess.new("personal messages are disabled.") + end + + message_notifications = Notification.unread_type( + current_user, + Notification.types[:private_message], + USER_MENU_LIST_LIMIT + ) + + if message_notifications.size < USER_MENU_LIST_LIMIT + exclude_topic_ids = message_notifications.map(&:topic_id).uniq + messages_list = TopicQuery.new( + current_user, + per_page: USER_MENU_LIST_LIMIT - message_notifications.size + ).list_private_messages(current_user) do |query| + if exclude_topic_ids.present? + query.where("topics.id NOT IN (?)", exclude_topic_ids) + else + query + end + end + end + + if message_notifications.present? + serialized_notifications = ActiveModel::ArraySerializer.new( + message_notifications, + each_serializer: NotificationSerializer, + scope: guardian + ) + end + + if messages_list + serialized_messages = serialize_data( + messages_list, + TopicListSerializer, + scope: guardian, + root: false + )[:topics] + end + + render json: { + notifications: serialized_notifications || [], + topics: serialized_messages || [] + } + end + private def clean_custom_field_values(field) @@ -1973,4 +2023,13 @@ class UsersController < ApplicationController { users: ActiveModel::ArraySerializer.new(users, each_serializer: each_serializer).as_json } end + + def find_unread_notifications_of_type(type, limit) + current_user + .notifications + .visible + .includes(:topic) + .where(read: false, notification_type: type) + .limit(limit) + end end diff --git a/app/models/notification.rb b/app/models/notification.rb index 1989a07d474..70bf5547751 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -15,6 +15,9 @@ class Notification < ActiveRecord::Base scope :recent, lambda { |n = nil| n ||= 10; order('notifications.created_at desc').limit(n) } scope :visible , lambda { joins('LEFT JOIN topics ON notifications.topic_id = topics.id') .where('topics.id IS NULL OR topics.deleted_at IS NULL') } + scope :unread_type, ->(user, type, limit = 20) do + where(user_id: user.id, read: false, notification_type: type).visible.includes(:topic).limit(limit) + end attr_accessor :skip_send_email diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index cfccf9f90ff..5365a990bc5 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1123,6 +1123,7 @@ en: dismiss_notifications: "Dismiss All" dismiss_notifications_tooltip: "Mark all unread notifications as read" dismiss_bookmarks_tooltip: "Mark all unread bookmark reminders as read" + dismiss_messages_tooltip: "Mark all unread personal messages notifications as read" no_messages_title: "You don’t have any messages" no_messages_body: > Need to have a direct personal conversation with someone, outside the normal conversational flow? Message them by selecting their avatar and using the %{icon} message button.

@@ -2371,6 +2372,9 @@ en: bookmarks: one: "Are you sure? You have %{count} unread bookmark reminder." other: "Are you sure? You have %{count} unread bookmark reminders." + messages: + one: "Are you sure? You have %{count} unread personal message." + other: "Are you sure? You have %{count} unread personal messages." dismiss: "Dismiss" cancel: "Cancel" @@ -2564,6 +2568,7 @@ en: sr_menu_tabs: "Menu tabs" view_all_notifications: "view all notifications" view_all_bookmarks: "view all bookmarks" + view_all_messages: "view all personal messages" reviewable: view_all: "view all review items" queue: "Queue" diff --git a/config/routes.rb b/config/routes.rb index b9f388fb54f..49c00253646 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -520,6 +520,7 @@ Discourse::Application.routes.draw do get "#{root_path}/:username/badges" => "users#badges", constraints: { username: RouteFormat.username } get "#{root_path}/:username/bookmarks" => "users#bookmarks", constraints: { username: RouteFormat.username, format: /(json|ics)/ } get "#{root_path}/:username/user-menu-bookmarks" => "users#user_menu_bookmarks", constraints: { username: RouteFormat.username, format: :json } + get "#{root_path}/:username/user-menu-private-messages" => "users#user_menu_messages", constraints: { username: RouteFormat.username, format: :json } get "#{root_path}/:username/notifications" => "users#show", constraints: { username: RouteFormat.username } get "#{root_path}/:username/notifications/:filter" => "users#show", constraints: { username: RouteFormat.username } delete "#{root_path}/:username" => "users#destroy", constraints: { username: RouteFormat.username } diff --git a/lib/topic_query/private_message_lists.rb b/lib/topic_query/private_message_lists.rb index 84b413fdcf1..cdf5201b020 100644 --- a/lib/topic_query/private_message_lists.rb +++ b/lib/topic_query/private_message_lists.rb @@ -2,7 +2,7 @@ class TopicQuery module PrivateMessageLists - def list_private_messages(user) + def list_private_messages(user, &blk) list = private_messages_for(user, :user) list = not_archived(list, user) @@ -14,7 +14,7 @@ class TopicQuery ) SQL - create_list(:private_messages, {}, list) + create_list(:private_messages, {}, list, &blk) end def list_private_messages_archive(user) diff --git a/spec/fabricators/post_fabricator.rb b/spec/fabricators/post_fabricator.rb index 3a23e045c6b..4b35bc721c4 100644 --- a/spec/fabricators/post_fabricator.rb +++ b/spec/fabricators/post_fabricator.rb @@ -127,6 +127,7 @@ Fabricator(:post_with_external_links, from: :post) do end Fabricator(:private_message_post, from: :post) do + transient :recipient user topic do |attrs| Fabricate(:private_message_topic, @@ -135,7 +136,7 @@ Fabricator(:private_message_post, from: :post) do subtype: TopicSubtype.user_to_user, topic_allowed_users: [ Fabricate.build(:topic_allowed_user, user: attrs[:user]), - Fabricate.build(:topic_allowed_user, user: Fabricate(:user)) + Fabricate.build(:topic_allowed_user, user: attrs[:recipient] || Fabricate(:user)) ] ) end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 632cc38cb44..6a75373b0b0 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -5690,14 +5690,14 @@ RSpec.describe UsersController do ) end - it "fills up the remaining of the USER_MENU_BOOKMARKS_LIST_LIMIT limit with bookmarks" do + it "fills up the remaining of the USER_MENU_LIST_LIMIT limit with bookmarks" do bookmark2 = Fabricate( :bookmark, user: user, bookmarkable: Fabricate(:post, topic: topic) ) - stub_const(UsersController, "USER_MENU_BOOKMARKS_LIST_LIMIT", 2) do + stub_const(UsersController, "USER_MENU_LIST_LIMIT", 2) do get "/u/#{user.username}/user-menu-bookmarks" end expect(response.status).to eq(200) @@ -5708,7 +5708,7 @@ RSpec.describe UsersController do bookmarks = response.parsed_body["bookmarks"] expect(bookmarks.size).to eq(1) - stub_const(UsersController, "USER_MENU_BOOKMARKS_LIST_LIMIT", 3) do + stub_const(UsersController, "USER_MENU_LIST_LIMIT", 3) do get "/u/#{user.username}/user-menu-bookmarks" end expect(response.status).to eq(200) @@ -5721,7 +5721,7 @@ RSpec.describe UsersController do BookmarkReminderNotificationHandler.new(bookmark2).send_notification - stub_const(UsersController, "USER_MENU_BOOKMARKS_LIST_LIMIT", 3) do + stub_const(UsersController, "USER_MENU_LIST_LIMIT", 3) do get "/u/#{user.username}/user-menu-bookmarks" end expect(response.status).to eq(200) @@ -5735,6 +5735,138 @@ RSpec.describe UsersController do end end + describe "#user_menu_messages" do + fab!(:message_without_notification) { Fabricate(:private_message_post, recipient: user).topic } + fab!(:message_with_read_notification) { Fabricate(:private_message_post, recipient: user).topic } + fab!(:message_with_unread_notification) { Fabricate(:private_message_post, recipient: user).topic } + + fab!(:user1_message_without_notification) do + Fabricate(:private_message_post, recipient: user1).topic + end + fab!(:user1_message_with_read_notification) do + Fabricate(:private_message_post, recipient: user1).topic + end + fab!(:user1_message_with_unread_notification) do + Fabricate(:private_message_post, recipient: user1).topic + end + + fab!(:unread_notification) do + Fabricate( + :private_message_notification, + read: false, + user: user, + topic: message_with_unread_notification + ) + end + fab!(:read_notification) do + Fabricate( + :private_message_notification, + read: true, + user: user, + topic: message_with_read_notification + ) + end + + fab!(:user1_unread_notification) do + Fabricate( + :private_message_notification, + read: false, + user: user1, + topic: user1_message_with_unread_notification + ) + end + fab!(:user1_read_notification) do + Fabricate( + :private_message_notification, + read: true, + user: user1, + topic: user1_message_with_read_notification + ) + end + + context "when logged out" do + it "responds with 404" do + get "/u/#{user.username}/user-menu-private-messages" + expect(response.status).to eq(404) + end + end + + context "when logged in" do + before do + sign_in(user) + end + + it "responds with 403 when requesting messages list of another user" do + get "/u/#{user1.username}/user-menu-private-messages" + expect(response.status).to eq(403) + end + + it "responds with 403 if private messages are disabled and the user isn't staff" do + SiteSetting.enable_personal_messages = false + get "/u/#{user.username}/user-menu-private-messages" + expect(response.status).to eq(403) + end + + it "doesn't respond with 403 if private messages are disabled and the user is staff" do + SiteSetting.enable_personal_messages = false + user.update!(moderator: true) + get "/u/#{user.username}/user-menu-private-messages" + expect(response.status).to eq(200) + end + + it "sends an array of unread private_message notifications" do + get "/u/#{user.username}/user-menu-private-messages" + expect(response.status).to eq(200) + + notifications = response.parsed_body["notifications"] + expect(notifications.map { |notification| notification["id"] }).to contain_exactly( + unread_notification.id + ) + end + + it "responds with an array of PM topics that are not associated with any of the unread private_message notifications" do + get "/u/#{user.username}/user-menu-private-messages" + expect(response.status).to eq(200) + + topics = response.parsed_body["topics"] + expect(topics.map { |topic| topic["id"] }).to contain_exactly( + message_without_notification.id, + message_with_read_notification.id + ) + end + + it "fills up the remaining of the USER_MENU_LIST_LIMIT limit with PM topics" do + stub_const(UsersController, "USER_MENU_LIST_LIMIT", 2) do + get "/u/#{user.username}/user-menu-private-messages" + end + expect(response.status).to eq(200) + notifications = response.parsed_body["notifications"] + expect(notifications.size).to eq(1) + + topics = response.parsed_body["topics"] + expect(topics.size).to eq(1) + + message2 = Fabricate(:private_message_post, recipient: user).topic + Fabricate( + :private_message_notification, + read: false, + user: user, + topic: message2 + ) + + stub_const(UsersController, "USER_MENU_LIST_LIMIT", 2) do + get "/u/#{user.username}/user-menu-private-messages" + end + expect(response.status).to eq(200) + notifications = response.parsed_body["notifications"] + expect(notifications.size).to eq(2) + + topics = response.parsed_body["topics"] + expect(topics.size).to eq(0) + end + end + end + def create_second_factor_security_key sign_in(user1) stub_secure_session_confirmed