Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEATURE: prioritize_solved_topics_in_search to prioritize solved topics #236

Merged
merged 4 commits into from
May 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config/locales/server.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions config/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ plugins:
- "never"
- "always"
- "answered only"
prioritize_solved_topics_in_search: false
enable_solved_tags:
type: tag_list
default: ""
Expand Down
60 changes: 42 additions & 18 deletions plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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!(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
)",
)
Expand All @@ -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
)",
)
Expand All @@ -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
)",
)
Expand All @@ -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
)",
)
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -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] }
Expand Down
32 changes: 32 additions & 0 deletions spec/integration/solved_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down