mirror of
https://github.com/discourse/discourse.git
synced 2026-08-01 09:08:08 -05:00
FIX: Resolve tag synonyms in search and filter queries (#37628)
What is the problem?
Discourse allows admins to mark a tag as a synonym of another tag. For
example, "brunch" can be made a synonym of "lunch". When this happens,
all topics tagged with "brunch" are automatically retagged with "lunch",
and the `tags.target_tag_id` column on the synonym tag record is set to
point to the target tag.
However, several code paths did not account for synonyms:
1. **Search:** `Search#search_tags` and the hashtag advanced filter did
not resolve synonyms. When a user searched using a synonym name (e.g.
`tags:brunch`, `tags:brunch+eggs`, or `#brunch`), no results were
returned because:
- The `tags:` comma path queries `topic_tags` joined with `tags` by
name, but topics are tagged with the target tag "lunch", not the
synonym "brunch".
- The `tags:` plus path aggregates tag names per topic into a
tsvector and matches against the searched name, but the aggregated
names are target tag names, so "brunch" never matches.
- The `#` hashtag path picks the synonym tag's own `id` and queries
`topic_tags` by that ID, but topics store the target tag's ID.
2. **Filter route:** `TopicsFilter#tag_ids_from_tag_names` concatenated
both the synonym's own ID and the target tag ID. For match-all
queries (e.g. `tag:brunch`), this required a topic to have both IDs
in `topic_tags`, which never happens since only the target tag ID is
stored. For negation queries (e.g. `-tag:brunch`), the exclusion
targeted the synonym ID rather than the target, so no topics were
excluded.
What is the solution?
In `Search#search_tags`, add a synonym resolution step before the
existing comma/plus branching logic. It splits the match string into
individual tag names, queries for any that are synonyms via
`Tag.where_name(tag_names).where.not(target_tag_id: nil)`, builds a
name mapping, and replaces synonym names with their target tag names.
The replacement operates on the split array elements rather than using
substring replacement to avoid corrupting tag names that may contain
other tag names as substrings.
In the hashtag advanced filter, pick both `:id` and `:target_tag_id`
from the tag lookup and prefer `target_tag_id` when present, so the
query uses the target tag's ID instead of the synonym's own ID.
In `TopicsFilter#tag_ids_from_tag_names`, replace the transpose/concat
approach with `.map { |id, target_id| target_id || id }` to resolve
each tag to its canonical ID — the target for synonyms, or the tag's
own ID otherwise.
A partial index on `tags.target_tag_id` is added (scoped to
`WHERE target_tag_id IS NOT NULL`) to support efficient synonym lookups.
This commit is contained in:
+4
-3
@@ -312,7 +312,8 @@ end
|
||||
#
|
||||
# Indexes
|
||||
#
|
||||
# index_tags_on_lower_name (lower((name)::text)) UNIQUE
|
||||
# index_tags_on_name (name) UNIQUE
|
||||
# index_tags_on_slug (slug) WHERE ((slug)::text <> ''::text)
|
||||
# index_tags_on_lower_name (lower((name)::text)) UNIQUE
|
||||
# index_tags_on_name (name) UNIQUE
|
||||
# index_tags_on_slug (slug) WHERE ((slug)::text <> ''::text)
|
||||
# index_tags_on_target_tag_id (target_tag_id) WHERE (target_tag_id IS NOT NULL)
|
||||
#
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
# frozen_string_literal: true
|
||||
class AddIndexToTagsTargetTagId < ActiveRecord::Migration[8.0]
|
||||
def change
|
||||
add_index :tags, :target_tag_id, where: "target_tag_id IS NOT NULL"
|
||||
end
|
||||
end
|
||||
+17
-3
@@ -666,7 +666,8 @@ class Search
|
||||
posts.where("topics.category_id IN (?)", category_ids)
|
||||
else
|
||||
# try a possible tag match
|
||||
tag_id = Tag.where_name(category_slug).pick(:id)
|
||||
tag_id, target_tag_id = Tag.where_name(category_slug).pick(:id, :target_tag_id)
|
||||
tag_id = target_tag_id || tag_id
|
||||
if (tag_id)
|
||||
posts.where(<<~SQL, tag_id)
|
||||
topics.id IN (
|
||||
@@ -928,7 +929,7 @@ class Search
|
||||
modifier = positive ? "" : "NOT"
|
||||
|
||||
if match.include?("+")
|
||||
tags = match.split("+")
|
||||
tags = resolve_tag_synonyms(match.split("+"))
|
||||
|
||||
posts.where(
|
||||
"topics.id #{modifier} IN (
|
||||
@@ -941,7 +942,7 @@ class Search
|
||||
Search.unaccent(tags.join("&")),
|
||||
)
|
||||
else
|
||||
tags = match.split(",")
|
||||
tags = resolve_tag_synonyms(match.split(","))
|
||||
|
||||
posts.where(
|
||||
"topics.id #{modifier} IN (
|
||||
@@ -954,6 +955,19 @@ class Search
|
||||
end
|
||||
end
|
||||
|
||||
def resolve_tag_synonyms(tag_names)
|
||||
synonym_map =
|
||||
Tag
|
||||
.where_name(tag_names)
|
||||
.where.not(target_tag_id: nil)
|
||||
.includes(:target_tag)
|
||||
.to_h { |synonym| [synonym.name.downcase, synonym.target_tag.name.downcase] }
|
||||
|
||||
return tag_names if synonym_map.empty?
|
||||
|
||||
tag_names.map { |name| synonym_map[name] || name }
|
||||
end
|
||||
|
||||
def process_advanced_search!(term)
|
||||
term
|
||||
.to_s
|
||||
|
||||
+8
-18
@@ -847,25 +847,15 @@ class TopicsFilter
|
||||
category_ids
|
||||
end
|
||||
|
||||
# Accepts an array of tag names and returns an array of tag ids and the tag ids of aliases for the tag names which the user can see.
|
||||
# If a block is given, it will be called with the tag ids and alias tag ids as arguments.
|
||||
# Accepts an array of tag names and returns an array of resolved tag IDs.
|
||||
# Synonym tags are resolved to their target tag ID.
|
||||
def tag_ids_from_tag_names(tag_names)
|
||||
tag_ids, alias_tag_ids =
|
||||
DiscourseTagging
|
||||
.filter_visible(Tag, @guardian)
|
||||
.where_name(tag_names)
|
||||
.pluck(:id, :target_tag_id)
|
||||
.transpose
|
||||
|
||||
tag_ids ||= []
|
||||
alias_tag_ids ||= []
|
||||
|
||||
yield(tag_ids, alias_tag_ids) if block_given?
|
||||
|
||||
all_tag_ids = tag_ids.concat(alias_tag_ids)
|
||||
all_tag_ids.compact!
|
||||
all_tag_ids.uniq!
|
||||
all_tag_ids
|
||||
DiscourseTagging
|
||||
.filter_visible(Tag, @guardian)
|
||||
.where_name(tag_names)
|
||||
.pluck(:id, :target_tag_id)
|
||||
.map { |id, target_id| target_id || id }
|
||||
.uniq
|
||||
end
|
||||
|
||||
def filter_tag_groups(values:)
|
||||
|
||||
@@ -2832,6 +2832,46 @@ RSpec.describe Search do
|
||||
[post7.id, post8.id],
|
||||
)
|
||||
end
|
||||
|
||||
context "with tag synonyms" do
|
||||
fab!(:synonym_tag) { Fabricate(:tag, name: "brunch", target_tag: tag1) }
|
||||
fab!(:topic_with_synonym_target_tag) do
|
||||
Fabricate(:topic, tags: [synonym_tag.target_tag, tag2])
|
||||
end
|
||||
fab!(:post_in_topic_with_synonym_target_tag) do
|
||||
indexed_post(topic: topic_with_synonym_target_tag)
|
||||
end
|
||||
|
||||
it "can find posts by tag synonym using comma syntax" do
|
||||
results = Search.execute("tags:brunch")
|
||||
expect(results.posts.map(&:id)).to include(post_in_topic_with_synonym_target_tag.id)
|
||||
end
|
||||
|
||||
it "can find posts with mixed synonym and regular tag using comma syntax" do
|
||||
results = Search.execute("tags:brunch,sandwiches")
|
||||
expect(results.posts.map(&:id)).to include(post_in_topic_with_synonym_target_tag.id)
|
||||
end
|
||||
|
||||
it "can exclude posts by tag synonym using negation with comma syntax" do
|
||||
results = Search.execute("tags:eggs -tags:brunch")
|
||||
expect(results.posts.map(&:id)).not_to include(post_in_topic_with_synonym_target_tag.id)
|
||||
end
|
||||
|
||||
it "can find posts by tag synonym using plus syntax" do
|
||||
results = Search.execute("tags:brunch+eggs")
|
||||
expect(results.posts.map(&:id)).to include(post_in_topic_with_synonym_target_tag.id)
|
||||
end
|
||||
|
||||
it "can exclude posts by tag synonym using negation with plus syntax" do
|
||||
results = Search.execute("tags:eggs -tags:brunch+sandwiches")
|
||||
expect(results.posts.map(&:id)).not_to include(post4.id)
|
||||
end
|
||||
|
||||
it "can find posts by tag synonym using hashtag syntax" do
|
||||
results = Search.execute("#brunch")
|
||||
expect(results.posts.map(&:id)).to include(post_in_topic_with_synonym_target_tag.id)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
it "can find posts which contains filetypes" do
|
||||
|
||||
@@ -1203,6 +1203,40 @@ RSpec.describe TopicsFilter do
|
||||
fab!(:topic_with_tag_and_tag2) { Fabricate(:topic, tags: [tag, tag2]) }
|
||||
fab!(:topic_with_tag2) { Fabricate(:topic, tags: [tag2]) }
|
||||
fab!(:topic_with_group_only_tag) { Fabricate(:topic, tags: [group_only_tag]) }
|
||||
fab!(:tag_synonym) { Fabricate(:tag, name: "synonym1", target_tag_id: tag.id) }
|
||||
|
||||
describe "when filtering by a tag synonym" do
|
||||
it "should return topics tagged with the target tag when query string is `tag:synonym1`" do
|
||||
expect(
|
||||
TopicsFilter
|
||||
.new(guardian: Guardian.new)
|
||||
.filter_from_query_string("tag:synonym1")
|
||||
.pluck(:id),
|
||||
).to contain_exactly(topic_with_tag.id, topic_with_tag_and_tag2.id)
|
||||
end
|
||||
|
||||
it "should return topics tagged with the target tag or other tags when query string is `tags:synonym1,tag2`" do
|
||||
expect(
|
||||
TopicsFilter
|
||||
.new(guardian: Guardian.new)
|
||||
.filter_from_query_string("tags:synonym1,#{tag2.name}")
|
||||
.pluck(:id),
|
||||
).to contain_exactly(topic_with_tag.id, topic_with_tag_and_tag2.id, topic_with_tag2.id)
|
||||
end
|
||||
|
||||
it "should exclude topics tagged with the target tag when query string is `-tag:synonym1`" do
|
||||
expect(
|
||||
TopicsFilter
|
||||
.new(guardian: Guardian.new)
|
||||
.filter_from_query_string("-tag:synonym1")
|
||||
.pluck(:id),
|
||||
).to contain_exactly(
|
||||
topic_without_tag.id,
|
||||
topic_with_tag2.id,
|
||||
topic_with_group_only_tag.id,
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
it "should not filter any topics by tags when tagging is disabled" do
|
||||
SiteSetting.tagging_enabled = false
|
||||
|
||||
Reference in New Issue
Block a user