From ebe4896e489179bd3bda1f4c9287396e8c44ddb6 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Wed, 23 Dec 2020 15:14:41 +0800 Subject: [PATCH] FEATURE: Change very high/low search priority to rank at absolute ends. Prior to this change, we had weights for very_high, high, low and very_low. This means there were 4 weights to tweak and what weights to use for `very_high/high` and `very_low/low` pair was hard to explain. This change makes it such that `very_high` search priority will always ensure that the posts are ranked at the top while `very_low` search priority will ensure that the posts are ranked at the very bottom. --- config/locales/server.en.yml | 8 +-- config/site_settings.yml | 8 --- ...ry_search_priorities_from_site_settings.rb | 13 ++++ lib/search.rb | 20 +++++-- ...egory_search_priority_weights_validator.rb | 8 +-- ..._search_priority_weights_validator_spec.rb | 12 +--- spec/requests/search_controller_spec.rb | 60 +++++++++++++++---- 7 files changed, 80 insertions(+), 49 deletions(-) create mode 100644 db/migrate/20201223071241_delete_stale_category_search_priorities_from_site_settings.rb diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index b2f4128f9c5..64df440feed 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1470,10 +1470,8 @@ en: search_query_log_max_size: "Maximum amount of search queries to keep" search_query_log_max_retention_days: "Maximum amount of time to keep search queries, in days." search_ignore_accents: "Ignore accents when searching for text." - category_search_priority_very_low_weight: "Weight applied to ranking for very low category search priority." category_search_priority_low_weight: "Weight applied to ranking for low category search priority." category_search_priority_high_weight: "Weight applied to ranking for high category search priority." - category_search_priority_very_high_weight: "Weight applied to ranking for very high category search priority." allow_uncategorized_topics: "Allow topics to be created without a category. WARNING: If there are any uncategorized topics, you must recategorize them before turning this off." allow_duplicate_topic_titles: "Allow topics with identical, duplicate titles." allow_duplicate_topic_titles_category: "Allow topics with identical, duplicate titles if the category is different. allow_duplicate_topic_titles must be false." @@ -2329,10 +2327,8 @@ en: invalid_hex_value: "Color values have to be 6-digit hexadecimal codes." empty_selectable_avatars: "You must first upload at least two selectable avatars before enabling this setting." category_search_priority: - very_low_weight_invalid: "You cannot set the weight to be greater than 'category_search_priority_low_weight'." - low_weight_invalid: "You cannot set the weight to be greater or equal to 1 or smaller than 'category_search_priority_very_low_weight'." - high_weight_invalid: "You cannot set the weight to be smaller or equal to 1 or greater than 'category_search_priority_very_high_weight'." - very_high_weight_invalid: "You cannot set the weight to be smaller than 'category_search_priority_high_weight'." + low_weight_invalid: "You cannot set the weight to be greater or equal to 1." + high_weight_invalid: "You cannot set the weight to be smaller or equal to 1." allowed_unicode_usernames: regex_invalid: "The regular expression is invalid: %{error}" leading_trailing_slash: "The regular expression must not start and end with a slash." diff --git a/config/site_settings.yml b/config/site_settings.yml index 5a6ef69af44..53c19ee4276 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1950,10 +1950,6 @@ search: ro: true sk: true tr_TR: true - category_search_priority_very_low_weight: - default: 0.6 - hidden: true - validator: "CategorySearchPriorityWeightsValidator" category_search_priority_low_weight: default: 0.8 hidden: true @@ -1962,10 +1958,6 @@ search: default: 1.2 hidden: true validator: "CategorySearchPriorityWeightsValidator" - category_search_priority_very_high_weight: - default: 1.4 - hidden: true - validator: "CategorySearchPriorityWeightsValidator" uncategorized: version_checks: diff --git a/db/migrate/20201223071241_delete_stale_category_search_priorities_from_site_settings.rb b/db/migrate/20201223071241_delete_stale_category_search_priorities_from_site_settings.rb new file mode 100644 index 00000000000..a6998b8f2f5 --- /dev/null +++ b/db/migrate/20201223071241_delete_stale_category_search_priorities_from_site_settings.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class DeleteStaleCategorySearchPrioritiesFromSiteSettings < ActiveRecord::Migration[6.0] + def up + execute <<~SQL + DELETE FROM site_settings WHERE name IN ('category_search_priority_very_low_weight', 'category_search_priority_very_high_weight') + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/search.rb b/lib/search.rb index 934660f06d0..2370877977b 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -1019,17 +1019,25 @@ class Search ) SQL + category_search_priority = <<~SQL + ( + CASE categories.search_priority + WHEN #{Searchable::PRIORITIES[:very_high]} + THEN 3 + WHEN #{Searchable::PRIORITIES[:very_low]} + THEN 1 + ELSE 2 + END + ) + SQL + category_priority_weights = <<~SQL ( CASE categories.search_priority - WHEN #{Searchable::PRIORITIES[:very_low]} - THEN #{SiteSetting.category_search_priority_very_low_weight} WHEN #{Searchable::PRIORITIES[:low]} THEN #{SiteSetting.category_search_priority_low_weight} WHEN #{Searchable::PRIORITIES[:high]} THEN #{SiteSetting.category_search_priority_high_weight} - WHEN #{Searchable::PRIORITIES[:very_high]} - THEN #{SiteSetting.category_search_priority_very_high_weight} ELSE CASE WHEN topics.closed THEN 0.9 @@ -1048,9 +1056,9 @@ class Search posts = if aggregate_search - posts.order("MAX(#{data_ranking}) DESC") + posts.order("MAX(#{category_search_priority}) DESC", "MAX(#{data_ranking}) DESC") else - posts.order("#{data_ranking} DESC") + posts.order("#{category_search_priority} DESC", "#{data_ranking} DESC") end posts = posts.order("topics.bumped_at DESC") diff --git a/lib/validators/category_search_priority_weights_validator.rb b/lib/validators/category_search_priority_weights_validator.rb index 565a0b327c1..ef4e337c6ae 100644 --- a/lib/validators/category_search_priority_weights_validator.rb +++ b/lib/validators/category_search_priority_weights_validator.rb @@ -9,14 +9,10 @@ class CategorySearchPriorityWeightsValidator val = val.to_f case @name - when "category_search_priority_very_low_weight" - val < SiteSetting.category_search_priority_low_weight when "category_search_priority_low_weight" - val < 1 && val > SiteSetting.category_search_priority_very_low_weight + val < 1 when "category_search_priority_high_weight" - val > 1 && val < SiteSetting.category_search_priority_very_high_weight - when "category_search_priority_very_high_weight" - val > SiteSetting.category_search_priority_high_weight + val > 1 end end diff --git a/spec/components/validators/category_search_priority_weights_validator_spec.rb b/spec/components/validators/category_search_priority_weights_validator_spec.rb index d0023703858..9db8f0b84e7 100644 --- a/spec/components/validators/category_search_priority_weights_validator_spec.rb +++ b/spec/components/validators/category_search_priority_weights_validator_spec.rb @@ -5,24 +5,16 @@ require 'validators/category_search_priority_weights_validator' RSpec.describe CategorySearchPriorityWeightsValidator do it "should validate the results correctly" do - expect do - SiteSetting.category_search_priority_very_low_weight = 0.9 - end.to raise_error(Discourse::InvalidParameters) - - [1, 0].each do |value| + [1, 1.1].each do |value| expect do SiteSetting.category_search_priority_low_weight = value end.to raise_error(Discourse::InvalidParameters) end - ['0.2', 10].each do |value| + [1, "0.9"].each do |value| expect do SiteSetting.category_search_priority_high_weight = value end.to raise_error(Discourse::InvalidParameters) end - - expect do - SiteSetting.category_search_priority_very_high_weight = 1.1 - end.to raise_error(Discourse::InvalidParameters) end end diff --git a/spec/requests/search_controller_spec.rb b/spec/requests/search_controller_spec.rb index 990bc15d97a..6af41183206 100644 --- a/spec/requests/search_controller_spec.rb +++ b/spec/requests/search_controller_spec.rb @@ -448,66 +448,100 @@ describe SearchController do end context "search priority" do - fab!(:low_priority_category) do + fab!(:very_low_priority_category) do Fabricate( :category, search_priority: Searchable::PRIORITIES[:very_low] ) end + + fab!(:low_priority_category) do + Fabricate( + :category, + search_priority: Searchable::PRIORITIES[:low] + ) + end + fab!(:high_priority_category) do Fabricate( :category, search_priority: Searchable::PRIORITIES[:high] ) end + fab!(:very_high_priority_category) do Fabricate( :category, search_priority: Searchable::PRIORITIES[:very_high] ) end + + fab!(:very_low_priority_topic) { Fabricate(:topic, category: very_low_priority_category) } fab!(:low_priority_topic) { Fabricate(:topic, category: low_priority_category) } fab!(:high_priority_topic) { Fabricate(:topic, category: high_priority_category) } fab!(:very_high_priority_topic) { Fabricate(:topic, category: very_high_priority_category) } + + fab!(:very_low_priority_post) do + SearchIndexer.enable + Fabricate(:post, topic: very_low_priority_topic, raw: "This is a very Low Priority Post") + end + fab!(:low_priority_post) do SearchIndexer.enable - Fabricate(:post, topic: low_priority_topic, raw: "This is a Low Priority Post") + + Fabricate(:post, + topic: low_priority_topic, + raw: "This is a Low Priority Post", + created_at: 1.day.ago, + ) end - fab!(:hight_priority_post) do + + fab!(:high_priority_post) do SearchIndexer.enable Fabricate(:post, topic: high_priority_topic, raw: "This is a High Priority Post") end - fab!(:old_very_hight_priority_post) do + + fab!(:very_high_priority_post) do SearchIndexer.enable - Fabricate(:old_post, topic: very_high_priority_topic, raw: "This is a Old but Very High Priority Post") + + Fabricate(:post, + topic: very_high_priority_topic, + raw: "This is a Old but Very High Priority Post", + created_at: 2.days.ago, + ) end it "sort posts with search priority when search term is empty" do get "/search.json", params: { q: 'status:open' } expect(response.status).to eq(200) data = response.parsed_body - post1 = data["posts"].find { |e| e["id"] == old_very_hight_priority_post.id } - post2 = data["posts"].find { |e| e["id"] == low_priority_post.id } - expect(data["posts"][0]["id"]).to eq(old_very_hight_priority_post.id) + post1 = data["posts"].find { |e| e["id"] == very_high_priority_post.id } + post2 = data["posts"].find { |e| e["id"] == very_low_priority_post.id } + expect(data["posts"][0]["id"]).to eq(very_high_priority_post.id) expect(post1["id"]).to be > post2["id"] end it "sort posts with search priority when no order query" do + SiteSetting.category_search_priority_high_weight = 999999 + SiteSetting.category_search_priority_low_weight = 0 + get "/search.json", params: { q: 'status:open Priority Post' } expect(response.status).to eq(200) data = response.parsed_body - expect(data["posts"][0]["id"]).to eq(old_very_hight_priority_post.id) - expect(data["posts"][1]["id"]).to eq(hight_priority_post.id) + expect(data["posts"][0]["id"]).to eq(very_high_priority_post.id) + expect(data["posts"][1]["id"]).to eq(high_priority_post.id) expect(data["posts"][2]["id"]).to eq(low_priority_post.id) + expect(data["posts"][3]["id"]).to eq(very_low_priority_post.id) end it "doesn't sort posts with search piority when query with order" do get "/search.json", params: { q: 'status:open order:latest Priority Post' } expect(response.status).to eq(200) data = response.parsed_body - expect(data["posts"][0]["id"]).to eq(hight_priority_post.id) - expect(data["posts"][1]["id"]).to eq(low_priority_post.id) - expect(data["posts"][2]["id"]).to eq(old_very_hight_priority_post.id) + expect(data["posts"][0]["id"]).to eq(high_priority_post.id) + expect(data["posts"][1]["id"]).to eq(very_low_priority_post.id) + expect(data["posts"][2]["id"]).to eq(low_priority_post.id) + expect(data["posts"][3]["id"]).to eq(very_high_priority_post.id) end end