From b4a740382f2a9c708e4afe72aa91658aedce4c85 Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 3 May 2023 11:56:10 +1000 Subject: [PATCH] FEATURE: prioritize_solved_topics_in_search to prioritize solved topics (#236) Many consumers of Discourse solved may want solved topics to show up more prominently in search. New setting `prioritize_solved_topics_in_search` (default off) allows bumping these topics to the top. Co-authored-by: Alan Guo Xiang Tan --- config/locales/server.en.yml | 1 + config/settings.yml | 1 + plugin.rb | 60 +++++++++++++++++++++++---------- spec/integration/solved_spec.rb | 32 ++++++++++++++++++ 4 files changed, 76 insertions(+), 18 deletions(-) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 8f5f281..823c4e2 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -14,6 +14,7 @@ en: accept_solutions_topic_author: "Allow the topic author to accept a solution." solved_add_schema_markup: "Add QAPage schema markup to HTML." enable_solved_tags: "Tags that will allow users to select solutions." + prioritize_solved_topics_in_search: "Prioritize solved topics in search results." reports: accepted_solutions: diff --git a/config/settings.yml b/config/settings.yml index f939b35..1212edc 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -34,6 +34,7 @@ plugins: - "never" - "always" - "answered only" + prioritize_solved_topics_in_search: false enable_solved_tags: type: tag_list default: "" diff --git a/plugin.rb b/plugin.rb index a712428..d2b8a37 100644 --- a/plugin.rb +++ b/plugin.rb @@ -15,7 +15,7 @@ register_svg_icon "far fa-square" end -PLUGIN_NAME = "discourse_solved".freeze +PLUGIN_NAME = "discourse_solved" register_asset "stylesheets/solutions.scss" register_asset "stylesheets/mobile/solutions.scss", :mobile @@ -74,13 +74,14 @@ class Engine < ::Rails::Engine isolate_namespace DiscourseSolved end - AUTO_CLOSE_TOPIC_TIMER_CUSTOM_FIELD = "solved_auto_close_topic_timer_id".freeze + AUTO_CLOSE_TOPIC_TIMER_CUSTOM_FIELD = "solved_auto_close_topic_timer_id" + ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD = "accepted_answer_post_id" def self.accept_answer!(post, acting_user, topic: nil) topic ||= post.topic DistributedMutex.synchronize("discourse_solved_toggle_answer_#{topic.id}") do - accepted_id = topic.custom_fields["accepted_answer_post_id"].to_i + accepted_id = topic.custom_fields[ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD].to_i if accepted_id > 0 if p2 = Post.find_by(id: accepted_id) @@ -94,7 +95,7 @@ def self.accept_answer!(post, acting_user, topic: nil) end post.custom_fields["is_accepted_answer"] = "true" - topic.custom_fields["accepted_answer_post_id"] = post.id + topic.custom_fields[ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD] = post.id if defined?(UserAction::SOLVED) UserAction.log_action!( @@ -172,7 +173,7 @@ def self.unaccept_answer!(post, topic: nil) DistributedMutex.synchronize("discourse_solved_toggle_answer_#{topic.id}") do post.custom_fields.delete("is_accepted_answer") - topic.custom_fields.delete("accepted_answer_post_id") + topic.custom_fields.delete(ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD) if timer_id = topic.custom_fields[AUTO_CLOSE_TOPIC_TIMER_CUSTOM_FIELD] topic_timer = TopicTimer.find_by(id: timer_id) @@ -299,7 +300,10 @@ def before_head_close_meta(controller) }, } - if accepted_answer = Post.find_by(id: topic.custom_fields["accepted_answer_post_id"]) + if accepted_answer = + Post.find_by( + id: topic.custom_fields[::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD], + ) question_json["answerCount"] = 1 question_json[:acceptedAnswer] = { "@type" => "Answer", @@ -343,7 +347,8 @@ def before_head_close_meta(controller) Report.add_report("accepted_solutions") do |report| report.data = [] - accepted_solutions = TopicCustomField.where(name: "accepted_answer_post_id") + accepted_solutions = + TopicCustomField.where(name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD) category_id, include_subcategories = report.add_category_filter if category_id @@ -375,6 +380,25 @@ def before_head_close_meta(controller) end end + if respond_to?(:register_modifier) + register_modifier(:search_rank_sort_priorities) do |priorities, search| + if SiteSetting.prioritize_solved_topics_in_search + condition = <<~SQL + EXISTS + ( + SELECT 1 FROM topic_custom_fields f + WHERE topics.id = f.topic_id + AND f.name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' + ) + SQL + + priorities.push([condition, 1.1]) + else + priorities + end + end + end + if defined?(UserAction::SOLVED) require_dependency "user_summary" class ::UserSummary @@ -444,7 +468,7 @@ def accepted_answer_post_info end def accepted_answer_post_id - id = object.topic.custom_fields["accepted_answer_post_id"] + id = object.topic.custom_fields[::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD] # a bit messy but race conditions can give us an array here, avoid begin id && id.to_i @@ -533,7 +557,7 @@ def accepted_answer def topic_accepted_answer if topic = (topic_view && topic_view.topic) || object.topic - topic.custom_fields["accepted_answer_post_id"].present? + topic.custom_fields[::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD].present? end end end @@ -547,7 +571,7 @@ def topic_accepted_answer "topics.id IN ( SELECT tc.topic_id FROM topic_custom_fields tc - WHERE tc.name = 'accepted_answer_post_id' AND + WHERE tc.name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' AND tc.value IS NOT NULL )", ) @@ -558,7 +582,7 @@ def topic_accepted_answer "topics.id NOT IN ( SELECT tc.topic_id FROM topic_custom_fields tc - WHERE tc.name = 'accepted_answer_post_id' AND + WHERE tc.name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' AND tc.value IS NOT NULL )", ) @@ -575,7 +599,7 @@ def topic_accepted_answer "topics.id IN ( SELECT tc.topic_id FROM topic_custom_fields tc - WHERE tc.name = 'accepted_answer_post_id' AND + WHERE tc.name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' AND tc.value IS NOT NULL )", ) @@ -585,7 +609,7 @@ def topic_accepted_answer "topics.id NOT IN ( SELECT tc.topic_id FROM topic_custom_fields tc - WHERE tc.name = 'accepted_answer_post_id' AND + WHERE tc.name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' AND tc.value IS NOT NULL )", ) @@ -620,13 +644,13 @@ class ::ListableTopicSerializer end if TopicList.respond_to? :preloaded_custom_fields - TopicList.preloaded_custom_fields << "accepted_answer_post_id" + TopicList.preloaded_custom_fields << ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD end if Site.respond_to? :preloaded_category_custom_fields Site.preloaded_category_custom_fields << "enable_accepted_answers" end if Search.respond_to? :preloaded_topic_custom_fields - Search.preloaded_topic_custom_fields << "accepted_answer_post_id" + Search.preloaded_topic_custom_fields << ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD end if CategoryList.respond_to?(:preloaded_topic_custom_fields) @@ -637,7 +661,7 @@ class ::ListableTopicSerializer NOT EXISTS( SELECT 1 FROM topic_custom_fields WHERE topic_id = topics.id - AND name = 'accepted_answer_post_id' + AND name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' AND value IS NOT NULL ) SQL @@ -722,7 +746,7 @@ class ::ListableTopicSerializer add_to_class(:composer_messages_finder, :check_topic_is_solved) do return if !SiteSetting.solved_enabled || SiteSetting.disable_solved_education_message return if !replying? || @topic.blank? || @topic.private_message? - return if @topic.custom_fields["accepted_answer_post_id"].blank? + return if @topic.custom_fields[::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD].blank? { id: "solved_topic", @@ -749,7 +773,7 @@ class ::Topic answer_post_ids = TopicCustomField .select("value::INTEGER") - .where(name: "accepted_answer_post_id") + .where(name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD) .where(topic_id: topics.map(&:id)) answer_user_ids = Post.where(id: answer_post_ids).pluck(:topic_id, :user_id).to_h topics.each { |topic| topic.accepted_answer_user_id = answer_user_ids[topic.id] } diff --git a/spec/integration/solved_spec.rb b/spec/integration/solved_spec.rb index 0de7146..2710c28 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -9,6 +9,38 @@ before { SiteSetting.allow_solved_on_all_topics = true } + describe "search" do + before { SearchIndexer.enable } + + after { SearchIndexer.disable } + + it "can prioritize solved topics in search" do + normal_post = + Fabricate( + :post, + raw: "My reply carrot", + topic: Fabricate(:topic, title: "A topic that is not solved but open"), + ) + + solved_post = + Fabricate( + :post, + raw: "My solution carrot", + topic: Fabricate(:topic, title: "A topic that will be closed", closed: true), + ) + + DiscourseSolved.accept_answer!(solved_post, Discourse.system_user) + + result = Search.execute("carrot") + expect(result.posts.pluck(:id)).to eq([normal_post.id, solved_post.id]) + + SiteSetting.prioritize_solved_topics_in_search = true + + result = Search.execute("carrot") + expect(result.posts.pluck(:id)).to eq([solved_post.id, normal_post.id]) + end + end + describe "auto bump" do it "does not automatically bump solved topics" do category = Fabricate(:category_with_definition)