From e0d7431292892324922cbf41e00febd8519fdda0 Mon Sep 17 00:00:00 2001 From: Isaac Janzen <50783505+janzenisaac@users.noreply.github.com> Date: Fri, 22 Apr 2022 10:20:24 -0500 Subject: [PATCH] FIX: Use username for nested quotes (#16523) There was an edge when a user re-quoted a nested quote that it would return an incorrect `full name` but the correct `username` for the original quote. This PR updates the logic to fall back to using the OP user's username. The complexity of the changes required to allow for full names to be displayed on nested quotes far outweighs how rare quoting nested quotes is. --- .../javascripts/discourse/app/lib/quote.js | 15 +++-- .../tests/acceptance/composer-actions-test.js | 10 ++++ .../discourse/tests/fixtures/topic.js | 58 ++++++++++++++++++- 3 files changed, 75 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/discourse/app/lib/quote.js b/app/assets/javascripts/discourse/app/lib/quote.js index 1a035c4bea8..0013508cab4 100644 --- a/app/assets/javascripts/discourse/app/lib/quote.js +++ b/app/assets/javascripts/discourse/app/lib/quote.js @@ -9,10 +9,15 @@ export function buildQuote(post, contents, opts = {}) { return ""; } - const name = prioritizeNameFallback( - post.name, - opts.username || post.username - ); + let fullName = post.name; + // if the quote username data attr is present but it does not + // match the post username then fallback to the quote username instead of fetching + // the full name from the post + if (opts.username && opts.username !== post.username) { + fullName = null; + } + + const name = prioritizeNameFallback(fullName, opts.username || post.username); const params = [ name, @@ -26,7 +31,7 @@ export function buildQuote(post, contents, opts = {}) { if ( helperContext().siteSettings.display_name_on_posts && !helperContext().siteSettings.prioritize_username_in_ux && - post.name + fullName ) { params.push(`username:${opts.username || post.username}`); } diff --git a/app/assets/javascripts/discourse/tests/acceptance/composer-actions-test.js b/app/assets/javascripts/discourse/tests/acceptance/composer-actions-test.js index aecda8fe687..d0bb7d62ab4 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/composer-actions-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/composer-actions-test.js @@ -611,6 +611,16 @@ acceptance("Prioritize Full Name", function (needs) { '[quote="james, john, the third, post:2, topic:54079, full:true, username:james_john"]\nThis is a short topic.\n[/quote]' ); }); + + test("Quoting a nested quote returns the correct username", async function (assert) { + await visit("/t/short-topic-with-two-posts/54079"); + await selectText("#post_4 p"); + await click(".insert-quote"); + assert.strictEqual( + queryAll(".d-editor-input").val().trim(), + '[quote="james_john, post:2, topic:54079"]\nThis is a short topic.\n[/quote]' + ); + }); }); acceptance("Prioritizing Name fall back", function (needs) { diff --git a/app/assets/javascripts/discourse/tests/fixtures/topic.js b/app/assets/javascripts/discourse/tests/fixtures/topic.js index d7067a18e69..59f37f91476 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/topic.js +++ b/app/assets/javascripts/discourse/tests/fixtures/topic.js @@ -6449,7 +6449,7 @@ export default { id: 398, name: "james, john, the third", username: "james_john", - avatar_template: "/images/avatar.png", + avatar_template: "/letter_avatar_proxy/v4/letter/j/3be4f8/{size}.png", uploaded_avatar_id: 5697, created_at: "2013-02-05T21:29:00.280Z", cooked: "

This is a short topic.

", @@ -6500,7 +6500,7 @@ export default { id: 410, name: "james, john, the third", username: "james_john", - avatar_template: "/images/avatar.png", + avatar_template: "/letter_avatar_proxy/v4/letter/j/3be4f8/{size}.png", uploaded_avatar_id: 5697, created_at: "2013-02-05T21:29:00.280Z", cooked: "

This is a short topic.

", @@ -6551,7 +6551,7 @@ export default { id: 419, name: "Tim Stone", username: "tms", - avatar_template: "/images/avatar.png", + avatar_template: "/letter_avatar_proxy/v4/letter/t/3be4f8/{size}.png", uploaded_avatar_id: 40181, created_at: "2013-02-05T21:32:47.649Z", cooked: "

Yeah it is a short one

", @@ -6598,6 +6598,58 @@ export default { can_view_edit_history: true, wiki: false, }, + { + id: 421, + name: "Tim Stone", + username: "tms", + avatar_template: "/letter_avatar_proxy/v4/letter/t/3be4f8/{size}.png", + uploaded_avatar_id: 40181, + created_at: "2013-02-05T21:32:47.649Z", + cooked: "

Yeah it is a short one

", + cooked: '

there is a quote above

', + post_number: 4, + post_type: 1, + updated_at: "2013-02-06T10:15:27.965Z", + like_count: 4, + reply_count: 0, + reply_to_post_number: null, + quote_count: 1, + incoming_link_count: 16, + reads: 460, + score: 308.35, + yours: false, + topic_id: 54079, + topic_slug: "short-topic-with-two-posts", + display_username: "Tim Stone", + primary_group_name: null, + version: 2, + can_edit: true, + can_delete: true, + can_recover: true, + link_counts: [], + read: true, + user_title: "Great contributor", + actions_summary: [ + { + id: 2, + count: 4, + hidden: false, + can_act: true, + }, + ], + moderator: false, + admin: false, + staff: false, + user_id: 9, + hidden: false, + hidden_reason_id: null, + trust_level: 2, + deleted_at: null, + user_deleted: false, + edit_reason: null, + can_view_edit_history: true, + wiki: false, + }, ], stream: [398, 419], gaps: { before: {}, after: { 398: [419] } },