FIX: Standardise the definition of what a solution is (#352)

There are three locations where a user's solution query is defined
- user summary
- user card
- user directory

This PR updates the queries to use data from the new `SolvedTopics` table instead of the `UserActions` table. And adds testssssss

Author: nattsw

lib/discourse_solved/user_summary_extension.rb

@@ -4,6 +4,9 @@ module DiscourseSolved::UserSummaryExtension
   extend ActiveSupport::Concern
 
   def solved_count
-    DiscourseSolved::SolvedTopic.where(accepter: @user).count
+    DiscourseSolved::SolvedTopic
+      .joins("JOIN posts ON posts.id = discourse_solved_solved_topics.answer_post_id")
+      .where(posts: { user_id: @user.id })
+      .count
   end
 end

plugin.rb

@@ -240,12 +240,10 @@ def self.skip_db?
   end
 
   add_to_serializer(:user_card, :accepted_answers) do
-    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")
+    DiscourseSolved::SolvedTopic
+      .joins(answer_post: :user, topic: {})
+      .where(posts: { user_id: object.id, deleted_at: nil })
+      .where(topics: { archetype: Archetype.default, deleted_at: nil })
       .count
   end
   add_to_serializer(:user_summary, :solved_count) { object.solved_count }
@@ -288,24 +286,23 @@ def self.skip_db?
 
   query = <<~SQL
     WITH x AS (
-      SELECT u.id user_id, COUNT(DISTINCT ua.id) AS solutions
-      FROM users AS u
-      LEFT JOIN user_actions AS ua
-         ON ua.user_id = u.id
-        AND ua.action_type = #{UserAction::SOLVED}
-        AND COALESCE(ua.created_at, :since) > :since
+      SELECT p.user_id, COUNT(DISTINCT st.id) AS solutions
+      FROM discourse_solved_solved_topics AS st
+      JOIN posts AS p
+         ON p.id = st.answer_post_id
+        AND COALESCE(p.created_at, :since) > :since
+        AND p.deleted_at IS NULL
       JOIN topics AS t
-         ON t.id = ua.target_topic_id
+         ON t.id = st.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
+      JOIN users AS u
+         ON u.id = p.user_id
       WHERE u.id > 0
         AND u.active
         AND u.silenced_till IS NULL
         AND u.suspended_till IS NULL
-      GROUP BY u.id
+      GROUP BY p.user_id
     )
     UPDATE directory_items di
     SET solutions = x.solutions

spec/models/directory_item_spec.rb

@@ -0,0 +1,105 @@
+# frozen_string_literal: true
+
+describe DirectoryItem, type: :model do
+  describe "Updating user directory with solutions count" do
+    fab!(:user)
+    fab!(:admin)
+
+    fab!(:topic1) { Fabricate(:topic, archetype: "regular", user:) }
+    fab!(:topic_post1) { Fabricate(:post, topic: topic1, user:) }
+
+    fab!(:topic2) { Fabricate(:topic, archetype: "regular", user:) }
+    fab!(:topic_post2) { Fabricate(:post, topic: topic2, user:) }
+
+    fab!(:pm) { Fabricate(:topic, archetype: "private_message", user:, category_id: nil) }
+    fab!(:pm_post) { Fabricate(:post, topic: pm, user:) }
+
+    before { SiteSetting.solved_enabled = true }
+
+    it "excludes PM post solutions from solutions" do
+      DiscourseSolved.accept_answer!(topic_post1, admin)
+      DiscourseSolved.accept_answer!(pm_post, admin)
+
+      DirectoryItem.refresh!
+
+      expect(
+        DirectoryItem.find_by(
+          user_id: user.id,
+          period_type: DirectoryItem.period_types[:all],
+        ).solutions,
+      ).to eq(1)
+    end
+
+    it "excludes deleted posts from solutions" do
+      DiscourseSolved.accept_answer!(topic_post1, admin)
+      DiscourseSolved.accept_answer!(topic_post2, admin)
+      topic_post2.update(deleted_at: Time.zone.now)
+
+      DirectoryItem.refresh!
+
+      expect(
+        DirectoryItem.find_by(
+          user_id: user.id,
+          period_type: DirectoryItem.period_types[:all],
+        ).solutions,
+      ).to eq(1)
+    end
+
+    it "excludes deleted topics from solutions" do
+      DiscourseSolved.accept_answer!(topic_post1, admin)
+      DiscourseSolved.accept_answer!(topic_post2, admin)
+      topic2.update(deleted_at: Time.zone.now)
+
+      DirectoryItem.refresh!
+
+      expect(
+        DirectoryItem.find_by(
+          user_id: user.id,
+          period_type: DirectoryItem.period_types[:all],
+        ).solutions,
+      ).to eq(1)
+    end
+
+    it "excludes solutions for silenced users" do
+      user.update(silenced_till: Time.zone.now + 1.day)
+
+      DiscourseSolved.accept_answer!(topic_post1, admin)
+
+      DirectoryItem.refresh!
+
+      expect(
+        DirectoryItem.find_by(
+          user_id: user.id,
+          period_type: DirectoryItem.period_types[:all],
+        )&.solutions,
+      ).to eq(nil)
+    end
+
+    it "excludes solutions for suspended users" do
+      DiscourseSolved.accept_answer!(topic_post1, admin)
+      user.update(suspended_till: Time.zone.now + 1.day)
+
+      DirectoryItem.refresh!
+
+      expect(
+        DirectoryItem.find_by(
+          user_id: user.id,
+          period_type: DirectoryItem.period_types[:all],
+        )&.solutions,
+      ).to eq(0)
+    end
+
+    it "includes solutions for active users" do
+      DiscourseSolved.accept_answer!(topic_post1, admin)
+
+      DirectoryItem.refresh!
+
+      expect(
+        DirectoryItem.find_by(
+          user_id: user.id,
+          period_type: DirectoryItem.period_types[:all],
+        ).solutions,
+      ).to eq(1)
+    end
+  end
+end

spec/models/user_summary_spec.rb

@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+describe UserSummary do
+  describe "solved_count" do
+    it "indicates the number of times a user's post is a topic's solution" do
+      topic = Fabricate(:topic)
+      Fabricate(:post, topic:)
+      user = Fabricate(:user)
+      admin = Fabricate(:admin)
+      post = Fabricate(:post, topic:, user:)
+
+      user_summary = UserSummary.new(user, Guardian.new)
+      admin_summary = UserSummary.new(admin, Guardian.new)
+
+      expect(user_summary.solved_count).to eq(0)
+      expect(admin_summary.solved_count).to eq(0)
+
+      DiscourseSolved.accept_answer!(post, admin)
+
+      expect(user_summary.solved_count).to eq(1)
+      expect(admin_summary.solved_count).to eq(0)
+    end
+  end
+end

spec/serializers/user_card_serializer_spec.rb

@@ -1,7 +1,5 @@
 # frozen_string_literal: true
 
-require "rails_helper"
-
 describe UserCardSerializer do
   let(:user) { Fabricate(:user) }
   let(:serializer) { described_class.new(user, scope: Guardian.new, root: false) }