FIX: don't allow or count solutions in PMs

Pretty straightforward, this ensures we don't allow users to mark a post
as a solution in a PM.

This also ensures we don't count solutions in topics that were converted
to a PM.

Internal ref t/146766

app/lib/guardian_extensions.rb

@@ -21,7 +21,7 @@ module DiscourseSolved
 
     def can_accept_answer?(topic, post)
       return false if !authenticated?
-      return false if !topic || !post || post.whisper?
+      return false if !topic || topic.private_message? || !post || post.whisper? 
       return false if !allow_accepted_answers?(topic.category_id, topic.tags.map(&:name))
 
       return true if is_staff?

plugin.rb

@@ -272,19 +272,20 @@ after_initialize do
     report.data = []
 
     accepted_solutions =
-      TopicCustomField.where(name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD)
+      TopicCustomField
+        .joins(:topic)
+        .where("topics.archetype <> ?", Archetype.private_message)
+        .where(name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD)
 
     category_id, include_subcategories = report.add_category_filter
     if category_id
       if include_subcategories
         accepted_solutions =
-          accepted_solutions.joins(:topic).where(
+          accepted_solutions
-            "topics.category_id IN (?)",
+            .where("topics.category_id IN (?)", Category.subcategory_ids(category_id))
-            Category.subcategory_ids(category_id),
-          )
       else
         accepted_solutions =
-          accepted_solutions.joins(:topic).where("topics.category_id = ?", category_id)
+          accepted_solutions.where("topics.category_id = ?", category_id)
       end
     end
 
@@ -345,7 +346,7 @@ after_initialize do
       )
     SQL
 
-    scope.where(sql)
+    scope.where(sql).where("topics.archetype <> ?", Archetype.private_message)
   end
 
   unsolved_callback = ->(scope) do
@@ -379,7 +380,7 @@ after_initialize do
       SQL
     end
 
-    scope
+    scope.where("topics.archetype <> ?", Archetype.private_message)
   end
 
   register_custom_filter_by_status("solved", &solved_callback)
@@ -398,6 +399,12 @@ after_initialize do
     end
   end
 
+  register_modifier(:user_action_stream_builder) do |builder|
+    builder
+      .where("t.deleted_at IS NULL")
+      .where("t.archetype <> ?", Archetype.private_message)
+  end
+
   TopicList.preloaded_custom_fields << ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD
   Site.preloaded_category_custom_fields << ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD
   Search.preloaded_topic_custom_fields << ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD
@@ -477,26 +484,36 @@ after_initialize do
     end
   end
 
-  query =
+  query = <<~SQL
-    "
+    WITH x AS (
-    WITH x AS (SELECT
+      SELECT u.id user_id, COUNT(DISTINCT ua.id) AS solutions
-      u.id user_id,
-      COUNT(DISTINCT ua.id) AS solutions
       FROM users AS u
-      LEFT OUTER JOIN user_actions AS ua ON ua.user_id = u.id AND ua.action_type = #{UserAction::SOLVED} AND COALESCE(ua.created_at, :since) > :since
+      LEFT JOIN user_actions AS ua 
-      WHERE u.active
+         ON ua.user_id = u.id 
+        AND ua.action_type = #{UserAction::SOLVED} 
+        AND COALESCE(ua.created_at, :since) > :since
+      JOIN topics AS t 
+         ON t.id = ua.target_topic_id
+        AND t.archetype <> 'private_message'
+        AND t.deleted_at IS NULL
+      JOIN posts AS p 
+         ON p.id = ua.target_post_id
+        AND p.deleted_at IS NULL
+      WHERE u.id > 0
+        AND u.active
         AND u.silenced_till IS NULL
-        AND u.id > 0
+        AND u.suspended_till IS NULL
       GROUP BY u.id
     )
-    UPDATE directory_items di SET
+    UPDATE directory_items di 
-      solutions = x.solutions
+    SET solutions = x.solutions
     FROM x
     WHERE x.user_id = di.user_id
-    AND di.period_type = :period_type
+      AND di.period_type = :period_type
-    AND di.solutions <> x.solutions
+      AND di.solutions <> x.solutions
-  "
+  SQL
-  add_directory_column("solutions", query: query)
+
+  add_directory_column("solutions", query:)
 
   add_to_class(:composer_messages_finder, :check_topic_is_solved) do
     return if !SiteSetting.solved_enabled || SiteSetting.disable_solved_education_message
@@ -514,7 +531,13 @@ after_initialize do
   end
 
   add_to_serializer(:user_card, :accepted_answers) do
-    UserAction.where(user_id: object.id).where(action_type: UserAction::SOLVED).count
+    UserAction
+      .where(user_id: object.id)
+      .where(action_type: UserAction::SOLVED)
+      .joins("JOIN topics ON topics.id = user_actions.target_topic_id")
+      .where("topics.archetype <> ?", Archetype.private_message)
+      .where("topics.deleted_at IS NULL")
+      .count
   end
 
   register_topic_list_preload_user_ids do |topics, user_ids, topic_list|

spec/integration/solved_spec.rb

@@ -47,6 +47,16 @@ RSpec.describe "Managing Posts solved status" do
       )
     end
 
