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

@@ -76,6 +76,7 @@ class Engine < ::Rails::Engine
 
     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 @@ class ::Guardian
     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 @@ def topic_accepted_answer
     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 @@ class ::ListableTopicSerializer
     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 @@ class ::ListableTopicSerializer
         )
       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 @@ class ::ListableTopicSerializer
       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
 

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

spec/integration/solved_spec.rb

@@ -4,7 +4,7 @@
 
 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 @@
       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