DEV: Prioritize full name when display_name_on_posts active (#16078)

See: https://meta.discourse.org/t/display-full-name-not-username-when-attributing-quote-or-reply/203533?u=isaacjanzen for context

The initial release [broke quoting](https://meta.discourse.org/t/quoting-broken-when-name-matches-username/217633?u=isaacjanzen) but we now pass the username when 
```
siteSettings.display_name_on_posts && !siteSettings.prioritize_username_in_ux && post.name
```
as well as the full name to guarantee that we are not getting any mismatches when querying for user / avatar.

eg. 
```
[quote="Isaac Janzen, post:3, topic:7, full:true, username:isaac.janzen"]
bing bong
[/quote]
```
This commit is contained in:
Isaac Janzen 2022-04-20 10:07:51 -05:00 committed by GitHub
parent 5d00f7bc0a
commit 196b791365
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 601 additions and 9 deletions

View File

@ -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`;
}

View File

@ -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;

View File

@ -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;
}

View File

@ -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,
};
}

View File

@ -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)),
];
},

View File

@ -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]'
);
});
});

View File

@ -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) {

View File

@ -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: "<p>This is a short topic.</p>",
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: "<p>This is a short topic.</p>",
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: "<p>Yeah it is a short one</p>",
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,
},
};

View File

@ -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,
};

View File

@ -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);

View File

@ -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

View File

@ -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: "<sunday><monday>")
cp = CookedPostProcessor.new(post)
cp.post_process
expect(cp.html).to eq("<p></p>")
end
it "works only on new posts" do
Fabricate(:post, topic: topic, hidden: true, raw: "this is the second post after")
Fabricate(:post, topic: topic, post_type: Post.types[:small_action])
reply = PostCreator.create!(topic.user, topic_id: topic.id, raw: raw)
stub_image_size
CookedPostProcessor.new(reply).post_process
expect(reply.raw).to eq(raw)
PostRevisor.new(reply).revise!(Discourse.system_user, raw: raw, edit_reason: "put back full quote")
stub_image_size
CookedPostProcessor.new(reply).post_process(new_post: true)
expect(reply.raw).to eq("and this is the third reply")
end
it "works with nested quotes" do
reply1 = Fabricate(:post, user: user, topic: topic, raw: raw)
reply2 = Fabricate(:post, topic: topic, raw: <<~RAW.strip)
[quote="#{reply1.user.name}, post:#{reply1.post_number}, topic:#{topic.id}, username:#{reply1.user.username}"]
#{raw}
[/quote]
quoting a post with a quote
RAW
CookedPostProcessor.new(reply2).remove_full_quote_on_direct_reply
expect(reply2.raw).to eq('quoting a post with a quote')
end
end
context "prioritizes full name in quotes" 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') }
before do
SiteSetting.display_name_on_posts = true
SiteSetting.prioritize_username_in_ux = false
end
it "maintains full name post processing" do
reply = Fabricate(:post, user: user, topic: topic, raw: <<~RAW.strip)
[quote="#{user.name}, post:#{post.id}, topic:#{topic.id}, username:#{user.username}"]
quoting a post with a quote
[/quote]
quoting a post with a quote
RAW
doc = Nokogiri::HTML5::fragment(CookedPostProcessor.new(reply).html)
expect(doc.css('.title').text).to eq("\n\n #{user.name}:")
end
end
context "#html" do
it "escapes attributes" do
post = Fabricate(:post, raw: '<img alt="<something>">')
@ -1877,5 +2032,4 @@ describe CookedPostProcessor do
expect(CookedPostProcessor.new(post).html).to eq('<p><img alt="&lt;something&gt;"></p>')
end
end
end

View File

@ -129,6 +129,9 @@
"username": {
"type": "string"
},
"name": {
"type": "string"
},
"avatar_template": {
"type": "string"
}