Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FIX: Regression in TopicTrackingState MessageBus message scope. (#19835) 

0403cda1d1 introduced a regression where
topics in non read-restricted categories have its TopicTrackingState
MessageBus messages published with the `group_ids: [nil]` option. This
essentially means that no one would be able to view the message.
This commit is contained in:
Alan Guo Xiang Tan 2023-01-12 08:22:28 +08:00 committed by GitHub
parent 387693e889
commit 9fcd8336e4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 98 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
app/models/topic_tracking_state.rb
View File

@ -95,6 +95,8 @@ class TopicTrackingState
end
def self.publish_muted(topic)
return unless topic.regular?
user_ids =
topic
.topic_users
@ -110,6 +112,8 @@ class TopicTrackingState
end
def self.publish_unmuted(topic)
return unless topic.regular?
user_ids =
User
.watching_topic(topic)
@ -172,6 +176,8 @@ class TopicTrackingState
end
def self.publish_recover(topic)
return unless topic.regular?
group_ids = secure_category_group_ids(topic)
message = { topic_id: topic.id, message_type: RECOVER_MESSAGE_TYPE }
@ -180,6 +186,8 @@ class TopicTrackingState
end
def self.publish_delete(topic)
return unless topic.regular?
group_ids = secure_category_group_ids(topic)
message = { topic_id: topic.id, message_type: DELETE_MESSAGE_TYPE }
@ -188,6 +196,8 @@ class TopicTrackingState
end
def self.publish_destroy(topic)
return unless topic.regular?
group_ids = secure_category_group_ids(topic)
message = { topic_id: topic.id, message_type: DESTROY_MESSAGE_TYPE }
@ -549,12 +559,14 @@ class TopicTrackingState
end
def self.secure_category_group_ids(topic)
ids = topic&.category&.secure_group_ids
category = topic.category
if ids.blank?
[Group::AUTO_GROUPS[:admin]]
if category.read_restricted
ids = [Group::AUTO_GROUPS[:admins]]
ids.push(*category.secure_group_ids)
ids.uniq
else
ids
nil
end
end
private_class_method :secure_category_group_ids

156
spec/models/topic_tracking_state_spec.rb
View File

@ -3,33 +3,91 @@
RSpec.describe TopicTrackingState do
fab!(:user) { Fabricate(:user) }
fab!(:whisperers_group) { Fabricate(:group) }
let(:post) { create_post }
let(:topic) { post.topic }
fab!(:private_message_post) { Fabricate(:private_message_post) }
let(:private_message_topic) { private_message_post.topic }
let(:post) { create_post }
let(:topic) { post.topic }
fab!(:read_restricted_category) { Fabricate(:category, read_restricted: true) }
fab!(:read_restricted_topic) { Fabricate(:topic, category: read_restricted_category) }
shared_examples "does not publish message for private topics" do |method|
it "should not publish any message for a private topic" do
messages =
MessageBus.track_publish { described_class.public_send(method, private_message_topic) }
describe ".publish_new" do
it "should publish message only to admin group when category is read restricted but no groups have been granted access" do
expect(messages).to eq([])
end
end
shared_examples "publishes message to right groups and users" do |message_bus_channel, method|
fab!(:public_category) { Fabricate(:category, read_restricted: false) }
fab!(:topic_in_public_category) { Fabricate(:topic, category: public_category) }
fab!(:group) { Fabricate(:group) }
fab!(:read_restricted_category_with_groups) { Fabricate(:private_category, group: group) }
fab!(:topic_in_read_restricted_category_with_groups) do
Fabricate(:topic, category: read_restricted_category_with_groups)
end
fab!(:read_restricted_category_with_no_groups) { Fabricate(:category, read_restricted: true) }
fab!(:topic_in_read_restricted_category_with_no_groups) do
Fabricate(:topic, category: read_restricted_category_with_no_groups)
end
it "should publish message to everyone for a topic in a category that is not read restricted" do
message =
MessageBus
.track_publish("/new") { described_class.publish_new(read_restricted_topic) }
.track_publish(message_bus_channel) do
described_class.public_send(method, topic_in_public_category)
end
.first
data = message.data
expect(data["topic_id"]).to eq(read_restricted_topic.id)
expect(data["message_type"]).to eq(described_class::NEW_TOPIC_MESSAGE_TYPE)
expect(message.group_ids).to contain_exactly(Group::AUTO_GROUPS[:admin])
expect(data["topic_id"]).to eq(topic_in_public_category.id)
expect(message.group_ids).to eq(nil)
expect(message.user_ids).to eq(nil)
end
it "should publish message only to admin group and groups that have permission to read a category when topic is in category that is restricted to certain groups" do
message =
MessageBus
.track_publish(message_bus_channel) do
described_class.public_send(method, topic_in_read_restricted_category_with_groups)
end
.first
data = message.data
expect(data["topic_id"]).to eq(topic_in_read_restricted_category_with_groups.id)
expect(message.group_ids).to contain_exactly(Group::AUTO_GROUPS[:admins], group.id)
expect(message.user_ids).to eq(nil)
end
it "should publish message only to admin group when topic is in category that is read restricted but no groups have been granted access" do
message =
MessageBus
.track_publish(message_bus_channel) do
described_class.public_send(method, topic_in_read_restricted_category_with_no_groups)
end
.first
data = message.data
expect(data["topic_id"]).to eq(topic_in_read_restricted_category_with_no_groups.id)
expect(message.group_ids).to contain_exactly(Group::AUTO_GROUPS[:admins])
expect(message.user_ids).to eq(nil)
end
end
describe ".publish_new" do
include_examples("publishes message to right groups and users", "/new", :publish_new)
include_examples("does not publish message for private topics", :publish_new)
end
describe ".publish_latest" do
include_examples("publishes message to right groups and users", "/latest", :publish_latest)
include_examples("does not publish message for private topics", :publish_latest)
it "can correctly publish latest" do
message = MessageBus.track_publish("/latest") { described_class.publish_latest(topic) }.first
@ -38,6 +96,8 @@ RSpec.describe TopicTrackingState do
expect(data["topic_id"]).to eq(topic.id)
expect(data["message_type"]).to eq(described_class::LATEST_MESSAGE_TYPE)
expect(data["payload"]["archetype"]).to eq(Archetype.default)
expect(message.group_ids).to eq(nil)
expect(message.user_ids).to eq(nil)
end
it "publishes whisper post to staff users and members of whisperers group" do
@ -54,29 +114,6 @@ RSpec.describe TopicTrackingState do
expect(message.group_ids).to contain_exactly(whisperers_group.id, Group::AUTO_GROUPS[:staff])
end
it "should publish message only to admin group when category is read restricted but no groups have been granted access" do
message =
MessageBus
.track_publish("/latest") { described_class.publish_latest(read_restricted_topic) }
.first
data = message.data
expect(data["topic_id"]).to eq(read_restricted_topic.id)
expect(data["message_type"]).to eq(described_class::LATEST_MESSAGE_TYPE)
expect(message.group_ids).to contain_exactly(Group::AUTO_GROUPS[:admin])
expect(message.user_ids).to eq(nil)
end
describe "private message" do
it "should not publish any message" do
messages =
MessageBus.track_publish { described_class.publish_latest(private_message_topic) }
expect(messages).to eq([])
end
end
end
describe ".publish_read" do
@ -241,6 +278,8 @@ RSpec.describe TopicTrackingState do
let(:user) { Fabricate(:user, last_seen_at: Date.today) }
let(:post) { create_post(user: user) }
include_examples("does not publish message for private topics", :publish_muted)
it "can correctly publish muted" do
TopicUser.find_by(topic: topic, user: post.user).update(notification_level: 0)
messages = MessageBus.track_publish("/latest") { TopicTrackingState.publish_muted(topic) }
@ -273,6 +312,8 @@ RSpec.describe TopicTrackingState do
let(:third_user) { Fabricate(:user, last_seen_at: Date.today) }
let(:post) { create_post(user: user) }
include_examples("does not publish message for private topics", :publish_unmuted)
it "can correctly publish unmuted" do
Fabricate(:topic_tag, topic: topic)
SiteSetting.mute_all_categories_by_default = true
@ -734,50 +775,17 @@ RSpec.describe TopicTrackingState do
end
describe ".publish_recover" do
it "should publish message only to admin group when category is read restricted but no groups have been granted access" do
message =
MessageBus
.track_publish("/recover") { described_class.publish_recover(read_restricted_topic) }
.first
data = message.data
expect(data["topic_id"]).to eq(read_restricted_topic.id)
expect(data["message_type"]).to eq(described_class::RECOVER_MESSAGE_TYPE)
expect(message.group_ids).to contain_exactly(Group::AUTO_GROUPS[:admin])
expect(message.user_ids).to eq(nil)
end
include_examples("publishes message to right groups and users", "/recover", :publish_recover)
include_examples("does not publish message for private topics", :publish_recover)
end
describe ".publish_delete" do
it "should publish message only to admin group when category is read restricted but no groups have been granted access" do
message =
MessageBus
.track_publish("/delete") { described_class.publish_delete(read_restricted_topic) }
.first
data = message.data
expect(data["topic_id"]).to eq(read_restricted_topic.id)
expect(data["message_type"]).to eq(described_class::DELETE_MESSAGE_TYPE)
expect(message.group_ids).to contain_exactly(Group::AUTO_GROUPS[:admin])
expect(message.user_ids).to eq(nil)
end
include_examples("publishes message to right groups and users", "/delete", :publish_delete)
include_examples("does not publish message for private topics", :publish_delete)
end
describe ".publish_destroy" do
it "should publish message only to admin group when category is read restricted but no groups have been granted access" do
message =
MessageBus
.track_publish("/destroy") { described_class.publish_destroy(read_restricted_topic) }
.first
data = message.data
expect(data["topic_id"]).to eq(read_restricted_topic.id)
expect(data["message_type"]).to eq(described_class::DESTROY_MESSAGE_TYPE)
expect(message.group_ids).to contain_exactly(Group::AUTO_GROUPS[:admin])
expect(message.user_ids).to eq(nil)
end
include_examples("publishes message to right groups and users", "/destroy", :publish_destroy)
include_examples("does not publish message for private topics", :publish_destroy)
end
end