-
Notifications
You must be signed in to change notification settings - Fork 82
FIX: User directory for solutions should update when value changes from positive value to zero #372
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…om positive to zero
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the issue where users with zero solutions still showed a stale count by revising the directory query to include all users and adding a cross-date refresh test.
- Added a new spec to verify that a user's solution count drops from 1 to 0 when the time window moves past their only solution.
- Updated the SQL in
plugin.rb
to left-join users/posts/solutions so users with no recent solutions are included (counting zero).
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
spec/models/directory_item_spec.rb | Added a when refreshing across dates context to test solution 0. |
plugin.rb | Switched to left joins and a COUNT(DISTINCT CASE …) to include zero solutions. |
plugin.rb
Outdated
SELECT users.id AS user_id, | ||
COUNT(DISTINCT st.topic_id) FILTER (WHERE | ||
st.topic_id IS NOT NULL AND | ||
t.id IS NOT NULL AND | ||
t.archetype <> 'private_message' AND | ||
t.deleted_at IS NULL AND | ||
p.deleted_at IS NULL | ||
) AS solutions | ||
FROM users | ||
LEFT JOIN posts p ON p.user_id = users.id | ||
LEFT JOIN discourse_solved_solved_topics st | ||
ON st.answer_post_id = p.id | ||
AND st.created_at >= :since | ||
LEFT JOIN topics t ON t.id = st.topic_id | ||
WHERE users.id > 0 | ||
AND users.active | ||
AND users.silenced_till IS NULL | ||
AND users.suspended_till IS NULL | ||
GROUP BY users.id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some performance concerns about the new query because it has to join the entire users table against the posts
table. The previous query only looped through the discourse_solved_solved_topics
which is going to have way less rows as compared to the users
table.
To account for users with zero solutions, did you consider just setting the count for all users to zero first in an initial query before filling up the correct count using the original query?
Special thanks to Moin for pointing this out.
Bug
If John created a post which is a solution in March, the user directory would show that John solution = 1.
In April, if John has not solved a topic, he would still have solution = 1 in the user directory.
Screenshot to user directory /u (does not show bug, just a reference):

Fix
The directory column query needs to include users with 0 solutions, so that the value would be updated.