FEATURE: Display unread and new counts for messages. (#14059)

There are certain design decisions that were made in this commit.

Private messages implements its own version of topic tracking state because there are significant differences between regular and private_message topics. Regular topics have to track categories and tags while private messages do not. It is much easier to design the new topic tracking state if we maintain two different classes, instead of trying to mash this two worlds together.

One MessageBus channel per user and one MessageBus channel per group. This allows each user and each group to have their own channel backlog instead of having one global channel which requires the client to filter away unrelated messages.
This commit is contained in:
Alan Guo Xiang Tan
2021-08-25 11:17:56 +08:00
committed by GitHub
parent 4387bc1261
commit f66007ec83
32 changed files with 1149 additions and 362 deletions

View File

@@ -0,0 +1,54 @@
# frozen_string_literal: true
require 'rails_helper'
describe GroupArchivedMessage do
fab!(:user) { Fabricate(:user) }
fab!(:user_2) { Fabricate(:user) }
fab!(:group) do
Fabricate(:group, messageable_level: Group::ALIAS_LEVELS[:everyone]).tap do |g|
g.add(user_2)
end
end
fab!(:group_message) do
create_post(
user: user,
target_group_names: [group.name],
archetype: Archetype.private_message
).topic
end
describe '.move_to_inbox!' do
it 'should unarchive the topic correctly' do
described_class.archive!(group.id, group_message)
messages = MessageBus.track_publish(PrivateMessageTopicTrackingState.group_channel(group.id)) do
described_class.move_to_inbox!(group.id, group_message)
end
expect(messages.present?).to eq(true)
expect(GroupArchivedMessage.exists?(
topic: group_message,
group: group
)).to eq(false)
end
end
describe '.archive!' do
it 'should archive the topic correctly' do
messages = MessageBus.track_publish(PrivateMessageTopicTrackingState.group_channel(group.id)) do
described_class.archive!(group.id, group_message)
end
expect(GroupArchivedMessage.exists?(
topic: group_message,
group: group
)).to eq(true)
expect(messages.present?).to eq(true)
end
end
end

View File

@@ -0,0 +1,206 @@
# frozen_string_literal: true
require 'rails_helper'
describe PrivateMessageTopicTrackingState do
fab!(:user) { Fabricate(:user) }
fab!(:user_2) { Fabricate(:user) }
fab!(:group) do
Fabricate(:group, messageable_level: Group::ALIAS_LEVELS[:everyone]).tap do |g|
g.add(user_2)
end
end
fab!(:group_message) do
create_post(
user: user,
target_group_names: [group.name],
archetype: Archetype.private_message
).topic
end
fab!(:private_message) do
create_post(
user: user,
target_usernames: [user_2.username],
archetype: Archetype.private_message
).topic
end
fab!(:private_message_2) do
create_post(
user: user,
target_usernames: [Fabricate(:user).username],
archetype: Archetype.private_message
).topic
end
describe '.report' do
it 'returns the right tracking state' do
TopicUser.find_by(user: user_2, topic: group_message).update!(
last_read_post_number: 1
)
expect(described_class.report(user_2).map(&:topic_id))
.to contain_exactly(private_message.id)
create_post(user: user, topic: group_message)
report = described_class.report(user_2)
expect(report.map(&:topic_id)).to contain_exactly(
group_message.id,
private_message.id
)
state = report.first
expect(state.topic_id).to eq(private_message.id)
expect(state.user_id).to eq(user_2.id)
expect(state.last_read_post_number).to eq(nil)
expect(state.notification_level).to eq(NotificationLevels.all[:watching])
expect(state.highest_post_number).to eq(1)
expect(state.group_ids).to eq([])
expect(report.last.group_ids).to contain_exactly(group.id)
end
it 'returns the right tracking state when topics contain whispers' do
TopicUser.find_by(user: user_2, topic: private_message).update!(
last_read_post_number: 1
)
create_post(
raw: "this is a test post",
topic: private_message,
post_type: Post.types[:whisper],
user: Fabricate(:admin)
)
expect(described_class.report(user_2).map(&:topic_id))
.to contain_exactly(group_message.id)
user_2.grant_admin!
tracking_state = described_class.report(user_2)
expect(tracking_state.map { |topic| [topic.topic_id, topic.highest_post_number] })
.to contain_exactly(
[group_message.id, 1],
[private_message.id, 2]
)
end
it 'returns the right tracking state when topics have been dismissed' do
DismissedTopicUser.create!(
user_id: user_2.id,
topic_id: group_message.id
)
expect(described_class.report(user_2).map(&:topic_id))
.to contain_exactly(private_message.id)
end
end
describe '.publish_new' do
it 'should publish the right message_bus message' do
messages = MessageBus.track_publish do
described_class.publish_new(private_message)
end
expect(messages.map(&:channel)).to contain_exactly(
described_class.user_channel(user.id),
described_class.user_channel(user_2.id)
)
data = messages.find do |message|
message.channel == described_class.user_channel(user.id)
end.data
expect(data['message_type']).to eq(described_class::NEW_MESSAGE_TYPE)
end
it 'should publish the right message_bus message for a group message' do
messages = MessageBus.track_publish do
described_class.publish_new(group_message)
end
expect(messages.map(&:channel)).to contain_exactly(
described_class.group_channel(group.id),
described_class.user_channel(user.id)
)
data = messages.find do |message|
message.channel == described_class.group_channel(group.id)
end.data
expect(data['message_type']).to eq(described_class::NEW_MESSAGE_TYPE)
expect(data['topic_id']).to eq(group_message.id)
expect(data['payload']['last_read_post_number']).to eq(nil)
expect(data['payload']['highest_post_number']).to eq(1)
expect(data['payload']['group_ids']).to eq([group.id])
end
end
describe '.publish_unread' do
it 'should publish the right message_bus message' do
messages = MessageBus.track_publish do
described_class.publish_unread(private_message.first_post)
end
expect(messages.map(&:channel)).to contain_exactly(
described_class.user_channel(user.id),
described_class.user_channel(user_2.id)
)
data = messages.find do |message|
message.channel == described_class.user_channel(user.id)
end.data
expect(data['message_type']).to eq(described_class::UNREAD_MESSAGE_TYPE)
expect(data['topic_id']).to eq(private_message.id)
expect(data['payload']['last_read_post_number']).to eq(1)
expect(data['payload']['highest_post_number']).to eq(1)
expect(data['payload']['notification_level'])
.to eq(NotificationLevels.all[:watching])
expect(data['payload']['group_ids']).to eq([])
end
end
describe '.publish_user_archived' do
it 'should publish the right message_bus message' do
message = MessageBus.track_publish(described_class.user_channel(user.id)) do
described_class.publish_user_archived(private_message, user.id)
end.first
data = message.data
expect(data['topic_id']).to eq(private_message.id)
expect(data['message_type']).to eq(described_class::ARCHIVE_MESSAGE_TYPE)
end
end
describe '.publish_group_archived' do
it 'should publish the right message_bus message' do
user_3 = Fabricate(:user)
group.add(user_3)
messages = MessageBus.track_publish do
described_class.publish_group_archived(group_message, group.id)
end
expect(messages.map(&:channel)).to contain_exactly(
described_class.group_channel(group.id)
)
data = messages.find do |message|
message.channel == described_class.group_channel(group.id)
end.data
expect(data['message_type']).to eq(described_class::GROUP_ARCHIVE_MESSAGE_TYPE)
expect(data['topic_id']).to eq(group_message.id)
expect(data['payload']['group_ids']).to contain_exactly(group.id)
end
end
end

View File

@@ -208,186 +208,6 @@ describe TopicTrackingState do
end
end
describe '#publish_private_message' do
fab!(:admin) { Fabricate(:admin) }
describe 'normal topic' do
it 'should publish the right message' do
allowed_users = private_message_topic.allowed_users
messages = MessageBus.track_publish do
TopicTrackingState.publish_private_message(private_message_topic)
end
expect(messages.count).to eq(1)
message = messages.first
expect(message.channel).to eq('/private-messages/inbox')
expect(message.data["topic_id"]).to eq(private_message_topic.id)
expect(message.user_ids).to contain_exactly(*allowed_users.map(&:id))
end
end
describe 'topic with groups' do
fab!(:group1) { Fabricate(:group, users: [Fabricate(:user)]) }
fab!(:group2) { Fabricate(:group, users: [Fabricate(:user), Fabricate(:user)]) }
before do
[group1, group2].each do |group|
private_message_topic.allowed_groups << group
end
end
it "should publish the right message" do
messages = MessageBus.track_publish do
TopicTrackingState.publish_private_message(
private_message_topic
)
end
expect(messages.map(&:channel)).to contain_exactly(
'/private-messages/inbox',
"/private-messages/group/#{group1.name}/inbox",
"/private-messages/group/#{group2.name}/inbox"
)
message = messages.find do |m|
m.channel == '/private-messages/inbox'
end
expect(message.data["topic_id"]).to eq(private_message_topic.id)
expect(message.user_ids).to eq(private_message_topic.allowed_users.map(&:id))
[group1, group2].each do |group|
message = messages.find do |m|
m.channel == "/private-messages/group/#{group.name}/inbox"
end
expect(message.data["topic_id"]).to eq(private_message_topic.id)
expect(message.user_ids).to eq(group.users.map(&:id))
end
end
describe "archiving topic" do
it "should publish the right message" do
messages = MessageBus.track_publish do
TopicTrackingState.publish_private_message(
private_message_topic,
group_archive: true
)
end
expect(messages.map(&:channel)).to contain_exactly(
'/private-messages/inbox',
"/private-messages/group/#{group1.name}/inbox",
"/private-messages/group/#{group1.name}/archive",
"/private-messages/group/#{group2.name}/inbox",
"/private-messages/group/#{group2.name}/archive",
)
message = messages.find { |m| m.channel == '/private-messages/inbox' }
expect(message.data["topic_id"]).to eq(private_message_topic.id)
expect(message.user_ids).to eq(private_message_topic.allowed_users.map(&:id))
[group1, group2].each do |group|
[
"/private-messages/group/#{group.name}/inbox",
"/private-messages/group/#{group.name}/archive"
].each do |channel|
message = messages.find { |m| m.channel == channel }
expect(message.data["topic_id"]).to eq(private_message_topic.id)
expect(message.user_ids).to eq(group.users.map(&:id))
end
end
end
end
end
describe 'topic with new post' do
let(:user) { private_message_topic.allowed_users.last }
let!(:post) do
Fabricate(:post,
topic: private_message_topic,
user: user
)
end
let!(:group) do
group = Fabricate(:group, users: [Fabricate(:user)])
private_message_topic.allowed_groups << group
group
end
it 'should publish the right message' do
messages = MessageBus.track_publish do
TopicTrackingState.publish_private_message(
private_message_topic,
post: post
)
end
expected_channels = [
'/private-messages/inbox',
'/private-messages/sent',
"/private-messages/group/#{group.name}/inbox"
]
expect(messages.map(&:channel)).to contain_exactly(*expected_channels)
expected_channels.zip([
private_message_topic.allowed_users.map(&:id),
[user.id],
[group.users.first.id]
]).each do |channel, user_ids|
message = messages.find { |m| m.channel == channel }
expect(message.data["topic_id"]).to eq(private_message_topic.id)
expect(message.user_ids).to eq(user_ids)
end
end
end
describe 'archived topic' do
it 'should publish the right message' do
messages = MessageBus.track_publish do
TopicTrackingState.publish_private_message(
private_message_topic,
archive_user_id: private_message_post.user_id,
)
end
expected_channels = [
"/private-messages/archive",
"/private-messages/inbox",
"/private-messages/sent",
]
expect(messages.map(&:channel)).to eq(expected_channels)
expected_channels.each do |channel|
message = messages.find { |m| m.channel = channel }
expect(message.data["topic_id"]).to eq(private_message_topic.id)
expect(message.user_ids).to eq([private_message_post.user_id])
end
end
end
describe 'for a regular topic' do
it 'should not publish any message' do
topic.allowed_users << Fabricate(:user)
messages = MessageBus.track_publish do
TopicTrackingState.publish_private_message(topic)
end
expect(messages).to eq([])
end
end
end
describe '#publish_read_private_message' do
fab!(:group) { Fabricate(:group) }
let(:read_topic_key) { "/private-messages/unread-indicator/#{group_message.id}" }

View File

@@ -3,20 +3,51 @@
require 'rails_helper'
describe UserArchivedMessage do
it 'Does not move archived muted messages back to inbox' do
user = Fabricate(:admin)
user2 = Fabricate(:admin)
fab!(:user) { Fabricate(:user) }
fab!(:user_2) { Fabricate(:user) }
topic = create_post(user: user,
skip_validations: true,
target_usernames: [user2.username, user.username].join(","),
archetype: Archetype.private_message).topic
UserArchivedMessage.archive!(user.id, topic)
expect(topic.message_archived?(user)).to eq(true)
TopicUser.change(user.id, topic.id, notification_level: TopicUser.notification_levels[:muted])
UserArchivedMessage.move_to_inbox!(user.id, topic)
expect(topic.message_archived?(user)).to eq(true)
fab!(:private_message) do
create_post(
user: user,
skip_validations: true,
target_usernames: [user_2.username, user.username].join(","),
archetype: Archetype.private_message
).topic
end
describe '.move_to_inbox!' do
it 'moves topic back to inbox correctly' do
UserArchivedMessage.archive!(user.id, private_message)
expect do
messages = MessageBus.track_publish(PrivateMessageTopicTrackingState.user_channel(user.id)) do
UserArchivedMessage.move_to_inbox!(user.id, private_message)
end
expect(messages.present?).to eq(true)
end.to change { private_message.message_archived?(user) }.from(true).to(false)
end
it 'does not move archived muted messages back to inbox' do
UserArchivedMessage.archive!(user.id, private_message)
expect(private_message.message_archived?(user)).to eq(true)
TopicUser.change(user.id, private_message.id, notification_level: TopicUser.notification_levels[:muted])
UserArchivedMessage.move_to_inbox!(user.id, private_message)
expect(private_message.message_archived?(user)).to eq(true)
end
end
describe '.archive' do
it 'archives message correctly' do
messages = MessageBus.track_publish(PrivateMessageTopicTrackingState.user_channel(user.id)) do
UserArchivedMessage.archive!(user.id, private_message)
end
expect(messages.present?).to eq(true)
end
end
end

View File

@@ -5064,6 +5064,43 @@ describe UsersController do
end
end
describe "#private_message_topic_tracking_state" do
fab!(:user) { Fabricate(:user) }
fab!(:user_2) { Fabricate(:user) }
fab!(:private_message) do
create_post(
user: user,
target_usernames: [user_2.username],
archetype: Archetype.private_message
).topic
end
before do
sign_in(user_2)
end
it 'does not allow an unauthorized user to access the state of another user' do
get "/u/#{user.username}/private-message-topic-tracking-state.json"
expect(response.status).to eq(403)
end
it 'returns the right response' do
get "/u/#{user_2.username}/private-message-topic-tracking-state.json"
expect(response.status).to eq(200)
topic_state = response.parsed_body.first
expect(topic_state["topic_id"]).to eq(private_message.id)
expect(topic_state["highest_post_number"]).to eq(1)
expect(topic_state["last_read_post_number"]).to eq(nil)
expect(topic_state["notification_level"]).to eq(NotificationLevels.all[:watching])
expect(topic_state["group_ids"]).to eq([])
end
end
def create_second_factor_security_key
sign_in(user)
stub_secure_session_confirmed