0
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2026-08-05 17:25:34 +08:00
discourse/app/models/topic_converter.rb
Régis Hanol b8077beba5
FIX: Allow selecting tags that are only used in personal messages (#41918)
Previously, 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
2026-07-24 12:27:03 +02:00

184 lines
5.1 KiB
Ruby
Vendored

# frozen_string_literal: true
class TopicConverter
attr_reader :topic
def initialize(topic, user, silent: false)
@topic = topic
@user = user
@silent = silent
end
def convert_to_public_topic(category_id = nil)
Topic.transaction do
category_id ||=
SiteSetting.uncategorized_category_id if SiteSetting.allow_uncategorized_topics
@category = Category.find_by(id: category_id) if category_id
@category ||=
Category
.where(read_restricted: false)
.where.not(id: SiteSetting.uncategorized_category_id)
.first
revised =
PostRevisor.new(@topic.first_post, @topic).revise!(
@user,
{ category_id: @category.id, archetype: Archetype.default },
revise_opts,
)
raise ActiveRecord::Rollback if !revised || !@topic.valid?
update_user_stats
update_post_uploads_secure_status
add_small_action("public_topic") unless @silent
update_tag_counters(1, include_public: !@category.read_restricted)
Jobs.enqueue(:topic_action_converter, topic_id: @topic.id)
Jobs.enqueue(:delete_inaccessible_notifications, topic_id: @topic.id)
watch_topic(@topic) unless @silent
end
@topic
end
def convert_to_private_message
if exceeds_recipient_cap?
@topic.errors.add(
:base,
I18n.t(
"topic_converter.too_many_recipients",
max: SiteSetting.max_allowed_message_recipients,
),
)
return @topic
end
Topic.transaction do
was_public = !@topic.category.read_restricted
@topic.update_category_topic_count_by(-1) if @topic.visible
revised =
PostRevisor.new(@topic.first_post, @topic).revise!(
@user,
{ category_id: nil, archetype: Archetype.private_message },
revise_opts,
)
raise ActiveRecord::Rollback if !revised || !@topic.valid?
add_allowed_users
update_post_uploads_secure_status
add_small_action("private_topic") unless @silent
update_tag_counters(-1, include_public: was_public)
UserProfile.remove_featured_topic_from_all_profiles(@topic)
Jobs.enqueue(:topic_action_converter, topic_id: @topic.id)
Jobs.enqueue(:delete_inaccessible_notifications, topic_id: @topic.id)
watch_topic(@topic) unless @silent
end
@topic
end
private
def update_tag_counters(topic_count_delta, include_public:)
counters = { staff_topic_count: topic_count_delta, pm_topic_count: -topic_count_delta }
counters[:public_topic_count] = topic_count_delta if include_public
Tag.update_counters(@topic.tags, counters)
end
def revise_opts
{ bypass_bump: @silent, silent: @silent, hidden: true }
end
def posters
@posters ||=
@topic
.posts
.where.not(post_type: [Post.types[:small_action], Post.types[:whisper]])
.distinct
.pluck(:user_id)
end
def increment_users_post_count
update_users_post_count(:increment)
end
def decrement_users_post_count
update_users_post_count(:decrement)
end
def update_users_post_count(action)
operation = action == :increment ? "+" : "-"
# NOTE that DirectoryItem.refresh will overwrite this by counting UserAction records.
#
# Changes user_stats (post_count) by the number of posts in the topic.
# First post, hidden posts and non-regular posts are ignored.
DB.exec <<~SQL
UPDATE user_stats
SET post_count = post_count #{operation} X.count
FROM (
SELECT
us.user_id,
COUNT(*) AS count
FROM user_stats us
INNER JOIN posts ON posts.topic_id = #{@topic.id.to_i} AND posts.user_id = us.user_id
WHERE posts.post_number > 1
AND NOT posts.hidden
AND posts.post_type = #{Post.types[:regular].to_i}
GROUP BY us.user_id
) X
WHERE X.user_id = user_stats.user_id
SQL
end
def update_user_stats
increment_users_post_count
UserStatCountUpdater.increment!(@topic.first_post)
end
def add_allowed_users
decrement_users_post_count
UserStatCountUpdater.decrement!(@topic.first_post)
existing_allowed_users = @topic.topic_allowed_users.pluck(:user_id)
users_to_allow = posters << @user.id
(users_to_allow - existing_allowed_users).uniq.each do |user_id|
@topic.topic_allowed_users.build(user_id: user_id)
end
@topic.save!
end
def exceeds_recipient_cap?
allowed_users = @topic.topic_allowed_users.pluck(:user_id)
total_recipients = (posters | allowed_users | [@user.id]).size
total_recipients > SiteSetting.max_allowed_message_recipients
end
def watch_topic(topic)
@topic.notifier.watch_topic!(topic.user_id)
@topic.reload.topic_allowed_users.each do |tau|
next if tau.user_id < 0 || tau.user_id == topic.user_id
topic.notifier.watch!(tau.user_id)
end
end
def update_post_uploads_secure_status
DB.after_commit { Jobs.enqueue(:update_topic_upload_security, topic_id: @topic.id) }
end
def add_small_action(action_code)
DB.after_commit { @topic.add_small_action(@user, action_code) }
end
end