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 908c88f0db6..85b4aa071a7 100644 --- a/app/assets/javascripts/discourse/app/initializers/subscribe-user-notifications.js +++ b/app/assets/javascripts/discourse/app/initializers/subscribe-user-notifications.js @@ -53,6 +53,8 @@ export default { read_first_notification: data.read_first_notification, all_unread_notifications_count: data.all_unread_notifications_count, grouped_unread_notifications: data.grouped_unread_notifications, + new_personal_messages_notifications_count: + data.new_personal_messages_notifications_count, }); if ( diff --git a/app/assets/javascripts/discourse/app/widgets/header.js b/app/assets/javascripts/discourse/app/widgets/header.js index cd32571a51e..81f705a2e69 100644 --- a/app/assets/javascripts/discourse/app/widgets/header.js +++ b/app/assets/javascripts/discourse/app/widgets/header.js @@ -82,25 +82,48 @@ createWidget("header-notifications", { contents.push(h("div.do-not-disturb-background", iconNode("moon"))); } else { if (this.currentUser.redesigned_user_menu_enabled) { - const unread = user.all_unread_notifications_count || 0; - const reviewables = user.unseen_reviewable_count || 0; - const count = unread + reviewables; - if (count > 0) { - if (this._shouldHighlightAvatar()) { - contents.push(h("span.ring")); - } - + let ringClass = null; + if (user.new_personal_messages_notifications_count) { + ringClass = "personal-messages"; + contents.push( + this.attach("link", { + action: attrs.action, + className: "badge-notification with-icon new-pms", + icon: "envelope", + omitSpan: true, + title: "notifications.tooltip.new_message_notification", + titleOptions: { + count: user.new_personal_messages_notifications_count, + }, + }) + ); + } else if (user.unseen_reviewable_count) { + contents.push( + this.attach("link", { + action: attrs.action, + className: "badge-notification with-icon new-reviewables", + icon: "flag", + omitSpan: true, + title: "notifications.tooltip.new_reviewable", + titleOptions: { count: user.unseen_reviewable_count }, + }) + ); + } else if (user.all_unread_notifications_count) { + ringClass = "regular-notifications"; contents.push( this.attach("link", { action: attrs.action, className: "badge-notification unread-notifications", - rawLabel: count, + rawLabel: user.all_unread_notifications_count, omitSpan: true, title: "notifications.tooltip.regular", - titleOptions: { count }, + titleOptions: { count: user.all_unread_notifications_count }, }) ); } + if (ringClass && this._shouldHighlightAvatar()) { + contents.push(h(`span.ring.revamped.${ringClass}`)); + } } else { const unreadNotifications = user.unread_notifications; if (!!unreadNotifications) { diff --git a/app/assets/javascripts/discourse/tests/integration/components/site-header-test.js b/app/assets/javascripts/discourse/tests/integration/components/site-header-test.js index 15f1cdd5c99..7027735604f 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/site-header-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/site-header-test.js @@ -9,6 +9,7 @@ import { } from "@ember/test-helpers"; import { exists, query } from "discourse/tests/helpers/qunit-helpers"; import { hbs } from "ember-cli-htmlbars"; +import I18n from "I18n"; module("Integration | Component | site-header", function (hooks) { setupRenderingTest(hooks); @@ -18,7 +19,7 @@ module("Integration | Component | site-header", function (hooks) { this.currentUser.set("read_first_notification", false); }); - test("displaying unread and reviewable notifications count when user's notifications and reviewables count are updated", async function (assert) { + test("unread notifications count rerenders when user's notifications count is updated", async function (assert) { this.currentUser.set("all_unread_notifications_count", 1); this.currentUser.set("redesigned_user_menu_enabled", true); @@ -35,14 +36,6 @@ module("Integration | Component | site-header", function (hooks) { ".header-dropdown-toggle.current-user .unread-notifications" ); assert.strictEqual(unreadBadge.textContent, "5"); - - this.currentUser.set("unseen_reviewable_count", 3); - await settled(); - - unreadBadge = query( - ".header-dropdown-toggle.current-user .unread-notifications" - ); - assert.strictEqual(unreadBadge.textContent, "8"); }); test("hamburger menu icon shows pending reviewables count", async function (assert) { @@ -133,4 +126,121 @@ module("Integration | Component | site-header", function (hooks) { "the up arrow key moves the focus in the opposite direction" ); }); + + test("new personal messages bubble is prioritized over unseen reviewables and regular notifications bubbles", async function (assert) { + this.currentUser.set("redesigned_user_menu_enabled", true); + this.currentUser.set("all_unread_notifications_count", 5); + this.currentUser.set("new_personal_messages_notifications_count", 2); + this.currentUser.set("unseen_reviewable_count", 3); + + await render(hbs``); + + assert.notOk( + exists( + ".header-dropdown-toggle.current-user .badge-notification.unread-notifications" + ), + "regular notifications bubble isn't displayed when there are new personal messages notifications" + ); + + assert.notOk( + exists( + ".header-dropdown-toggle.current-user .badge-notification.with-icon.new-reviewables" + ), + "reviewables bubble isn't displayed when there are new personal messages notifications" + ); + + const pmsBubble = query( + ".header-dropdown-toggle.current-user .badge-notification.with-icon.new-pms" + ); + assert.strictEqual( + pmsBubble.textContent.trim(), + "", + "personal messages bubble has no count" + ); + assert.ok( + pmsBubble.querySelector(".d-icon-envelope"), + "personal messages bubble has envelope icon" + ); + assert.strictEqual( + pmsBubble.title, + I18n.t("notifications.tooltip.new_message_notification", { count: 2 }), + "personal messages bubble bubble has a title" + ); + }); + + test("unseen reviewables bubble is prioritized over regular notifications", async function (assert) { + this.currentUser.set("redesigned_user_menu_enabled", true); + this.currentUser.set("all_unread_notifications_count", 5); + this.currentUser.set("new_personal_messages_notifications_count", 0); + this.currentUser.set("unseen_reviewable_count", 3); + await render(hbs``); + + assert.notOk( + exists( + ".header-dropdown-toggle.current-user .badge-notification.unread-notifications" + ), + "regular notifications bubble isn't displayed when there are unseen reviewables notifications" + ); + + const reviewablesBubble = query( + ".header-dropdown-toggle.current-user .badge-notification.with-icon.new-reviewables" + ); + assert.strictEqual( + reviewablesBubble.textContent.trim(), + "", + "reviewables bubble has no count" + ); + assert.ok( + reviewablesBubble.querySelector(".d-icon-flag"), + "reviewables bubble has flag icon" + ); + assert.strictEqual( + reviewablesBubble.title, + I18n.t("notifications.tooltip.new_reviewable", { count: 3 }), + "reviewables bubble has a title" + ); + + assert.notOk( + exists( + ".header-dropdown-toggle.current-user .badge-notification.with-icon.new-pms" + ), + "personal messages bubble isn't displayed" + ); + }); + + test("regular notifications bubble is shown if there are neither new personal messages nor unseen reviewables", async function (assert) { + this.currentUser.set("redesigned_user_menu_enabled", true); + this.currentUser.set("all_unread_notifications_count", 5); + this.currentUser.set("new_personal_messages_notifications_count", 0); + this.currentUser.set("unseen_reviewable_count", 0); + await render(hbs``); + + const regularNotificationsBubble = query( + ".header-dropdown-toggle.current-user .badge-notification.unread-notifications" + ); + assert.strictEqual( + regularNotificationsBubble.textContent, + "5", + "regular notifications bubble has a count" + ); + assert.strictEqual( + regularNotificationsBubble.title, + I18n.t("notifications.tooltip.regular", { count: 5 }), + "regular notifications bubble has a title" + ); + + assert.notOk( + exists( + ".header-dropdown-toggle.current-user .badge-notification.with-icon.new-reviewables" + ), + "reviewables bubble isn't displayed" + ); + + assert.notOk( + exists( + ".header-dropdown-toggle.current-user .badge-notification.with-icon.new-pms" + ), + "personal messages bubble isn't displayed" + ); + }); }); diff --git a/app/assets/stylesheets/common/base/discourse.scss b/app/assets/stylesheets/common/base/discourse.scss index 39c1bb8f87b..f4a449faebd 100644 --- a/app/assets/stylesheets/common/base/discourse.scss +++ b/app/assets/stylesheets/common/base/discourse.scss @@ -486,21 +486,6 @@ table { } } -.ring { - $gradient-start: transparent; - $gradient-end: #090; - background: radial-gradient($gradient-start, $gradient-end); - top: -11px !important; - right: 23.5px !important; - border-radius: 100%; - width: 20px; - height: 20px; - transform-origin: center; - animation-iteration-count: infinite; - animation-duration: 3s; - animation-name: ping; -} - .fade { opacity: 0; transition: opacity 0.15s linear; diff --git a/app/assets/stylesheets/common/base/header.scss b/app/assets/stylesheets/common/base/header.scss index c52ed610a10..0c96429274a 100644 --- a/app/assets/stylesheets/common/base/header.scss +++ b/app/assets/stylesheets/common/base/header.scss @@ -141,7 +141,7 @@ border-top: 1px solid transparent; border-left: 1px solid transparent; border-right: 1px solid transparent; - .d-icon { + > .d-icon { color: var(--primary-medium); } } @@ -158,7 +158,7 @@ border-top: 1px solid var(--primary-low); border-left: 1px solid var(--primary-low); border-right: 1px solid var(--primary-low); - .d-icon { + > .d-icon { color: var(--primary-medium); } &:after { @@ -185,14 +185,36 @@ display: inline-block; color: var(--header_primary-low-mid); } + .notifications { position: relative; } .ring { position: absolute; - top: -9px; + top: -11px; + right: 23.5px; z-index: z("base"); margin-left: 0; + background: radial-gradient(transparent, var(--success)); + border-radius: 100%; + width: 20px; + height: 20px; + transform-origin: center; + animation-iteration-count: infinite; + animation-duration: 3s; + animation-name: ping; + + &.revamped { + right: -4px; + top: -6px; + + &.regular-notifications { + background: radial-gradient( + transparent, + var(--tertiary-med-or-tertiary) + ); + } + } } .header-dropdown-toggle { position: relative; @@ -203,10 +225,24 @@ left: 0; top: -4px; min-width: 0.6em; - } - .unread-notifications { left: auto; right: -3px; + + &.with-icon { + &.new-pms { + background-color: var(--success); + } + &.new-reviewables { + background-color: var(--danger); + } + .d-icon { + color: var(--secondary); + font-size: var(--font-down-1); + width: 1em; + } + } + } + .unread-notifications { background-color: var(--tertiary-med-or-tertiary); } .unread-high-priority-notifications, diff --git a/app/models/user.rb b/app/models/user.rb index 8400c30a4e0..0b98b5f65cf 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -597,6 +597,23 @@ class User < ActiveRecord::Base @unread_high_prios ||= unread_notifications_of_priority(high_priority: true) end + def new_personal_messages_notifications_count + args = { + user_id: self.id, + seen_notification_id: self.seen_notification_id, + private_message: Notification.types[:private_message] + } + + DB.query_single(<<~SQL, args).first + SELECT COUNT(*) + FROM notifications + WHERE user_id = :user_id + AND id > :seen_notification_id + AND NOT read + AND notification_type = :private_message + SQL + end + # PERF: This safeguard is in place to avoid situations where # a user with enormous amounts of unread data can issue extremely # expensive queries @@ -775,6 +792,7 @@ class User < ActiveRecord::Base if self.redesigned_user_menu_enabled? payload[:all_unread_notifications_count] = all_unread_notifications_count payload[:grouped_unread_notifications] = grouped_unread_notifications + payload[:new_personal_messages_notifications_count] = new_personal_messages_notifications_count 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 70920d01a51..b4077b05ebe 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -46,6 +46,7 @@ class CurrentUserSerializer < BasicUserSerializer :is_anonymous, :reviewable_count, :unseen_reviewable_count, + :new_personal_messages_notifications_count, :read_faq?, :automatically_unpin_topics, :mailing_list_mode, @@ -374,6 +375,10 @@ class CurrentUserSerializer < BasicUserSerializer redesigned_user_menu_enabled end + def include_new_personal_messages_notifications_count? + redesigned_user_menu_enabled + end + def redesigned_user_page_nav_enabled if SiteSetting.enable_new_user_profile_nav_groups.present? object.in_any_groups?(SiteSetting.enable_new_user_profile_nav_groups_map) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 8006523356e..c3527e044b3 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2383,6 +2383,12 @@ en: high_priority: one: "%{count} unread high priority notification" other: "%{count} unread high priority notifications" + new_message_notification: + one: "%{count} new message notification" + other: "%{count} new message notifications" + new_reviewable: + one: "%{count} new reviewable" + other: "%{count} new reviewables" title: "notifications of @name mentions, replies to your posts and topics, messages, etc" none: "Unable to load notifications at this time." empty: "No notifications found." diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 26af6b62289..dc9bab85caa 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2176,6 +2176,7 @@ RSpec.describe User do user.update!(admin: true) Fabricate(:notification, user: user, notification_type: 1) Fabricate(:notification, notification_type: 15, high_priority: true, read: false, user: user) + Fabricate(:notification, user: user, notification_type: Notification.types[:private_message], read: false) messages = MessageBus.track_publish("/notification/#{user.id}") do user.publish_notifications_state @@ -2185,8 +2186,9 @@ RSpec.describe User do message = messages.first - expect(message.data[:all_unread_notifications_count]).to eq(2) - expect(message.data[:grouped_unread_notifications]).to eq({ 1 => 1, 15 => 1 }) + expect(message.data[:all_unread_notifications_count]).to eq(3) + expect(message.data[:grouped_unread_notifications]).to eq({ 1 => 1, 15 => 1, Notification.types[:private_message] => 1 }) + expect(message.data[:new_personal_messages_notifications_count]).to eq(1) end end end @@ -3116,6 +3118,53 @@ RSpec.describe User do expect(admin.secure_category_ids).not_to include(private_category.id) end end + end + describe '#new_personal_messages_notifications_count' do + it "returns count of new and unread private_message notifications of the user" do + another_user = Fabricate(:user) + + Fabricate(:notification, user: user, read: false) + + last_seen_id = Fabricate( + :notification, + user: user, + read: false, + notification_type: Notification.types[:private_message] + ).id + + expect(user.new_personal_messages_notifications_count).to eq(1) + + Fabricate( + :notification, + user: user, + read: false, + notification_type: Notification.types[:private_message] + ) + + Fabricate( + :notification, + user: another_user, + read: false, + notification_type: Notification.types[:private_message] + ) + + Fabricate( + :notification, + user: user, + read: true, + notification_type: Notification.types[:private_message] + ) + + Fabricate( + :notification, + user: user, + read: false, + notification_type: Notification.types[:replied] + ) + + user.update!(seen_notification_id: last_seen_id) + expect(user.new_personal_messages_notifications_count).to eq(1) + end end end diff --git a/spec/serializers/current_user_serializer_spec.rb b/spec/serializers/current_user_serializer_spec.rb index b25f9c94e09..0c6f92b3a60 100644 --- a/spec/serializers/current_user_serializer_spec.rb +++ b/spec/serializers/current_user_serializer_spec.rb @@ -390,5 +390,19 @@ RSpec.describe CurrentUserSerializer do end end + describe "#new_personal_messages_notifications_count" do + fab!(:notification) { Fabricate(:notification, user: user, read: false, notification_type: Notification.types[:private_message]) } + + it "isn't included when enable_experimental_sidebar_hamburger is disabled" do + SiteSetting.enable_experimental_sidebar_hamburger = false + expect(serializer.as_json[:new_personal_messages_notifications_count]).to be_nil + end + + it "is included when enable_experimental_sidebar_hamburger is enabled" do + SiteSetting.enable_experimental_sidebar_hamburger = true + expect(serializer.as_json[:new_personal_messages_notifications_count]).to eq(1) + end + end + include_examples "#display_sidebar_tags", described_class end