From 4032a1e35f4cf6a42b5cbedd780e8344dc442346 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20David=20Mart=C3=ADnez=20Cubillos?= Date: Tue, 20 Jun 2023 09:52:02 -0500 Subject: [PATCH] FIX: Search bug for status:unsolved returns topics from non-solution enabled categories (#241) * FIX: Search bug for status:unsolved returns topics from non-solution enabled categories * fixed rubocop issues --- plugin.rb | 48 +++++++--- spec/fabricators/extend_topic_fabricator.rb | 14 +++ spec/integration/solved_spec.rb | 99 ++++++++++++++++++++- 3 files changed, 146 insertions(+), 15 deletions(-) create mode 100644 spec/fabricators/extend_topic_fabricator.rb diff --git a/plugin.rb b/plugin.rb index 2018d09..09410d5 100644 --- a/plugin.rb +++ b/plugin.rb @@ -76,6 +76,7 @@ SQL AUTO_CLOSE_TOPIC_TIMER_CUSTOM_FIELD = "solved_auto_close_topic_timer_id" ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD = "accepted_answer_post_id" + ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD = "enable_accepted_answers" def self.accept_answer!(post, acting_user, topic: nil) topic ||= post.topic @@ -502,9 +503,10 @@ SQL def self.reset_accepted_answer_cache @@allowed_accepted_cache["allowed"] = begin Set.new( - CategoryCustomField.where(name: "enable_accepted_answers", value: "true").pluck( - :category_id, - ), + CategoryCustomField.where( + name: ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD, + value: "true", + ).pluck(:category_id), ) end end @@ -586,14 +588,32 @@ SQL end Search.advanced_filter(/status:unsolved/) do |posts| - posts.where( - "topics.id NOT IN ( - SELECT tc.topic_id - FROM topic_custom_fields tc - WHERE tc.name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' AND - tc.value IS NOT NULL - )", - ) + if SiteSetting.allow_solved_on_all_topics + posts.where( + "topics.id NOT IN ( + SELECT tc.topic_id + FROM topic_custom_fields tc + WHERE tc.name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' AND + tc.value IS NOT NULL + )", + ) + else + posts.where( + "topics.id NOT IN ( + SELECT tc.topic_id + FROM topic_custom_fields tc + WHERE tc.name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' AND + tc.value IS NOT NULL + ) AND topics.id IN ( + SELECT top.id + FROM topics top + INNER JOIN category_custom_fields cc + ON top.category_id = cc.category_id + WHERE cc.name = '#{::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD}' AND + cc.value = 'true' + )", + ) + end end end @@ -655,7 +675,7 @@ SQL 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" + Site.preloaded_category_custom_fields << ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD end if Search.respond_to? :preloaded_topic_custom_fields Search.preloaded_topic_custom_fields << ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD @@ -696,7 +716,7 @@ SQL ) CategoryCustomField.create!( category_id: solved_category.id, - name: "enable_accepted_answers", + name: ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD, value: "true", ) puts "discourse-solved enabled on category '#{solved_category.name}' (#{solved_category.id})." @@ -706,7 +726,7 @@ SQL unless SiteSetting.allow_solved_on_all_topics solved_category_id = CategoryCustomField - .where(name: "enable_accepted_answers", value: "true") + .where(name: ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD, value: "true") .first .category_id diff --git a/spec/fabricators/extend_topic_fabricator.rb b/spec/fabricators/extend_topic_fabricator.rb new file mode 100644 index 0000000..c41e4b6 --- /dev/null +++ b/spec/fabricators/extend_topic_fabricator.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true +Fabricator(:custom_topic, from: :topic) do + transient :custom_topic_name + transient :value + after_create do |top, transients| + custom_topic = + TopicCustomField.new( + topic_id: top.id, + name: transients[:custom_topic_name], + value: transients[:value], + ) + custom_topic.save + end +end diff --git a/spec/integration/solved_spec.rb b/spec/integration/solved_spec.rb index 2710c28..80052fe 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -4,7 +4,7 @@ require "rails_helper" RSpec.describe "Managing Posts solved status" do let(:topic) { Fabricate(:topic) } - let(:user) { Fabricate(:trust_level_4) } + fab!(:user) { Fabricate(:trust_level_4) } let(:p1) { Fabricate(:post, topic: topic) } before { SiteSetting.allow_solved_on_all_topics = true } @@ -39,6 +39,103 @@ RSpec.describe "Managing Posts solved status" do result = Search.execute("carrot") expect(result.posts.pluck(:id)).to eq([solved_post.id, normal_post.id]) end + + describe "#advanced_search" do + fab!(:category_enabled) do + category = Fabricate(:category) + category_custom_field = + CategoryCustomField.new( + category_id: category.id, + name: ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD, + value: "true", + ) + category_custom_field.save + category + end + fab!(:category_disabled) do + category = Fabricate(:category) + category_custom_field = + CategoryCustomField.new( + category_id: category.id, + name: ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD, + value: "false", + ) + category_custom_field.save + category + end + fab!(:topic_unsolved) do + Fabricate( + :custom_topic, + user: user, + category: category_enabled, + custom_topic_name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD, + ) + end + fab!(:topic_solved) do + Fabricate( + :custom_topic, + user: user, + category: category_enabled, + custom_topic_name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD, + ) + end + fab!(:topic_disabled_1) do + Fabricate( + :custom_topic, + user: user, + category: category_disabled, + custom_topic_name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD, + ) + end + fab!(:topic_disabled_2) do + Fabricate( + :custom_topic, + user: user, + category: category_disabled, + custom_topic_name: "another_custom_field", + ) + end + fab!(:post_unsolved) { Fabricate(:post, topic: topic_unsolved) } + fab!(:post_solved) do + post = Fabricate(:post, topic: topic_solved) + DiscourseSolved.accept_answer!(post, Discourse.system_user) + post + end + fab!(:post_disabled_1) { Fabricate(:post, topic: topic_disabled_1) } + fab!(:post_disabled_2) { Fabricate(:post, topic: topic_disabled_2) } + + before do + SearchIndexer.enable + Jobs.run_immediately! + + SearchIndexer.index(topic_unsolved, force: true) + SearchIndexer.index(topic_solved, force: true) + SearchIndexer.index(topic_disabled_1, force: true) + SearchIndexer.index(topic_disabled_2, force: true) + end + + after { SearchIndexer.disable } + + describe "searches for unsolved topics" do + describe "when allow solved on all topics is disabled" do + before { SiteSetting.allow_solved_on_all_topics = false } + + it "only returns posts where 'Allow topic owner and staff to mark a reply as the solution' is enabled and post is not solved" do + result = Search.execute("status:unsolved") + expect(result.posts.pluck(:id)).to match_array([post_unsolved.id]) + end + end + describe "when allow solved on all topics is enabled" do + before { SiteSetting.allow_solved_on_all_topics = true } + it "only returns posts where the post is not solved" do + result = Search.execute("status:unsolved") + expect(result.posts.pluck(:id)).to match_array( + [post_unsolved.id, post_disabled_1.id, post_disabled_2.id], + ) + end + end + end + end end describe "auto bump" do