FIX: Use SolvedTopics to list posts in /activity/solved instead of user actions (#376)

In https://github.com/discourse/discourse-solved/pull/342 we moved solutions away from topic_custom_fields into proper tables, with the tables as the proper source of truth to a topic's solution.

The user's /my/activity/solved route uses user_actions which is not accurate, and a user has reported a bug where their solution is not reflected there (user actions are not a good representation of what a topic's solution is). 

This commit introduces 
- a new route to get solutions, and is mindful `hide_user_profiles_from_public` and such settings
- also mindful of PMs and private categories
- a new template that makes use of the `<UserStream>` to load posts safely and avoid reimplementation

app/controllers/discourse_solved/solved_topics_controller.rb

@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+class DiscourseSolved::SolvedTopicsController < ::ApplicationController
+  requires_plugin DiscourseSolved::PLUGIN_NAME
+
+  def by_user
+    params.require(:username)
+    user =
+      fetch_user_from_params(
+        include_inactive:
+          current_user.try(:staff?) || (current_user && SiteSetting.show_inactive_accounts),
+      )
+    raise Discourse::NotFound unless guardian.public_can_see_profiles?
+    raise Discourse::NotFound unless guardian.can_see_profile?(user)
+
+    offset = [0, params[:offset].to_i].max
+    limit = params.fetch(:limit, 30).to_i
+
+    posts =
+      Post
+        .joins(
+          "INNER JOIN discourse_solved_solved_topics ON discourse_solved_solved_topics.answer_post_id = posts.id",
+        )
+        .joins(:topic)
+        .joins("LEFT JOIN categories ON categories.id = topics.category_id")
+        .where(user_id: user.id, deleted_at: nil)
+        .where(topics: { archetype: Archetype.default, deleted_at: nil })
+        .where(
+          "topics.category_id IS NULL OR NOT categories.read_restricted OR topics.category_id IN (:secure_category_ids)",
+          secure_category_ids: guardian.secure_category_ids,
+        )
+        .includes(:user, topic: %i[category tags])
+        .order("discourse_solved_solved_topics.created_at DESC")
+        .offset(offset)
+        .limit(limit)
+
+    render_serialized(posts, DiscourseSolved::SolvedPostSerializer, root: "user_solved_posts")
+  end
+end

app/serializers/discourse_solved/solved_post_serializer.rb

@@ -0,0 +1,87 @@
+# frozen_string_literal: true
+
+class DiscourseSolved::SolvedPostSerializer < ApplicationSerializer
+  attributes :created_at,
+             :archived,
+             :avatar_template,
+             :category_id,
+             :closed,
+             :cooked,
+             :excerpt,
+             :name,
+             :post_id,
+             :post_number,
+             :post_type,
+             :raw,
+             :slug,
+             :topic_id,
+             :topic_title,
+             :truncated,
+             :url,
+             :user_id,
+             :username
+
+  def archived
+    object.topic.archived
+  end
+
+  def avatar_template
+    object.user&.avatar_template
+  end
+
+  def category_id
+    object.topic.category_id
+  end
+
+  def closed
+    object.topic.closed
+  end
+
+  def excerpt
+    @excerpt ||= PrettyText.excerpt(cooked, 300, keep_emoji_images: true)
+  end
+
+  def name
+    object.user&.name
+  end
+
+  def include_name?
+    SiteSetting.enable_names?
+  end
+
+  def post_id
+    object.id
+  end
+
+  def slug
+    Slug.for(object.topic.title)
+  end
+
+  def include_slug?
+    object.topic.title.present?
+  end
+
+  def topic_title
+    object.topic.title
+  end
+
+  def truncated
+    true
+  end
+
+  def include_truncated?
+    cooked.length > 300
+  end
+
+  def url
+    "#{Discourse.base_url}#{object.url}"
+  end
+
+  def user_id
+    object.user_id
+  end
+
+  def username
+    object.user&.username
+  end
+end

assets/javascripts/discourse/routes/user-activity-solved.js

@@ -1,15 +1,110 @@
-import UserActivityStreamRoute from "discourse/routes/user-activity-stream";
+import { tracked } from "@glimmer/tracking";
+import EmberObject from "@ember/object";
+import { service } from "@ember/service";
+import { Promise } from "rsvp";
+import { ajax } from "discourse/lib/ajax";
+import DiscourseRoute from "discourse/routes/discourse";
 import { i18n } from "discourse-i18n";
 
