Continuing from Configuring Twitter login (and rich embeds) for Discourse - #114 by Hifihedgehog
I got an email about this and was very confused as I didn’t recall making a post like this.
I’m curious how the “taylor” got swapped here in the quote reply? Is this some manual edit mistake (I’m not sure how to do a quote block with a source manually, or if that is even possible). Or is there a bug somewhere in the reply logic? @tshenry
I’ve just tried quoting a few posts, and I think something funky is going on with username vs fullname. Probably this recent commit:
Hi @JammyDodger,
can you give me some more info into
With the site settings of prioritize username in ux turned on and display name on posts turned off you should see the username in the quote, and vice versa for seeing the full name.
Sorry, I could have been much more helpful there. 
I just tried quoting several of the posts in this topic, and there were some odd results in my composer:
@Hifihedgehog’s post:
[quote=", post:114, topic:13395"]
you right! That took all of a couple minutes and I was instantly approved. Simple fixes are always nice! :smile:
[/quote]
@tshenry’s post:
[quote="Taylor, post:113, topic:13395"]
You can gain access to the v1.1 API for free under the Elevated plan. You simply need to apply from your developer dashboard. I found it very quick and easy. Discourse Twitter logins worked flawlessly after that :tada:
[/quote]
(that’s the style of one @taylor got a notification for)
Ah, thanks for pointing this out… yah, this is not desirable / intended. I will look into this!
So the reason @taylor got tagged in this quote (and got an email) is because his username - Taylor is the same as @tshenry’s fullname - Taylor 
which (the fullname) is being prioritized properly in quotes because of the changes added in DEV: Prioritize full name when setting active (#15820) · discourse/discourse@5a93ce4 · GitHub.
Anyways, thanks for tip, I’ll get this fixed!
FWIW, It also leaves quotes without fullnames blank:
Are we good on this now @isaac?
Yes we reverted the change, and I am working on fixing logic before merging again
In updating the quote logic to allow for us to toggle username or full name via a site setting I have run into a problem (part of the problem was noted above), let me create a hypothetical scenario…
Rebaking all of the quotes is also a no go. If this site setting was toggled multiple times, and we rebake multiple times, a quote that is being converted from a full name → username is not unique and could find multiple users.
The avenue I see us taking is adding an additional data attribute to the quote of data-full-name="true" (or something) to check if we should replace the username with the full name in the ux, instead of manipulating the data-username
eg.
data-username="always.users.username" data-post="1" data-topic="1" data-full-name="true"
and save ourselves the hassle of worrying if we are querying or sending notifications on username or full name. Thoughts? Is this worth the effort of updating PrettyText and our quote builder for this change?
I prefer not to add support for a switchover. This is a decision you make early in your community setup, supporting super smooth switchover is extremely expensive and comes with a bunch of trade-offs.
If I quote you with username prioritized we render this markdown
[quote="isaacjanzen, post:12, topic:217633"]
Thoughts?
[/quote]
If i quote you with name prioritized we render this markdown
[quote="Isaac Janzen, post:12, topic:217633"]
Thoughts?
[/quote]
Got to be careful with , in full names - but I support stripping them out for this use case or I guess you can introduce some escaping trick.
Are you saying you would recommend we scrap trying to add the ability to display quotes with full names? We are inevitably going to run into these switchover cases… eg. any existing community with a quote
Nope, not fully scratch it… meta has prioritize username in ux disabled. The quote I just made should be: (which is missing an avatar which needs fixing)
The markup here should be:
[quote="Isaac Janzen, post:14, topic:217633"]
Are you saying you would recommend we scrap trying to add the ability to display quotes with full names?
[/quote]
Only change needed it to make the quote widget be aware of the setting so it fills in full name instead of username. Do not worry about migration and history.
I am not sure we are on the same page here… when we merged DEV: Prioritize full name when setting active (#15820) · discourse/discourse@5a93ce4 · GitHub we achieved
# quote.js
# opts.displayName = true if site settings prioritize full name
const name = opts.displayName
? opts.name || post.name
: opts.username || post.username;
# build quote markdown
const params = [
name,
`post:${opts.post || post.post_number}`,
`topic:${opts.topic || post.topic_id}`,
];
to get the output of
but the problem was that the user lookup based on full name was breaking existing quotes that were baked with username.
So it feels that it is impossible to ignore
I am super confused cause meta has:
So why is the name missing from the quote above? Was the change rolled back?
Agree we need to do more here:
We need to grab the username from “post / topic” combination and place it in the cooked markdown (we should do that unconditionally), otherwise we can not display avatars for the full name.
It also lets us fix this edge case, so it does not show my avatar on this mis-quote. (or even highlights a misquote)
Ah, yah
I agree, and can add this.
I am totally open to fix a few edge cases while there:
I think the best thing to do for now is just “override”. If the post has a name / username, use it over the one that was supplied in the quote. Cook in current name / current username.
Alright, I have a PR up that clears things up.
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]
What are your thoughts @sam?
I guess I can accept a markdown change so we work around needing to go into a rabbit hole of an enormous security adventure. However the change needs to be hyper surgical.
Can you confirm that the new verbose syntax only ever happens if both siteSettings.display_name_on_posts && !siteSettings.prioritize_username_in_ux ?
[quote="Isaac Janzen, post:3, topic:7, username:isaac.janzen"]
....
[/quote]
Meaning that if you do not enable both display_name_on_posts AND disable prioritize_username_in_ux
Then the old quote format of should remain:
[quote="isaac.janzen, post:3, topic:7"]
....
[/quote]
I am willing to accept this as an intermediate step for now, but our long term goal would be to remove this noise from the quote block (tighten security, restrict abuse) and support the abridged quote format we always had, working through the security can of snakes, just like we are forced to now for inline oneboxes eg: Quoting broken when name matches username - #20 by isaac ← https://meta.discourse.org/t/quoting-broken-when-name-matches-username/217633/20?u=sam
Mentioning @tobiaseigen here for visibility cause this relates to previous discussions.
Bottom line:
[quote="TOTALLY OPTIONAL WILL BE FISHED FROM POST IF SECURITY LINES UP AND INFO ON POST IS PRIORITIZED OVER THIS TEXT, post:3, topic:7"]. - recommend we wait a while on this.Merged and deployed DEV: Prioritize full name when display_name_on_posts active by janzenisaac · Pull Request #16078 · discourse/discourse · GitHub
I can confirm.