FEATURE: Show 'marked solved by' in OP when topic is solved (#343)

Depends on: #342

This feature adds the "Marked solved as" information to the solved post appended to OP.

Originally, I had moved the widget usage to a [component](https://github.com/discourse/discourse-solved/blob/39baa0be4a889fdbff108e887a677d9a298d27d4/assets/javascripts/discourse/components/solved-post.gjs), but due to "cooking quotes", after some internal discussion (t/95318/25) we will stick to widgets for now as the post-stream gets modernized.

Author: nattsw

assets/javascripts/discourse/components/solved-accept-answer-button.gjs

@@ -19,7 +19,7 @@ export default class SolvedAcceptAnswerButton extends Component {
 
   @action
   acceptAnswer() {
-    acceptAnswer(this.args.post, this.appEvents);
+    acceptAnswer(this.args.post, this.appEvents, this.currentUser);
   }
 
   <template>
@@ -34,9 +34,9 @@ export default class SolvedAcceptAnswerButton extends Component {
   </template>
 }
 
-export function acceptAnswer(post, appEvents) {
+export function acceptAnswer(post, appEvents, acceptingUser) {
   // TODO (glimmer-post-menu): Remove this exported function and move the code into the button action after the widget code is removed
-  acceptPost(post);
+  acceptPost(post, acceptingUser);
 
   appEvents.trigger("discourse-solved:solution-toggled", post);
 
@@ -46,7 +46,7 @@ export function acceptAnswer(post, appEvents) {
   });
 }
 
-function acceptPost(post) {
+function acceptPost(post, acceptingUser) {
   const topic = post.topic;
 
   clearAccepted(topic);
@@ -62,6 +62,8 @@ function acceptPost(post) {
     name: post.name,
     post_number: post.post_number,
     excerpt: post.cooked,
+    accepter_username: acceptingUser.username,
+    accepter_name: acceptingUser.name,
   });
 
   ajax("/solution/accept", {

assets/javascripts/discourse/initializers/extend-for-solved-button.js

@@ -36,38 +36,32 @@ function initializeWithApi(api) {
         const topic = postModel.topic;
         if (topic.accepted_answer) {
           const hasExcerpt = !!topic.accepted_answer.excerpt;
-
-          const withExcerpt = `
-            <aside class='quote accepted-answer' data-post="${
-              topic.get("accepted_answer").post_number
-            }" data-topic="${topic.id}">
-              <div class='title'>
-                ${topic.acceptedAnswerHtml} <div class="quote-controls"><\/div>
+          const excerpt = hasExcerpt
+            ? ` <blockquote> ${topic.accepted_answer.excerpt} </blockquote> `
+            : "";
+          const solvedQuote = `
+            <aside class='quote accepted-answer' data-post="${topic.get("accepted_answer").post_number}" data-topic="${topic.id}">
+              <div class='title ${hasExcerpt ? "" : "title-only"}'>
+                <div class="accepted-answer--solver">
+                  ${topic.solvedByHtml}
+                <\/div>
+                <div class="accepted-answer--accepter">
+                  ${topic.accepterHtml}
+                <\/div>
+                <div class="quote-controls"><\/div>
               </div>
-              <blockquote>
-                ${topic.accepted_answer.excerpt}
-              </blockquote>
+              ${excerpt}
             </aside>`;
 
-          const withoutExcerpt = `
-            <aside class='quote accepted-answer'>
-              <div class='title title-only'>
-                ${topic.acceptedAnswerHtml}
-              </div>
-            </aside>`;
-
-          const cooked = new PostCooked(
-            { cooked: hasExcerpt ? withExcerpt : withoutExcerpt },
-            dec
-          );
+          const cooked = new PostCooked({ cooked: solvedQuote }, dec);
           return dec.rawHtml(cooked.init());
         }
       }
     }
   });
 
   api.attachWidgetAction("post", "acceptAnswer", function () {
-    acceptAnswer(this.model, this.appEvents);
+    acceptAnswer(this.model, this.appEvents, this.currentUser);
   });
 
   api.attachWidgetAction("post", "unacceptAnswer", function () {
@@ -173,7 +167,7 @@ export default {
   initialize() {
     Topic.reopen({
       // keeping this here cause there is complex localization
-      acceptedAnswerHtml: computed("accepted_answer", "id", function () {
+      solvedByHtml: computed("accepted_answer", "id", function () {
         const username = this.get("accepted_answer.username");
         const name = this.get("accepted_answer.name");
         const postNumber = this.get("accepted_answer.post_number");
@@ -196,6 +190,18 @@ export default {
           user_path: User.create({ username }).path,
         });
       }),
+      accepterHtml: computed("accepted_answer", function () {
+        const username = this.get("accepted_answer.accepter_username");
+        const name = this.get("accepted_answer.accepter_name");
+        const formattedUsername =
+          this.siteSettings.display_name_on_posts && name
+            ? name
+            : formatUsername(username);
+        return i18n("solved.marked_solved_by", {
+          username: formattedUsername,
+          username_lower: username.toLowerCase(),
+        });
+      }),
     });
 
     withPluginApi("2.0.0", (api) => {

assets/stylesheets/solutions.scss

@@ -62,8 +62,37 @@ $solved-color: green;
   font-size: 13px;
 }
 
-aside.quote .title.title-only {
-  padding: 12px;
+aside.quote.accepted-answer {
+  .title {
+    display: flex;
+    flex-wrap: wrap;
+
+    &.title-only {
+      padding: 12px;
+    }
+  }
+
+  .accepted-answer--solver {
+    margin-right: auto;
+  }
+
+  .accepted-answer--accepter {
+    font-size: var(--font-down-1);
+    margin-left: auto;
+    margin-top: auto;
+  }
+
+  @media screen and (max-width: 768px) {
+    .accepted-answer--accepter {
+      width: 100%;
+      margin-top: 0.25em;
+      order: 3;
+    }
+
+    .quote-controls {
+      order: 2;
+    }
+  }
 }
 
 .user-card-metadata-outlet.accepted-answers {

config/locales/client.en.yml

@@ -33,6 +33,7 @@ en:
       no_solved_topics_title: "You haven’t solved any topics yet"
       no_solved_topics_title_others: "%{username} has not solved any topics yet"
       no_solved_topics_body: "When you provide a helpful reply to a topic, your reply might be selected as the solution by the topic owner or staff."
+      marked_solved_by: "Marked as solved by <a href data-user-card='%{username_lower}'>%{username}</a></span>"
 
       no_answer:
         title: Has your question been answered?

lib/discourse_solved/topic_view_serializer_extension.rb

@@ -6,43 +6,46 @@ module DiscourseSolved::TopicViewSerializerExtension
   prepended { attributes :accepted_answer }
 
   def include_accepted_answer?
-    SiteSetting.solved_enabled? && accepted_answer_post_id
+    SiteSetting.solved_enabled? && object.topic.solved.present?
   end
 
   def accepted_answer
-    if info = accepted_answer_post_info
-      { post_number: info[0], username: info[1], excerpt: info[2], name: info[3] }
-    end
+    accepted_answer_post_info
   end
 
   private
 
   def accepted_answer_post_info
-    post_info =
-      if post = object.posts.find { |p| p.post_number == accepted_answer_post_id }
-        [post.post_number, post.user.username, post.cooked, post.user.name]
-      else
-        Post
-          .where(id: accepted_answer_post_id, topic_id: object.topic.id)
-          .joins(:user)
-          .pluck("post_number", "username", "cooked", "name")
-          .first
-      end
-
-    if post_info
-      post_info[2] = if SiteSetting.solved_quote_length > 0
-        PrettyText.excerpt(post_info[2], SiteSetting.solved_quote_length, keep_emoji_images: true)
+    solved = object.topic.solved
+    answer_post = solved.answer_post
+    answer_post_user = answer_post.user
+    accepter = solved.accepter
+
+    excerpt =
+      if SiteSetting.solved_quote_length > 0
+        PrettyText.excerpt(
+          answer_post.cooked,
+          SiteSetting.solved_quote_length,
+          keep_emoji_images: true,
+        )
       else
         nil
       end
 
-      post_info[3] = nil if !SiteSetting.enable_names || !SiteSetting.display_name_on_posts
-
-      post_info
+    accepted_answer = {
+      post_number: answer_post.post_number,
+      username: answer_post_user.username,
+      name: answer_post_user.name,
+      accepter_username: accepter.username,
+      accepter_name: accepter.name,
+      excerpt:,
+    }
+
+    if !SiteSetting.enable_names || !SiteSetting.display_name_on_posts
+      accepted_answer[:name] = nil
+      accepted_answer[:accepter_name] = nil
     end
-  end
 
-  def accepted_answer_post_id
-    object.topic.solved&.answer_post_id
+    accepted_answer
   end
 end

spec/requests/topics_controller_spec.rb

@@ -65,18 +65,21 @@ def schema_json(answerCount)
 
     it "should include user name in output with the corresponding site setting" do
       SiteSetting.display_name_on_posts = true
-      Fabricate(:solved_topic, topic: topic, answer_post: p2)
+      accepter = Fabricate(:user)
+      Fabricate(:solved_topic, topic: topic, answer_post: p2, accepter:)
 
       get "/t/#{topic.slug}/#{topic.id}.json"
 
       expect(response.parsed_body["accepted_answer"]["name"]).to eq(p2.user.name)
       expect(response.parsed_body["accepted_answer"]["username"]).to eq(p2.user.username)
+      expect(response.parsed_body["accepted_answer"]["accepter_name"]).to eq(accepter.name)
+      expect(response.parsed_body["accepted_answer"]["accepter_username"]).to eq(accepter.username)
 
       # enable_names is default ON, this ensures disabling it also disables names here
       SiteSetting.enable_names = false
       get "/t/#{topic.slug}/#{topic.id}.json"
       expect(response.parsed_body["accepted_answer"]["name"]).to eq(nil)
-      expect(response.parsed_body["accepted_answer"]["username"]).to eq(p2.user.username)
+      expect(response.parsed_body["accepted_answer"]["accepter_name"]).to eq(nil)
     end
 
     it "should not include user name when site setting is disabled" do

spec/system/solved_spec.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+describe "About page", 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") }
+  let(:topic_page) { PageObjects::Pages::Topic.new }
+
+  before do
+    SiteSetting.solved_enabled = true
+    SiteSetting.allow_solved_on_all_topics = true
+    SiteSetting.accept_all_solutions_allowed_groups = Group::AUTO_GROUPS[:everyone]
+  end
+
+  it "accepts post as solution and shows in OP" do
+    sign_in(accepter)
+
+    topic_page.visit_topic(topic, post_number: 2)
+
+    expect(topic_page).to have_css(".post-action-menu__solved-unaccepted")
+
+    find(".post-action-menu__solved-unaccepted").click
+
+    expect(topic_page).to have_css(".post-action-menu__solved-accepted")
+    expect(topic_page.find(".title .accepted-answer--solver")).to have_content(
+      "Solved by #{solver.username}",
+    )
+    expect(topic_page.find(".title .accepted-answer--accepter")).to have_content(
+      "Marked as solved by #{accepter.username}",
+    )
+    expect(topic_page.find("blockquote")).to have_content("The answer is 42")
+  end
+end

test/javascripts/helpers/discourse-solved-helpers.js

@@ -199,6 +199,12 @@ export const postStreamWithAcceptedAnswerExcerpt = (excerpt) => {
     featured_link: null,
     topic_timer: null,
     message_bus_last_id: 0,
-    accepted_answer: { post_number: 2, username: "kzh", excerpt },
+    accepted_answer: {
+      post_number: 2,
+      username: "kzh",
+      excerpt,
+      accepter_username: "tomtom",
+      accepter_name: "Tomtom",
+    },
   };
 };