SECURITY: Default tags to show count of topics in unrestricted categories (#19929)

Currently, `Tag#topic_count` is a count of all regular topics regardless of whether the topic is in a read restricted category or not. As a result, any users can technically poll a sensitive tag to determine if a new topic is created in a category which the user has not excess to. We classify this as a minor leak in sensitive information.

The following changes are introduced in this commit:

1. Introduce `Tag#public_topic_count` which only count topics which have been tagged with a given tag in public categories.
2. Rename `Tag#topic_count` to `Tag#staff_topic_count` which counts the same way as `Tag#topic_count`. In other words, it counts all topics tagged with a given tag regardless of the category the topic is in. The rename is also done so that we indicate that this column contains sensitive information.
3. Change all previous spots which relied on `Topic#topic_count` to rely on `Tag.topic_column_count(guardian)` which will return the right "topic count" column to use based on the current scope.
4. Introduce `SiteSetting.include_secure_categories_in_tag_counts` site setting to allow site administrators to always display the tag topics count using `Tag#staff_topic_count` instead.
This commit is contained in:
Alan Guo Xiang Tan 2023-01-20 11:59:37 +08:00 committed by GitHub
parent c84f189011
commit 0e69aeb276
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
28 changed files with 602 additions and 127 deletions

View File

@ -40,7 +40,7 @@ class TagsController < ::ApplicationController
if SiteSetting.tags_listed_by_group if SiteSetting.tags_listed_by_group
ungrouped_tags = Tag.where("tags.id NOT IN (SELECT tag_id FROM tag_group_memberships)") ungrouped_tags = Tag.where("tags.id NOT IN (SELECT tag_id FROM tag_group_memberships)")
ungrouped_tags = ungrouped_tags.where("tags.topic_count > 0") unless show_all_tags ungrouped_tags = ungrouped_tags.used_tags_in_regular_topics(guardian) unless show_all_tags
grouped_tag_counts = grouped_tag_counts =
TagGroup TagGroup
@ -51,18 +51,14 @@ class TagsController < ::ApplicationController
{ {
id: tag_group.id, id: tag_group.id,
name: tag_group.name, name: tag_group.name,
tags: tags: self.class.tag_counts_json(tag_group.tags.where(target_tag_id: nil), guardian),
self.class.tag_counts_json(
tag_group.tags.where(target_tag_id: nil),
show_pm_tags: guardian.can_tag_pms?,
),
} }
end end
@tags = self.class.tag_counts_json(ungrouped_tags, show_pm_tags: guardian.can_tag_pms?) @tags = self.class.tag_counts_json(ungrouped_tags, guardian)
@extras = { tag_groups: grouped_tag_counts } @extras = { tag_groups: grouped_tag_counts }
else else
tags = show_all_tags ? Tag.all : Tag.where("tags.topic_count > 0") tags = show_all_tags ? Tag.all : Tag.used_tags_in_regular_topics(guardian)
unrestricted_tags = DiscourseTagging.filter_visible(tags.where(target_tag_id: nil), guardian) unrestricted_tags = DiscourseTagging.filter_visible(tags.where(target_tag_id: nil), guardian)
categories = categories =
@ -77,13 +73,14 @@ class TagsController < ::ApplicationController
category_tags = category_tags =
self.class.tag_counts_json( self.class.tag_counts_json(
DiscourseTagging.filter_visible(c.tags.where(target_tag_id: nil), guardian), DiscourseTagging.filter_visible(c.tags.where(target_tag_id: nil), guardian),
guardian,
) )
next if category_tags.empty? next if category_tags.empty?
{ id: c.id, tags: category_tags } { id: c.id, tags: category_tags }
end end
.compact .compact
@tags = self.class.tag_counts_json(unrestricted_tags, show_pm_tags: guardian.can_tag_pms?) @tags = self.class.tag_counts_json(unrestricted_tags, guardian)
@extras = { categories: category_tag_counts } @extras = { categories: category_tag_counts }
end end
@ -264,7 +261,7 @@ class TagsController < ::ApplicationController
tags_with_counts, filter_result_context = tags_with_counts, filter_result_context =
DiscourseTagging.filter_allowed_tags(guardian, **filter_params, with_context: true) DiscourseTagging.filter_allowed_tags(guardian, **filter_params, with_context: true)
tags = self.class.tag_counts_json(tags_with_counts, show_pm_tags: guardian.can_tag_pms?) tags = self.class.tag_counts_json(tags_with_counts, guardian)
json_response = { results: tags } json_response = { results: tags }
@ -388,18 +385,22 @@ class TagsController < ::ApplicationController
end end
end end
def self.tag_counts_json(tags, show_pm_tags: true) def self.tag_counts_json(tags, guardian)
show_pm_tags = guardian.can_tag_pms?
target_tags = Tag.where(id: tags.map(&:target_tag_id).compact.uniq).select(:id, :name) target_tags = Tag.where(id: tags.map(&:target_tag_id).compact.uniq).select(:id, :name)
tags tags
.map do |t| .map do |t|
next if t.topic_count == 0 && t.pm_topic_count > 0 && !show_pm_tags topic_count = t.public_send(Tag.topic_count_column(guardian))
next if topic_count == 0 && t.pm_topic_count > 0 && !show_pm_tags
{ {
id: t.name, id: t.name,
text: t.name, text: t.name,
name: t.name, name: t.name,
description: t.description, description: t.description,
count: t.topic_count, count: topic_count,
pm_count: show_pm_tags ? t.pm_topic_count : 0, pm_count: show_pm_tags ? t.pm_topic_count : 0,
target_tag: target_tag:
t.target_tag_id ? target_tags.find { |x| x.id == t.target_tag_id }&.name : nil, t.target_tag_id ? target_tags.find { |x| x.id == t.target_tag_id }&.name : nil,

View File

@ -4,6 +4,10 @@ class Tag < ActiveRecord::Base
include Searchable include Searchable
include HasDestroyedWebHook include HasDestroyedWebHook
self.ignored_columns = [
"topic_count", # TODO(tgxworld): Remove on 1 July 2023
]
RESERVED_TAGS = [ RESERVED_TAGS = [
"none", "none",
"constructor", # prevents issues with javascript's constructor of objects "constructor", # prevents issues with javascript's constructor of objects
@ -25,11 +29,14 @@ class Tag < ActiveRecord::Base
# tags that have never been used and don't belong to a tag group # tags that have never been used and don't belong to a tag group
scope :unused, scope :unused,
-> { -> {
where(topic_count: 0, pm_topic_count: 0).joins( where(staff_topic_count: 0, pm_topic_count: 0).joins(
"LEFT JOIN tag_group_memberships tgm ON tags.id = tgm.tag_id", "LEFT JOIN tag_group_memberships tgm ON tags.id = tgm.tag_id",
).where("tgm.tag_id IS NULL") ).where("tgm.tag_id IS NULL")
} }
scope :used_tags_in_regular_topics,
->(guardian) { where("tags.#{Tag.topic_count_column(guardian)} > 0") }
scope :base_tags, -> { where(target_tag_id: nil) } scope :base_tags, -> { where(target_tag_id: nil) }
has_many :tag_users, dependent: :destroy # notification settings has_many :tag_users, dependent: :destroy # notification settings
@ -62,7 +69,7 @@ class Tag < ActiveRecord::Base
def self.update_topic_counts def self.update_topic_counts
DB.exec <<~SQL DB.exec <<~SQL
UPDATE tags t UPDATE tags t
SET topic_count = x.topic_count SET staff_topic_count = x.topic_count
FROM ( FROM (
SELECT COUNT(topics.id) AS topic_count, tags.id AS tag_id SELECT COUNT(topics.id) AS topic_count, tags.id AS tag_id
FROM tags FROM tags
@ -73,7 +80,31 @@ class Tag < ActiveRecord::Base
GROUP BY tags.id GROUP BY tags.id
) x ) x
WHERE x.tag_id = t.id WHERE x.tag_id = t.id
AND x.topic_count <> t.topic_count AND x.topic_count <> t.staff_topic_count
SQL
DB.exec <<~SQL
UPDATE tags t
SET public_topic_count = x.topic_count
FROM (
WITH tags_with_public_topics AS (
SELECT
COUNT(topics.id) AS topic_count,
tags.id AS tag_id
FROM tags
INNER JOIN topic_tags ON tags.id = topic_tags.tag_id
INNER JOIN topics ON topics.id = topic_tags.topic_id AND topics.deleted_at IS NULL AND topics.archetype != 'private_message'
INNER JOIN categories ON categories.id = topics.category_id AND NOT categories.read_restricted
GROUP BY tags.id
)
SELECT
COALESCE(tags_with_public_topics.topic_count, 0 ) AS topic_count,
tags.id AS tag_id
FROM tags
LEFT JOIN tags_with_public_topics ON tags_with_public_topics.tag_id = tags.id
) x
WHERE x.tag_id = t.id
AND x.topic_count <> t.public_topic_count;
SQL SQL
DB.exec <<~SQL DB.exec <<~SQL
@ -97,19 +128,18 @@ class Tag < ActiveRecord::Base
self.find_by("lower(name) = ?", name.downcase) self.find_by("lower(name) = ?", name.downcase)
end end
def self.top_tags(limit_arg: nil, category: nil, guardian: nil) def self.top_tags(limit_arg: nil, category: nil, guardian: Guardian.new)
# we add 1 to max_tags_in_filter_list to efficiently know we have more tags # we add 1 to max_tags_in_filter_list to efficiently know we have more tags
# than the limit. Frontend is responsible to enforce limit. # than the limit. Frontend is responsible to enforce limit.
limit = limit_arg || (SiteSetting.max_tags_in_filter_list + 1) limit = limit_arg || (SiteSetting.max_tags_in_filter_list + 1)
scope_category_ids = (guardian || Guardian.new).allowed_category_ids scope_category_ids = guardian.allowed_category_ids
scope_category_ids &= ([category.id] + category.subcategories.pluck(:id)) if category scope_category_ids &= ([category.id] + category.subcategories.pluck(:id)) if category
return [] if scope_category_ids.empty? return [] if scope_category_ids.empty?
filter_sql = filter_sql =
( (
if guardian&.is_staff? if guardian.is_staff?
"" ""
else else
" AND tags.id IN (#{DiscourseTagging.visible_tags(guardian).select(:id).to_sql})" " AND tags.id IN (#{DiscourseTagging.visible_tags(guardian).select(:id).to_sql})"
@ -130,6 +160,14 @@ class Tag < ActiveRecord::Base
tag_names_with_counts.map { |row| row.tag_name } tag_names_with_counts.map { |row| row.tag_name }
end end
def self.topic_count_column(guardian)
if guardian&.is_staff? || SiteSetting.include_secure_categories_in_tag_counts
"staff_topic_count"
else
"public_topic_count"
end
end
def self.pm_tags(limit: 1000, guardian: nil, allowed_user: nil) def self.pm_tags(limit: 1000, guardian: nil, allowed_user: nil)
return [] if allowed_user.blank? || !(guardian || Guardian.new).can_tag_pms? return [] if allowed_user.blank? || !(guardian || Guardian.new).can_tag_pms?
user_id = allowed_user.id user_id = allowed_user.id
@ -216,12 +254,13 @@ end
# #
# id :integer not null, primary key # id :integer not null, primary key
# name :string not null # name :string not null
# topic_count :integer default(0), not null
# created_at :datetime not null # created_at :datetime not null
# updated_at :datetime not null # updated_at :datetime not null
# pm_topic_count :integer default(0), not null # pm_topic_count :integer default(0), not null
# target_tag_id :integer # target_tag_id :integer
# description :string # description :string
# public_topic_count :integer default(0), not null
# staff_topic_count :integer default(0), not null
# #
# Indexes # Indexes
# #

View File

@ -958,6 +958,7 @@ class Topic < ActiveRecord::Base
def changed_to_category(new_category) def changed_to_category(new_category)
return true if new_category.blank? || Category.exists?(topic_id: id) return true if new_category.blank? || Category.exists?(topic_id: id)
if new_category.id == SiteSetting.uncategorized_category_id && if new_category.id == SiteSetting.uncategorized_category_id &&
!SiteSetting.allow_uncategorized_topics !SiteSetting.allow_uncategorized_topics
return false return false
@ -971,6 +972,15 @@ class Topic < ActiveRecord::Base
if old_category if old_category
Category.where(id: old_category.id).update_all("topic_count = topic_count - 1") Category.where(id: old_category.id).update_all("topic_count = topic_count - 1")
count =
if old_category.read_restricted && !new_category.read_restricted
1
elsif !old_category.read_restricted && new_category.read_restricted
-1
end
Tag.update_counters(self.tags, { public_topic_count: count }) if count
end end
# when a topic changes category we may have to start watching it # when a topic changes category we may have to start watching it
@ -1781,12 +1791,15 @@ class Topic < ActiveRecord::Base
def convert_to_public_topic(user, category_id: nil) def convert_to_public_topic(user, category_id: nil)
public_topic = TopicConverter.new(self, user).convert_to_public_topic(category_id) public_topic = TopicConverter.new(self, user).convert_to_public_topic(category_id)
Tag.update_counters(public_topic.tags, { public_topic_count: 1 }) if !category.read_restricted
add_small_action(user, "public_topic") if public_topic add_small_action(user, "public_topic") if public_topic
public_topic public_topic
end end
def convert_to_private_message(user) def convert_to_private_message(user)
read_restricted = category.read_restricted
private_topic = TopicConverter.new(self, user).convert_to_private_message private_topic = TopicConverter.new(self, user).convert_to_private_message
Tag.update_counters(private_topic.tags, { public_topic_count: -1 }) if !read_restricted
add_small_action(user, "private_topic") if private_topic add_small_action(user, "private_topic") if private_topic
private_topic private_topic
end end

View File

@ -9,14 +9,18 @@ class TopicTag < ActiveRecord::Base
if topic.archetype == Archetype.private_message if topic.archetype == Archetype.private_message
tag.increment!(:pm_topic_count) tag.increment!(:pm_topic_count)
else else
tag.increment!(:topic_count) counters_to_update = { staff_topic_count: 1 }
if Category.exists?(id: topic.category_id, read_restricted: false)
counters_to_update[:public_topic_count] = 1
end
Tag.update_counters(tag.id, counters_to_update)
if topic.category_id
if stat = CategoryTagStat.find_by(tag_id: tag_id, category_id: topic.category_id) if stat = CategoryTagStat.find_by(tag_id: tag_id, category_id: topic.category_id)
stat.increment!(:topic_count) stat.increment!(:topic_count)
else else
CategoryTagStat.create(tag_id: tag_id, category_id: topic.category_id, topic_count: 1) CategoryTagStat.create!(tag_id: tag_id, category_id: topic.category_id, topic_count: 1)
end
end end
end end
end end
@ -27,12 +31,17 @@ class TopicTag < ActiveRecord::Base
if topic.archetype == Archetype.private_message if topic.archetype == Archetype.private_message
tag.decrement!(:pm_topic_count) tag.decrement!(:pm_topic_count)
else else
if topic.category_id && if stat = CategoryTagStat.find_by(tag_id: tag_id, category: topic.category_id)
stat = CategoryTagStat.find_by(tag_id: tag_id, category: topic.category_id) stat.topic_count == 1 ? stat.destroy! : stat.decrement!(:topic_count)
stat.topic_count == 1 ? stat.destroy : stat.decrement!(:topic_count)
end end
tag.decrement!(:topic_count) counters_to_update = { staff_topic_count: -1 }
if Category.exists?(id: topic.category_id, read_restricted: false)
counters_to_update[:public_topic_count] = -1
end
Tag.update_counters(tag.id, counters_to_update)
end end
end end
end end

View File

@ -32,7 +32,8 @@ module TopicTagsMixin
if SiteSetting.tags_sort_alphabetically if SiteSetting.tags_sort_alphabetically
topic.tags.sort_by(&:name) topic.tags.sort_by(&:name)
else else
topic.tags.sort_by(&:topic_count).reverse topic_count_column = Tag.topic_count_column(scope)
topic.tags.sort_by { |tag| tag.public_send(topic_count_column) }.reverse
end end
) )
tags = tags.reject { |tag| scope.hidden_tag_names.include?(tag[:name]) } if !scope.is_staff? tags = tags.reject { |tag| scope.hidden_tag_names.include?(tag[:name]) } if !scope.is_staff?

View File

@ -2,9 +2,11 @@
module UserSidebarMixin module UserSidebarMixin
def sidebar_tags def sidebar_tags
topic_count_column = Tag.topic_count_column(scope)
object object
.visible_sidebar_tags(scope) .visible_sidebar_tags(scope)
.pluck(:name, :topic_count, :pm_topic_count) .pluck(:name, topic_count_column, :pm_topic_count)
.reduce([]) do |tags, sidebar_tag| .reduce([]) do |tags, sidebar_tag|
tags.push(name: sidebar_tag[0], pm_only: sidebar_tag[1] == 0 && sidebar_tag[2] > 0) tags.push(name: sidebar_tag[0], pm_only: sidebar_tag[1] == 0 && sidebar_tag[2] > 0)
end end

View File

@ -6,7 +6,7 @@ class DetailedTagSerializer < TagSerializer
has_many :categories, serializer: BasicCategorySerializer has_many :categories, serializer: BasicCategorySerializer
def synonyms def synonyms
TagsController.tag_counts_json(object.synonyms) TagsController.tag_counts_json(object.synonyms, scope)
end end
def categories def categories

View File

@ -3,6 +3,10 @@
class TagSerializer < ApplicationSerializer class TagSerializer < ApplicationSerializer
attributes :id, :name, :topic_count, :staff, :description attributes :id, :name, :topic_count, :staff, :description
def topic_count
object.public_send(Tag.topic_count_column(scope))
end
def staff def staff
DiscourseTagging.staff_tag_names.include?(name) DiscourseTagging.staff_tag_names.include?(name)
end end

View File

@ -12,26 +12,30 @@ class TagHashtagDataSource
"tag" "tag"
end end
def self.tag_to_hashtag_item(tag) def self.tag_to_hashtag_item(tag, guardian)
tag = Tag.new(tag.slice(:id, :name, :description).merge(topic_count: tag[:count])) if tag.is_a?( topic_count_column = Tag.topic_count_column(guardian)
Hash,
) tag =
Tag.new(
tag.slice(:id, :name, :description).merge(topic_count_column => tag[:count]),
) if tag.is_a?(Hash)
HashtagAutocompleteService::HashtagItem.new.tap do |item| HashtagAutocompleteService::HashtagItem.new.tap do |item|
item.text = tag.name item.text = tag.name
item.secondary_text = "x#{tag.topic_count}" item.secondary_text = "x#{tag.public_send(topic_count_column)}"
item.description = tag.description item.description = tag.description
item.slug = tag.name item.slug = tag.name
item.relative_url = tag.url item.relative_url = tag.url
item.icon = icon item.icon = icon
end end
end end
private_class_method :tag_to_hashtag_item
def self.lookup(guardian, slugs) def self.lookup(guardian, slugs)
return [] if !SiteSetting.tagging_enabled return [] if !SiteSetting.tagging_enabled
DiscourseTagging DiscourseTagging
.filter_visible(Tag.where_name(slugs), guardian) .filter_visible(Tag.where_name(slugs), guardian)
.map { |tag| tag_to_hashtag_item(tag) } .map { |tag| tag_to_hashtag_item(tag, guardian) }
end end
def self.search( def self.search(
@ -60,9 +64,9 @@ class TagHashtagDataSource
) )
TagsController TagsController
.tag_counts_json(tags_with_counts) .tag_counts_json(tags_with_counts, guardian)
.take(limit) .take(limit)
.map { |tag| tag_to_hashtag_item(tag) } .map { |tag| tag_to_hashtag_item(tag, guardian) }
end end
def self.search_sort(search_results, _) def self.search_sort(search_results, _)
@ -82,8 +86,8 @@ class TagHashtagDataSource
) )
TagsController TagsController
.tag_counts_json(tags_with_counts) .tag_counts_json(tags_with_counts, guardian)
.take(limit) .take(limit)
.map { |tag| tag_to_hashtag_item(tag) } .map { |tag| tag_to_hashtag_item(tag, guardian) }
end end
end end

View File

@ -1651,6 +1651,7 @@ en:
content_security_policy_frame_ancestors: "Restrict who can embed this site in iframes via CSP. Control allowed hosts on <a href='%{base_path}/admin/customize/embedding'>Embedding</a>" content_security_policy_frame_ancestors: "Restrict who can embed this site in iframes via CSP. Control allowed hosts on <a href='%{base_path}/admin/customize/embedding'>Embedding</a>"
content_security_policy_script_src: "Additional allowlisted script sources. The current host and CDN are included by default. See <a href='https://meta.discourse.org/t/mitigate-xss-attacks-with-content-security-policy/104243' target='_blank'>Mitigate XSS Attacks with Content Security Policy.</a>" content_security_policy_script_src: "Additional allowlisted script sources. The current host and CDN are included by default. See <a href='https://meta.discourse.org/t/mitigate-xss-attacks-with-content-security-policy/104243' target='_blank'>Mitigate XSS Attacks with Content Security Policy.</a>"
invalidate_inactive_admin_email_after_days: "Admin accounts that have not visited the site in this number of days will need to re-validate their email address before logging in. Set to 0 to disable." invalidate_inactive_admin_email_after_days: "Admin accounts that have not visited the site in this number of days will need to re-validate their email address before logging in. Set to 0 to disable."
include_secure_categories_in_tag_counts: "When enabled, count of topics for a tag will include topics that are in read restricted categories for all users. When disabled, normal users are only shown a count of topics for a tag where all the topics are in public categories."
top_menu: "Determine which items appear in the homepage navigation, and in what order. Example latest|new|unread|categories|top|read|posted|bookmarks" top_menu: "Determine which items appear in the homepage navigation, and in what order. Example latest|new|unread|categories|top|read|posted|bookmarks"
post_menu: "Determine which items appear on the post menu, and in what order. Example like|edit|flag|delete|share|bookmark|reply" post_menu: "Determine which items appear on the post menu, and in what order. Example like|edit|flag|delete|share|bookmark|reply"
post_menu_hidden_items: "The menu items to hide by default in the post menu unless an expansion ellipsis is clicked on." post_menu_hidden_items: "The menu items to hide by default in the post menu unless an expansion ellipsis is clicked on."

View File

@ -1783,6 +1783,8 @@ security:
suppress_secured_categories_from_admin: suppress_secured_categories_from_admin:
default: false default: false
hidden: true hidden: true
include_secure_categories_in_tag_counts:
default: false
onebox: onebox:
post_onebox_maxlength: post_onebox_maxlength:

View File

@ -0,0 +1,28 @@
# frozen_string_literal: true
class AddPublicTopicCountToTags < ActiveRecord::Migration[7.0]
def up
add_column :tags, :public_topic_count, :integer, default: 0, null: false
execute <<~SQL
UPDATE tags t
SET public_topic_count = x.topic_count
FROM (
SELECT
COUNT(topics.id) AS topic_count,
tags.id AS tag_id
FROM tags
INNER JOIN topic_tags ON tags.id = topic_tags.tag_id
INNER JOIN topics ON topics.id = topic_tags.topic_id AND topics.deleted_at IS NULL AND topics.archetype != 'private_message'
INNER JOIN categories ON categories.id = topics.category_id AND NOT categories.read_restricted
GROUP BY tags.id
) x
WHERE x.tag_id = t.id
AND x.topic_count <> t.public_topic_count;
SQL
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -0,0 +1,27 @@
# frozen_string_literal: true
class AddStaffTopicCountToTags < ActiveRecord::Migration[7.0]
def up
add_column :tags, :staff_topic_count, :integer, default: 0, null: false
execute <<~SQL
UPDATE tags t
SET staff_topic_count = x.topic_count
FROM (
SELECT COUNT(topics.id) AS topic_count, tags.id AS tag_id
FROM tags
LEFT JOIN topic_tags ON tags.id = topic_tags.tag_id
LEFT JOIN topics ON topics.id = topic_tags.topic_id
AND topics.deleted_at IS NULL
AND topics.archetype != 'private_message'
GROUP BY tags.id
) x
WHERE x.tag_id = t.id
AND x.topic_count <> t.staff_topic_count
SQL
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -0,0 +1,15 @@
# frozen_string_literal: true
require "migration/column_dropper"
class RemoveTopicCountFromTags < ActiveRecord::Migration[7.0]
DROPPED_COLUMNS ||= { tags: %i[topic_count] }
def up
DROPPED_COLUMNS.each { |table, columns| Migration::ColumnDropper.execute_drop(table, columns) }
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -323,17 +323,19 @@ module DiscourseTagging
outer_join = category.nil? || category.allow_global_tags || !category_has_restricted_tags outer_join = category.nil? || category.allow_global_tags || !category_has_restricted_tags
topic_count_column = Tag.topic_count_column(guardian)
distinct_clause = distinct_clause =
if opts[:order_popularity] if opts[:order_popularity]
"DISTINCT ON (topic_count, name)" "DISTINCT ON (#{topic_count_column}, name)"
elsif opts[:order_search_results] && opts[:term].present? elsif opts[:order_search_results] && opts[:term].present?
"DISTINCT ON (lower(name) = lower(:cleaned_term), topic_count, name)" "DISTINCT ON (lower(name) = lower(:cleaned_term), #{topic_count_column}, name)"
else else
"" ""
end end
sql << <<~SQL sql << <<~SQL
SELECT #{distinct_clause} t.id, t.name, t.topic_count, t.pm_topic_count, t.description, SELECT #{distinct_clause} t.id, t.name, t.#{topic_count_column}, t.pm_topic_count, t.description,
tgr.tgm_id as tgm_id, tgr.tag_group_id as tag_group_id, tgr.parent_tag_id as parent_tag_id, tgr.tgm_id as tgm_id, tgr.tag_group_id as tag_group_id, tgr.parent_tag_id as parent_tag_id,
tgr.one_per_topic as one_per_topic, t.target_tag_id tgr.one_per_topic as one_per_topic, t.target_tag_id
FROM tags t FROM tags t
@ -479,9 +481,9 @@ module DiscourseTagging
end end
if opts[:order_popularity] if opts[:order_popularity]
builder.order_by("topic_count DESC, name") builder.order_by("#{topic_count_column} DESC, name")
elsif opts[:order_search_results] && !term.blank? elsif opts[:order_search_results] && !term.blank?
builder.order_by("lower(name) = lower(:cleaned_term) DESC, topic_count DESC, name") builder.order_by("lower(name) = lower(:cleaned_term) DESC, #{topic_count_column} DESC, name")
end end
result = builder.query(builder_params).uniq { |t| t.id } result = builder.query(builder_params).uniq { |t| t.id }

View File

@ -250,7 +250,8 @@ class TopicView
@topic.category @topic.category
title += " - #{@topic.category.name}" title += " - #{@topic.category.name}"
elsif SiteSetting.tagging_enabled && @topic.tags.exists? elsif SiteSetting.tagging_enabled && @topic.tags.exists?
title += " - #{@topic.tags.order("tags.topic_count DESC").first.name}" title +=
" - #{@topic.tags.order("tags.#{Tag.topic_count_column(@guardian)} DESC").first.name}"
end end
end end
title title

View File

@ -0,0 +1,104 @@
# frozen_string_literal: true
RSpec.describe "Updating tag counts" do
fab!(:tag1) { Fabricate(:tag) }
fab!(:tag2) { Fabricate(:tag) }
fab!(:group) { Fabricate(:group) }
fab!(:public_category) { Fabricate(:category) }
fab!(:public_category2) { Fabricate(:category) }
fab!(:private_category) { Fabricate(:private_category, group: group) }
fab!(:private_category2) { Fabricate(:private_category, group: group) }
fab!(:topic_in_public_category) do
Fabricate(:topic, category: public_category, tags: [tag1, tag2]).tap do |topic|
Fabricate(:post, topic: topic)
end
end
fab!(:topic_in_private_category) do
Fabricate(:topic, category: private_category, tags: [tag1, tag2]).tap do |topic|
Fabricate(:post, topic: topic)
end
end
fab!(:private_message) do
topic = Fabricate(:private_message_post).topic
topic.update!(tags: [tag1, tag2])
topic
end
before do
expect(tag1.public_topic_count).to eq(1)
expect(tag1.staff_topic_count).to eq(2)
expect(tag1.pm_topic_count).to eq(1)
expect(tag2.reload.public_topic_count).to eq(1)
expect(tag2.staff_topic_count).to eq(2)
expect(tag2.pm_topic_count).to eq(1)
end
it "should decrease Tag#public_topic_count for all tags when topic's category is changed from a public category to a read restricted category" do
expect { topic_in_public_category.change_category_to_id(private_category.id) }.to change {
tag1.reload.public_topic_count
}.by(-1).and change { tag2.reload.public_topic_count }.by(-1)
end
it "should increase Tag#public_topic_count for all tags when topic's category is changed from a read restricted category to a public category" do
expect { topic_in_private_category.change_category_to_id(public_category.id) }.to change {
tag1.reload.public_topic_count
}.by(1).and change { tag2.reload.public_topic_count }.by(1)
end
it "should not change Tag#public_topic_count for all tags when topic's category is changed from a public category to another public category" do
expect do
topic_in_public_category.change_category_to_id(public_category2.id)
end.to not_change { tag1.reload.public_topic_count }.and not_change {
tag2.reload.public_topic_count
}
end
it "should not change Tag#public_topic_count for all tags when topic's category is changed from a read restricted category to another read restricted category" do
expect do
topic_in_private_category.change_category_to_id(private_category2.id)
end.to not_change { tag1.reload.public_topic_count }.and not_change {
tag2.reload.public_topic_count
}
end
it "increases Tag#public_topic_count for all tags when topic is converted from private message to a regular topic in a public category" do
expect do
private_message.convert_to_public_topic(
Discourse.system_user,
category_id: public_category.id,
)
end.to change { tag1.reload.public_topic_count }.by(1).and change {
tag2.reload.public_topic_count
}.by(1)
end
it "should not change Tag#public_topic_count for all tags when topic is converted from private message to a regular topic in a read restricted category" do
expect do
private_message.convert_to_public_topic(
Discourse.system_user,
category_id: private_category.id,
)
end.to not_change { tag1.reload.public_topic_count }.and not_change {
tag2.reload.public_topic_count
}
end
it "should decrease Tag#public_topic_count for all tags when regular topic in public category is converted to a private message" do
expect do
topic_in_public_category.convert_to_private_message(Discourse.system_user)
end.to change { tag1.reload.public_topic_count }.by(-1).and change {
tag2.reload.public_topic_count
}.by(-1)
end
it "should not change Tag#public_topic_count for all tags when regular topic in read restricted category is converted to a private message" do
expect do
topic_in_private_category.convert_to_private_message(Discourse.system_user)
end.to not_change { tag1.reload.public_topic_count }.and not_change {
tag2.reload.public_topic_count
}
end
end

View File

@ -641,9 +641,11 @@ RSpec.describe DiscourseTagging do
it "user does not get an error when editing their topic with a hidden tag" do it "user does not get an error when editing their topic with a hidden tag" do
PostRevisor.new(post).revise!(admin, raw: post.raw, tags: [hidden_tag.name]) PostRevisor.new(post).revise!(admin, raw: post.raw, tags: [hidden_tag.name])
expect( expect(
PostRevisor.new(post).revise!(topic.user, raw: post.raw + " edit", tags: []), PostRevisor.new(post).revise!(topic.user, raw: post.raw + " edit", tags: []),
).to be_truthy ).to be_truthy
expect(topic.reload.tags).to eq([hidden_tag]) expect(topic.reload.tags).to eq([hidden_tag])
end end
end end
@ -967,8 +969,16 @@ RSpec.describe DiscourseTagging do
topic = Fabricate(:topic, tags: [tag2]) topic = Fabricate(:topic, tags: [tag2])
expect(DiscourseTagging.add_or_create_synonyms_by_name(tag1, [tag2.name])).to eq(true) expect(DiscourseTagging.add_or_create_synonyms_by_name(tag1, [tag2.name])).to eq(true)
expect_same_tag_names(topic.reload.tags, [tag1]) expect_same_tag_names(topic.reload.tags, [tag1])
expect(tag1.reload.topic_count).to eq(1)
expect(tag2.reload.topic_count).to eq(0) tag1.reload
expect(tag1.public_topic_count).to eq(1)
expect(tag1.staff_topic_count).to eq(1)
tag2.reload
expect(tag2.public_topic_count).to eq(0)
expect(tag2.staff_topic_count).to eq(0)
end end
end end
end end

View File

@ -750,8 +750,8 @@ RSpec.describe TopicView do
end end
describe "page_title" do describe "page_title" do
fab!(:tag1) { Fabricate(:tag) } fab!(:tag1) { Fabricate(:tag, staff_topic_count: 0, public_topic_count: 0) }
fab!(:tag2) { Fabricate(:tag, topic_count: 2) } fab!(:tag2) { Fabricate(:tag, staff_topic_count: 2, public_topic_count: 2) }
fab!(:op_post) { Fabricate(:post, topic: topic) } fab!(:op_post) { Fabricate(:post, topic: topic) }
fab!(:post1) { Fabricate(:post, topic: topic) } fab!(:post1) { Fabricate(:post, topic: topic) }
fab!(:whisper) { Fabricate(:post, topic: topic, post_type: Post.types[:whisper]) } fab!(:whisper) { Fabricate(:post, topic: topic, post_type: Post.types[:whisper]) }

View File

@ -202,29 +202,101 @@ RSpec.describe Tag do
end end
end end
describe "topic counts" do describe ".ensure_consistency!" do
it "should exclude private message topics" do it "should exclude private message topics" do
topic topic
Fabricate(:private_message_topic, tags: [tag]) Fabricate(:private_message_topic, tags: [tag])
Tag.ensure_consistency! Tag.ensure_consistency!
tag.reload tag.reload
expect(tag.topic_count).to eq(1) expect(tag.staff_topic_count).to eq(1)
expect(tag.public_topic_count).to eq(1)
end
it "should update Tag#topic_count and Tag#public_topic_count correctly" do
tag = Fabricate(:tag, name: "tag1")
tag2 = Fabricate(:tag, name: "tag2")
tag3 = Fabricate(:tag, name: "tag3")
group = Fabricate(:group)
category = Fabricate(:category)
private_category = Fabricate(:private_category, group: group)
private_category2 = Fabricate(:private_category, group: group)
_topic_with_tag = Fabricate(:topic, category: category, tags: [tag])
_topic_with_tag_in_private_category =
Fabricate(:topic, category: private_category, tags: [tag])
_topic_with_tag2_in_private_category2 =
Fabricate(:topic, category: private_category2, tags: [tag2])
tag.update!(staff_topic_count: 123, public_topic_count: 456)
tag2.update!(staff_topic_count: 123, public_topic_count: 456)
tag3.update!(staff_topic_count: 123, public_topic_count: 456)
Tag.ensure_consistency!
tag.reload
tag2.reload
tag3.reload
expect(tag.staff_topic_count).to eq(2)
expect(tag.public_topic_count).to eq(1)
expect(tag2.staff_topic_count).to eq(1)
expect(tag2.public_topic_count).to eq(0)
expect(tag3.staff_topic_count).to eq(0)
expect(tag3.public_topic_count).to eq(0)
end end
end end
describe "unused tags scope" do describe "unused tags scope" do
let!(:tags) do let!(:tags) do
[ [
Fabricate(:tag, name: "used_publically", topic_count: 2, pm_topic_count: 0), Fabricate(
Fabricate(:tag, name: "used_privately", topic_count: 0, pm_topic_count: 3), :tag,
Fabricate(:tag, name: "used_everywhere", topic_count: 0, pm_topic_count: 3), name: "used_publically",
Fabricate(:tag, name: "unused1", topic_count: 0, pm_topic_count: 0), staff_topic_count: 2,
Fabricate(:tag, name: "unused2", topic_count: 0, pm_topic_count: 0), public_topic_count: 2,
pm_topic_count: 0,
),
Fabricate(
:tag,
name: "used_privately",
staff_topic_count: 0,
public_topic_count: 0,
pm_topic_count: 3,
),
Fabricate(
:tag,
name: "used_everywhere",
staff_topic_count: 0,
public_topic_count: 0,
pm_topic_count: 3,
),
Fabricate(
:tag,
name: "unused1",
staff_topic_count: 0,
public_topic_count: 0,
pm_topic_count: 0,
),
Fabricate(
:tag,
name: "unused2",
staff_topic_count: 0,
public_topic_count: 0,
pm_topic_count: 0,
),
] ]
end end
let(:tag_in_group) do let(:tag_in_group) do
Fabricate(:tag, name: "unused_in_group", topic_count: 0, pm_topic_count: 0) Fabricate(
:tag,
name: "unused_in_group",
public_topic_count: 0,
staff_topic_count: 0,
pm_topic_count: 0,
)
end end
let!(:tag_group) { Fabricate(:tag_group, tag_names: [tag_in_group.name]) } let!(:tag_group) { Fabricate(:tag_group, tag_names: [tag_in_group.name]) }
@ -292,4 +364,22 @@ RSpec.describe Tag do
expect(category.reload.tags).to include(tag2) expect(category.reload.tags).to include(tag2)
end end
end end
describe ".topic_count_column" do
fab!(:admin) { Fabricate(:admin) }
it "returns 'staff_topic_count' when user is staff" do
expect(Tag.topic_count_column(Guardian.new(admin))).to eq("staff_topic_count")
end
it "returns 'public_topic_count' when user is not staff" do
expect(Tag.topic_count_column(Guardian.new(user))).to eq("public_topic_count")
end
it "returns 'staff_topic_count' when user is not staff but `include_secure_categories_in_tag_counts` site setting is enabled" do
SiteSetting.include_secure_categories_in_tag_counts = true
expect(Tag.topic_count_column(Guardian.new(user))).to eq("staff_topic_count")
end
end
end end

View File

@ -1,33 +1,56 @@
# frozen_string_literal: true # frozen_string_literal: true
RSpec.describe TopicTag do RSpec.describe TopicTag do
fab!(:group) { Fabricate(:group) }
fab!(:private_category) { Fabricate(:private_category, group: group) }
fab!(:topic) { Fabricate(:topic) } fab!(:topic) { Fabricate(:topic) }
fab!(:topic_in_private_category) { Fabricate(:topic, category: private_category) }
fab!(:tag) { Fabricate(:tag) } fab!(:tag) { Fabricate(:tag) }
let(:topic_tag) { Fabricate(:topic_tag, topic: topic, tag: tag) } let(:topic_tag) { Fabricate(:topic_tag, topic: topic, tag: tag) }
describe "#after_create" do describe "#after_create" do
it "tag topic_count should be increased" do it "should increase Tag#staff_topic_count and Tag#public_topic_count for a regular topic in a public category" do
expect { topic_tag }.to change(tag, :topic_count).by(1) expect { topic_tag }.to change { tag.reload.staff_topic_count }.by(1).and change {
tag.reload.public_topic_count
}.by(1)
end end
it "tag topic_count should not be increased" do it "should only increase Tag#staff_topic_count for a regular topic in a read restricted category" do
expect { Fabricate(:topic_tag, topic: topic_in_private_category, tag: tag) }.to change {
tag.reload.staff_topic_count
}.by(1)
expect(tag.reload.public_topic_count).to eq(0)
end
it "should increase Tag#pm_topic_count for a private message topic" do
topic.archetype = Archetype.private_message topic.archetype = Archetype.private_message
expect { topic_tag }.not_to change(tag, :topic_count) expect { topic_tag }.to change { tag.reload.pm_topic_count }.by(1)
end end
end end
describe "#after_destroy" do describe "#after_destroy" do
it "tag topic_count should be decreased" do it "should decrease Tag#staff_topic_count and Tag#public_topic_count for a regular topic in a public category" do
topic_tag topic_tag
expect { topic_tag.destroy }.to change(tag, :topic_count).by(-1)
expect { topic_tag.destroy! }.to change { tag.reload.staff_topic_count }.by(-1).and change {
tag.reload.public_topic_count
}.by(-1)
end end
it "tag topic_count should not be decreased" do it "should only decrease Topic#topic_count for a regular topic in a read restricted category" do
topic_tag = Fabricate(:topic_tag, topic: topic_in_private_category, tag: tag)
expect { topic_tag.destroy! }.to change { tag.reload.staff_topic_count }.by(-1)
expect(tag.reload.public_topic_count).to eq(0)
end
it "should decrease Tag#pm_topic_count for a private message topic" do
topic.archetype = Archetype.private_message topic.archetype = Archetype.private_message
topic_tag topic_tag
expect { topic_tag.destroy }.not_to change(tag, :topic_count) expect { topic_tag.destroy! }.to change { tag.reload.pm_topic_count }.by(-1)
end end
end end
end end

View File

@ -12,18 +12,43 @@ RSpec.describe TagsController do
describe "#index" do describe "#index" do
fab!(:test_tag) { Fabricate(:tag, name: "test") } fab!(:test_tag) { Fabricate(:tag, name: "test") }
fab!(:topic_tag) { Fabricate(:tag, name: "topic-test", topic_count: 1) } fab!(:topic_tag) do
Fabricate(:tag, name: "topic-test", public_topic_count: 1, staff_topic_count: 1)
end
fab!(:synonym) { Fabricate(:tag, name: "synonym", target_tag: topic_tag) } fab!(:synonym) { Fabricate(:tag, name: "synonym", target_tag: topic_tag) }
shared_examples "successfully retrieve tags with topic_count > 0" do shared_examples "retrieves the right tags" do
it "should return the right response" do it "retrieves all tags as a staff user" do
sign_in(admin)
get "/tags.json"
expect(response.status).to eq(200)
tags = response.parsed_body["tags"]
expect(tags[0]["name"]).to eq(test_tag.name)
expect(tags[0]["count"]).to eq(0)
expect(tags[0]["pm_count"]).to eq(0)
expect(tags[1]["name"]).to eq(topic_tag.name)
expect(tags[1]["count"]).to eq(1)
expect(tags[1]["pm_count"]).to eq(0)
end
it "only retrieve tags that have been used in public topics for non-staff user" do
sign_in(user)
get "/tags.json" get "/tags.json"
expect(response.status).to eq(200) expect(response.status).to eq(200)
tags = response.parsed_body["tags"] tags = response.parsed_body["tags"]
expect(tags.length).to eq(1) expect(tags.length).to eq(1)
expect(tags[0]["text"]).to eq("topic-test")
expect(tags[0]["name"]).to eq(topic_tag.name)
expect(tags[0]["count"]).to eq(1)
expect(tags[0]["pm_count"]).to eq(0)
end end
end end
@ -76,7 +101,7 @@ RSpec.describe TagsController do
context "with tags_listed_by_group enabled" do context "with tags_listed_by_group enabled" do
before { SiteSetting.tags_listed_by_group = true } before { SiteSetting.tags_listed_by_group = true }
include_examples "successfully retrieve tags with topic_count > 0" include_examples "retrieves the right tags"
it "works for tags in groups" do it "works for tags in groups" do
tag_group = Fabricate(:tag_group, tags: [test_tag, topic_tag, synonym]) tag_group = Fabricate(:tag_group, tags: [test_tag, topic_tag, synonym])
@ -94,20 +119,7 @@ RSpec.describe TagsController do
context "with tags_listed_by_group disabled" do context "with tags_listed_by_group disabled" do
before { SiteSetting.tags_listed_by_group = false } before { SiteSetting.tags_listed_by_group = false }
include_examples "successfully retrieve tags with topic_count > 0" include_examples "retrieves the right tags"
end
context "when user can admin tags" do
it "successfully retrieve all tags" do
sign_in(admin)
get "/tags.json"
expect(response.status).to eq(200)
tags = response.parsed_body["tags"]
expect(tags.length).to eq(2)
end
end end
context "with hidden tags" do context "with hidden tags" do
@ -721,10 +733,10 @@ RSpec.describe TagsController do
expect(response.parsed_body["results"].map { |j| j["id"] }.sort).to eq(%w[stuff stumped]) expect(response.parsed_body["results"].map { |j| j["id"] }.sort).to eq(%w[stuff stumped])
end end
it "returns tags ordered by topic_count, and prioritises exact matches" do it "returns tags ordered by public_topic_count, and prioritises exact matches" do
Fabricate(:tag, name: "tag1", topic_count: 10) Fabricate(:tag, name: "tag1", public_topic_count: 10, staff_topic_count: 10)
Fabricate(:tag, name: "tag2", topic_count: 100) Fabricate(:tag, name: "tag2", public_topic_count: 100, staff_topic_count: 100)
Fabricate(:tag, name: "tag", topic_count: 1) Fabricate(:tag, name: "tag", public_topic_count: 1, staff_topic_count: 1)
get "/tags/filter/search.json", params: { q: "tag", limit: 2 } get "/tags/filter/search.json", params: { q: "tag", limit: 2 }
expect(response.status).to eq(200) expect(response.status).to eq(200)
@ -934,11 +946,41 @@ RSpec.describe TagsController do
context "with some tags" do context "with some tags" do
let!(:tags) do let!(:tags) do
[ [
Fabricate(:tag, name: "used_publically", topic_count: 2, pm_topic_count: 0), Fabricate(
Fabricate(:tag, name: "used_privately", topic_count: 0, pm_topic_count: 3), :tag,
Fabricate(:tag, name: "used_everywhere", topic_count: 0, pm_topic_count: 3), name: "used_publically",
Fabricate(:tag, name: "unused1", topic_count: 0, pm_topic_count: 0), public_topic_count: 2,
Fabricate(:tag, name: "unused2", topic_count: 0, pm_topic_count: 0), staff_topic_count: 2,
pm_topic_count: 0,
),
Fabricate(
:tag,
name: "used_privately",
public_topic_count: 0,
staff_topic_count: 0,
pm_topic_count: 3,
),
Fabricate(
:tag,
name: "used_everywhere",
public_topic_count: 0,
staff_topic_count: 0,
pm_topic_count: 3,
),
Fabricate(
:tag,
name: "unused1",
public_topic_count: 0,
staff_topic_count: 0,
pm_topic_count: 0,
),
Fabricate(
:tag,
name: "unused2",
public_topic_count: 0,
staff_topic_count: 0,
pm_topic_count: 0,
),
] ]
end end

View File

@ -0,0 +1,25 @@
# frozen_string_literal: true
RSpec.describe TagSerializer do
fab!(:user) { Fabricate(:user) }
fab!(:admin) { Fabricate(:admin) }
fab!(:tag) { Fabricate(:tag) }
fab!(:group) { Fabricate(:group) }
fab!(:private_category) { Fabricate(:private_category, group: group) }
fab!(:topic_in_public_category) { Fabricate(:topic, tags: [tag]) }
fab!(:topic_in_private_category) { Fabricate(:topic, category: private_category, tags: [tag]) }
describe "#topic_count" do
it "should return the value of `Tag#public_topic_count` for a non-staff user" do
serialized = described_class.new(tag, scope: Guardian.new(user), root: false).as_json
expect(serialized[:topic_count]).to eq(1)
end
it "should return the vavlue of `Tag#topic_count` for a staff user" do
serialized = described_class.new(tag, scope: Guardian.new(admin), root: false).as_json
expect(serialized[:topic_count]).to eq(2)
end
end
end

View File

@ -279,9 +279,33 @@ RSpec.describe TopicViewSerializer do
end end
describe "tags order" do describe "tags order" do
fab!(:tag1) { Fabricate(:tag, name: "ctag", description: "c description", topic_count: 5) } fab!(:tag1) do
fab!(:tag2) { Fabricate(:tag, name: "btag", description: "b description", topic_count: 9) } Fabricate(
fab!(:tag3) { Fabricate(:tag, name: "atag", description: "a description", topic_count: 3) } :tag,
name: "ctag",
description: "c description",
staff_topic_count: 5,
public_topic_count: 5,
)
end
fab!(:tag2) do
Fabricate(
:tag,
name: "btag",
description: "b description",
staff_topic_count: 9,
public_topic_count: 9,
)
end
fab!(:tag3) do
Fabricate(
:tag,
name: "atag",
description: "a description",
staff_topic_count: 3,
public_topic_count: 3,
)
end
before do before do
topic.tags << tag1 topic.tags << tag1

View File

@ -3,7 +3,9 @@
RSpec.describe HashtagAutocompleteService do RSpec.describe HashtagAutocompleteService do
fab!(:user) { Fabricate(:user) } fab!(:user) { Fabricate(:user) }
fab!(:category1) { Fabricate(:category, name: "The Book Club", slug: "the-book-club") } fab!(:category1) { Fabricate(:category, name: "The Book Club", slug: "the-book-club") }
fab!(:tag1) { Fabricate(:tag, name: "great-books", topic_count: 22) } fab!(:tag1) do
Fabricate(:tag, name: "great-books", staff_topic_count: 22, public_topic_count: 22)
end
fab!(:topic1) { Fabricate(:topic) } fab!(:topic1) { Fabricate(:topic) }
let(:guardian) { Guardian.new(user) } let(:guardian) { Guardian.new(user) }
@ -68,7 +70,7 @@ RSpec.describe HashtagAutocompleteService do
end end
it "includes the tag count" do it "includes the tag count" do
tag1.update!(topic_count: 78) tag1.update!(staff_topic_count: 78, public_topic_count: 78)
expect(subject.search("book", %w[tag category]).map(&:text)).to eq( expect(subject.search("book", %w[tag category]).map(&:text)).to eq(
["great-books", "The Book Club"], ["great-books", "The Book Club"],
) )
@ -149,8 +151,8 @@ RSpec.describe HashtagAutocompleteService do
category6 = Fabricate(:category, name: "Book Reviews", slug: "book-reviews") category6 = Fabricate(:category, name: "Book Reviews", slug: "book-reviews")
Fabricate(:category, name: "Good Books", slug: "book", parent_category: category6) Fabricate(:category, name: "Good Books", slug: "book", parent_category: category6)
Fabricate(:tag, name: "bookmania", topic_count: 15) Fabricate(:tag, name: "bookmania", staff_topic_count: 15, public_topic_count: 15)
Fabricate(:tag, name: "awful-books", topic_count: 56) Fabricate(:tag, name: "awful-books", staff_topic_count: 56, public_topic_count: 56)
expect(subject.search("book", %w[category tag]).map(&:ref)).to eq( expect(subject.search("book", %w[category tag]).map(&:ref)).to eq(
[ [
@ -220,9 +222,13 @@ RSpec.describe HashtagAutocompleteService do
end end
fab!(:category4) { Fabricate(:category, name: "Bookworld", slug: "book", topic_count: 56) } fab!(:category4) { Fabricate(:category, name: "Bookworld", slug: "book", topic_count: 56) }
fab!(:category5) { Fabricate(:category, name: "Media", slug: "media", topic_count: 446) } fab!(:category5) { Fabricate(:category, name: "Media", slug: "media", topic_count: 446) }
fab!(:tag2) { Fabricate(:tag, name: "mid-books", topic_count: 33) } fab!(:tag2) do
fab!(:tag3) { Fabricate(:tag, name: "terrible-books", topic_count: 2) } Fabricate(:tag, name: "mid-books", staff_topic_count: 33, public_topic_count: 33)
fab!(:tag4) { Fabricate(:tag, name: "book", topic_count: 1) } end
fab!(:tag3) do
Fabricate(:tag, name: "terrible-books", staff_topic_count: 2, public_topic_count: 2)
end
fab!(:tag4) { Fabricate(:tag, name: "book", staff_topic_count: 1, public_topic_count: 1) }
it "returns the 'most polular' categories and tags (based on topic_count) that the user can access" do it "returns the 'most polular' categories and tags (based on topic_count) that the user can access" do
category1.update!(read_restricted: true) category1.update!(read_restricted: true)

View File

@ -1,16 +1,16 @@
# frozen_string_literal: true # frozen_string_literal: true
RSpec.describe TagHashtagDataSource do RSpec.describe TagHashtagDataSource do
fab!(:tag1) { Fabricate(:tag, name: "fact", topic_count: 0) } fab!(:tag1) { Fabricate(:tag, name: "fact", public_topic_count: 0) }
fab!(:tag2) { Fabricate(:tag, name: "factor", topic_count: 5) } fab!(:tag2) { Fabricate(:tag, name: "factor", public_topic_count: 5) }
fab!(:tag3) { Fabricate(:tag, name: "factory", topic_count: 4) } fab!(:tag3) { Fabricate(:tag, name: "factory", public_topic_count: 4) }
fab!(:tag4) { Fabricate(:tag, name: "factorio", topic_count: 3) } fab!(:tag4) { Fabricate(:tag, name: "factorio", public_topic_count: 3) }
fab!(:tag5) { Fabricate(:tag, name: "factz", topic_count: 1) } fab!(:tag5) { Fabricate(:tag, name: "factz", public_topic_count: 1) }
fab!(:user) { Fabricate(:user) } fab!(:user) { Fabricate(:user) }
let(:guardian) { Guardian.new(user) } let(:guardian) { Guardian.new(user) }
describe "#search" do describe "#search" do
it "orders tag results by exact search match, then topic count, then name" do it "orders tag results by exact search match, then public topic count, then name" do
expect(described_class.search(guardian, "fact", 5).map(&:slug)).to eq( expect(described_class.search(guardian, "fact", 5).map(&:slug)).to eq(
%w[fact factor factory factorio factz], %w[fact factor factory factorio factz],
) )
@ -31,7 +31,7 @@ RSpec.describe TagHashtagDataSource do
) )
end end
it "includes the topic count for the text of the tag in secondary text" do it "includes the public topic count for the text of the tag in secondary text" do
expect(described_class.search(guardian, "fact", 5).map(&:secondary_text)).to eq( expect(described_class.search(guardian, "fact", 5).map(&:secondary_text)).to eq(
%w[x0 x5 x4 x3 x1], %w[x0 x5 x4 x3 x1],
) )
@ -52,7 +52,7 @@ RSpec.describe TagHashtagDataSource do
end end
describe "#search_without_term" do describe "#search_without_term" do
it "returns distinct tags sorted by topic_count" do it "returns distinct tags sorted by public topic count" do
expect(described_class.search_without_term(guardian, 5).map(&:slug)).to eq( expect(described_class.search_without_term(guardian, 5).map(&:slug)).to eq(
%w[factor factory factorio factz fact], %w[factor factory factorio factz fact],
) )

View File

@ -65,7 +65,9 @@ RSpec.shared_examples "User Sidebar Serializer Attributes" do |serializer_klass|
describe "#sidebar_tags" do describe "#sidebar_tags" do
fab!(:tag) { Fabricate(:tag, name: "foo") } fab!(:tag) { Fabricate(:tag, name: "foo") }
fab!(:pm_tag) { Fabricate(:tag, name: "bar", pm_topic_count: 5, topic_count: 0) } fab!(:pm_tag) do
Fabricate(:tag, name: "bar", pm_topic_count: 5, staff_topic_count: 0, public_topic_count: 0)
end
fab!(:hidden_tag) { Fabricate(:tag, name: "secret") } fab!(:hidden_tag) { Fabricate(:tag, name: "secret") }
fab!(:staff_tag_group) do fab!(:staff_tag_group) do
Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: ["secret"]) Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: ["secret"])

View File

@ -10,8 +10,8 @@ describe "Using #hashtag autocompletion to search for and lookup categories and
fab!(:category2) do fab!(:category2) do
Fabricate(:category, name: "Other Category", slug: "other-cat", topic_count: 23) Fabricate(:category, name: "Other Category", slug: "other-cat", topic_count: 23)
end end
fab!(:tag) { Fabricate(:tag, name: "cooltag", topic_count: 324) } fab!(:tag) { Fabricate(:tag, name: "cooltag", staff_topic_count: 324, public_topic_count: 324) }
fab!(:tag2) { Fabricate(:tag, name: "othertag", topic_count: 66) } fab!(:tag2) { Fabricate(:tag, name: "othertag", staff_topic_count: 66, public_topic_count: 66) }
fab!(:topic) { Fabricate(:topic, category: category, tags: [tag]) } fab!(:topic) { Fabricate(:topic, category: category, tags: [tag]) }
fab!(:post) { Fabricate(:post, topic: topic) } fab!(:post) { Fabricate(:post, topic: topic) }
let(:uncategorized_category) { Category.find(SiteSetting.uncategorized_category_id) } let(:uncategorized_category) { Category.find(SiteSetting.uncategorized_category_id) }