mirror of
https://github.com/discourse/discourse.git
synced 2026-08-06 02:19:55 +08:00
b8077beba5Previously, a tag whose only usage was in personal messages was silently dropped by `TagsController.tag_counts_json` — a display rule from 2020 meant to keep such tags off the `/tags` browse page for users who cannot tag messages (and `pm_tags_allowed_for_groups` has no staff bypass, so by default that includes admins). Every surface reusing that method as a plain serializer inherited the rule by accident: - the composer tag search treated the missing row as unauthorized and showed the tag disabled with a bogus **"Can't be used in this category"** reason (the reported bug), - every "show all tags" chooser (tag groups, synonyms, watched tags, category allowed tags, webhooks, automations, …) silently refused to offer such tags at all, - the `#` autocomplete would not suggest a tag that nonetheless cooked into a working hashtag link when typed in full. This change makes `tag_counts_json` a pure serializer and moves the rule into an explicit, named helper (`DiscourseTagging.without_pm_only_tags`) applied only where it belongs — the `/tags` browse lists — with an exemption for the admin "show all tags" view so the admin inventory is complete. Selection and search surfaces now offer every tag the user is allowed to use, and tag-group visibility rules still apply everywhere. It also fixes two adjacent inconsistencies uncovered along the way: - **Topic→message conversion counter drift.** Converting only adjusted `public_topic_count`, so a converted topic's tags kept working until the periodic consistency job recounted them into the broken state — the "worked at first, broke a day later" in the report. The converter now moves all three counters immediately, and rolls back cleanly when the underlying post revision fails (its return value was previously ignored, and `Topic#valid?` clears the errors it adds, so a failed conversion still applied its side effects). - **Crawler/print tag leak.** The crawler layout leaked a message's tag names in the page title and `og:article:tag` metadata to participants the serializer already hides tags from; both now flow through `TopicView#visible_tags`, gated on `guardian.can_see_tags?`. Reported in https://meta.discourse.org/t/407050
405 lines
16 KiB
Ruby
Vendored
405 lines
16 KiB
Ruby
Vendored
# frozen_string_literal: true
|
|
|
|
RSpec.describe TopicConverter do
|
|
describe "convert_to_public_topic" do
|
|
fab!(:admin)
|
|
fab!(:author, :user)
|
|
fab!(:category) { Fabricate(:category, topic_count: 1) }
|
|
fab!(:private_message) { Fabricate(:private_message_topic, user: author) } # creates a topic without a first post
|
|
let(:first_post) do
|
|
create_post(user: author, topic: private_message, allow_uncategorized_topics: false)
|
|
end
|
|
let(:other_user) { private_message.topic_allowed_users.find { |u| u.user != author }.user }
|
|
|
|
let(:uncategorized_category) { Category.find(SiteSetting.uncategorized_category_id) }
|
|
|
|
context "with success" do
|
|
it "converts private message to regular topic" do
|
|
SiteSetting.allow_uncategorized_topics = true
|
|
topic = nil
|
|
|
|
_pm_post_2 = Fabricate(:post, topic: private_message, user: author)
|
|
_pm_post_3 = Fabricate(:post, topic: private_message, user: author)
|
|
|
|
other_pm = Fabricate(:private_message_post).topic
|
|
other_pm_post = Fabricate(:private_message_post, topic: other_pm)
|
|
other_pm_post_2 =
|
|
Fabricate(:private_message_post, topic: other_pm, user: other_pm_post.user)
|
|
|
|
expect do
|
|
topic = TopicConverter.new(first_post.topic, admin).convert_to_public_topic
|
|
topic.reload
|
|
end.to change { uncategorized_category.reload.topic_count }.by(1).and change {
|
|
author.reload.topic_count
|
|
}.from(0).to(1).and change { author.reload.post_count }.from(0).to(2)
|
|
|
|
# Ensure query does not affect users from other topics or posts as DB query to update count is quite complex.
|
|
expect(other_pm.user.topic_count).to eq(0)
|
|
expect(other_pm.user.post_count).to eq(0)
|
|
expect(other_pm_post.user.topic_count).to eq(0)
|
|
expect(other_pm_post.user.post_count).to eq(0)
|
|
|
|
expect(topic).to be_valid
|
|
expect(topic.archetype).to eq("regular")
|
|
expect(topic.category_id).to eq(SiteSetting.uncategorized_category_id)
|
|
end
|
|
|
|
it "moves the tags' personal message counts to their topic counts" do
|
|
SiteSetting.allow_uncategorized_topics = true
|
|
tag = Fabricate(:tag)
|
|
first_post.topic.tags << tag
|
|
expect(tag.reload.pm_topic_count).to eq(1)
|
|
|
|
TopicConverter.new(first_post.topic, admin).convert_to_public_topic
|
|
|
|
tag.reload
|
|
expect(tag.pm_topic_count).to eq(0)
|
|
expect(tag.staff_topic_count).to eq(1)
|
|
expect(tag.public_topic_count).to eq(1)
|
|
end
|
|
|
|
it "does not change the tags' public topic counts when converting into a read-restricted category" do
|
|
private_category = Fabricate(:private_category, group: Group[:staff])
|
|
tag = Fabricate(:tag)
|
|
first_post.topic.tags << tag
|
|
expect(tag.reload.pm_topic_count).to eq(1)
|
|
|
|
TopicConverter.new(first_post.topic, admin).convert_to_public_topic(private_category.id)
|
|
|
|
tag.reload
|
|
expect(tag.pm_topic_count).to eq(0)
|
|
expect(tag.staff_topic_count).to eq(1)
|
|
expect(tag.public_topic_count).to eq(0)
|
|
end
|
|
|
|
it "does not update tag counters when the post revision fails" do
|
|
tag = Fabricate(:tag)
|
|
first_post.topic.tags << tag
|
|
expect(tag.reload.pm_topic_count).to eq(1)
|
|
|
|
required_tag_group = Fabricate(:tag_group, tags: [Fabricate(:tag)])
|
|
category.category_required_tag_groups.create!(tag_group: required_tag_group, min_count: 1)
|
|
moderator = Fabricate(:moderator)
|
|
|
|
TopicConverter.new(first_post.topic, moderator).convert_to_public_topic(category.id)
|
|
|
|
expect(first_post.topic.reload.archetype).to eq(Archetype.private_message)
|
|
tag.reload
|
|
expect(tag.pm_topic_count).to eq(1)
|
|
expect(tag.staff_topic_count).to eq(0)
|
|
expect(tag.public_topic_count).to eq(0)
|
|
end
|
|
|
|
context "when uncategorized category is not allowed" do
|
|
before do
|
|
SiteSetting.allow_uncategorized_topics = false
|
|
category.update!(read_restricted: false)
|
|
end
|
|
|
|
it "should convert private message into the right category" do
|
|
topic = TopicConverter.new(first_post.topic, admin).convert_to_public_topic
|
|
topic.reload
|
|
|
|
expect(topic).to be_valid
|
|
expect(topic.archetype).to eq("regular")
|
|
|
|
first_category =
|
|
Category
|
|
.where.not(id: SiteSetting.uncategorized_category_id)
|
|
.where(read_restricted: false)
|
|
.order("id asc")
|
|
.first
|
|
|
|
expect(topic.category_id).to eq(first_category.id)
|
|
expect(topic.category.topic_count).to eq(2)
|
|
end
|
|
end
|
|
|
|
context "when a custom category_id is given" do
|
|
it "should convert private message into the right category" do
|
|
topic = TopicConverter.new(first_post.topic, admin).convert_to_public_topic(category.id)
|
|
|
|
expect(topic.reload.category).to eq(category)
|
|
expect(topic.category.topic_count).to eq(2)
|
|
end
|
|
end
|
|
|
|
it "updates user stats" do
|
|
first_post
|
|
topic_user = TopicUser.find_by(user_id: author.id, topic_id: private_message.id)
|
|
expect(private_message.user.user_stat.topic_count).to eq(0)
|
|
expect(private_message.user.user_stat.post_count).to eq(0)
|
|
private_message.convert_to_public_topic(admin)
|
|
expect(private_message.reload.user.user_stat.topic_count).to eq(1)
|
|
expect(private_message.user.user_stat.post_count).to eq(0)
|
|
expect(topic_user.reload.notification_level).to eq(TopicUser.notification_levels[:watching])
|
|
end
|
|
|
|
context "with a reply" do
|
|
before do
|
|
Jobs.run_immediately!
|
|
UserActionManager.enable
|
|
first_post
|
|
create_post(topic: private_message, user: other_user)
|
|
private_message.reload
|
|
end
|
|
|
|
it "updates UserActions" do
|
|
TopicConverter.new(private_message, admin).convert_to_public_topic
|
|
expect(
|
|
author.user_actions.where(action_type: UserAction::NEW_PRIVATE_MESSAGE).count,
|
|
).to eq(0)
|
|
expect(author.user_actions.where(action_type: UserAction::NEW_TOPIC).count).to eq(1)
|
|
expect(
|
|
other_user.user_actions.where(action_type: UserAction::NEW_PRIVATE_MESSAGE).count,
|
|
).to eq(0)
|
|
expect(
|
|
other_user.user_actions.where(action_type: UserAction::GOT_PRIVATE_MESSAGE).count,
|
|
).to eq(0)
|
|
expect(other_user.user_actions.where(action_type: UserAction::REPLY).count).to eq(1)
|
|
end
|
|
end
|
|
|
|
it "creates small action and revision when not silent" do
|
|
Jobs.run_immediately!
|
|
first_post
|
|
|
|
expect do
|
|
TopicConverter.new(private_message, admin).convert_to_public_topic(category.id)
|
|
end.to change { PostRevision.count }.by(1)
|
|
|
|
expect(
|
|
private_message.posts.where(
|
|
post_type: Post.types[:small_action],
|
|
action_code: "public_topic",
|
|
),
|
|
).to be_present
|
|
end
|
|
|
|
it "records a revision but skips small action and bump when silent" do
|
|
Jobs.run_immediately!
|
|
first_post
|
|
bumped_at = private_message.reload.bumped_at
|
|
|
|
expect do
|
|
TopicConverter.new(private_message, admin, silent: true).convert_to_public_topic(
|
|
category.id,
|
|
)
|
|
end.to change { PostRevision.count }.by(1)
|
|
|
|
expect(private_message.first_post.revisions.last.hidden).to eq(true)
|
|
expect(private_message.posts.where(post_type: Post.types[:small_action])).to be_empty
|
|
expect(private_message.reload.bumped_at).to be_within(1.second).of(bumped_at)
|
|
end
|
|
|
|
it "deletes notifications for users not allowed to see the topic" do
|
|
staff_category = Fabricate(:private_category, group: Group[:staff])
|
|
user_notification =
|
|
Fabricate(:mentioned_notification, post: first_post, user: Fabricate(:user))
|
|
admin_notification =
|
|
Fabricate(:mentioned_notification, post: first_post, user: Fabricate(:admin))
|
|
|
|
Jobs.run_immediately!
|
|
TopicConverter.new(first_post.topic, admin).convert_to_public_topic(staff_category.id)
|
|
|
|
expect(Notification.exists?(user_notification.id)).to eq(false)
|
|
expect(Notification.exists?(admin_notification.id)).to eq(true)
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "convert_to_private_message" do
|
|
fab!(:admin)
|
|
fab!(:author, :user)
|
|
fab!(:category)
|
|
fab!(:topic) { Fabricate(:topic, user: author, category_id: category.id) }
|
|
fab!(:post) { Fabricate(:post, topic: topic, user: topic.user) }
|
|
|
|
context "with success" do
|
|
it "converts regular topic to private message" do
|
|
private_message = topic.convert_to_private_message(admin)
|
|
expect(private_message).to be_valid
|
|
expect(topic.archetype).to eq("private_message")
|
|
expect(topic.category_id).to eq(nil)
|
|
expect(category.reload.topic_count).to eq(0)
|
|
end
|
|
|
|
it "moves the tags' topic counts to their personal message counts" do
|
|
tag = Fabricate(:tag)
|
|
topic.tags << tag
|
|
expect(tag.reload.staff_topic_count).to eq(1)
|
|
|
|
topic.convert_to_private_message(admin)
|
|
|
|
tag.reload
|
|
expect(tag.public_topic_count).to eq(0)
|
|
expect(tag.staff_topic_count).to eq(0)
|
|
expect(tag.pm_topic_count).to eq(1)
|
|
end
|
|
|
|
it "does not change the tags' public topic counts when converting from a read-restricted category" do
|
|
private_category = Fabricate(:private_category, group: Group[:staff])
|
|
restricted_topic = Fabricate(:topic, user: author, category: private_category)
|
|
Fabricate(:post, topic: restricted_topic, user: author)
|
|
tag = Fabricate(:tag)
|
|
restricted_topic.tags << tag
|
|
expect(tag.reload.staff_topic_count).to eq(1)
|
|
expect(tag.public_topic_count).to eq(0)
|
|
|
|
restricted_topic.convert_to_private_message(admin)
|
|
|
|
tag.reload
|
|
expect(tag.public_topic_count).to eq(0)
|
|
expect(tag.staff_topic_count).to eq(0)
|
|
expect(tag.pm_topic_count).to eq(1)
|
|
end
|
|
|
|
it "converts unlisted topic to private message" do
|
|
topic.update_status("visible", false, admin)
|
|
private_message = topic.convert_to_private_message(admin)
|
|
|
|
expect(private_message).to be_valid
|
|
expect(topic.archetype).to eq("private_message")
|
|
expect(topic.category_id).to eq(nil)
|
|
expect(topic.user.post_count).to eq(0)
|
|
expect(topic.user.topic_count).to eq(0)
|
|
expect(category.reload.topic_count).to eq(0)
|
|
end
|
|
|
|
it "updates user stats when converting topic to private message" do
|
|
_post_2 = Fabricate(:post, topic: topic, user: author)
|
|
_post_3 = Fabricate(:post, topic: topic, user: author)
|
|
|
|
other_topic = Fabricate(:post).topic
|
|
other_post = Fabricate(:post, topic: other_topic)
|
|
|
|
topic_user = TopicUser.create!(user_id: author.id, topic_id: topic.id, posted: true)
|
|
|
|
expect do topic.convert_to_private_message(admin) end.to change {
|
|
author.reload.post_count
|
|
}.from(2).to(0).and change { author.reload.topic_count }.from(1).to(0)
|
|
|
|
# Ensure query does not affect users from other topics or posts as DB query to update count is quite complex.
|
|
expect(other_topic.user.post_count).to eq(0)
|
|
expect(other_topic.user.topic_count).to eq(1)
|
|
expect(other_post.user.post_count).to eq(1)
|
|
expect(other_post.user.topic_count).to eq(0)
|
|
|
|
expect(topic.reload.topic_allowed_users.where(user_id: author.id).count).to eq(1)
|
|
expect(topic_user.reload.notification_level).to eq(TopicUser.notification_levels[:watching])
|
|
end
|
|
|
|
it "invites only users with regular posts" do
|
|
post2 = Fabricate(:post, topic: topic)
|
|
Fabricate(:post, topic: topic, post_type: Post.types[:whisper])
|
|
Fabricate(:post, topic: topic, post_type: Post.types[:small_action])
|
|
|
|
topic.convert_to_private_message(admin)
|
|
|
|
expect(topic.reload.topic_allowed_users.pluck(:user_id)).to contain_exactly(
|
|
admin.id,
|
|
post.user_id,
|
|
post2.user_id,
|
|
)
|
|
end
|
|
|
|
it "changes user_action type" do
|
|
Jobs.run_immediately!
|
|
UserActionManager.enable
|
|
topic.convert_to_private_message(admin)
|
|
expect(author.user_actions.where(action_type: UserAction::NEW_TOPIC).count).to eq(0)
|
|
expect(author.user_actions.where(action_type: UserAction::NEW_PRIVATE_MESSAGE).count).to eq(
|
|
1,
|
|
)
|
|
end
|
|
|
|
it "deletes notifications for users not allowed to see the message" do
|
|
user_notification = Fabricate(:mentioned_notification, post: post, user: Fabricate(:user))
|
|
admin_notification = Fabricate(:mentioned_notification, post: post, user: Fabricate(:admin))
|
|
|
|
Jobs.run_immediately!
|
|
topic.convert_to_private_message(admin)
|
|
|
|
expect(Notification.exists?(user_notification.id)).to eq(false)
|
|
expect(Notification.exists?(admin_notification.id)).to eq(true)
|
|
end
|
|
|
|
it "fails with an error when posters exceed max_allowed_message_recipients" do
|
|
SiteSetting.max_allowed_message_recipients = 2
|
|
Fabricate(:post, topic: topic)
|
|
Fabricate(:post, topic: topic)
|
|
|
|
result = TopicConverter.new(topic, admin).convert_to_private_message
|
|
|
|
expect(result.errors[:base]).to be_present
|
|
expect(topic.reload.archetype).to eq(Archetype.default)
|
|
end
|
|
|
|
it "creates small action and revision when not silent" do
|
|
Jobs.run_immediately!
|
|
|
|
expect do TopicConverter.new(topic, admin).convert_to_private_message end.to change {
|
|
PostRevision.count
|
|
}.by(1)
|
|
|
|
expect(
|
|
topic.posts.where(post_type: Post.types[:small_action], action_code: "private_topic"),
|
|
).to be_present
|
|
end
|
|
|
|
it "records a revision but skips small action and bump when silent" do
|
|
Jobs.run_immediately!
|
|
bumped_at = topic.bumped_at
|
|
|
|
expect do
|
|
TopicConverter.new(topic, admin, silent: true).convert_to_private_message
|
|
end.to change { PostRevision.count }.by(1)
|
|
|
|
expect(topic.first_post.revisions.last.hidden).to eq(true)
|
|
expect(topic.posts.where(post_type: Post.types[:small_action])).to be_empty
|
|
expect(topic.reload.bumped_at).to be_within(1.second).of(bumped_at)
|
|
end
|
|
|
|
it "includes the poster of a single-post topic" do
|
|
moderator = Fabricate(:moderator)
|
|
private_message = topic.convert_to_private_message(moderator)
|
|
expect(private_message.allowed_users).to match_array([topic.user, moderator])
|
|
end
|
|
end
|
|
|
|
context "when topic has replies" do
|
|
let(:replied_user) { Fabricate(:coding_horror) }
|
|
|
|
before do
|
|
create_post(topic: topic, user: replied_user)
|
|
topic.reload
|
|
end
|
|
|
|
it "adds users who replied to topic in Private Message" do
|
|
topic.convert_to_private_message(admin)
|
|
|
|
expect(topic.reload.topic_allowed_users.where(user_id: replied_user.id).count).to eq(1)
|
|
expect(topic.reload.user.user_stat.post_count).to eq(0)
|
|
end
|
|
end
|
|
|
|
context "when user already exists in topic_allowed_users table" do
|
|
before { topic.topic_allowed_users.create!(user_id: admin.id) }
|
|
|
|
it "works" do
|
|
topic.convert_to_private_message(admin)
|
|
|
|
expect(topic.reload.archetype).to eq("private_message")
|
|
end
|
|
end
|
|
|
|
context "with user_profiles with newly converted PM as featured topic" do
|
|
it "sets all matching user_profile featured topic ids to nil" do
|
|
author.user_profile.update(featured_topic: topic)
|
|
topic.convert_to_private_message(admin)
|
|
|
|
expect(author.user_profile.reload.featured_topic).to eq(nil)
|
|
end
|
|
end
|
|
end
|
|
end
|
