mirror of
https://github.com/discourse/discourse.git
synced 2025-05-23 16:31:09 +08:00
FEATURE: Redesign discourse-presence to track state on the client side. (#9487)
Before this commit, the presence state of users were stored on the server side and any updates to the state meant we had to publish the entire state to the clients. Also, the way the state of users were stored on the server side meant we didn't have a way to differentiate between replying users and whispering users. In this redesign, we decided to move the tracking of users state to the client side and have the server publish client events instead. As a result of this change, we're able to remove the number of opened connections needed to track presence and also reduce the payload that is sent for each event. At the same time, we've also improved on the restrictions when publishing message_bus messages. Users that do not have permission to see certain events will not receive messages for those events.
This commit is contained in:
Alan Guo Xiang Tan
committed by
GitHub
GitHub
parent
5503eba924
commit
301a0fa54e
@ -3,170 +3,442 @@
|
||||
require 'rails_helper'
|
||||
|
||||
describe ::Presence::PresencesController do
|
||||
before do
|
||||
SiteSetting.presence_enabled = true
|
||||
end
|
||||
describe '#handle_message' do
|
||||
context 'when not logged in' do
|
||||
it 'should raise the right error' do
|
||||
post '/presence/publish.json'
|
||||
|
||||
let(:user1) { Fabricate(:user) }
|
||||
let(:user2) { Fabricate(:user) }
|
||||
let(:user3) { Fabricate(:user) }
|
||||
expect(response.status).to eq(403)
|
||||
end
|
||||
end
|
||||
|
||||
let(:post1) { Fabricate(:post) }
|
||||
let(:post2) { Fabricate(:post) }
|
||||
context 'when logged in' do
|
||||
fab!(:user) { Fabricate(:user) }
|
||||
fab!(:user2) { Fabricate(:user) }
|
||||
fab!(:admin) { Fabricate(:admin) }
|
||||
|
||||
let(:manager) { ::Presence::PresenceManager }
|
||||
fab!(:group) do
|
||||
group = Fabricate(:group)
|
||||
group.add(user)
|
||||
group
|
||||
end
|
||||
|
||||
after do
|
||||
Discourse.redis.del("presence:topic:#{post1.topic.id}")
|
||||
Discourse.redis.del("presence:topic:#{post2.topic.id}")
|
||||
Discourse.redis.del("presence:post:#{post1.id}")
|
||||
Discourse.redis.del("presence:post:#{post2.id}")
|
||||
end
|
||||
fab!(:category) { Fabricate(:private_category, group: group) }
|
||||
fab!(:private_topic) { Fabricate(:topic, category: category) }
|
||||
fab!(:public_topic) { Fabricate(:topic, first_post: Fabricate(:post)) }
|
||||
|
||||
context 'when not logged in' do
|
||||
it 'should raise the right error' do
|
||||
post '/presence/publish.json'
|
||||
expect(response.status).to eq(403)
|
||||
fab!(:private_message) do
|
||||
Fabricate(:private_message_topic,
|
||||
allowed_groups: [group]
|
||||
)
|
||||
end
|
||||
|
||||
before do
|
||||
sign_in(user)
|
||||
end
|
||||
|
||||
it 'returns the right response when user disables the presence feature' do
|
||||
user.user_option.update_column(:hide_profile_and_presence, true)
|
||||
|
||||
post '/presence/publish.json'
|
||||
|
||||
expect(response.status).to eq(404)
|
||||
end
|
||||
|
||||
it 'returns the right response when the presence site settings is disabled' do
|
||||
SiteSetting.presence_enabled = false
|
||||
|
||||
post '/presence/publish.json'
|
||||
|
||||
expect(response.status).to eq(404)
|
||||
end
|
||||
|
||||
it 'returns the right response if required params are missing' do
|
||||
post '/presence/publish.json'
|
||||
|
||||
expect(response.status).to eq(400)
|
||||
end
|
||||
|
||||
it 'returns the right response if topic_id is invalid' do
|
||||
post '/presence/publish.json', params: { topic_id: -999, state: 'replying' }
|
||||
|
||||
expect(response.status).to eq(400)
|
||||
end
|
||||
|
||||
it 'returns the right response when user does not have access to the topic' do
|
||||
group.remove(user)
|
||||
|
||||
post '/presence/publish.json', params: { topic_id: private_topic.id, state: 'replying' }
|
||||
|
||||
expect(response.status).to eq(403)
|
||||
end
|
||||
|
||||
it 'returns the right response when an invalid state is provided with a post_id' do
|
||||
post '/presence/publish.json', params: {
|
||||
topic_id: public_topic.id,
|
||||
post_id: public_topic.first_post.id,
|
||||
state: 'some state'
|
||||
}
|
||||
|
||||
expect(response.status).to eq(400)
|
||||
end
|
||||
|
||||
it 'returns the right response when user can not edit a post' do
|
||||
Fabricate(:post, topic: private_topic, user: private_topic.user)
|
||||
|
||||
post '/presence/publish.json', params: {
|
||||
topic_id: private_topic.id,
|
||||
post_id: private_topic.first_post.id,
|
||||
state: 'editing'
|
||||
}
|
||||
|
||||
expect(response.status).to eq(403)
|
||||
end
|
||||
|
||||
it 'returns the right response when an invalid post_id is given' do
|
||||
post '/presence/publish.json', params: {
|
||||
topic_id: public_topic.id,
|
||||
post_id: -9,
|
||||
state: 'editing'
|
||||
}
|
||||
|
||||
expect(response.status).to eq(400)
|
||||
end
|
||||
|
||||
it 'publishes the right message for a public topic' do
|
||||
freeze_time
|
||||
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: { topic_id: public_topic.id, state: 'replying' }
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
end
|
||||
|
||||
expect(messages.length).to eq(1)
|
||||
|
||||
message = messages.first
|
||||
|
||||
expect(message.channel).to eq("/presence/#{public_topic.id}")
|
||||
expect(message.data.dig(:user, :id)).to eq(user.id)
|
||||
expect(message.data[:published_at]).to eq(Time.zone.now.to_i)
|
||||
expect(message.group_ids).to eq(nil)
|
||||
expect(message.user_ids).to eq(nil)
|
||||
end
|
||||
|
||||
it 'publishes the right message for a restricted topic' do
|
||||
freeze_time
|
||||
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: {
|
||||
topic_id: private_topic.id,
|
||||
state: 'replying'
|
||||
}
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
end
|
||||
|
||||
expect(messages.length).to eq(1)
|
||||
|
||||
message = messages.first
|
||||
|
||||
expect(message.channel).to eq("/presence/#{private_topic.id}")
|
||||
expect(message.data.dig(:user, :id)).to eq(user.id)
|
||||
expect(message.data[:published_at]).to eq(Time.zone.now.to_i)
|
||||
expect(message.group_ids).to contain_exactly(group.id)
|
||||
expect(message.user_ids).to eq(nil)
|
||||
end
|
||||
|
||||
it 'publishes the right message for a private message' do
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: {
|
||||
topic_id: private_message.id,
|
||||
state: 'replying'
|
||||
}
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
end
|
||||
|
||||
expect(messages.length).to eq(1)
|
||||
|
||||
message = messages.first
|
||||
|
||||
expect(message.group_ids).to contain_exactly(
|
||||
group.id,
|
||||
Group::AUTO_GROUPS[:staff]
|
||||
)
|
||||
|
||||
expect(message.user_ids).to contain_exactly(
|
||||
*private_message.topic_allowed_users.pluck(:user_id)
|
||||
)
|
||||
end
|
||||
|
||||
it 'publishes the message to staff group when user is whispering' do
|
||||
SiteSetting.enable_whispers = true
|
||||
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: {
|
||||
topic_id: public_topic.id,
|
||||
state: 'replying',
|
||||
is_whisper: true
|
||||
}
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
end
|
||||
|
||||
expect(messages.length).to eq(1)
|
||||
|
||||
message = messages.first
|
||||
|
||||
expect(message.group_ids).to contain_exactly(Group::AUTO_GROUPS[:staff])
|
||||
expect(message.user_ids).to eq(nil)
|
||||
end
|
||||
|
||||
it 'publishes the message to staff group when a staff is editing a whisper' do
|
||||
SiteSetting.enable_whispers = true
|
||||
sign_in(admin)
|
||||
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: {
|
||||
topic_id: public_topic.id,
|
||||
post_id: public_topic.first_post.id,
|
||||
state: 'editing',
|
||||
is_whisper: true
|
||||
}
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
end
|
||||
|
||||
expect(messages.length).to eq(1)
|
||||
|
||||
message = messages.first
|
||||
|
||||
expect(message.group_ids).to contain_exactly(Group::AUTO_GROUPS[:staff])
|
||||
expect(message.user_ids).to eq(nil)
|
||||
end
|
||||
|
||||
it 'publishes the message to staff group when a staff is editing a locked post' do
|
||||
SiteSetting.enable_whispers = true
|
||||
sign_in(admin)
|
||||
locked_post = Fabricate(:post, topic: public_topic, locked_by_id: admin.id)
|
||||
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: {
|
||||
topic_id: public_topic.id,
|
||||
post_id: locked_post.id,
|
||||
state: 'editing',
|
||||
}
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
end
|
||||
|
||||
expect(messages.length).to eq(1)
|
||||
|
||||
message = messages.first
|
||||
|
||||
expect(message.group_ids).to contain_exactly(Group::AUTO_GROUPS[:staff])
|
||||
expect(message.user_ids).to eq(nil)
|
||||
end
|
||||
|
||||
it 'publishes the message to author, staff group and TL4 group when editing a public post' do
|
||||
post = Fabricate(:post, topic: public_topic, user: user)
|
||||
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: {
|
||||
topic_id: public_topic.id,
|
||||
post_id: post.id,
|
||||
state: 'editing',
|
||||
}
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
end
|
||||
|
||||
expect(messages.length).to eq(1)
|
||||
|
||||
message = messages.first
|
||||
|
||||
expect(message.group_ids).to contain_exactly(
|
||||
Group::AUTO_GROUPS[:trust_level_4],
|
||||
Group::AUTO_GROUPS[:staff]
|
||||
)
|
||||
|
||||
expect(message.user_ids).to contain_exactly(user.id)
|
||||
end
|
||||
|
||||
it 'publishes the message to author and staff group when editing a public post ' \
|
||||
'if SiteSettings.trusted_users_can_edit_others is set to false' do
|
||||
|
||||
post = Fabricate(:post, topic: public_topic, user: user)
|
||||
SiteSetting.trusted_users_can_edit_others = false
|
||||
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: {
|
||||
topic_id: public_topic.id,
|
||||
post_id: post.id,
|
||||
state: 'editing',
|
||||
}
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
end
|
||||
|
||||
expect(messages.length).to eq(1)
|
||||
|
||||
message = messages.first
|
||||
|
||||
expect(message.group_ids).to contain_exactly(Group::AUTO_GROUPS[:staff])
|
||||
expect(message.user_ids).to contain_exactly(user.id)
|
||||
end
|
||||
|
||||
it 'publishes the message to SiteSetting.min_trust_to_edit_wiki_post group ' \
|
||||
'and staff group when editing a wiki in a public topic' do
|
||||
|
||||
post = Fabricate(:post, topic: public_topic, user: user, wiki: true)
|
||||
SiteSetting.min_trust_to_edit_wiki_post = TrustLevel.levels[:basic]
|
||||
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: {
|
||||
topic_id: public_topic.id,
|
||||
post_id: post.id,
|
||||
state: 'editing',
|
||||
}
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
end
|
||||
|
||||
expect(messages.length).to eq(1)
|
||||
|
||||
message = messages.first
|
||||
|
||||
expect(message.group_ids).to contain_exactly(
|
||||
Group::AUTO_GROUPS[:trust_level_1],
|
||||
Group::AUTO_GROUPS[:staff]
|
||||
)
|
||||
|
||||
expect(message.user_ids).to contain_exactly(user.id)
|
||||
end
|
||||
|
||||
it 'publishes the message to author and staff group when editing a private message' do
|
||||
post = Fabricate(:post, topic: private_message, user: user)
|
||||
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: {
|
||||
topic_id: private_message.id,
|
||||
post_id: post.id,
|
||||
state: 'editing',
|
||||
}
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
end
|
||||
|
||||
expect(messages.length).to eq(1)
|
||||
|
||||
message = messages.first
|
||||
|
||||
expect(message.group_ids).to contain_exactly(
|
||||
Group::AUTO_GROUPS[:staff],
|
||||
)
|
||||
|
||||
expect(message.user_ids).to contain_exactly(user.id)
|
||||
end
|
||||
|
||||
it 'publishes the message to users with trust levels of SiteSetting.min_trust_to_edit_wiki_post ' \
|
||||
'and staff group when editing a wiki in a private message' do
|
||||
|
||||
post = Fabricate(:post,
|
||||
topic: private_message,
|
||||
user: private_message.user,
|
||||
wiki: true
|
||||
)
|
||||
|
||||
user2.update!(trust_level: TrustLevel.levels[:newuser])
|
||||
group.add(user2)
|
||||
|
||||
SiteSetting.min_trust_to_edit_wiki_post = TrustLevel.levels[:basic]
|
||||
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: {
|
||||
topic_id: private_message.id,
|
||||
post_id: post.id,
|
||||
state: 'editing',
|
||||
}
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
end
|
||||
|
||||
expect(messages.length).to eq(1)
|
||||
|
||||
message = messages.first
|
||||
|
||||
expect(message.group_ids).to contain_exactly(
|
||||
Group::AUTO_GROUPS[:staff],
|
||||
group.id
|
||||
)
|
||||
|
||||
expect(message.user_ids).to contain_exactly(
|
||||
*private_message.allowed_users.pluck(:id)
|
||||
)
|
||||
end
|
||||
|
||||
it 'publises the right message when closing composer in public topic' do
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: {
|
||||
topic_id: public_topic.id,
|
||||
state: described_class::CLOSED_STATE,
|
||||
}
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
end
|
||||
|
||||
expect(messages.length).to eq(1)
|
||||
|
||||
message = messages.first
|
||||
|
||||
expect(message.group_ids).to eq(nil)
|
||||
expect(message.user_ids).to eq(nil)
|
||||
end
|
||||
|
||||
it 'publises the right message when closing composer in private topic' do
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: {
|
||||
topic_id: private_topic.id,
|
||||
state: described_class::CLOSED_STATE,
|
||||
}
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
end
|
||||
|
||||
expect(messages.length).to eq(1)
|
||||
|
||||
message = messages.first
|
||||
|
||||
expect(message.group_ids).to contain_exactly(group.id)
|
||||
expect(message.user_ids).to eq(nil)
|
||||
end
|
||||
|
||||
it 'publises the right message when closing composer in private message' do
|
||||
post = Fabricate(:post, topic: private_message, user: user)
|
||||
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: {
|
||||
topic_id: private_message.id,
|
||||
state: described_class::CLOSED_STATE,
|
||||
}
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
end
|
||||
|
||||
expect(messages.length).to eq(1)
|
||||
|
||||
message = messages.first
|
||||
|
||||
expect(message.group_ids).to contain_exactly(
|
||||
Group::AUTO_GROUPS[:staff],
|
||||
group.id
|
||||
)
|
||||
|
||||
expect(message.user_ids).to contain_exactly(
|
||||
*private_message.allowed_users.pluck(:id)
|
||||
)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when logged in' do
|
||||
before do
|
||||
sign_in(user1)
|
||||
end
|
||||
|
||||
it "doesn't produce an error" do
|
||||
expect { post '/presence/publish.json' }.not_to raise_error
|
||||
end
|
||||
|
||||
it "does not publish for users with disabled presence features" do
|
||||
user1.user_option.update_column(:hide_profile_and_presence, true)
|
||||
post '/presence/publish.json'
|
||||
expect(response.code).to eq("404")
|
||||
end
|
||||
|
||||
it "uses guardian to secure endpoint" do
|
||||
private_post = Fabricate(:private_message_post)
|
||||
|
||||
post '/presence/publish.json', params: {
|
||||
current: { action: 'edit', post_id: private_post.id }
|
||||
}
|
||||
|
||||
expect(response.code.to_i).to eq(403)
|
||||
|
||||
group = Fabricate(:group)
|
||||
category = Fabricate(:private_category, group: group)
|
||||
private_topic = Fabricate(:topic, category: category)
|
||||
|
||||
post '/presence/publish.json', params: {
|
||||
current: { action: 'edit', topic_id: private_topic.id }
|
||||
}
|
||||
|
||||
expect(response.code.to_i).to eq(403)
|
||||
end
|
||||
|
||||
it "returns a response when requested" do
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: {
|
||||
current: { compose_state: 'open', action: 'edit', post_id: post1.id }, response_needed: true
|
||||
}
|
||||
end
|
||||
|
||||
expect(messages.count).to eq(1)
|
||||
|
||||
data = JSON.parse(response.body)
|
||||
|
||||
expect(data['messagebus_channel']).to eq("/presence/post/#{post1.id}")
|
||||
expect(data['messagebus_id']).to eq(MessageBus.last_id("/presence/post/#{post1.id}"))
|
||||
expect(data['users'][0]["id"]).to eq(user1.id)
|
||||
end
|
||||
|
||||
it "doesn't return a response when not requested" do
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: {
|
||||
current: { compose_state: 'open', action: 'edit', post_id: post1.id }
|
||||
}
|
||||
end
|
||||
|
||||
expect(messages.count).to eq(1)
|
||||
|
||||
data = JSON.parse(response.body)
|
||||
expect(data).to eq({})
|
||||
end
|
||||
|
||||
it "does send duplicate messagebus messages" do
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: {
|
||||
current: { compose_state: 'open', action: 'edit', post_id: post1.id }
|
||||
}
|
||||
end
|
||||
|
||||
expect(messages.count).to eq(1)
|
||||
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: {
|
||||
current: { compose_state: 'open', action: 'edit', post_id: post1.id }
|
||||
}
|
||||
end
|
||||
|
||||
# we do this cause we also publish time
|
||||
expect(messages.count).to eq(1)
|
||||
end
|
||||
|
||||
it "clears 'previous' state when supplied" do
|
||||
messages = MessageBus.track_publish do
|
||||
post '/presence/publish.json', params: {
|
||||
current: { compose_state: 'open', action: 'edit', post_id: post1.id }
|
||||
}
|
||||
|
||||
post '/presence/publish.json', params: {
|
||||
current: { compose_state: 'open', action: 'edit', post_id: post2.id },
|
||||
previous: { compose_state: 'open', action: 'edit', post_id: post1.id }
|
||||
}
|
||||
end
|
||||
|
||||
expect(messages.count).to eq(3)
|
||||
end
|
||||
|
||||
it 'cleans up old users when requested' do
|
||||
freeze_time Time.zone.now do
|
||||
manager.add('topic', post1.topic.id, user2.id)
|
||||
end
|
||||
|
||||
# Anything older than 20 seconds should be cleaned up
|
||||
freeze_time 30.seconds.from_now do
|
||||
post '/presence/publish.json', params: {
|
||||
current: { compose_state: 'open', action: 'reply', topic_id: post1.topic.id }, response_needed: true
|
||||
}
|
||||
|
||||
data = JSON.parse(response.body)
|
||||
|
||||
expect(data['users'].length).to eq(1)
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
describe 'when post has been deleted' do
|
||||
it 'should return an empty response' do
|
||||
post1.destroy!
|
||||
|
||||
post '/presence/publish.json', params: {
|
||||
current: { compose_state: 'open', action: 'edit', post_id: post1.id }
|
||||
}
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
expect(JSON.parse(response.body)).to eq({})
|
||||
|
||||
post '/presence/publish.json', params: {
|
||||
current: { compose_state: 'open', action: 'edit', post_id: post2.id },
|
||||
previous: { compose_state: 'open', action: 'edit', post_id: post1.id }
|
||||
}
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
expect(JSON.parse(response.body)).to eq({})
|
||||
end
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user