From 29416b8406990f28ebcd137d3513d18fd3303b16 Mon Sep 17 00:00:00 2001 From: Nat Date: Fri, 21 Mar 2025 23:38:47 +0800 Subject: [PATCH] FEATURE: Show 'marked as solved by' in OP when a topic is solved --- .../discourse/components/solved-post.gjs | 100 ++++++++++++++++++ .../initializers/extend-for-solved-button.js | 72 +------------ assets/stylesheets/solutions.scss | 33 +++++- config/locales/client.en.yml | 1 + .../topic_view_serializer_extension.rb | 47 ++++---- spec/requests/topics_controller_spec.rb | 7 +- .../integratin/discourse-solved-post-test.js | 89 ++++++++++++++++ 7 files changed, 254 insertions(+), 95 deletions(-) create mode 100644 assets/javascripts/discourse/components/solved-post.gjs create mode 100644 test/javascripts/integratin/discourse-solved-post-test.js diff --git a/assets/javascripts/discourse/components/solved-post.gjs b/assets/javascripts/discourse/components/solved-post.gjs new file mode 100644 index 0000000..0ddd09c --- /dev/null +++ b/assets/javascripts/discourse/components/solved-post.gjs @@ -0,0 +1,100 @@ +import Component from "@glimmer/component"; +import { service } from "@ember/service"; +import { htmlSafe } from "@ember/template"; +import { not } from "truth-helpers"; +import concatClass from "discourse/helpers/concat-class"; +import { iconHTML } from "discourse/lib/icon-library"; +import { formatUsername } from "discourse/lib/utilities"; +import User from "discourse/models/user"; +import { i18n } from "discourse-i18n"; + +export default class SolvedPost extends Component { + static shouldRender(args) { + return args.post?.post_number === 1 && args.post?.topic?.accepted_answer; + } + + @service siteSettings; + + get answerPostPath() { + return `${this.args.outletArgs.post.topic.url}/${this.answerPostNumber}`; + } + + get acceptedAnswer() { + return this.args.outletArgs.post.topic.accepted_answer; + } + + get answerPostNumber() { + return this.acceptedAnswer?.post_number; + } + + get topicId() { + return this.args.outletArgs.post.topic.id; + } + + get hasExcerpt() { + return !!this.solvedExcerpt; + } + + get solvedExcerpt() { + return this.acceptedAnswer?.excerpt; + } + + get username() { + return this.acceptedAnswer?.username; + } + + get displayedUser() { + const { name, username } = this.acceptedAnswer || {}; + return this.siteSettings.display_name_on_posts && name + ? name + : formatUsername(username); + } + + get title() { + return i18n("solved.accepted_html", { + icon: iconHTML("square-check", { class: "accepted" }), + username_lower: this.username?.toLowerCase(), + username: this.displayedUser, + post_path: this.answerPostPath, + post_number: this.answerPostNumber, + user_path: User.create({ username: this.username }).path, + }); + } + + get accepter() { + const accepterUsername = this.acceptedAnswer?.accepter_username; + const accepterName = this.acceptedAnswer?.accepter_name; + const formattedUsername = this.siteSettings.display_name_on_posts && accepterName + ? accepterName + : formatUsername(accepterUsername); + return i18n("solved.marked_solved_by", { + username: formattedUsername, + username_lower: accepterUsername.toLowerCase() + }); + } + + +} diff --git a/assets/javascripts/discourse/initializers/extend-for-solved-button.js b/assets/javascripts/discourse/initializers/extend-for-solved-button.js index 3130262..e0d4a70 100644 --- a/assets/javascripts/discourse/initializers/extend-for-solved-button.js +++ b/assets/javascripts/discourse/initializers/extend-for-solved-button.js @@ -1,16 +1,12 @@ -import { computed } from "@ember/object"; import discourseComputed from "discourse/lib/decorators"; import { withSilencedDeprecations } from "discourse/lib/deprecated"; -import { iconHTML, iconNode } from "discourse/lib/icon-library"; +import { iconNode } from "discourse/lib/icon-library"; import { withPluginApi } from "discourse/lib/plugin-api"; -import { formatUsername } from "discourse/lib/utilities"; -import Topic from "discourse/models/topic"; -import User from "discourse/models/user"; -import PostCooked from "discourse/widgets/post-cooked"; import { i18n } from "discourse-i18n"; import SolvedAcceptAnswerButton, { acceptAnswer, } from "../components/solved-accept-answer-button"; +import SolvedPost from "../components/solved-post"; import SolvedUnacceptAnswerButton, { unacceptAnswer, } from "../components/solved-unaccept-answer-button"; @@ -29,42 +25,7 @@ function initializeWithApi(api) { api.addDiscoveryQueryParam("solved", { replace: true, refreshModel: true }); } - api.decorateWidget("post-contents:after-cooked", (dec) => { - if (dec.attrs.post_number === 1) { - const postModel = dec.getModel(); - if (postModel) { - const topic = postModel.topic; - if (topic.accepted_answer) { - const hasExcerpt = !!topic.accepted_answer.excerpt; - - const withExcerpt = ` - `; - - const withoutExcerpt = ` - `; - - const cooked = new PostCooked( - { cooked: hasExcerpt ? withExcerpt : withoutExcerpt }, - dec - ); - return dec.rawHtml(cooked.init()); - } - } - } - }); + api.renderBeforeWrapperOutlet("post-menu", SolvedPost); api.attachWidgetAction("post", "acceptAnswer", function () { acceptAnswer(this.model, this.appEvents); @@ -171,33 +132,6 @@ function customizeWidgetPostMenu(api) { export default { name: "extend-for-solved-button", initialize() { - Topic.reopen({ - // keeping this here cause there is complex localization - acceptedAnswerHtml: 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"); - - if (!username || !postNumber) { - return ""; - } - - const displayedUser = - this.siteSettings.display_name_on_posts && name - ? name - : formatUsername(username); - - return i18n("solved.accepted_html", { - icon: iconHTML("square-check", { class: "accepted" }), - username_lower: username.toLowerCase(), - username: displayedUser, - post_path: `${this.url}/${postNumber}`, - post_number: postNumber, - user_path: User.create({ username }).path, - }); - }), - }); - withPluginApi("2.0.0", (api) => { withSilencedDeprecations("discourse.hbr-topic-list-overrides", () => { let topicStatusIcons; diff --git a/assets/stylesheets/solutions.scss b/assets/stylesheets/solutions.scss index 40888e0..fba5370 100644 --- a/assets/stylesheets/solutions.scss +++ b/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 { diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 167b92a..f1c36b8 100644 --- a/config/locales/client.en.yml +++ b/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 %{username}" no_answer: title: Has your question been answered? diff --git a/lib/discourse_solved/topic_view_serializer_extension.rb b/lib/discourse_solved/topic_view_serializer_extension.rb index 22807fe..f0b1c2d 100644 --- a/lib/discourse_solved/topic_view_serializer_extension.rb +++ b/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 + solved = object.topic.solved + answer_post = solved.answer_post + answer_post_user = answer_post.user + accepter = solved.accepter - 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) + 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 + 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:, + } - post_info + 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 diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 608f305..7d0633c 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -65,18 +65,21 @@ RSpec.describe TopicsController do 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 diff --git a/test/javascripts/integratin/discourse-solved-post-test.js b/test/javascripts/integratin/discourse-solved-post-test.js new file mode 100644 index 0000000..0c51703 --- /dev/null +++ b/test/javascripts/integratin/discourse-solved-post-test.js @@ -0,0 +1,89 @@ +import { render } from "@ember/test-helpers"; +import { hbs } from "ember-cli-htmlbars"; +import { module, test } from "qunit"; +import { setupRenderingTest } from "discourse/tests/helpers/component-test"; + +module("Integration | Component | solved-post", function (hooks) { + setupRenderingTest(hooks); + + test("renders solved post information", async function (assert) { + this.siteSettings = { + display_name_on_posts: true + }; + + const post = { + post_number: 1, + topic: { + id: 123, + url: "/t/topic/123", + accepted_answer: { + username: "solver", + name: "Solver Person", + post_number: 7, + excerpt: "This is the solution", + accepter_username: "accepter", + accepter_name: "Accepter Person" + } + } + }; + + this.set("outletArgs", { post }); + + await render(hbs``); + + assert.dom(".accepted-answer").exists("shows accepted answer section"); + assert.dom(".accepted-answer[data-post='7']").exists("has correct post number"); + assert.dom(".accepted-answer[data-topic='123']").exists("has correct topic id"); + assert.dom(".title .accepted-answer--solver").includesText("Solved by solver in post #7", "shows solver name"); + assert.dom(".title .accepted-answer--accepter").includesText("Marked as solved by accepter", "shows accepter name"); + assert.dom("blockquote").hasText("This is the solution", "shows excerpt"); + }); + + test("handles missing excerpt", async function (assert) { + const post = { + post_number: 1, + topic: { + id: 123, + accepted_answer: { + username: "solver", + post_number: 7, + accepter_username: "accepter" + } + } + }; + + this.set("outletArgs", { post }); + + await render(hbs``); + + assert.dom(".title").hasClass("title-only", "adds title-only class when no excerpt"); + assert.dom("blockquote").doesNotExist("doesn't show blockquote without excerpt"); + }); + + test("uses username when display_name_on_posts is false", async function (assert) { + this.siteSettings = { + display_name_on_posts: false + }; + + const post = { + post_number: 1, + topic: { + id: 123, + accepted_answer: { + username: "solver", + name: "Solver Person", + post_number: 7, + accepter_username: "accepter", + accepter_name: "Accepter Person" + } + } + }; + + this.set("outletArgs", { post }); + + await render(hbs``); + + assert.dom(".title .accepted-answer--solver").includesText("solver", "shows username"); + assert.dom(".title .accepted-answer--accepter").includesText("accepter", "shows accepter username"); + }); +});