Skip to content

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

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

Merged
merged 2 commits into from
Mar 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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);

Expand All @@ -46,7 +46,7 @@ export function acceptAnswer(post, appEvents) {
});
}

function acceptPost(post) {
function acceptPost(post, acceptingUser) {
const topic = post.topic;

clearAccepted(topic);
Expand All @@ -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", {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The need for functionality within PostCooked here is why we can't yet switch to a glimmer component, so we will just update the widget as-is for now.

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 () {
Expand Down Expand Up @@ -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");
Expand All @@ -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) => {
Expand Down
33 changes: 31 additions & 2 deletions assets/stylesheets/solutions.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions config/locales/client.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
51 changes: 27 additions & 24 deletions lib/discourse_solved/topic_view_serializer_extension.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +44 to +46
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is tied to my other comment. We are already handling some of the logic here so could centralize them in a single place.

end
end

def accepted_answer_post_id
object.topic.solved&.answer_post_id
accepted_answer
end
end
7 changes: 5 additions & 2 deletions spec/requests/topics_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 35 additions & 0 deletions spec/system/solved_spec.rb
Original file line number Diff line number Diff line change
@@ -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
8 changes: 7 additions & 1 deletion test/javascripts/helpers/discourse-solved-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
};
};