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
This commit is contained in:
Natalie Tay 2025-03-28 09:49:02 +08:00 committed by GitHub
parent 9e9ac2862c
commit be5798f6d8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 147 additions and 20 deletions

View File

@ -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

View File

@ -240,12 +240,10 @@ after_initialize do
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 @@ after_initialize do
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

View File

@ -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

View File

@ -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

View File

@ -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) }