diff --git a/app/services/hashtag_autocomplete_service.rb b/app/services/hashtag_autocomplete_service.rb index b13940382b5..6e92b2ee272 100644 --- a/app/services/hashtag_autocomplete_service.rb +++ b/app/services/hashtag_autocomplete_service.rb @@ -273,7 +273,7 @@ class HashtagAutocompleteService # Any items that are _not_ the top-ranked type (which could possibly not be # the same as the first item in the types_in_priority_order if there was # no data for that type) that have conflicting slugs with other items for - # other types need to have a ::type suffix added to their ref. + # other higher-ranked types need to have a ::type suffix added to their ref. # # This will be used for the lookup method above if one of these items is # chosen in the UI, otherwise there is no way to determine whether a hashtag is @@ -281,7 +281,7 @@ class HashtagAutocompleteService # # For example, if there is a category with the slug #general and a tag # with the slug #general, then the tag will have its ref changed to #general::tag - append_types_to_conflicts(limited_results, top_ranked_type, limit) + append_types_to_conflicts(limited_results, top_ranked_type, types_in_priority_order, limit) end # TODO (martin) Remove this once plugins are not relying on the old lookup @@ -425,13 +425,22 @@ class HashtagAutocompleteService ) end - def append_types_to_conflicts(limited_results, top_ranked_type, limit) + def append_types_to_conflicts(limited_results, top_ranked_type, types_in_priority_order, limit) limited_results.each do |hashtag_item| next if hashtag_item.type == top_ranked_type - other_slugs = limited_results.reject { |r| r.type === hashtag_item.type }.map(&:slug) - if other_slugs.include?(hashtag_item.slug) - hashtag_item.ref = "#{hashtag_item.slug}::#{hashtag_item.type}" + # We only need to change the ref to include the type if there is a + # higher-ranked hashtag slug that conflicts with this one. + higher_ranked_types = + types_in_priority_order.slice(0, types_in_priority_order.index(hashtag_item.type)) + higher_ranked_slugs = + limited_results + .reject { |r| r.type === hashtag_item.type } + .select { |r| higher_ranked_types.include?(r.type) } + .map(&:slug) + + if higher_ranked_slugs.include?(hashtag_item.slug) + hashtag_item.ref = "#{hashtag_item.ref}::#{hashtag_item.type}" end end diff --git a/spec/services/hashtag_autocomplete_service_spec.rb b/spec/services/hashtag_autocomplete_service_spec.rb index eeb4dd74b8c..9c2382816ba 100644 --- a/spec/services/hashtag_autocomplete_service_spec.rb +++ b/spec/services/hashtag_autocomplete_service_spec.rb @@ -140,6 +140,44 @@ RSpec.describe HashtagAutocompleteService do ) end + it "does not add a type suffix where + 1. a subcategory name conflicts with an existing tag name and + 2. the category is not the top ranked type" do + parent = Fabricate(:category, name: "Hobbies", slug: "hobbies") + category1.update!(parent_category: parent) + Fabricate(:tag, name: "the-book-club") + + Fabricate(:bookmark, user: user, name: "book club") + guardian.user.reload + + DiscoursePluginRegistry.register_hashtag_autocomplete_data_source( + FakeBookmarkHashtagDataSource, + stub(enabled?: true), + ) + + expect(subject.search("book", %w[bookmark category tag]).map(&:ref)).to eq( + %w[book-club hobbies:the-book-club great-books the-book-club::tag], + ) + end + + it "handles the type suffix where the top ranked type conflicts with a subcategory" do + parent = Fabricate(:category, name: "Hobbies", slug: "hobbies") + category1.update!(parent_category: parent) + Fabricate(:tag, name: "the-book-club") + + Fabricate(:bookmark, user: user, name: "the book club") + guardian.user.reload + + DiscoursePluginRegistry.register_hashtag_autocomplete_data_source( + FakeBookmarkHashtagDataSource, + stub(enabled?: true), + ) + + expect(subject.search("book", %w[bookmark category tag]).map(&:ref)).to eq( + %w[the-book-club hobbies:the-book-club::category great-books the-book-club::tag], + ) + end + it "orders results by (with type ordering within each section): 1. exact match on slug (ignoring parent/child distinction for categories) 2. slugs that start with the term @@ -342,6 +380,18 @@ RSpec.describe HashtagAutocompleteService do category1.update!(parent_category: nil) end + it "handles parent:child category lookups with type suffix" do + parent_category = Fabricate(:category, name: "Media", slug: "media") + category1.update!(parent_category: parent_category) + result = subject.lookup(%w[media:the-book-club::category], %w[category tag]) + expect(result[:category].map(&:slug)).to eq(["the-book-club"]) + expect(result[:category].map(&:ref)).to eq(["media:the-book-club::category"]) + expect(result[:category].map(&:relative_url)).to eq( + ["/c/media/the-book-club/#{category1.id}"], + ) + category1.update!(parent_category: nil) + end + it "does not return the category if the parent does not match the child" do parent_category = Fabricate(:category, name: "Media", slug: "media") category1.update!(parent_category: parent_category)