diff --git a/app/assets/javascripts/discourse/app/lib/quote.js b/app/assets/javascripts/discourse/app/lib/quote.js index d965833a6ee..1a035c4bea8 100644 --- a/app/assets/javascripts/discourse/app/lib/quote.js +++ b/app/assets/javascripts/discourse/app/lib/quote.js @@ -1,3 +1,6 @@ +import { prioritizeNameFallback } from "discourse/lib/settings"; +import { helperContext } from "discourse-common/lib/helpers"; + export const QUOTE_REGEXP = /\[quote=([^\]]*)\]((?:[\s\S](?!\[quote=[^\]]*\]))*?)\[\/quote\]/im; // Build the BBCode quote around the selected text @@ -6,8 +9,13 @@ export function buildQuote(post, contents, opts = {}) { return ""; } + const name = prioritizeNameFallback( + post.name, + opts.username || post.username + ); + const params = [ - opts.username || post.username, + name, `post:${opts.post || post.post_number}`, `topic:${opts.topic || post.topic_id}`, ]; @@ -15,6 +23,13 @@ export function buildQuote(post, contents, opts = {}) { if (opts.full) { params.push("full:true"); } + if ( + helperContext().siteSettings.display_name_on_posts && + !helperContext().siteSettings.prioritize_username_in_ux && + post.name + ) { + params.push(`username:${opts.username || post.username}`); + } return `[quote="${params.join(", ")}"]\n${contents.trim()}\n[/quote]\n\n`; } diff --git a/app/assets/javascripts/discourse/app/lib/settings.js b/app/assets/javascripts/discourse/app/lib/settings.js index fe3da6cfd3a..5450941c782 100644 --- a/app/assets/javascripts/discourse/app/lib/settings.js +++ b/app/assets/javascripts/discourse/app/lib/settings.js @@ -8,6 +8,18 @@ export function prioritizeNameInUx(name) { ); } +export function prioritizeNameFallback(name, username) { + let siteSettings = helperContext().siteSettings; + if ( + siteSettings.display_name_on_posts && + !siteSettings.prioritize_username_in_ux + ) { + return name || username; + } else { + return username; + } +} + export function emojiBasePath() { let siteSettings = helperContext().siteSettings; diff --git a/app/assets/javascripts/discourse/app/lib/transform-post.js b/app/assets/javascripts/discourse/app/lib/transform-post.js index ff5516da228..0709b17ab23 100644 --- a/app/assets/javascripts/discourse/app/lib/transform-post.js +++ b/app/assets/javascripts/discourse/app/lib/transform-post.js @@ -74,6 +74,7 @@ export function transformBasicPost(post) { actionsSummary: null, read: post.read, replyToUsername: null, + replyToName: null, replyToAvatarTemplate: null, reply_to_post_number: post.reply_to_post_number, cooked_hidden: !!post.cooked_hidden, @@ -228,6 +229,7 @@ export default function transformPost( const replyToUser = post.get("reply_to_user"); if (replyToUser) { postAtts.replyToUsername = replyToUser.username; + postAtts.replyToName = replyToUser.name; postAtts.replyToAvatarTemplate = replyToUser.avatar_template; } diff --git a/app/assets/javascripts/discourse/app/models/composer.js b/app/assets/javascripts/discourse/app/models/composer.js index 68566db1cb1..d5bed57f0f5 100644 --- a/app/assets/javascripts/discourse/app/models/composer.js +++ b/app/assets/javascripts/discourse/app/models/composer.js @@ -23,6 +23,7 @@ import deprecated from "discourse-common/lib/deprecated"; import { isEmpty } from "@ember/utils"; import { propertyNotEqual } from "discourse/lib/computed"; import { throwAjaxError } from "discourse/lib/ajax-error"; +import { prioritizeNameFallback } from "discourse/lib/settings"; let _customizations = []; export function registerCustomizationCallback(cb) { @@ -362,9 +363,11 @@ const Composer = RestModel.extend({ anchor: I18n.t("post.post_number", { number: postNumber }), }; + const name = prioritizeNameFallback(post.name, post.username); + options.userLink = { href: `${topic.url}/${postNumber}`, - anchor: post.username, + anchor: name, }; } diff --git a/app/assets/javascripts/discourse/app/widgets/post.js b/app/assets/javascripts/discourse/app/widgets/post.js index 3ba650dd683..a6f58ae04e7 100644 --- a/app/assets/javascripts/discourse/app/widgets/post.js +++ b/app/assets/javascripts/discourse/app/widgets/post.js @@ -17,7 +17,10 @@ import { h } from "virtual-dom"; import hbs from "discourse/widgets/hbs-compiler"; import { iconNode } from "discourse-common/lib/icon-library"; import { postTransformCallbacks } from "discourse/widgets/post-stream"; -import { prioritizeNameInUx } from "discourse/lib/settings"; +import { + prioritizeNameFallback, + prioritizeNameInUx, +} from "discourse/lib/settings"; import { relativeAgeMediumSpan } from "discourse/lib/formatter"; import { transformBasicPost } from "discourse/lib/transform-post"; import autoGroupFlairForUser from "discourse/lib/avatar-flair"; @@ -140,16 +143,20 @@ createWidget("reply-to-tab", { html(attrs, state) { const icon = state.loading ? h("div.spinner.small") : iconNode("share"); + const name = prioritizeNameFallback( + attrs.replyToName, + attrs.replyToUsername + ); return [ icon, " ", avatarImg("small", { template: attrs.replyToAvatarTemplate, - username: attrs.replyToUsername, + username: name, }), " ", - h("span", formatUsername(attrs.replyToUsername)), + h("span", formatUsername(name)), ]; }, 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 acc54f4a38e..aecda8fe687 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/composer-actions-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/composer-actions-test.js @@ -4,6 +4,7 @@ import { exists, query, queryAll, + selectText, updateCurrentUser, } from "discourse/tests/helpers/qunit-helpers"; import { click, fillIn, visit } from "@ember/test-helpers"; @@ -18,7 +19,11 @@ import { toggleCheckDraftPopup } from "discourse/controllers/composer"; acceptance("Composer Actions", function (needs) { needs.user(); - needs.settings({ enable_whispers: true }); + needs.settings({ + prioritize_username_in_ux: true, + display_name_on_post: false, + enable_whispers: true, + }); needs.site({ can_tag_topics: true }); test("creating new topic and then reply_as_private_message keeps attributes", async function (assert) { @@ -551,3 +556,78 @@ acceptance("Composer Actions With New Topic Draft", function (needs) { sinon.restore(); }); }); + +acceptance("Prioritize Username", function (needs) { + needs.user(); + needs.settings({ + prioritize_username_in_ux: true, + display_name_on_post: false, + }); + + test("Reply to post use username", async function (assert) { + await visit("/t/short-topic-with-two-posts/54079"); + await click("article#post_2 button.reply"); + + assert.strictEqual( + queryAll(".action-title .user-link").text().trim(), + "james_john" + ); + }); + + test("Quotes use username", async function (assert) { + await visit("/t/short-topic-with-two-posts/54079"); + await selectText("#post_2 p"); + await click(".insert-quote"); + assert.strictEqual( + queryAll(".d-editor-input").val().trim(), + '[quote="james_john, post:2, topic:54079, full:true"]\nThis is a short topic.\n[/quote]' + ); + }); +}); + +acceptance("Prioritize Full Name", function (needs) { + needs.user(); + needs.settings({ + prioritize_username_in_ux: false, + display_name_on_post: true, + }); + + test("Reply to post use full name", async function (assert) { + await visit("/t/short-topic-with-two-posts/54079"); + await click("article#post_2 button.reply"); + + assert.strictEqual( + queryAll(".action-title .user-link").text().trim(), + "james, john, the third" + ); + }); + + test("Quotes use full name", async function (assert) { + await visit("/t/short-topic-with-two-posts/54079"); + await selectText("#post_2 p"); + await click(".insert-quote"); + assert.strictEqual( + queryAll(".d-editor-input").val().trim(), + '[quote="james, john, the third, post:2, topic:54079, full:true, username:james_john"]\nThis is a short topic.\n[/quote]' + ); + }); +}); + +acceptance("Prioritizing Name fall back", function (needs) { + needs.user(); + needs.settings({ + prioritize_username_in_ux: false, + display_name_on_post: true, + }); + + test("Quotes fall back to username if name is not present", async function (assert) { + await visit("/t/internationalization-localization/130"); + // select a user with no name + await selectText("#post_1 p"); + await click(".insert-quote"); + assert.strictEqual( + queryAll(".d-editor-input").val().trim(), + '[quote="bianca, post:1, topic:130, full:true"]\nLorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas a varius ipsum. Nunc euismod, metus non vulputate malesuada, ligula metus pharetra tortor, vel sodales arcu lacus sed mauris. Nam semper, orci vitae fringilla placerat, dui tellus convallis felis, ultricies laoreet sapien mi et metus. Mauris facilisis, mi fermentum rhoncus feugiat, dolor est vehicula leo, id porta leo ex non enim. In a ligula vel tellus commodo scelerisque non in ex. Pellentesque semper leo quam, nec varius est viverra eget. Donec vehicula sem et massa faucibus tempus.\n[/quote]' + ); + }); +}); diff --git a/app/assets/javascripts/discourse/tests/acceptance/topic-test.js b/app/assets/javascripts/discourse/tests/acceptance/topic-test.js index bbbbeaf5fa9..83beb16e6cb 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/topic-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/topic-test.js @@ -263,6 +263,8 @@ acceptance("Topic featured links", function (needs) { topic_featured_link_enabled: true, max_topic_title_length: 80, exclude_rel_nofollow_domains: "example.com", + display_name_on_posts: false, + prioritize_username_in_ux: true, }); test("remove nofollow attribute", async function (assert) { diff --git a/app/assets/javascripts/discourse/tests/fixtures/topic.js b/app/assets/javascripts/discourse/tests/fixtures/topic.js index 909483a1b15..d7067a18e69 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/topic.js +++ b/app/assets/javascripts/discourse/tests/fixtures/topic.js @@ -6441,4 +6441,298 @@ export default { ], tags: null, }, + "/t/54079.json": { + pending_posts: [], + post_stream: { + posts: [ + { + id: 398, + name: "james, john, the third", + username: "james_john", + avatar_template: "/images/avatar.png", + uploaded_avatar_id: 5697, + created_at: "2013-02-05T21:29:00.280Z", + cooked: "
This is a short topic.
", + post_number: 1, + post_type: 1, + updated_at: "2013-02-05T21:29:00.280Z", + like_count: 0, + reply_count: 1, + reply_to_post_number: null, + quote_count: 0, + incoming_link_count: 314, + reads: 475, + score: 1702.25, + yours: false, + topic_id: 54079, + topic_slug: "short-topic-with-two-posts", + display_username: "james, john, the third", + primary_group_name: null, + version: 1, + can_edit: true, + can_delete: false, + can_recover: true, + link_counts: [], + read: true, + user_title: null, + actions_summary: [ + { + id: 2, + count: 0, + hidden: false, + can_act: true, + }, + ], + moderator: false, + admin: false, + staff: false, + user_id: 255, + 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, + }, + { + id: 410, + name: "james, john, the third", + username: "james_john", + avatar_template: "/images/avatar.png", + uploaded_avatar_id: 5697, + created_at: "2013-02-05T21:29:00.280Z", + cooked: "This is a short topic.
", + post_number: 2, + post_type: 1, + updated_at: "2013-02-05T21:29:00.280Z", + like_count: 0, + reply_count: 1, + reply_to_post_number: null, + quote_count: 0, + incoming_link_count: 314, + reads: 475, + score: 1702.25, + yours: false, + topic_id: 54079, + topic_slug: "short-topic-with-two-posts", + display_username: "james, john, the third", + primary_group_name: null, + version: 1, + can_edit: true, + can_delete: false, + can_recover: true, + link_counts: [], + read: true, + user_title: null, + actions_summary: [ + { + id: 2, + count: 0, + hidden: false, + can_act: true, + }, + ], + moderator: false, + admin: false, + staff: false, + user_id: 255, + 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, + }, + { + id: 419, + name: "Tim Stone", + username: "tms", + avatar_template: "/images/avatar.png", + uploaded_avatar_id: 40181, + created_at: "2013-02-05T21:32:47.649Z", + cooked: "Yeah it is a short one
", + post_number: 3, + post_type: 1, + updated_at: "2013-02-06T10:15:27.965Z", + like_count: 4, + reply_count: 0, + reply_to_post_number: null, + quote_count: 0, + 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] } }, + }, + id: 54079, + title: "Short topic with two posts", + fancy_title: "Short topic with two posts", + posts_count: 2, + created_at: "2013-02-05T21:29:00.174Z", + views: 5211, + reply_count: 1, + participant_count: 2, + like_count: 135, + last_posted_at: "2015-03-04T15:07:10.487Z", + visible: true, + closed: false, + archived: false, + has_summary: true, + archetype: "regular", + slug: "short-topic-with-two-posts", + category_id: 2, + word_count: 300, + deleted_at: null, + draft: null, + draft_key: "topic_54079", + draft_sequence: 3, + posted: true, + unpinned: null, + pinned_globally: false, + pinned: false, + pinned_at: null, + details: { + can_publish_page: true, + can_invite_via_email: true, + can_toggle_topic_visibility: true, + can_pin_unpin_topic: true, + auto_close_at: null, + auto_close_hours: null, + auto_close_based_on_last_post: false, + created_by: { + id: 255, + username: "james_john", + uploaded_avatar_id: 5697, + avatar_template: "/images/avatar.png", + }, + last_poster: { + id: 9, + username: "Tim Stone", + uploaded_avatar_id: 40181, + avatar_template: "/images/avatar.png", + }, + participants: [ + { + id: 9, + username: "tms", + uploaded_avatar_id: 40181, + avatar_template: "/images/avatar.png", + post_count: 2, + }, + { + id: 255, + username: "james_john", + uploaded_avatar_id: 5697, + avatar_template: "/images/avatar.png", + post_count: 1, + }, + ], + links: [], + notification_level: 2, + notifications_reason_id: 4, + can_move_posts: true, + can_edit: true, + can_delete: true, + can_recover: true, + can_remove_allowed_users: true, + can_invite_to: true, + can_create_post: true, + can_reply_as_new_topic: true, + can_flag_topic: true, + }, + highest_post_number: 2, + last_read_post_number: 2, + deleted_by: null, + has_deleted: true, + actions_summary: [ + { id: 4, count: 0, hidden: false, can_act: true }, + { id: 7, count: 0, hidden: false, can_act: true }, + { id: 8, count: 0, hidden: false, can_act: true }, + ], + chunk_size: 20, + bookmarked: false, + bookmarks: [], + suggested_topics: [ + { + id: 27331, + title: "Polls are still very buggy", + fancy_title: "Polls are still very buggy", + slug: "polls-are-still-very-buggy", + posts_count: 4, + reply_count: 1, + highest_post_number: 4, + image_url: "/uploads/default/_optimized/cd1/b8c/c162528887_690x401.png", + created_at: "2015-04-08T09:51:00.357Z", + last_posted_at: "2015-04-08T15:59:16.258Z", + bumped: true, + bumped_at: "2015-04-08T16:05:09.842Z", + unseen: false, + last_read_post_number: 3, + unread_posts: 1, + pinned: false, + unpinned: null, + visible: true, + closed: false, + archived: false, + notification_level: 2, + bookmarked: false, + bookmarks: [], + liked: false, + archetype: "regular", + like_count: 11, + views: 55, + category_id: 1, + posters: [ + { + extras: "latest single", + description: "Original Poster, Most Recent Poster", + user: { + id: 1, + username: "test", + avatar_template: "/images/avatar.png", + }, + }, + ], + }, + ], + tags: null, + }, }; diff --git a/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js b/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js index 827b01f5dea..630457df323 100644 --- a/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js +++ b/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js @@ -22,6 +22,8 @@ const rawOpts = { default_code_lang: "auto", enable_markdown_linkify: true, markdown_linkify_tlds: "com", + display_name_on_posts: false, + prioritize_username_in_ux: true, }, getURL: (url) => url, }; diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown/quotes.js b/app/assets/javascripts/pretty-text/engines/discourse-markdown/quotes.js index 037685f8236..5bd51dfb905 100644 --- a/app/assets/javascripts/pretty-text/engines/discourse-markdown/quotes.js +++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown/quotes.js @@ -36,6 +36,23 @@ const rule = { full = true; continue; } + + // if we have the additional attribute of username: because we are prioritizing full name + // then assign the name to be the displayName + if (split[i].indexOf("username:") === 0) { + // return users name by selecting all values from the first index to the post + // this protects us from when a user has a `,` in their name + displayName = split.slice(0, split.indexOf(`post:${postNumber}`)); + + // preserve `,` in a users name if they exist + if (displayName.length > 1) { + displayName = displayName.join(", "); + } + + // strip key of 'username:' and return username + username = split[i].slice(9); + continue; + } } } @@ -59,9 +76,9 @@ const rule = { } if (options.formatUsername) { - displayName = options.formatUsername(username); + displayName = displayName || options.formatUsername(username); } else { - displayName = username; + displayName = displayName || username; } let token = state.push("bbcode_open", "aside", 1); diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index c03537c7c74..b604b1b7faf 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -257,6 +257,7 @@ class PostSerializer < BasicPostSerializer def reply_to_user { username: object.reply_to_user.username, + name: object.reply_to_user.name, avatar_template: object.reply_to_user.avatar_template } end diff --git a/spec/lib/cooked_post_processor_spec.rb b/spec/lib/cooked_post_processor_spec.rb index 7800930a8c0..5f0718325b2 100644 --- a/spec/lib/cooked_post_processor_spec.rb +++ b/spec/lib/cooked_post_processor_spec.rb @@ -1870,6 +1870,161 @@ describe CookedPostProcessor do end end + context "full quote on direct reply with full name prioritization" do + fab!(:user) { Fabricate(:user, name: "james, john, the third") } + fab!(:topic) { Fabricate(:topic) } + let!(:post) { Fabricate(:post, user: user, topic: topic, raw: 'this is the "first" post') } + + let(:raw) do + <<~RAW.strip + [quote="#{post.user.name}, post:#{post.post_number}, topic:#{topic.id}, username:#{post.user.username}"] + + this is the “first” post + + [/quote] + + and this is the third reply + RAW + end + + let(:raw2) do + <<~RAW.strip + and this is the third reply + + [quote="#{post.user.name}, post:#{post.post_number}, topic:#{topic.id}, username:#{post.user.username}"] + this is the ”first” post + [/quote] + RAW + end + + let(:raw3) do + <<~RAW.strip + [quote="#{post.user.name}, post:#{post.post_number}, topic:#{topic.id}, username:#{post.user.username}"] + + this is the “first” post + + [/quote] + + [quote="#{post.user.name}, post:#{post.post_number}, topic:#{topic.id}, username:#{post.user.username}"] + + this is the “first” post + + [/quote] + + and this is the third reply + RAW + end + + before do + SiteSetting.remove_full_quote = true + SiteSetting.display_name_on_posts = true + SiteSetting.prioritize_username_in_ux = false + end + + it 'removes direct reply with full quotes' do + hidden = Fabricate(:post, topic: topic, hidden: true, raw: "this is the second post after") + small_action = Fabricate(:post, topic: topic, post_type: Post.types[:small_action]) + reply = Fabricate(:post, topic: topic, raw: raw) + + freeze_time do + topic.bumped_at = 1.day.ago + CookedPostProcessor.new(reply).remove_full_quote_on_direct_reply + + expect(topic.ordered_posts.pluck(:id)) + .to eq([post.id, hidden.id, small_action.id, reply.id]) + + expect(topic.bumped_at).to eq_time(1.day.ago) + expect(reply.raw).to eq("and this is the third reply") + expect(reply.revisions.count).to eq(1) + expect(reply.revisions.first.modifications["raw"]).to eq([raw, reply.raw]) + expect(reply.revisions.first.modifications["edit_reason"][1]).to eq(I18n.t(:removed_direct_reply_full_quotes)) + end + end + + it 'does nothing if there are multiple quotes' do + reply = Fabricate(:post, topic: topic, raw: raw3) + CookedPostProcessor.new(reply).remove_full_quote_on_direct_reply + expect(topic.ordered_posts.pluck(:id)).to eq([post.id, reply.id]) + expect(reply.raw).to eq(raw3) + end + + it 'does not delete quote if not first paragraph' do + reply = Fabricate(:post, topic: topic, raw: raw2) + CookedPostProcessor.new(reply).remove_full_quote_on_direct_reply + expect(topic.ordered_posts.pluck(:id)).to eq([post.id, reply.id]) + expect(reply.raw).to eq(raw2) + end + + it "does nothing when 'remove_full_quote' is disabled" do + SiteSetting.remove_full_quote = false + + reply = Fabricate(:post, topic: topic, raw: raw) + + CookedPostProcessor.new(reply).remove_full_quote_on_direct_reply + expect(reply.raw).to eq(raw) + end + + it "does not generate a blank HTML document" do + post = Fabricate(:post, topic: topic, raw: "