-export default class UserActivitySolved extends UserActivityStreamRoute {
+class SolvedPostsStream {
-  userActionType = 15;
+  @tracked content = [];
-  noContentHelpKey = "solved.no_solutions";
+  @tracked loading = false;
+  @tracked loaded = false;
+  @tracked itemsLoaded = 0;
+  @tracked canLoadMore = true;
+
+  constructor({ username, siteCategories }) {
+    this.username = username;
+    this.siteCategories = siteCategories;
+  }
+
+  get noContent() {
+    return this.loaded && this.content.length === 0;
+  }
+
+  findItems() {
+    if (this.loading || !this.canLoadMore) {
+      return Promise.resolve();
+    }
+
+    this.loading = true;
+
+    const limit = 20;
+    return ajax(
+      `/solution/by_user.json?username=${this.username}&offset=${this.itemsLoaded}&limit=${limit}`
+    )
+      .then((result) => {
+        const userSolvedPosts = result.user_solved_posts || [];
+
+        if (userSolvedPosts.length === 0) {
+          this.canLoadMore = false;
+          return;
+        }
+
+        const posts = userSolvedPosts.map((p) => {
+          const post = EmberObject.create(p);
+          post.set("titleHtml", post.topic_title);
+          post.set("postUrl", post.url);
+
+          if (post.category_id && this.siteCategories) {
+            post.set(
+              "category",
+              this.siteCategories.find((c) => c.id === post.category_id)
+            );
+          }
+          return post;
+        });
+
+        this.content = [...this.content, ...posts];
+        this.itemsLoaded = this.itemsLoaded + userSolvedPosts.length;
+
+        if (userSolvedPosts.length < limit) {
+          this.canLoadMore = false;
+        }
+      })
+      .finally(() => {
+        this.loaded = true;
+        this.loading = false;
+      });
+  }
+}
+
+export default class UserActivitySolved extends DiscourseRoute {
+  @service site;
+  @service currentUser;
+
+  model() {
+    const user = this.modelFor("user");
+
+    const stream = new SolvedPostsStream({
+      username: user.username,
+      siteCategories: this.site.categories,
+    });
+
+    return stream.findItems().then(() => {
+      return {
+        stream,
+        emptyState: this.emptyState(),
+      };
+    });
+  }
+
+  setupController(controller, model) {
+    controller.setProperties({
+      model,
+      emptyState: this.emptyState(),
+    });
+  }
+
+  renderTemplate() {
+    this.render("user-activity-solved");
+  }
 
   emptyState() {
     const user = this.modelFor("user");
 
     let title, body;
-    if (this.isCurrentUser(user)) {
+    if (this.currentUser && user.id === this.currentUser.id) {
       title = i18n("solved.no_solved_topics_title");
       body = i18n("solved.no_solved_topics_body");
     } else {

assets/javascripts/discourse/templates/user-activity-solved.gjs

@@ -0,0 +1,16 @@
+import RouteTemplate from "ember-route-template";
+import EmptyState from "discourse/components/empty-state";
+import UserStream from "discourse/components/user-stream";
+
+export default RouteTemplate(
+  <template>
+    {{#if @controller.model.stream.noContent}}
+      <EmptyState
+        @title={{@controller.model.emptyState.title}}
+        @body={{@controller.model.emptyState.body}}
+      />
+    {{else}}
+      <UserStream @stream={{@controller.model.stream}} />
+    {{/if}}
+  </template>
+);

config/routes.rb

@@ -3,6 +3,8 @@
 DiscourseSolved::Engine.routes.draw do
   post "/accept" => "answer#accept"
   post "/unaccept" => "answer#unaccept"
+
+  get "/by_user" => "solved_topics#by_user"
 end
 
 Discourse::Application.routes.draw { mount ::DiscourseSolved::Engine, at: "solution" }

spec/requests/solved_topics_controller_spec.rb

@@ -0,0 +1,117 @@
+# frozen_string_literal: true
+
+describe DiscourseSolved::SolvedTopicsController do
+  fab!(:user)
+  fab!(:another_user) { Fabricate(:user) }
+  fab!(:admin)
+  fab!(:topic)
+  fab!(:post) { Fabricate(:post, topic:) }
+  fab!(:answer_post) { Fabricate(:post, topic:, user:) }
+  fab!(:solved_topic) { Fabricate(:solved_topic, topic:, answer_post:) }
+
+  describe "#by_user" do
+    context "when accessing with username" do
+      it "returns solved posts for the specified user" do
+        sign_in(admin)
+
+        get "/solution/by_user.json", params: { username: user.username }
+
+        expect(response.status).to eq(200)
+        result = response.parsed_body
+        expect(result["user_solved_posts"]).to be_present
+        expect(result["user_solved_posts"].length).to eq(1)
+        expect(result["user_solved_posts"][0]["post_id"]).to eq(answer_post.id)
+      end
+
+      it "returns 404 for a non-existent user" do
+        sign_in(admin)
+
+        get "/solution/by_user.json", params: { username: "non-existent-user" }
+
+        expect(response.status).to eq(404)
+      end
+
+      it "correctly handles the offset parameter" do
+        sign_in(admin)
+
+        get "/solution/by_user.json", params: { username: user.username, offset: 1 }
+
+        expect(response.status).to eq(200)
+        result = response.parsed_body
+        expect(result["user_solved_posts"]).to be_empty
+      end
+
+      it "correctly handles the limit parameter" do
+        Fabricate(:solved_topic, answer_post: Fabricate(:post, user:))
+
+        sign_in(admin)
+
+        get "/solution/by_user.json", params: { username: user.username, limit: 1 }
+
+        expect(response.status).to eq(200)
+        result = response.parsed_body
+        expect(result["user_solved_posts"].length).to eq(1)
+      end
+    end
+
+    context "when accessing without username" do
+      it "returns 400 for the current user" do
+        sign_in(user)
+
+        get "/solution/by_user.json"
+
+        expect(response.status).to eq(400)
+      end
+
+      it "returns 400 if not logged in" do
+        get "/solution/by_user.json"
+
+        expect(response.status).to eq(400)
+      end
+    end
+
+    context "with visibility restrictions" do
+      context "with private category solved topic" do
+        fab!(:group) { Fabricate(:group).tap { |g| g.add(user) } }
+        fab!(:private_category) { Fabricate(:private_category, group:) }
+        fab!(:private_topic) { Fabricate(:topic, category: private_category) }
+        fab!(:private_post) { Fabricate(:post, topic: private_topic) }
+        fab!(:private_answer_post) { Fabricate(:post, topic: private_topic, user: user) }
+        fab!(:private_solved_topic) do
+          Fabricate(:solved_topic, topic: private_topic, answer_post: private_answer_post)
+        end
+
+        it "respects category permissions" do
+          sign_in(another_user)
+
+          get "/solution/by_user.json", params: { username: user.username }
+
+          expect(response.status).to eq(200)
+          result = response.parsed_body
+          # admin sees both solutions
+          expect(result["user_solved_posts"].length).to eq(1)
+
+          sign_in(user)
+
+          get "/solution/by_user.json", params: { username: user.username }
+
+          expect(response.status).to eq(200)
+          result = response.parsed_body
+          expect(result["user_solved_posts"].length).to eq(2)
+        end
+      end
+
+      it "does not return PMs" do
+        topic.update(archetype: Archetype.private_message, category: nil)
+
+        sign_in(user)
+
+        get "/solution/by_user.json", params: { username: user.username }
+
+        expect(response.status).to eq(200)
+        result = response.parsed_body
+        expect(result["user_solved_posts"]).to be_empty
+      end
+    end
+  end
+end

spec/system/solved_spec.rb

@@ -1,11 +1,11 @@
 # frozen_string_literal: true
 
-describe "About page", type: :system do
+describe "Solved", type: :system do
   fab!(:admin)
   fab!(:solver) { Fabricate(:user) }
   fab!(:accepter) { Fabricate(:user) }
   fab!(:topic) { Fabricate(:post, user: admin).topic }
-  fab!(:post1) { Fabricate(:post, topic:, user: solver, cooked: "The answer is 42") }
+  fab!(:solver_post) { Fabricate(:post, topic:, user: solver, cooked: "The answer is 42") }
   let(:topic_page) { PageObjects::Pages::Topic.new }
 
   before do
@@ -39,4 +39,13 @@ describe "About page", type: :system do
       end
     end
   end
+
+  it "shows the solved post in user activity at /my/activity/solved" do
+    Fabricate(:solved_topic, topic:, answer_post: solver_post, accepter:)
+
+    sign_in(solver)
+    visit "/my/activity/solved"
+
+    expect(page.find(".post-list")).to have_content(solver_post.cooked)
+  end
 end

test/javascripts/acceptance/user-activity-solved-test.js

@@ -9,8 +9,8 @@ acceptance(
     needs.user();
 
     needs.pretender((server, helper) => {
-      server.get("/user_actions.json", () =>
+      server.get("/solution/by_user.json", () =>
-        helper.response({ user_actions: [] })
+        helper.response({ user_solved_posts: [] })
       );
     });