Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FEATURE: unified user menu notifications count (#18132) 

Each new user menu notifications should have their own count. Therefore, we need to include all types to serializer and not only `grouped_unread_high_priority_notifications`

Additional PR will be created for chat and assign plugin, as they will have to switch to  `grouped_unread_notifications` as well.
This commit is contained in:
Krzysztof Kotlarek 2022-08-31 11:16:28 +10:00 committed by GitHub
parent 2e00d4d024
commit de8cd19438
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 81 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/assets/javascripts/discourse/app/components/user-menu/bookmarks-list.js
View File

@ -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

12
app/assets/javascripts/discourse/app/components/user-menu/menu.js
View File

@ -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 {

2
app/assets/javascripts/discourse/app/components/user-menu/messages-list.js
View File

@ -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

9
app/assets/javascripts/discourse/app/components/user-menu/notifications-list.js
View File

@ -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");

3
app/assets/javascripts/discourse/app/initializers/subscribe-user-notifications.js
View File

@ -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 (

15
app/assets/javascripts/discourse/app/lib/user-menu/notification-item.js
View File

@ -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("/") });
}

10
app/assets/javascripts/discourse/app/lib/user-menu/tab.js
View File

@ -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;
}
}

25
app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js
View File

@ -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,
},

4
app/assets/javascripts/discourse/tests/integration/components/user-menu/bookmarks-list-test.js
View File

@ -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(

4
app/assets/javascripts/discourse/tests/integration/components/user-menu/messages-list-test.js
View File

@ -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(

9
app/models/user.rb
View File

@ -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])

4
app/serializers/current_user_serializer.rb
View File

@ -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

12
spec/models/user_spec.rb
View File

@ -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