From 69397b001997d75f6717edbf7b119ca2eddff9b1 Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Tue, 2 May 2023 16:05:51 +1000 Subject: [PATCH 1/4] FEATURE: prioritize_solved_topics_in_search to prioritize solved topics Many consumers of Discourse solve may want solved topics to show up more prominently in search. New setting (default off) allows bumping these topics to the top. --- config/locales/server.en.yml | 1 + config/settings.yml | 1 + plugin.rb | 55 ++++++++++++++++++++++----------- spec/integration/solved_spec.rb | 29 +++++++++++++++++ 4 files changed, 68 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..83e6df2 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,20 @@ def before_head_close_meta(controller) end end + if respond_to?(:register_modifier) + register_modifier(:search_rank_sort_priorities) do |priorities, search| + priorities << [<<~SQL, 1.1] if SiteSetting.prioritize_solved_topics_in_search + 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 + end + end + if defined?(UserAction::SOLVED) require_dependency "user_summary" class ::UserSummary @@ -444,7 +463,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 +552,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 +566,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 +577,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 +594,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 +604,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 +639,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 +656,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 +741,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 +768,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..a9e1ac9 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -9,6 +9,35 @@ 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 + SiteSetting.prioritize_solved_topics_in_search = true + + 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([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) From c6c5f709b9406b9ef898b72821a628aaf36c8eab Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 2 May 2023 16:40:40 +1000 Subject: [PATCH 2/4] Update spec/integration/solved_spec.rb Co-authored-by: Alan Guo Xiang Tan --- spec/integration/solved_spec.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/spec/integration/solved_spec.rb b/spec/integration/solved_spec.rb index a9e1ac9..2710c28 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -15,8 +15,6 @@ after { SearchIndexer.disable } it "can prioritize solved topics in search" do - SiteSetting.prioritize_solved_topics_in_search = true - normal_post = Fabricate( :post, @@ -33,6 +31,11 @@ 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 From 067d7501e3cc89b1bba1ea0abbd9c675077bd53d Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 2 May 2023 17:36:25 +1000 Subject: [PATCH 3/4] Update plugin.rb Co-authored-by: Alan Guo Xiang Tan --- plugin.rb | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/plugin.rb b/plugin.rb index 83e6df2..a745e79 100644 --- a/plugin.rb +++ b/plugin.rb @@ -382,15 +382,20 @@ def before_head_close_meta(controller) if respond_to?(:register_modifier) register_modifier(:search_rank_sort_priorities) do |priorities, search| - priorities << [<<~SQL, 1.1] if SiteSetting.prioritize_solved_topics_in_search - 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 + 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 From 06d4c8b4709ecad92f153b761bd959d80a32ef9b Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Wed, 3 May 2023 11:47:10 +1000 Subject: [PATCH 4/4] lint --- plugin.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin.rb b/plugin.rb index a745e79..d2b8a37 100644 --- a/plugin.rb +++ b/plugin.rb @@ -382,12 +382,12 @@ def before_head_close_meta(controller) if respond_to?(:register_modifier) register_modifier(:search_rank_sort_priorities) do |priorities, search| - if SiteSetting.prioritize_solved_topics_in_search + if SiteSetting.prioritize_solved_topics_in_search condition = <<~SQL EXISTS ( SELECT 1 FROM topic_custom_fields f - WHERE topics.id = f.topic_id + WHERE topics.id = f.topic_id AND f.name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' ) SQL