FIX: Error when publishing TopicTrackingState updates for certain topics (#19812)

When a topic belongs to category that is read restricted but permission
has not been granted to any groups, publishing ceratin topic tracking state
updates for the topic will result in the `MessageBus::InvalidMessageTarget` error being raised
because we're passing `nil` to `group_ids` which is not support by
MessageBus.

This commit ensures that for said category above, we will publish the
updates to the admin groups.
This commit is contained in:
Alan Guo Xiang Tan 2023-01-11 06:15:52 +08:00 committed by GitHub
parent edbaa7cace
commit 0403cda1d1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 99 additions and 6 deletions

View File

@ -53,7 +53,7 @@ class TopicTrackingState
message = { topic_id: topic.id, message_type: NEW_TOPIC_MESSAGE_TYPE, payload: payload }
group_ids = topic.category && topic.category.secure_group_ids
group_ids = secure_category_group_ids(topic)
MessageBus.publish("/new", message.as_json, group_ids: group_ids)
publish_read(topic.id, 1, topic.user)
@ -84,8 +84,9 @@ class TopicTrackingState
if whisper
[Group::AUTO_GROUPS[:staff], *SiteSetting.whispers_allowed_group_ids]
else
topic.category && topic.category.secure_group_ids
secure_category_group_ids(topic)
end
MessageBus.publish("/latest", message.as_json, group_ids: group_ids)
end
@ -171,7 +172,7 @@ class TopicTrackingState
end
def self.publish_recover(topic)
group_ids = topic.category && topic.category.secure_group_ids
group_ids = secure_category_group_ids(topic)
message = { topic_id: topic.id, message_type: RECOVER_MESSAGE_TYPE }
@ -179,7 +180,7 @@ class TopicTrackingState
end
def self.publish_delete(topic)
group_ids = topic.category && topic.category.secure_group_ids
group_ids = secure_category_group_ids(topic)
message = { topic_id: topic.id, message_type: DELETE_MESSAGE_TYPE }
@ -187,7 +188,7 @@ class TopicTrackingState
end
def self.publish_destroy(topic)
group_ids = topic.category && topic.category.secure_group_ids
group_ids = secure_category_group_ids(topic)
message = { topic_id: topic.id, message_type: DESTROY_MESSAGE_TYPE }
@ -546,4 +547,15 @@ class TopicTrackingState
opts = { readers_count: post.readers_count, reader_id: user_id }
post.publish_change_to_clients!(:read, opts)
end
def self.secure_category_group_ids(topic)
ids = topic&.category&.secure_group_ids
if ids.blank?
[Group::AUTO_GROUPS[:admin]]
else
ids
end
end
private_class_method :secure_category_group_ids
end

View File

@ -10,7 +10,26 @@ RSpec.describe TopicTrackingState do
fab!(:private_message_post) { Fabricate(:private_message_post) }
let(:private_message_topic) { private_message_post.topic }
describe "#publish_latest" do
fab!(:read_restricted_category) { Fabricate(:category, read_restricted: true) }
fab!(:read_restricted_topic) { Fabricate(:topic, category: read_restricted_category) }
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
message =
MessageBus
.track_publish("/new") { described_class.publish_new(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::NEW_TOPIC_MESSAGE_TYPE)
expect(message.group_ids).to contain_exactly(Group::AUTO_GROUPS[:admin])
expect(message.user_ids).to eq(nil)
end
end
describe ".publish_latest" do
it "can correctly publish latest" do
message = MessageBus.track_publish("/latest") { described_class.publish_latest(topic) }.first
@ -36,6 +55,20 @@ 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 =
@ -699,4 +732,52 @@ RSpec.describe TopicTrackingState do
expect(state.map(&:topic_id)).to contain_exactly(topic.id)
end
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
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
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
end
end