From cad2fe60892c86416d33f8ac3e1b1aa4e3c9964c Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Fri, 25 Nov 2022 15:28:49 +1000 Subject: [PATCH] FIX: Push category hashtag slug match to top (#19174) When searching for categories it is possible for a child category to have a slug that matches the term exactly, but will not be found by .lookup since we don't return these categories unless the ref matches parent:child. Introduces a search_sort method to each hashtag data source so they can provide their custom sort logic of results, in category's case putting all matching slugs to the top regardless of parent/child relationship then sorting by text. --- app/services/category_hashtag_data_source.rb | 12 +++++-- app/services/hashtag_autocomplete_service.rb | 9 ++--- app/services/tag_hashtag_data_source.rb | 4 +++ .../lib/chat_channel_hashtag_data_source.rb | 4 +++ .../hashtag_autocomplete_service_spec.rb | 35 +++++++++++++++++++ 5 files changed, 58 insertions(+), 6 deletions(-) diff --git a/app/services/category_hashtag_data_source.rb b/app/services/category_hashtag_data_source.rb index 8869d7703b1..f2e7964d735 100644 --- a/app/services/category_hashtag_data_source.rb +++ b/app/services/category_hashtag_data_source.rb @@ -9,8 +9,7 @@ class CategoryHashtagDataSource end def self.category_to_hashtag_item(guardian_categories, category) - category = - Category.new(category.slice(:id, :slug, :name, :parent_category_id, :description)) + category = Category.new(category.slice(:id, :slug, :name, :parent_category_id, :description)) HashtagAutocompleteService::HashtagItem.new.tap do |item| item.text = category.name @@ -51,4 +50,13 @@ class CategoryHashtagDataSource .take(limit) .map { |category| category_to_hashtag_item(guardian_categories, category) } end + + def self.search_sort(search_results, term) + search_results + .select { |item| item.slug == term } + .sort_by { |item| item.text.downcase } + .concat( + search_results.select { |item| item.slug != term }.sort_by { |item| item.text.downcase }, + ) + end end diff --git a/app/services/hashtag_autocomplete_service.rb b/app/services/hashtag_autocomplete_service.rb index 085dd591ab0..77fd2c62be9 100644 --- a/app/services/hashtag_autocomplete_service.rb +++ b/app/services/hashtag_autocomplete_service.rb @@ -197,11 +197,12 @@ class HashtagAutocompleteService next if !all_data_items_valid?(search_results) search_results = - search_results - .reject do |item| + @@data_sources[type].search_sort( + search_results.reject do |item| limited_results.any? { |exact| exact.type == type && exact.slug === item.slug } - end - .sort_by { |item| item.text.downcase } + end, + term, + ) top_ranked_type = type if top_ranked_type.nil? limited_results.concat(search_results) diff --git a/app/services/tag_hashtag_data_source.rb b/app/services/tag_hashtag_data_source.rb index 8e6b838244c..b761949dab4 100644 --- a/app/services/tag_hashtag_data_source.rb +++ b/app/services/tag_hashtag_data_source.rb @@ -51,4 +51,8 @@ class TagHashtagDataSource .take(limit) .map { |tag| tag_to_hashtag_item(tag, include_count: true) } end + + def self.search_sort(search_results, _) + search_results.sort_by { |result| result.text.downcase } + end end diff --git a/plugins/chat/lib/chat_channel_hashtag_data_source.rb b/plugins/chat/lib/chat_channel_hashtag_data_source.rb index 06b9eba2a33..cd41895634d 100644 --- a/plugins/chat/lib/chat_channel_hashtag_data_source.rb +++ b/plugins/chat/lib/chat_channel_hashtag_data_source.rb @@ -40,4 +40,8 @@ class Chat::ChatChannelHashtagDataSource [] end end + + def self.search_sort(search_results, _) + search_results.sort_by { |result| result.text.downcase } + end end diff --git a/spec/services/hashtag_autocomplete_service_spec.rb b/spec/services/hashtag_autocomplete_service_spec.rb index 241c470ac6b..ef9b9d58692 100644 --- a/spec/services/hashtag_autocomplete_service_spec.rb +++ b/spec/services/hashtag_autocomplete_service_spec.rb @@ -43,6 +43,10 @@ RSpec.describe HashtagAutocompleteService do end end end + + def self.search_sort(search_results, _) + search_results.sort_by { |item| item.text.downcase } + end end describe ".contexts_with_ordered_types" do @@ -166,6 +170,37 @@ RSpec.describe HashtagAutocompleteService do ) end + it "orders categories by exact match on slug (ignoring parent/child distinction) then name, and then name for everything else" do + category2 = Fabricate(:category, name: "Book Library", slug: "book-library") + Fabricate(:category, name: "Horror", slug: "book", parent_category: category2) + Fabricate(:category, name: "Romance", slug: "romance-books") + Fabricate(:category, name: "Abstract Philosophy", slug: "abstract-philosophy-books") + category6 = Fabricate(:category, name: "Book Reviews", slug: "book-reviews") + Fabricate(:category, name: "Good Books", slug: "book", parent_category: category6) + expect(subject.search("book", %w[category]).map(&:ref)).to eq( + %w[ + book-reviews:book + book-library:book + abstract-philosophy-books + book-club + book-library + book-reviews + romance-books + ], + ) + expect(subject.search("book", %w[category]).map(&:text)).to eq( + [ + "Good Books", + "Horror", + "Abstract Philosophy", + "Book Club", + "Book Library", + "Book Reviews", + "Romance", + ], + ) + end + context "when multiple tags and categories are returned" do fab!(:category2) { Fabricate(:category, name: "Book Zone", slug: "book-zone") } fab!(:category3) { Fabricate(:category, name: "Book Dome", slug: "book-dome") }