diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index eb1214cb31c..f24aaaefa94 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1842,16 +1842,21 @@ class UsersController < ApplicationController if unread_notifications.size < USER_MENU_LIST_LIMIT exclude_topic_ids = unread_notifications.filter_map(&:topic_id).uniq limit = USER_MENU_LIST_LIMIT - unread_notifications.size + messages_list = TopicQuery .new(current_user, per_page: limit) - .list_private_messages_direct_and_groups(current_user) do |query| + .list_private_messages_direct_and_groups( + current_user, + groups_messages_notification_level: :watching, + ) do |query| if exclude_topic_ids.present? query.where("topics.id NOT IN (?)", exclude_topic_ids) else query end end + read_notifications = Notification .for_user_menu(current_user.id, limit: limit) diff --git a/lib/topic_query/private_message_lists.rb b/lib/topic_query/private_message_lists.rb index 932a76c6de5..87228950b49 100644 --- a/lib/topic_query/private_message_lists.rb +++ b/lib/topic_query/private_message_lists.rb @@ -3,15 +3,20 @@ class TopicQuery module PrivateMessageLists def list_private_messages(user, &blk) - list = private_messages_for(user, :user) + list = user_personal_private_messages(user) list = not_archived(list, user) list = have_posts_from_others(list, user) create_list(:private_messages, {}, list, &blk) end - def list_private_messages_direct_and_groups(user, &blk) - list = private_messages_for(user, :all) + def list_private_messages_direct_and_groups(user, groups_messages_notification_level: nil, &blk) + list = + user_personal_and_groups_private_messages( + user, + groups_messages_notification_level: groups_messages_notification_level, + ) + list = not_archived(list, user) list = not_archived_in_groups(list) list = have_posts_from_others(list, user) @@ -20,14 +25,16 @@ class TopicQuery end def list_private_messages_archive(user) - list = private_messages_for(user, :user) + list = user_personal_private_messages(user) + list = list.joins(:user_archived_messages).where("user_archived_messages.user_id = ?", user.id) + create_list(:private_messages, {}, list) end def list_private_messages_sent(user) - list = private_messages_for(user, :user) + list = user_personal_private_messages(user) list = list.where(<<~SQL, user.id) EXISTS ( @@ -54,7 +61,7 @@ class TopicQuery end def list_private_messages_group(user) - list = private_messages_for(user, :group) + list = user_groups_private_messages(user) list = list.joins(<<~SQL) LEFT JOIN group_archived_messages gm @@ -68,7 +75,7 @@ class TopicQuery end def list_private_messages_group_archive(user) - list = private_messages_for(user, :group) + list = user_groups_private_messages(user) list = list.joins(<<~SQL) INNER JOIN group_archived_messages gm @@ -96,7 +103,7 @@ class TopicQuery end def list_private_messages_warnings(user) - list = private_messages_for(user, :user) + list = user_personal_private_messages(user) list = list.where("topics.subtype = ?", TopicSubtype.moderator_warning) # Exclude official warnings that the user created, instead of received list = list.where("topics.user_id <> ?", user.id) @@ -104,79 +111,24 @@ class TopicQuery end def private_messages_for(user, type) - options = @options - options.reverse_merge!(per_page: per_page_setting) - - result = Topic.includes(:allowed_users) - result = result.includes(:tags) if SiteSetting.tagging_enabled - if type == :group - result = - result.joins( - "INNER JOIN topic_allowed_groups tag ON tag.topic_id = topics.id AND tag.group_id IN (SELECT id FROM groups WHERE LOWER(name) = '#{PG::Connection.escape_string(@options[:group_name].downcase)}')", - ) - - unless user.admin? - result = - result.joins( - "INNER JOIN group_users gu ON gu.group_id = tag.group_id AND gu.user_id = #{user.id.to_i}", - ) - end + user_groups_private_messages(user) elsif type == :user - result = - result.where( - "topics.id IN (SELECT topic_id FROM topic_allowed_users WHERE user_id = ?)", - user.id.to_i, - ) + user_personal_private_messages(user) elsif type == :all - group_ids = group_with_messages_ids(user) - - result = - if group_ids.present? - result.where(<<~SQL, user.id.to_i, group_ids) - topics.id IN ( - SELECT topic_id - FROM topic_allowed_users - WHERE user_id = ? - UNION ALL - SELECT topic_id FROM topic_allowed_groups - WHERE group_id IN (?) - ) - SQL - else - result.joins(<<~SQL) - INNER JOIN topic_allowed_users tau - ON tau.topic_id = topics.id - AND tau.user_id = #{user.id.to_i} - SQL - end + user_personal_and_groups_private_messages(user) end - - result = - result - .joins( - "LEFT OUTER JOIN topic_users AS tu ON (topics.id = tu.topic_id AND tu.user_id = #{user.id.to_i})", - ) - .order("topics.bumped_at DESC") - .private_messages - - result = result.limit(options[:per_page]) unless options[:limit] == false - result = result.visible if options[:visible] || @user.nil? || @user.regular? - - if options[:page] - offset = options[:page].to_i * options[:per_page] - result = result.offset(offset) if offset > 0 - end - result end def list_private_messages_tag(user) - list = private_messages_for(user, :all) + list = user_personal_and_groups_private_messages(user) + list = list.joins( "JOIN topic_tags tt ON tt.topic_id = topics.id JOIN tags t ON t.id = tt.tag_id AND t.name = '#{@options[:tags][0]}'", ) + create_list(:private_messages, {}, list) end @@ -296,5 +248,98 @@ class TopicQuery @group_with_messages_ids[user.id] = user.groups.where(has_messages: true).pluck(:id) end + + private + + def private_messages_default_scope(user) + options = @options + options.reverse_merge!(per_page: per_page_setting) + + result = + Topic + .private_messages + .includes(:allowed_users) + .joins( + "LEFT OUTER JOIN topic_users AS tu ON (topics.id = tu.topic_id AND tu.user_id = #{user.id.to_i})", + ) + .order("topics.bumped_at DESC") + + result = result.includes(:tags) if SiteSetting.tagging_enabled + result = result.limit(options[:per_page]) unless options[:limit] == false + result = result.visible if options[:visible] || @user.nil? || @user.regular? + + if options[:page] + offset = options[:page].to_i * options[:per_page] + result = result.offset(offset) if offset > 0 + end + + result + end + + def user_groups_private_messages(user) + result = private_messages_default_scope(user) + + result = + result.joins( + "INNER JOIN topic_allowed_groups tag ON tag.topic_id = topics.id AND tag.group_id IN (SELECT id FROM groups WHERE LOWER(name) = '#{PG::Connection.escape_string(@options[:group_name].downcase)}')", + ) + + unless user.admin? + result = + result.joins( + "INNER JOIN group_users gu ON gu.group_id = tag.group_id AND gu.user_id = #{user.id.to_i}", + ) + end + + result + end + + def user_personal_private_messages(user) + result = private_messages_default_scope(user) + + result.where( + "topics.id IN (SELECT topic_id FROM topic_allowed_users WHERE user_id = ?)", + user.id.to_i, + ) + end + + def user_personal_and_groups_private_messages(user, groups_messages_notification_level: nil) + result = private_messages_default_scope(user) + group_ids = group_with_messages_ids(user) + + topic_allowed_groups_scope = + if groups_messages_notification_level.present? && + notification_level = + NotificationLevels.topic_levels[groups_messages_notification_level] + <<~SQL + SELECT topic_allowed_groups.topic_id + FROM topic_allowed_groups + INNER JOIN topic_users ON topic_users.topic_id = topic_allowed_groups.topic_id AND topic_users.user_id = :user_id + WHERE group_id IN (:group_ids) + AND topic_users.notification_level >= #{notification_level.to_i} + SQL + else + "SELECT topic_id FROM topic_allowed_groups WHERE group_id IN (:group_ids)" + end + + result = + if group_ids.present? + result.where(<<~SQL, user_id: user.id.to_i, group_ids: group_ids) + topics.id IN ( + SELECT topic_id + FROM topic_allowed_users + WHERE user_id = :user_id + UNION ALL + #{topic_allowed_groups_scope} + ) + SQL + else + result.joins(<<~SQL) + INNER JOIN topic_allowed_users tau + ON tau.topic_id = topics.id + AND tau.user_id = #{user.id.to_i} + SQL + end + end end end diff --git a/spec/lib/topic_query/private_message_lists_spec.rb b/spec/lib/topic_query/private_message_lists_spec.rb index 402e7a303ef..136548de40f 100644 --- a/spec/lib/topic_query/private_message_lists_spec.rb +++ b/spec/lib/topic_query/private_message_lists_spec.rb @@ -320,4 +320,38 @@ RSpec.describe TopicQuery::PrivateMessageLists do expect(TopicQuery.new(user_4).private_messages_for(user_4, :all)).to eq([]) end end + + describe "#list_private_messages_direct_and_groups" do + it "returns a list of all personal and group private messages for a given user" do + expect( + TopicQuery.new(user_2).list_private_messages_direct_and_groups(user_2).topics, + ).to contain_exactly(private_message, group_message) + end + + it "returns a list of personal private messages and user watching group private messages for a given user when the `groups_notification_level` option is set" do + expect( + TopicQuery + .new(user_2) + .list_private_messages_direct_and_groups( + user_2, + groups_messages_notification_level: :watching, + ) + .topics, + ).to contain_exactly(private_message, group_message) + + TopicUser.find_by(user: user_2, topic: group_message).update!( + notification_level: NotificationLevels.topic_levels[:regular], + ) + + expect( + TopicQuery + .new(user_2) + .list_private_messages_direct_and_groups( + user_2, + groups_messages_notification_level: :watching, + ) + .topics, + ).to contain_exactly(private_message) + end + end end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index e7a7a6ec0c7..6af48bec505 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -6526,24 +6526,31 @@ RSpec.describe UsersController do ) end - it "responds with an array of PM topics that are not associated with any of the unread private_message notifications" do + it "responds with an array of personal messages and user watching group messages that are not associated with any of the unread private_message notifications" do group_message1.update!(bumped_at: 1.minutes.ago) message_without_notification.update!(bumped_at: 3.minutes.ago) group_message2.update!(bumped_at: 6.minutes.ago) message_with_read_notification.update!(bumped_at: 10.minutes.ago) read_group_message_summary_notification.destroy! + TopicUser.create!( + user: user, + topic: group_message1, + notification_level: TopicUser.notification_levels[:watching], + ) + TopicUser.create!( + user: user, + topic: group_message2, + notification_level: TopicUser.notification_levels[:regular], + ) + 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 eq( - [ - group_message1.id, - message_without_notification.id, - group_message2.id, - message_with_read_notification.id, - ], + [group_message1.id, message_without_notification.id, message_with_read_notification.id], ) end