From 5a5625460bd9ac3e156c7fa26739ef073143bdd0 Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Fri, 30 Sep 2022 08:44:04 +0300 Subject: [PATCH] DEV: Add group messages and group_message_summary notifications in the messages tab in the user menu (#18390) This commit adds non-archived group messages and `group_message_summary` notifications in the messages tab in the user menu. With this change, the messages tab in the user menu now includes 3 types of items: 1. Unread `private_message` notifications (notifications when you receive a reply in a PM) 2. Unread and read `group_message_summary` notifications (notifications when there's a new message in a group inbox that you track) 3. Non-archived personal and group messages Unread `private_message` notifications are always shown first, followed by unread `group_message_summary` notifications, and then everything else (messages and read `group_message_summary` notifications) sorted by recency (most recent first). Internal topic: t/72976. --- .../app/components/user-menu/menu.js | 2 +- .../app/components/user-menu/messages-list.js | 56 ++++- .../tests/acceptance/user-menu-test.js | 4 +- .../discourse/tests/fixtures/user-menu.js | 147 +++++------ .../user-menu/messages-list-test.js | 233 +++++++++++++++++- app/controllers/users_controller.rb | 61 +++-- app/models/notification.rb | 23 +- lib/topic_query/private_message_lists.rb | 30 ++- spec/fabricators/post_fabricator.rb | 19 ++ spec/requests/users_controller_spec.rb | 119 +++++++-- 10 files changed, 554 insertions(+), 140 deletions(-) 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 403737a8980..ff21b008a0e 100644 --- a/app/assets/javascripts/discourse/app/components/user-menu/menu.js +++ b/app/assets/javascripts/discourse/app/components/user-menu/menu.js @@ -170,7 +170,7 @@ const CORE_TOP_TABS = [ } get notificationTypes() { - return ["private_message"]; + return ["private_message", "group_message_summary"]; } get linkWhenActive() { 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 ac434948c08..7d916ec668b 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 @@ -7,9 +7,21 @@ import UserMenuNotificationItem from "discourse/lib/user-menu/notification-item" import UserMenuMessageItem from "discourse/lib/user-menu/message-item"; import Topic from "discourse/models/topic"; +function parseDateString(date) { + if (date) { + return new Date(date); + } +} + +async function initializeNotifications(rawList) { + const notifications = rawList.map((n) => Notification.create(n)); + await Notification.applyTransformations(notifications); + return notifications; +} + export default class UserMenuMessagesList extends UserMenuNotificationsList { get dismissTypes() { - return ["private_message"]; + return this.filterByTypes; } get showAllHref() { @@ -51,9 +63,10 @@ export default class UserMenuMessagesList extends UserMenuNotificationsList { ); const content = []; - const notifications = data.notifications.map((n) => Notification.create(n)); - await Notification.applyTransformations(notifications); - notifications.forEach((notification) => { + const unreadNotifications = await initializeNotifications( + data.unread_notifications + ); + unreadNotifications.forEach((notification) => { content.push( new UserMenuNotificationItem({ notification, @@ -66,12 +79,39 @@ export default class UserMenuMessagesList extends UserMenuNotificationsList { const topics = data.topics.map((t) => Topic.create(t)); await Topic.applyTransformations(topics); - content.push( - ...topics.map((topic) => { - return new UserMenuMessageItem({ message: topic }); - }) + + const readNotifications = await initializeNotifications( + data.read_notifications ); + let latestReadNotificationDate = parseDateString( + readNotifications[0]?.created_at + ); + let latestMessageDate = parseDateString(topics[0]?.bumped_at); + + while (latestReadNotificationDate || latestMessageDate) { + if ( + !latestReadNotificationDate || + (latestMessageDate && latestReadNotificationDate < latestMessageDate) + ) { + content.push(new UserMenuMessageItem({ message: topics[0] })); + topics.shift(); + latestMessageDate = parseDateString(topics[0]?.bumped_at); + } else { + content.push( + new UserMenuNotificationItem({ + notification: readNotifications[0], + currentUser: this.currentUser, + siteSettings: this.siteSettings, + site: this.site, + }) + ); + readNotifications.shift(); + latestReadNotificationDate = parseDateString( + readNotifications[0]?.created_at + ); + } + } return content; } 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 a6fba10a5e3..09f9a135f70 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-menu-test.js @@ -805,7 +805,7 @@ acceptance("User menu - Dismiss button", function (needs) { const copy = cloneJSON( UserMenuFixtures["/u/:username/user-menu-private-messages"] ); - copy.notifications = []; + copy.unread_notifications = []; return helper.response(copy); } else { return helper.response( @@ -941,7 +941,7 @@ acceptance("User menu - Dismiss button", function (needs) { assert.ok(markRead, "mark-read request is sent"); assert.strictEqual( markReadRequestBody, - "dismiss_types=private_message", + "dismiss_types=private_message%2Cgroup_message_summary", "mark-read request specifies private_message types" ); assert.notOk(exists(".user-menu .notifications-dismiss")); diff --git a/app/assets/javascripts/discourse/tests/fixtures/user-menu.js b/app/assets/javascripts/discourse/tests/fixtures/user-menu.js index a645cce16a0..63b8bdc4b93 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/user-menu.js +++ b/app/assets/javascripts/discourse/tests/fixtures/user-menu.js @@ -64,79 +64,80 @@ export default { ], }, "/u/:username/user-menu-private-messages": { - notifications: [ - { - id: 8315, - user_id: 1, - notification_type: 6, - read: false, - high_priority: true, - created_at: "2022-08-05T17:27:24.873Z", - post_number: 1, - topic_id: 249, - fancy_title: "Very secret message!", - slug: "very-secret-message", - data: { - topic_title: "very secret message!", - original_post_id: 1043, - original_post_type: 1, - original_username: "osama", - revision_number: null, - display_username: "osama" - }, + unread_notifications: [ + { + id: 8315, + user_id: 1, + notification_type: 6, + read: false, + high_priority: true, + created_at: "2022-08-05T17:27:24.873Z", + post_number: 1, + topic_id: 249, + fancy_title: "Very secret message!", + slug: "very-secret-message", + data: { + topic_title: "very secret message!", + original_post_id: 1043, + original_post_type: 1, + original_username: "osama", + revision_number: null, + display_username: "osama" }, - ], - topics: [ - { - id: 8092, - title: "BUG: Can not render emoji properly :/", - fancy_title: "BUG: Can not render emoji properly :confused:", - slug: "bug-can-not-render-emoji-properly", - posts_count: 1, - reply_count: 0, - highest_post_number: 2, - image_url: null, - created_at: "2019-07-26T01:29:24.008Z", - last_posted_at: "2019-07-26T01:29:24.177Z", - bumped: true, - bumped_at: "2019-07-26T01:29:24.177Z", - unseen: false, - last_read_post_number: 2, - unread_posts: 0, - pinned: false, - unpinned: null, - visible: true, - closed: false, - archived: false, - notification_level: 3, - bookmarked: false, - bookmarks: [], - liked: false, - views: 5, - like_count: 0, - has_summary: false, - archetype: "private_message", - last_poster_username: "mixtape", - category_id: null, - pinned_globally: false, - featured_link: null, - posters: [ - { - extras: "latest single", - description: "Original Poster, Most Recent Poster", - user_id: 13, - primary_group_id: null, - }, - ], - participants: [ - { - extras: "latest", - description: null, - user_id: 13, - primary_group_id: null, - }, - ], - } - ], + }, + ], + topics: [ + { + id: 8092, + title: "BUG: Can not render emoji properly :/", + fancy_title: "BUG: Can not render emoji properly :confused:", + slug: "bug-can-not-render-emoji-properly", + posts_count: 1, + reply_count: 0, + highest_post_number: 2, + image_url: null, + created_at: "2019-07-26T01:29:24.008Z", + last_posted_at: "2019-07-26T01:29:24.177Z", + bumped: true, + bumped_at: "2019-07-26T01:29:24.177Z", + unseen: false, + last_read_post_number: 2, + unread_posts: 0, + pinned: false, + unpinned: null, + visible: true, + closed: false, + archived: false, + notification_level: 3, + bookmarked: false, + bookmarks: [], + liked: false, + views: 5, + like_count: 0, + has_summary: false, + archetype: "private_message", + last_poster_username: "mixtape", + category_id: null, + pinned_globally: false, + featured_link: null, + posters: [ + { + extras: "latest single", + description: "Original Poster, Most Recent Poster", + user_id: 13, + primary_group_id: null, + }, + ], + participants: [ + { + extras: "latest", + description: null, + user_id: 13, + primary_group_id: null, + }, + ], + } + ], + read_notifications: [], } } 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 02fba00dc70..49de60fb0c8 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 @@ -5,14 +5,128 @@ import { render, settled } from "@ember/test-helpers"; import { NOTIFICATION_TYPES } from "discourse/tests/fixtures/concerns/notification-types"; import { hbs } from "ember-cli-htmlbars"; import pretender, { response } from "discourse/tests/helpers/create-pretender"; +import { cloneJSON, deepMerge } from "discourse-common/lib/object"; +import UserMenuFixtures from "discourse/tests/fixtures/user-menu"; import I18n from "I18n"; +function getMessage(overrides = {}) { + return deepMerge( + { + id: 8092, + title: "Test ToPic 4422", + fancy_title: "Test topic 4422", + slug: "test-topic-4422", + posts_count: 1, + reply_count: 0, + highest_post_number: 2, + image_url: null, + created_at: "2019-07-26T01:29:24.008Z", + last_posted_at: "2019-07-26T01:29:24.177Z", + bumped: true, + bumped_at: "2019-07-26T01:29:24.177Z", + unseen: false, + last_read_post_number: 2, + unread_posts: 0, + pinned: false, + unpinned: null, + visible: true, + closed: false, + archived: false, + notification_level: 3, + bookmarked: false, + bookmarks: [], + liked: false, + views: 5, + like_count: 0, + has_summary: false, + archetype: "private_message", + last_poster_username: "mixtape", + category_id: null, + pinned_globally: false, + featured_link: null, + posters: [ + { + extras: "latest single", + description: "Original Poster, Most Recent Poster", + user_id: 13, + primary_group_id: null, + }, + ], + participants: [ + { + extras: "latest", + description: null, + user_id: 13, + primary_group_id: null, + }, + ], + }, + overrides + ); +} + +function getGroupMessageSummaryNotification(overrides = {}) { + return deepMerge( + { + id: 9492, + user_id: 1, + notification_type: 16, + read: true, + high_priority: false, + created_at: "2022-08-05T17:27:24.873Z", + post_number: null, + topic_id: null, + fancy_title: null, + slug: null, + data: { + group_id: 1, + group_name: "jokers", + inbox_count: 4, + username: "joker.leader", + }, + }, + overrides + ); +} + module("Integration | Component | user-menu | messages-list", function (hooks) { setupRenderingTest(hooks); const template = hbs``; - test("renders notifications on top and messages on bottom", async function (assert) { + test("renders unread PM notifications first followed by messages and read group_message_summary notifications", async function (assert) { + pretender.get("/u/eviltrout/user-menu-private-messages", () => { + const copy = cloneJSON( + UserMenuFixtures["/u/:username/user-menu-private-messages"] + ); + copy.read_notifications = [getGroupMessageSummaryNotification()]; + return response(copy); + }); + await render(template); + const items = queryAll("ul li"); + + assert.strictEqual(items.length, 3); + + assert.ok(items[0].classList.contains("notification")); + assert.ok(items[0].classList.contains("unread")); + assert.ok(items[0].classList.contains("private-message")); + + assert.ok(items[1].classList.contains("notification")); + assert.ok(items[1].classList.contains("read")); + assert.ok(items[1].classList.contains("group-message-summary")); + + assert.ok(items[2].classList.contains("message")); + }); + + test("does not error when there are no group_message_summary notifications", async function (assert) { + pretender.get("/u/eviltrout/user-menu-private-messages", () => { + const copy = cloneJSON( + UserMenuFixtures["/u/:username/user-menu-private-messages"] + ); + copy.read_notifications = []; + return response(copy); + }); + await render(template); const items = queryAll("ul li"); @@ -25,6 +139,117 @@ module("Integration | Component | user-menu | messages-list", function (hooks) { assert.ok(items[1].classList.contains("message")); }); + test("does not error when there are no messages", async function (assert) { + pretender.get("/u/eviltrout/user-menu-private-messages", () => { + const copy = cloneJSON( + UserMenuFixtures["/u/:username/user-menu-private-messages"] + ); + copy.topics = []; + copy.read_notifications = [getGroupMessageSummaryNotification()]; + return response(copy); + }); + + await render(template); + const items = queryAll("ul li"); + + assert.strictEqual(items.length, 2); + + assert.ok(items[0].classList.contains("notification")); + assert.ok(items[0].classList.contains("unread")); + assert.ok(items[0].classList.contains("private-message")); + + assert.ok(items[1].classList.contains("notification")); + assert.ok(items[1].classList.contains("read")); + assert.ok(items[1].classList.contains("group-message-summary")); + }); + + test("merge-sorts group_message_summary notifications and messages", async function (assert) { + pretender.get("/u/eviltrout/user-menu-private-messages", () => { + const copy = cloneJSON( + UserMenuFixtures["/u/:username/user-menu-private-messages"] + ); + copy.unread_notifications = []; + copy.topics = [ + getMessage({ + bumped_at: "2014-07-26T01:29:24.177Z", + fancy_title: "Test Topic 0003", + }), + getMessage({ + bumped_at: "2012-07-26T01:29:24.177Z", + fancy_title: "Test Topic 0002", + }), + getMessage({ + bumped_at: "2010-07-26T01:29:24.177Z", + fancy_title: "Test Topic 0001", + }), + ]; + copy.read_notifications = [ + getGroupMessageSummaryNotification({ + created_at: "2015-07-26T01:29:24.177Z", + data: { + inbox_count: 13, + }, + }), + getGroupMessageSummaryNotification({ + created_at: "2013-07-26T01:29:24.177Z", + data: { + inbox_count: 12, + }, + }), + getGroupMessageSummaryNotification({ + created_at: "2011-07-26T01:29:24.177Z", + data: { + inbox_count: 11, + }, + }), + ]; + return response(copy); + }); + await render(template); + const items = queryAll("ul li"); + + assert.strictEqual(items.length, 6); + + assert.strictEqual( + items[0].textContent.trim(), + I18n.t("notifications.group_message_summary", { + count: 13, + group_name: "jokers", + }) + ); + + assert.strictEqual( + items[1].textContent.trim().replaceAll(/\s+/g, " "), + "mixtape Test Topic 0003" + ); + + assert.strictEqual( + items[2].textContent.trim(), + I18n.t("notifications.group_message_summary", { + count: 12, + group_name: "jokers", + }) + ); + + assert.strictEqual( + items[3].textContent.trim().replaceAll(/\s+/g, " "), + "mixtape Test Topic 0002" + ); + + assert.strictEqual( + items[4].textContent.trim(), + I18n.t("notifications.group_message_summary", { + count: 11, + group_name: "jokers", + }) + ); + + assert.strictEqual( + items[5].textContent.trim().replaceAll(/\s+/g, " "), + "mixtape Test Topic 0001" + ); + }); + test("show all link", async function (assert) { await render(template); const link = query(".panel-body-bottom .show-all"); @@ -66,7 +291,11 @@ module("Integration | Component | user-menu | messages-list", function (hooks) { test("empty state (aka blank page syndrome)", async function (assert) { pretender.get("/u/eviltrout/user-menu-private-messages", () => { - return response({ notifications: [], topics: [] }); + return response({ + unread_notifications: [], + topics: [], + read_notifications: [], + }); }); await render(template); assert.strictEqual( diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 9367dd73ffa..255da241f90 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1746,11 +1746,10 @@ class UsersController < ApplicationController raise Discourse::InvalidAccess.new("username doesn't match current_user's username") end - reminder_notifications = Notification.unread_type( - current_user, - Notification.types[:bookmark_reminder], - USER_MENU_LIST_LIMIT - ) + reminder_notifications = Notification + .for_user_menu(current_user.id, limit: USER_MENU_LIST_LIMIT) + .unread + .where(notification_type: Notification.types[:bookmark_reminder]) if reminder_notifications.size < USER_MENU_LIST_LIMIT exclude_bookmark_ids = reminder_notifications @@ -1803,29 +1802,37 @@ class UsersController < ApplicationController raise Discourse::InvalidAccess.new("personal messages are disabled.") end - message_notifications = Notification.unread_type( - current_user, - Notification.types[:private_message], - USER_MENU_LIST_LIMIT - ) + unread_notifications = Notification + .for_user_menu(current_user.id, limit: USER_MENU_LIST_LIMIT) + .unread + .where(notification_type: [Notification.types[:private_message], Notification.types[:group_message_summary]]) + .to_a - if message_notifications.size < USER_MENU_LIST_LIMIT - exclude_topic_ids = message_notifications.map(&:topic_id).uniq + 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: USER_MENU_LIST_LIMIT - message_notifications.size - ).list_private_messages(current_user) do |query| + per_page: limit + ).list_private_messages_direct_and_groups(current_user) 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) + .where( + read: true, + notification_type: Notification.types[:group_message_summary], + ) + .to_a end - if message_notifications.present? - serialized_notifications = ActiveModel::ArraySerializer.new( - message_notifications, + if unread_notifications.present? + serialized_unread_notifications = ActiveModel::ArraySerializer.new( + unread_notifications, each_serializer: NotificationSerializer, scope: guardian ) @@ -1840,8 +1847,17 @@ class UsersController < ApplicationController )[:topics] end + if read_notifications.present? + serialized_read_notifications = ActiveModel::ArraySerializer.new( + read_notifications, + each_serializer: NotificationSerializer, + scope: guardian + ) + end + render json: { - notifications: serialized_notifications || [], + unread_notifications: serialized_unread_notifications || [], + read_notifications: serialized_read_notifications || [], topics: serialized_messages || [] } end @@ -2027,13 +2043,4 @@ class UsersController < ApplicationController { users: ActiveModel::ArraySerializer.new(users, each_serializer: each_serializer).as_json } end - - def find_unread_notifications_of_type(type, limit) - current_user - .notifications - .visible - .includes(:topic) - .where(read: false, notification_type: type) - .limit(limit) - end end diff --git a/app/models/notification.rb b/app/models/notification.rb index fc4e8ffe69a..3c72cc2f1df 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -15,14 +15,26 @@ class Notification < ActiveRecord::Base scope :recent, lambda { |n = nil| n ||= 10; order('notifications.created_at desc').limit(n) } scope :visible , lambda { joins('LEFT JOIN topics ON notifications.topic_id = topics.id') .where('topics.id IS NULL OR topics.deleted_at IS NULL') } - scope :unread_type, ->(user, type, limit = 20) do - where(user_id: user.id, read: false, notification_type: type).visible.includes(:topic).limit(limit) + scope :unread_type, ->(user, type, limit = 30) do + unread_types(user, [type], limit) end - scope :prioritized, ->(limit = nil) do + scope :unread_types, ->(user, types, limit = 30) do + where(user_id: user.id, read: false, notification_type: types) + .visible + .includes(:topic) + .limit(limit) + end + scope :prioritized, ->() do order("notifications.high_priority AND NOT notifications.read DESC") .order("NOT notifications.read DESC") .order("notifications.created_at DESC") - .limit(limit || 30) + end + scope :for_user_menu, ->(user_id, limit: 30) do + where(user_id: user_id) + .visible + .prioritized + .includes(:topic) + .limit(limit) end attr_accessor :skip_send_email @@ -226,7 +238,8 @@ class Notification < ActiveRecord::Base notifications = user.notifications .includes(:topic) .visible - .prioritized(count) + .prioritized + .limit(count) if types.present? notifications = notifications.where(notification_type: types) diff --git a/lib/topic_query/private_message_lists.rb b/lib/topic_query/private_message_lists.rb index cdf5201b020..395b54fad50 100644 --- a/lib/topic_query/private_message_lists.rb +++ b/lib/topic_query/private_message_lists.rb @@ -5,14 +5,16 @@ class TopicQuery def list_private_messages(user, &blk) list = private_messages_for(user, :user) list = not_archived(list, user) + list = have_posts_from_others(list, user) - list = list.where(<<~SQL) - NOT ( - topics.participant_count = 1 - AND topics.user_id = #{user.id.to_i} - AND topics.moderator_posts_count = 0 - ) - SQL + create_list(:private_messages, {}, list, &blk) + end + + def list_private_messages_direct_and_groups(user, &blk) + list = private_messages_for(user, :all) + list = not_archived(list, user) + list = not_archived_in_groups(list) + list = have_posts_from_others(list, user) create_list(:private_messages, {}, list, &blk) end @@ -252,6 +254,20 @@ class TopicQuery .where('um.user_id IS NULL') end + def not_archived_in_groups(list) + list.left_joins(:group_archived_messages).where(group_archived_messages: { id: nil }) + end + + def have_posts_from_others(list, user) + list.where(<<~SQL) + NOT ( + topics.participant_count = 1 + AND topics.user_id = #{user.id.to_i} + AND topics.moderator_posts_count = 0 + ) + SQL + end + def group @group ||= begin Group diff --git a/spec/fabricators/post_fabricator.rb b/spec/fabricators/post_fabricator.rb index 4b35bc721c4..e18628a141a 100644 --- a/spec/fabricators/post_fabricator.rb +++ b/spec/fabricators/post_fabricator.rb @@ -143,6 +143,25 @@ Fabricator(:private_message_post, from: :post) do raw "Ssshh! This is our secret conversation!" end +Fabricator(:group_private_message_post, from: :post) do + transient :recipients + user + topic do |attrs| + Fabricate(:private_message_topic, + user: attrs[:user], + created_at: attrs[:created_at], + subtype: TopicSubtype.user_to_user, + topic_allowed_users: [ + Fabricate.build(:topic_allowed_user, user: attrs[:user]), + ], + topic_allowed_groups: [ + Fabricate.build(:topic_allowed_group, group: attrs[:recipients] || Fabricate(:group)) + ] + ) + end + raw "Ssshh! This is our group secret conversation!" +end + Fabricator(:private_message_post_one_user, from: :post) do user topic do |attrs| diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index ff74b1f41fa..ebcf3a0c568 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -5797,9 +5797,21 @@ RSpec.describe UsersController do end describe "#user_menu_messages" do + fab!(:group1) { Fabricate(:group, has_messages: true, users: [user]) } + fab!(:group2) { Fabricate(:group, has_messages: true, users: [user, user1]) } + fab!(:group3) { Fabricate(:group, has_messages: true, users: [user1]) } + fab!(:message_without_notification) { Fabricate(:private_message_post, recipient: user).topic } fab!(:message_with_read_notification) { Fabricate(:private_message_post, recipient: user).topic } fab!(:message_with_unread_notification) { Fabricate(:private_message_post, recipient: user).topic } + fab!(:archived_message) { Fabricate(:private_message_post, recipient: user).topic } + + fab!(:group_message1) { Fabricate(:group_private_message_post, recipients: group1).topic } + fab!(:group_message2) { Fabricate(:group_private_message_post, recipients: group2).topic } + fab!(:group_message3) { Fabricate(:group_private_message_post, recipients: group3).topic } + + fab!(:archived_group_message1) { Fabricate(:group_private_message_post, recipients: group1).topic } + fab!(:archived_group_message2) { Fabricate(:group_private_message_post, recipients: group2).topic } fab!(:user1_message_without_notification) do Fabricate(:private_message_post, recipient: user1).topic @@ -5810,16 +5822,18 @@ RSpec.describe UsersController do fab!(:user1_message_with_unread_notification) do Fabricate(:private_message_post, recipient: user1).topic end + fab!(:user1_archived_message) { Fabricate(:private_message_post, recipient: user1).topic } - fab!(:unread_notification) do + fab!(:unread_pm_notification) do Fabricate( :private_message_notification, read: false, user: user, - topic: message_with_unread_notification + topic: message_with_unread_notification, + created_at: 4.minutes.ago ) end - fab!(:read_notification) do + fab!(:read_pm_notification) do Fabricate( :private_message_notification, read: true, @@ -5828,7 +5842,27 @@ RSpec.describe UsersController do ) end - fab!(:user1_unread_notification) do + fab!(:unread_group_message_summary_notification) do + Fabricate( + :notification, + read: false, + user: user, + notification_type: Notification.types[:group_message_summary], + created_at: 2.minutes.ago + ) + end + + fab!(:read_group_message_summary_notification) do + Fabricate( + :notification, + read: true, + user: user, + notification_type: Notification.types[:group_message_summary], + created_at: 1.minutes.ago + ) + end + + fab!(:user1_unread_pm_notification) do Fabricate( :private_message_notification, read: false, @@ -5836,7 +5870,7 @@ RSpec.describe UsersController do topic: user1_message_with_unread_notification ) end - fab!(:user1_read_notification) do + fab!(:user1_read_pm_notification) do Fabricate( :private_message_notification, read: true, @@ -5845,6 +5879,30 @@ RSpec.describe UsersController do ) end + fab!(:user1_unread_group_message_summary_notification) do + Fabricate( + :notification, + read: false, + user: user1, + notification_type: Notification.types[:group_message_summary], + ) + end + fab!(:user1_read_group_message_summary_notification) do + Fabricate( + :notification, + read: true, + user: user1, + notification_type: Notification.types[:group_message_summary], + ) + end + + before do + UserArchivedMessage.archive!(user.id, archived_message) + UserArchivedMessage.archive!(user1.id, user1_archived_message) + GroupArchivedMessage.archive!(group1.id, archived_group_message1) + GroupArchivedMessage.archive!(group2.id, archived_group_message2) + end + context "when logged out" do it "responds with 404" do get "/u/#{user.username}/user-menu-private-messages" @@ -5873,33 +5931,62 @@ RSpec.describe UsersController do get "/u/#{user.username}/user-menu-private-messages" expect(response.status).to eq(200) - notifications = response.parsed_body["notifications"] - expect(notifications.map { |notification| notification["id"] }).to contain_exactly( - unread_notification.id + unread_notifications = response.parsed_body["unread_notifications"] + expect(unread_notifications.map { |notification| notification["id"] }).to eq([ + unread_pm_notification.id, + unread_group_message_summary_notification.id + ]) + end + + it "sends an array of read group_message_summary notifications" do + read_group_message_summary_notification2 = Fabricate( + :notification, + read: true, + user: user, + notification_type: Notification.types[:group_message_summary], + created_at: 5.minutes.ago ) + get "/u/#{user.username}/user-menu-private-messages" + expect(response.status).to eq(200) + + read_notifications = response.parsed_body["read_notifications"] + expect(read_notifications.map { |notification| notification["id"] }).to eq([ + read_group_message_summary_notification.id, + read_group_message_summary_notification2.id + ]) end it "responds with an array of PM topics 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! + 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 contain_exactly( + expect(topics.map { |topic| topic["id"] }).to eq([ + group_message1.id, message_without_notification.id, + group_message2.id, message_with_read_notification.id - ) + ]) end it "fills up the remaining of the USER_MENU_LIST_LIMIT limit with PM topics" do - stub_const(UsersController, "USER_MENU_LIST_LIMIT", 2) do + stub_const(UsersController, "USER_MENU_LIST_LIMIT", 3) do get "/u/#{user.username}/user-menu-private-messages" end expect(response.status).to eq(200) - notifications = response.parsed_body["notifications"] - expect(notifications.size).to eq(1) + unread_notifications = response.parsed_body["unread_notifications"] + expect(unread_notifications.size).to eq(2) topics = response.parsed_body["topics"] + read_notifications = response.parsed_body["read_notifications"] expect(topics.size).to eq(1) + expect(read_notifications.size).to eq(1) message2 = Fabricate(:private_message_post, recipient: user).topic Fabricate( @@ -5913,11 +6000,13 @@ RSpec.describe UsersController do get "/u/#{user.username}/user-menu-private-messages" end expect(response.status).to eq(200) - notifications = response.parsed_body["notifications"] - expect(notifications.size).to eq(2) + unread_notifications = response.parsed_body["unread_notifications"] + expect(unread_notifications.size).to eq(2) topics = response.parsed_body["topics"] + read_notifications = response.parsed_body["read_notifications"] expect(topics.size).to eq(0) + expect(read_notifications.size).to eq(0) end end end