From 8cf2f909f5053b13d8f6a79703aaf7fbdb3f6423 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Fri, 15 Mar 2024 12:08:37 -0400 Subject: [PATCH] DEV: Dedicated route for current user notification counts (#26106) Co-authored-by: Alan Guo Xiang Tan --- app/controllers/notifications_controller.rb | 4 + app/models/topic_tracking_state.rb | 38 +++++++++ .../user_notification_total_serializer.rb | 65 +++++++++++++++ config/routes.rb | 1 + plugins/chat/lib/chat/channel_fetcher.rb | 14 ++++ plugins/chat/plugin.rb | 10 +++ .../spec/lib/chat/channel_fetcher_spec.rb | 57 +++++++++++++ .../core_ext/notifications_controller_spec.rb | 59 +++++++++++++ spec/models/topic_tracking_state_spec.rb | 83 +++++++++++++++++++ .../requests/notifications_controller_spec.rb | 7 ++ ...user_notification_total_serializer_spec.rb | 61 ++++++++++++++ 11 files changed, 399 insertions(+) create mode 100644 app/serializers/user_notification_total_serializer.rb create mode 100644 plugins/chat/spec/requests/core_ext/notifications_controller_spec.rb create mode 100644 spec/serializers/user_notification_total_serializer_spec.rb diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 002385cc6ef..e8e0d111699 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -146,6 +146,10 @@ class NotificationsController < ApplicationController render json: success_json end + def totals + render_serialized(current_user, UserNotificationTotalSerializer, root: false) + end + private def set_notification diff --git a/app/models/topic_tracking_state.rb b/app/models/topic_tracking_state.rb index 71a04824bdd..770ded22e4e 100644 --- a/app/models/topic_tracking_state.rb +++ b/app/models/topic_tracking_state.rb @@ -577,6 +577,44 @@ class TopicTrackingState post.publish_change_to_clients!(:read, opts) end + def self.report_count_by_type(user, type:) + tag_ids = muted_tag_ids(user) + sql = + report_raw_sql( + topic_id: nil, + skip_unread: type == "new", + skip_new: type == "unread", + skip_order: true, + staff: user.staff?, + admin: user.admin?, + whisperer: user.whisperer?, + user: user, + muted_tag_ids: tag_ids, + ) + sql = tags_included_wrapped_sql(sql) + + DB.query( + sql + "\n\n LIMIT :max_topics", + { + user_id: user.id, + topic_id: nil, + min_new_topic_date: Time.at(SiteSetting.min_new_topics_time).to_datetime, + max_topics: TopicTrackingState::MAX_TOPICS, + user_first_unread_at: user.user_stat.first_unread_at, + }.merge(treat_as_new_topic_params), + ).count + end + + def self.report_totals(user) + if user.new_new_view_enabled? + { new: report(user).count } + else + new = report_count_by_type(user, type: "new") + unread = report_count_by_type(user, type: "unread") + { new: new, unread: unread } + end + end + def self.secure_category_group_ids(topic) category = topic.category diff --git a/app/serializers/user_notification_total_serializer.rb b/app/serializers/user_notification_total_serializer.rb new file mode 100644 index 00000000000..5faecb530b8 --- /dev/null +++ b/app/serializers/user_notification_total_serializer.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +class UserNotificationTotalSerializer < ApplicationSerializer + attributes :username, + :unread_notifications, + :unread_personal_messages, + :unseen_reviewables, + :topic_tracking, + :group_inboxes + + def unread_notifications + object.all_unread_notifications_count - new_personal_messages_notifications_count + end + + def include_unread_personal_messages? + object.in_any_groups?(SiteSetting.personal_message_enabled_groups_map) + end + + def unread_personal_messages + new_personal_messages_notifications_count + end + + def include_unseen_reviewables? + scope.user.staff? + end + + def unseen_reviewables + Reviewable.unseen_reviewable_count(object) + end + + def topic_tracking + TopicTrackingState.report_totals(object) + end + + def group_inboxes + group_inbox_data = + Notification + .unread + .where( + user_id: scope.user.id, + notification_type: Notification.types[:group_message_summary], + ) + .pluck(:data) + + results = [] + + return results if group_inbox_data.blank? + + group_inbox_data.map do |json| + data = JSON.parse(json, symbolize_names: true) + + results << { + group_id: data[:group_id], + group_name: data[:group_name], + count: data[:inbox_count], + } + end + + results + end + + def new_personal_messages_notifications_count + @new_personal_messages_notifications_count ||= object.new_personal_messages_notifications_count + end +end diff --git a/config/routes.rb b/config/routes.rb index 11bd8b70bfd..823a5d1dfa5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1129,6 +1129,7 @@ Discourse::Application.routes.draw do # creating an alias cause the api was extended to mark a single notification # this allows us to cleanly target it put "read" => "notifications#mark_read" + get "totals" => "notifications#totals" end end diff --git a/plugins/chat/lib/chat/channel_fetcher.rb b/plugins/chat/lib/chat/channel_fetcher.rb index 3a557af0dca..73e79c9f0eb 100644 --- a/plugins/chat/lib/chat/channel_fetcher.rb +++ b/plugins/chat/lib/chat/channel_fetcher.rb @@ -253,6 +253,20 @@ module Chat ).report end + def self.unreads_total(guardian) + result = 0 + + public_channels = secured_public_channels(guardian, status: :open, following: true) + publics = tracking_state(public_channels.map(&:id), guardian, include_threads: true) + publics.channel_tracking.each_value { |channel_info| result += channel_info[:mention_count] } + + direct_message_channels = secured_direct_message_channels(guardian.user.id, guardian) + directs = tracking_state(direct_message_channels.map(&:id), guardian) + directs.channel_tracking.each_value { |channel_info| result += channel_info[:unread_count] } + + result + end + def self.find_with_access_check(channel_id_or_slug, guardian) base_channel_relation = Chat::Channel.includes(:chatable) diff --git a/plugins/chat/plugin.rb b/plugins/chat/plugin.rb index 1e26ecc3575..b2728cb7473 100644 --- a/plugins/chat/plugin.rb +++ b/plugins/chat/plugin.rb @@ -246,6 +246,16 @@ after_initialize do .map { |row| { channel_id: row[0], data: row[1], thread_id: row[2] } } end + add_to_serializer( + :user_notification_total, + :chat_notifications, + include_condition: -> do + return @has_chat_enabled if defined?(@has_chat_enabled) + @has_chat_enabled = + SiteSetting.chat_enabled && scope.can_chat? && object.user_option.chat_enabled + end, + ) { Chat::ChannelFetcher.unreads_total(self.scope) } + add_to_serializer(:user_option, :chat_enabled) { object.chat_enabled } add_to_serializer( diff --git a/plugins/chat/spec/lib/chat/channel_fetcher_spec.rb b/plugins/chat/spec/lib/chat/channel_fetcher_spec.rb index 4664f7f3185..ad86be89c6f 100644 --- a/plugins/chat/spec/lib/chat/channel_fetcher_spec.rb +++ b/plugins/chat/spec/lib/chat/channel_fetcher_spec.rb @@ -415,6 +415,63 @@ describe Chat::ChannelFetcher do end end + describe ".unreads_total" do + it "returns correct totals for DMs and mentions" do + result = described_class.unreads_total(guardian) + + expect(result).to eq(0) + + Fabricate( + :user_chat_channel_membership_for_dm, + chat_channel: direct_message_channel1, + user: user1, + following: true, + ) + Chat::DirectMessageUser.create!(direct_message: dm_channel1, user: user1) + Chat::DirectMessageUser.create!(direct_message: dm_channel1, user: user2) + Fabricate( + :user_chat_channel_membership_for_dm, + chat_channel: direct_message_channel2, + user: user1, + following: true, + ) + Chat::DirectMessageUser.create!(direct_message: dm_channel2, user: user1) + Chat::DirectMessageUser.create!(direct_message: dm_channel2, user: user2) + + dm_1 = Fabricate(:chat_message, user: user1, chat_channel: direct_message_channel1) + dm_2 = Fabricate(:chat_message, user: user1, chat_channel: direct_message_channel2) + + direct_message_channel1.update!(last_message: dm_1) + direct_message_channel1.last_message.update!(created_at: 1.day.ago) + direct_message_channel2.update!(last_message: dm_2) + direct_message_channel2.last_message.update!(created_at: 1.hour.ago) + + result = described_class.unreads_total(guardian) + expect(result).to eq(2) + + membership = + Fabricate(:user_chat_channel_membership, chat_channel: category_channel, user: user1) + message = + Fabricate(:chat_message, chat_channel: category_channel, message: "bonjour", user: user2) + notification = + Notification.create!( + notification_type: Notification.types[:chat_mention], + user_id: user1.id, + data: { chat_message_id: message.id, chat_channel_id: category_channel.id }.to_json, + ) + Chat::UserMention.create!(notifications: [notification], user: user1, chat_message: message) + + result = described_class.unreads_total(guardian) + expect(result).to eq(3) # 2 DMs + 1 mention + + # mark mention as read + membership.update!(last_read_message_id: message.id) + + result = described_class.unreads_total(guardian) + expect(result).to eq(2) # only 2 DMs left unread + end + end + describe ".find_with_access_check" do it "raises NotFound if the channel does not exist" do category_channel.destroy! diff --git a/plugins/chat/spec/requests/core_ext/notifications_controller_spec.rb b/plugins/chat/spec/requests/core_ext/notifications_controller_spec.rb new file mode 100644 index 00000000000..ecd9bfa1238 --- /dev/null +++ b/plugins/chat/spec/requests/core_ext/notifications_controller_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +RSpec.describe NotificationsController do + context "when logged in" do + fab!(:chatters) { Fabricate(:group) } + fab!(:user) { Fabricate(:user, group_ids: [chatters.id]) } + fab!(:user2) { Fabricate(:user, group_ids: [chatters.id]) } + fab!(:dm1) { Fabricate(:direct_message) } + fab!(:direct_message_channel1) { Fabricate(:direct_message_channel, chatable: dm1) } + fab!(:dm2) { Fabricate(:direct_message) } + fab!(:direct_message_channel2) { Fabricate(:direct_message_channel, chatable: dm2) } + + before do + Jobs.run_immediately! + SiteSetting.chat_enabled = true + SiteSetting.chat_allowed_groups = [chatters.id] + sign_in(user) + end + + def create_dm(user, channel, dm) + Fabricate( + :user_chat_channel_membership_for_dm, + chat_channel: channel, + user: user, + following: true, + ) + Chat::DirectMessageUser.create!(direct_message: dm, user: user) + + msg = Fabricate(:chat_message, user: user, chat_channel: channel) + + channel.update!(last_message: msg) + channel.last_message.update!(created_at: 1.day.ago) + end + + describe "#totals" do + it "has a total of 0 chat notifications by default" do + get "/notifications/totals.json" + + expect(response.status).to eq(200) + expect(response.parsed_body["chat_notifications"]).to eq(0) + end + + it "returns the correct chat notifications count for unread DMs" do + create_dm(user, direct_message_channel1, dm1) + + get "/notifications/totals.json" + + expect(response.status).to eq(200) + expect(response.parsed_body["chat_notifications"]).to eq(1) + + create_dm(user, direct_message_channel2, dm2) + + get "/notifications/totals.json" + + expect(response.parsed_body["chat_notifications"]).to eq(2) + end + end + end +end diff --git a/spec/models/topic_tracking_state_spec.rb b/spec/models/topic_tracking_state_spec.rb index 42d778b0e46..c93fee99ad4 100644 --- a/spec/models/topic_tracking_state_spec.rb +++ b/spec/models/topic_tracking_state_spec.rb @@ -755,6 +755,89 @@ RSpec.describe TopicTrackingState do end end + describe ".report_totals" do + fab!(:user2) { Fabricate(:user) } + + it "correctly returns new/unread totals" do + report = TopicTrackingState.report_totals(user) + expect(report).to eq({ new: 0, unread: 0 }) + + post.topic.notifier.watch_topic!(post.topic.user_id) + + report = TopicTrackingState.report_totals(user) + expect(report).to eq({ new: 1, unread: 0 }) + + create_post(user: user, topic: post.topic) + + # when user replies, they have 0 new count + report = TopicTrackingState.report_totals(user) + expect(report).to eq({ new: 0, unread: 0 }) + + # when we reply the poster will have an unread item + report = TopicTrackingState.report_totals(post.user) + expect(report).to eq({ new: 0, unread: 1 }) + + create_post(user: user2, topic: post.topic) + + # when a third user replies, the original user should have an unread item + report = TopicTrackingState.report_totals(user) + expect(report).to eq({ new: 0, unread: 1 }) + + # the post user still has one unread + report = TopicTrackingState.report_totals(post.user) + expect(report).to eq({ new: 0, unread: 1 }) + + post2 = create_post + post2.topic.notifier.watch_topic!(user.id) + + # watching another new topic bumps the new count + report = TopicTrackingState.report_totals(user) + expect(report).to eq({ new: 1, unread: 1 }) + end + + it "respects treat_as_new_topic_start_date user option" do + report = TopicTrackingState.report_totals(user) + expect(report).to eq({ new: 0, unread: 0 }) + + post.topic.notifier.watch_topic!(post.topic.user_id) + + report = TopicTrackingState.report_totals(user) + expect(report).to eq({ new: 1, unread: 0 }) + + user.user_option.new_topic_duration_minutes = 5 + user.user_option.save + post.topic.created_at = 10.minutes.ago + post.topic.save + + report = TopicTrackingState.report_totals(user) + expect(report).to eq({ new: 0, unread: 0 }) + end + + it "respects new_new_view_enabled" do + new_new_group = Fabricate(:group) + SiteSetting.experimental_new_new_view_groups = new_new_group.name + user.groups << new_new_group + + report = TopicTrackingState.report_totals(user) + expect(report).to eq({ new: 0 }) + + post.topic.notifier.watch_topic!(post.topic.user_id) + + post2 = create_post + Fabricate(:post, topic: post2.topic) + + tracking = { + notification_level: TopicUser.notification_levels[:tracking], + last_read_post_number: 1, + } + + TopicUser.change(user.id, post2.topic_id, tracking) + + report = TopicTrackingState.report_totals(user) + expect(report).to eq({ new: 2 }) + end + end + describe ".publish_recover" do include_examples("publishes message to right groups and users", "/recover", :publish_recover) include_examples("does not publish message for private topics", :publish_recover) diff --git a/spec/requests/notifications_controller_spec.rb b/spec/requests/notifications_controller_spec.rb index 1c725999536..8634c32b1c5 100644 --- a/spec/requests/notifications_controller_spec.rb +++ b/spec/requests/notifications_controller_spec.rb @@ -618,5 +618,12 @@ RSpec.describe NotificationsController do delete_notification(403, :to) end end + + describe "#totals" do + it "can't see notification totals" do + get "/notifications/totals.json" + expect(response.status).to eq(403) + end + end end end diff --git a/spec/serializers/user_notification_total_serializer_spec.rb b/spec/serializers/user_notification_total_serializer_spec.rb new file mode 100644 index 00000000000..cf850763f58 --- /dev/null +++ b/spec/serializers/user_notification_total_serializer_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +RSpec.describe UserNotificationTotalSerializer do + fab!(:user) { Fabricate(:user, trust_level: 3) } + + fab!(:notification) { Fabricate(:notification, user: user, read: false) } + fab!(:pm_notification) do + Fabricate(:notification, user: user, notification_type: Notification.types[:private_message]) + end + fab!(:pm_notification2) do + Fabricate(:notification, user: user, notification_type: Notification.types[:private_message]) + end + fab!(:group_message_notification) do + Fabricate( + :notification, + user: user, + notification_type: Notification.types[:group_message_summary], + data: { group_id: 1, group_name: "Group", inbox_count: 5 }.to_json, + ) + end + fab!(:reviewable) + + let(:serializer) { described_class.new(user, scope: Guardian.new(user), root: false) } + let(:serialized_data) { serializer.as_json } + + it "includes the user's unread regular notifications count" do + # notification + group_message_notification - pm_notifications + expect(serialized_data[:unread_notifications]).to eq(2) + end + + it "includes the user's unread private messages count" do + expect(serialized_data[:unread_personal_messages]).to eq(2) + end + + context "when the user has PMs disabled" do + it "does not include the user's unread private messages count" do + SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:trust_level_4] + expect(serialized_data).not_to have_key(:unread_personal_messages) + end + end + + it "includes group inbox notification counts" do + expect(serialized_data[:group_inboxes]).to contain_exactly( + { group_id: 1, group_name: "Group", count: 5 }, + ) + end + + context "when the user is staff" do + before { user.update!(admin: true) } + + it "includes the count of unseen reviewables" do + expect(serialized_data[:unseen_reviewables]).to eq(1) + end + end + + context "when the user is not staff" do + it "does not include unseen reviewables counts" do + expect(serialized_data).not_to have_key(:unseen_reviewables) + end + end +end