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 b9f37162157..fcef1a4310b 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 @@ -37,7 +37,7 @@ export default class UserMenuBookmarksList extends UserMenuNotificationsList { } get #unreadBookmarkRemindersCount() { - const key = `grouped_unread_high_priority_notifications.${this.site.notification_types.bookmark_reminder}`; + const key = `grouped_unread_notifications.${this.site.notification_types.bookmark_reminder}`; // 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 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 b442692ecb5..caeaea7d2fc 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/menu.js +++ b/app/assets/javascripts/discourse/app/components/user-menu/menu.js @@ -37,6 +37,10 @@ const CORE_TOP_TABS = [ get panelComponent() { return "user-menu/replies-notifications-list"; } + + get count() { + return this.getUnreadCountForType("replied"); + } }, class extends UserMenuTab { @@ -51,6 +55,10 @@ const CORE_TOP_TABS = [ get panelComponent() { return "user-menu/mentions-notifications-list"; } + + get count() { + return this.getUnreadCountForType("mentioned"); + } }, class extends UserMenuTab { @@ -69,6 +77,10 @@ const CORE_TOP_TABS = [ get shouldDisplay() { return !this.currentUser.likes_notifications_disabled; } + + get count() { + return this.getUnreadCountForType("liked"); + } }, class extends UserMenuTab { 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 3fe2800da72..ac434948c08 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 @@ -37,7 +37,7 @@ export default class UserMenuMessagesList extends UserMenuNotificationsList { } get #unreadMessaagesNotifications() { - const key = `grouped_unread_high_priority_notifications.${this.site.notification_types.private_message}`; + const key = `grouped_unread_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 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 2c304d4ec97..2a56650c0c7 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 @@ -108,7 +108,7 @@ export default class UserMenuNotificationsList extends UserMenuItemsList { ajax("/notifications/mark-read", opts).then(() => { if (dismissTypes) { const unreadNotificationCountsHash = { - ...this.currentUser.grouped_unread_high_priority_notifications, + ...this.currentUser.grouped_unread_notifications, }; dismissTypes.forEach((type) => { const typeId = this.site.notification_types[type]; @@ -117,16 +117,13 @@ export default class UserMenuNotificationsList extends UserMenuItemsList { } }); this.currentUser.set( - "grouped_unread_high_priority_notifications", + "grouped_unread_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.currentUser.set("grouped_unread_notifications", {}); } this.refreshList(); postRNWebviewMessage("markRead", "1"); 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 0399ff9c1b1..1e17c8eb16a 100644 --- a/app/assets/javascripts/discourse/app/initializers/subscribe-user-notifications.js +++ b/app/assets/javascripts/discourse/app/initializers/subscribe-user-notifications.js @@ -51,8 +51,7 @@ export default { data.unread_high_priority_notifications, read_first_notification: data.read_first_notification, all_unread_notifications_count: data.all_unread_notifications_count, - grouped_unread_high_priority_notifications: - data.grouped_unread_high_priority_notifications, + grouped_unread_notifications: data.grouped_unread_notifications, }); if ( diff --git a/app/assets/javascripts/discourse/app/lib/user-menu/notification-item.js b/app/assets/javascripts/discourse/app/lib/user-menu/notification-item.js index 1ca293407e1..65717c73925 100644 --- a/app/assets/javascripts/discourse/app/lib/user-menu/notification-item.js +++ b/app/assets/javascripts/discourse/app/lib/user-menu/notification-item.js @@ -64,6 +64,21 @@ export default class UserMenuNotificationItem extends UserMenuBaseItem { onClick() { if (!this.notification.read) { this.notification.set("read", true); + + const groupedUnreadNotifications = + this.currentUser.grouped_unread_notifications; + const unreadCount = + groupedUnreadNotifications && + groupedUnreadNotifications[this.notification.notification_type]; + if (unreadCount > 0) { + groupedUnreadNotifications[this.notification.notification_type] = + unreadCount - 1; + this.currentUser.set( + "grouped_unread_notifications", + groupedUnreadNotifications + ); + } + setTransientHeader("Discourse-Clear-Notifications", this.notification.id); cookie("cn", this.notification.id, { path: getURL("/") }); } diff --git a/app/assets/javascripts/discourse/app/lib/user-menu/tab.js b/app/assets/javascripts/discourse/app/lib/user-menu/tab.js index b2969624d84..f33db82fb66 100644 --- a/app/assets/javascripts/discourse/app/lib/user-menu/tab.js +++ b/app/assets/javascripts/discourse/app/lib/user-menu/tab.js @@ -44,12 +44,18 @@ export default class UserMenuTab { } getUnreadCountForType(type) { - const key = `grouped_unread_high_priority_notifications.${this.site.notification_types[type]}`; + const key = `grouped_unread_notifications.${this.site.notification_types[type]}`; // 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; + + // TODO: remove old key fallback after plugins PRs are merged + // https://github.com/discourse/discourse-chat/pull/1208 + // https://github.com/discourse/discourse-assign/pull/373 + const oldKey = `grouped_unread_high_priority_notifications.${this.site.notification_types[type]}`; + + return this.currentUser.get(key) || this.currentUser.get(oldKey) || 0; } } 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 3b8514cadf8..fca2e21d099 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js @@ -23,6 +23,9 @@ acceptance("User menu", function (needs) { redesigned_user_menu_enabled: true, unread_high_priority_notifications: 73, trust_level: 3, + grouped_unread_notifications: { + [NOTIFICATION_TYPES.replied]: 2, + }, }); needs.settings({ @@ -51,6 +54,16 @@ acceptance("User menu", function (needs) { test("clicking on an unread notification", async function (assert) { await visit("/"); await click(".d-header-icons .current-user"); + + let repliesBadgeNotification = query( + "#user-menu-button-replies .badge-notification" + ); + assert.strictEqual( + repliesBadgeNotification.textContent.trim(), + "2", + "badge shows the right count" + ); + await click(".user-menu ul li.replied a"); assert.strictEqual( @@ -58,6 +71,16 @@ acceptance("User menu", function (needs) { 123, // id is from the fixtures in fixtures/notification-fixtures.js "the Discourse-Clear-Notifications request header is set to the notification id in the next ajax request" ); + + await click(".d-header-icons .current-user"); + repliesBadgeNotification = query( + "#user-menu-button-replies .badge-notification" + ); + assert.strictEqual( + repliesBadgeNotification.textContent.trim(), + "1", + "badge shows count reduced by one" + ); }); test("tabs added via the plugin API", async function (assert) { @@ -502,7 +525,7 @@ acceptance("User menu - Dismiss button", function (needs) { needs.user({ redesigned_user_menu_enabled: true, unread_high_priority_notifications: 10, - grouped_unread_high_priority_notifications: { + grouped_unread_notifications: { [NOTIFICATION_TYPES.bookmark_reminder]: 103, [NOTIFICATION_TYPES.private_message]: 89, }, diff --git a/app/assets/javascripts/discourse/tests/integration/components/user-menu/bookmarks-list-test.js b/app/assets/javascripts/discourse/tests/integration/components/user-menu/bookmarks-list-test.js index cecb3ed6965..f66752630fb 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/user-menu/bookmarks-list-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/user-menu/bookmarks-list-test.js @@ -42,7 +42,7 @@ module( }); test("dismiss button", async function (assert) { - this.currentUser.set("grouped_unread_high_priority_notifications", { + this.currentUser.set("grouped_unread_notifications", { [NOTIFICATION_TYPES.bookmark_reminder]: 72, }); await render(template); @@ -57,7 +57,7 @@ module( "dismiss button has a title" ); - this.currentUser.set("grouped_unread_high_priority_notifications", {}); + this.currentUser.set("grouped_unread_notifications", {}); await settled(); assert.notOk( 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 index 0b1e0491be7..02fba00dc70 100644 --- 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 @@ -40,7 +40,7 @@ module("Integration | Component | user-menu | messages-list", function (hooks) { }); test("dismiss button", async function (assert) { - this.currentUser.set("grouped_unread_high_priority_notifications", { + this.currentUser.set("grouped_unread_notifications", { [NOTIFICATION_TYPES.private_message]: 72, }); await render(template); @@ -55,7 +55,7 @@ module("Integration | Component | user-menu | messages-list", function (hooks) { "dismiss button has a title" ); - this.currentUser.set("grouped_unread_high_priority_notifications", {}); + this.currentUser.set("grouped_unread_notifications", {}); await settled(); assert.notOk( diff --git a/app/models/user.rb b/app/models/user.rb index 61feea4144f..94f691cb3ba 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -538,15 +538,14 @@ class User < ActiveRecord::Base DB.query_single(sql, user_id: id, high_priority: high_priority)[0].to_i end - MAX_UNREAD_HIGH_PRI_BACKLOG = 200 - def grouped_unread_high_priority_notifications - results = DB.query(<<~SQL, user_id: self.id, limit: MAX_UNREAD_HIGH_PRI_BACKLOG) + MAX_UNREAD_BACKLOG = 400 + def grouped_unread_notifications + results = DB.query(<<~SQL, user_id: self.id, limit: MAX_UNREAD_BACKLOG) SELECT X.notification_type AS type, COUNT(*) FROM ( SELECT n.notification_type FROM notifications n LEFT JOIN topics t ON t.id = n.topic_id WHERE t.deleted_at IS NULL - AND n.high_priority AND n.user_id = :user_id AND NOT n.read LIMIT :limit @@ -730,7 +729,7 @@ class User < ActiveRecord::Base if self.redesigned_user_menu_enabled? payload[:all_unread_notifications_count] = all_unread_notifications_count - payload[:grouped_unread_high_priority_notifications] = grouped_unread_high_priority_notifications + payload[:grouped_unread_notifications] = grouped_unread_notifications end MessageBus.publish("/notification/#{id}", payload, user_ids: [id]) diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index e926a4b8c84..5c891273d19 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -77,7 +77,7 @@ class CurrentUserSerializer < BasicUserSerializer :status, :sidebar_category_ids, :likes_notifications_disabled, - :grouped_unread_high_priority_notifications, + :grouped_unread_notifications, :redesigned_user_menu_enabled delegate :user_stat, to: :object, private: true @@ -338,7 +338,7 @@ class CurrentUserSerializer < BasicUserSerializer redesigned_user_menu_enabled end - def include_grouped_unread_high_priority_notifications? + def include_grouped_unread_notifications? redesigned_user_menu_enabled end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c8cfa914ff7..27d587595cd 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2067,10 +2067,10 @@ RSpec.describe User do end context "with redesigned_user_menu_enabled on" do - it "adds all_unread_notifications and grouped_unread_high_priority_notifications to the payload" do + it "adds all_unread_notifications and grouped_unread_notifications to the payload" do user.update!(admin: true) user.enable_redesigned_user_menu - Fabricate(:notification, user: user) + Fabricate(:notification, user: user, notification_type: 1) Fabricate(:notification, notification_type: 15, high_priority: true, read: false, user: user) messages = MessageBus.track_publish("/notification/#{user.id}") do user.publish_notifications_state @@ -2079,7 +2079,7 @@ RSpec.describe User do message = messages.first expect(message.data[:all_unread_notifications_count]).to eq(2) - expect(message.data[:grouped_unread_high_priority_notifications]).to eq({ 15 => 1 }) + expect(message.data[:grouped_unread_notifications]).to eq({ 1 => 1, 15 => 1 }) ensure user.disable_redesigned_user_menu end @@ -2808,8 +2808,8 @@ RSpec.describe User do end end - describe "#grouped_unread_high_priority_notifications" do - it "returns a map of high priority types to their unread count" do + describe "#grouped_unread_notifications" do + it "returns a map of types to their unread count" do Fabricate(:notification, user: user, notification_type: 1, high_priority: true, read: true) Fabricate(:notification, user: user, notification_type: 1, high_priority: true, read: false) Fabricate(:notification, user: user, notification_type: 1, high_priority: false, read: true) @@ -2826,7 +2826,7 @@ RSpec.describe User do # notification for another user. it shouldn't be included Fabricate(:notification, notification_type: 4, high_priority: true, read: false) - expect(user.grouped_unread_high_priority_notifications).to eq({ 1 => 1, 2 => 1 }) + expect(user.grouped_unread_notifications).to eq({ 1 => 2, 2 => 1 }) end end