+    fab!(:solved_pm) do
+      Fabricate(
+        :custom_topic,
+        category_id: nil,
+        archetype: Archetype.private_message,
+        custom_topic_name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD,
+        value: "42",
+      )
+    end
+
     fab!(:unsolved_in_category) { Fabricate(:topic, category: solvable_category) }
     fab!(:unsolved_in_tag) { Fabricate(:topic, tags: [solvable_tag]) }
 
@@ -572,4 +582,33 @@ RSpec.describe "Managing Posts solved status" do
       expect { DiscourseSolved.unaccept_answer!(reply) }.not_to raise_error
     end
   end
+
+  describe "user actions stream modifier" do
+    it "correctly list solutions" do
+      t1 = Fabricate(:topic)
+      t2 = Fabricate(:topic)
+      t3 = Fabricate(:topic)
+
+      p1 = Fabricate(:post, topic: t1, user:)
+      p2 = Fabricate(:post, topic: t2, user:)
+      p3 = Fabricate(:post, topic: t3, user:)
+
+      DiscourseSolved.accept_answer!(p1, Discourse.system_user)
+      DiscourseSolved.accept_answer!(p2, Discourse.system_user)
+      DiscourseSolved.accept_answer!(p3, Discourse.system_user)
+
+      t1.trash!(Discourse.system_user)
+      t2.convert_to_private_message(Discourse.system_user)
+
+      expect(
+        UserAction
+          .stream(
+            user_id: user.id,
+            action_types: [::UserAction::SOLVED],
+            guardian: user.guardian
+          )
+          .map(&:post_id)
+      ).to contain_exactly p3.id
+    end
+  end
 end

spec/lib/guardian_extensions_spec.rb

@@ -25,6 +25,11 @@ describe DiscourseSolved::GuardianExtensions do
       expect(guardian.can_accept_answer?(topic, post)).to eq(false)
     end
 
+    it "returns false for private messages" do
+      topic.update!(user:, category_id: nil, archetype: Archetype.private_message)
+      expect(guardian.can_accept_answer?(topic, post)).to eq(false)
+    end
+
     it "returns false if accepted answers are not allowed" do
       SiteSetting.allow_solved_on_all_topics = false
       expect(guardian.can_accept_answer?(topic, post)).to eq(false)

spec/serializers/user_card_serializer_spec.rb

@@ -8,6 +8,8 @@ describe UserCardSerializer do
   let(:json) { serializer.as_json }
 
   it "accepted_answers serializes number of accepted answers" do
+    expect(serializer.as_json[:accepted_answers]).to eq(0)
+
     post1 = Fabricate(:post, user: user)
     DiscourseSolved.accept_answer!(post1, Discourse.system_user)
     expect(serializer.as_json[:accepted_answers]).to eq(1)
@@ -16,7 +18,17 @@ describe UserCardSerializer do
     DiscourseSolved.accept_answer!(post2, Discourse.system_user)
     expect(serializer.as_json[:accepted_answers]).to eq(2)
 
+    post3 = Fabricate(:post, user: user)
+    DiscourseSolved.accept_answer!(post3, Discourse.system_user)
+    expect(serializer.as_json[:accepted_answers]).to eq(3)
+
     DiscourseSolved.unaccept_answer!(post1)
+    expect(serializer.as_json[:accepted_answers]).to eq(2)
+
+    post2.topic.trash!(Discourse.system_user)
     expect(serializer.as_json[:accepted_answers]).to eq(1)
+
+    post3.topic.convert_to_private_message(Discourse.system_user)
+    expect(serializer.as_json[:accepted_answers]).to eq(0)
   end
 end