FIX: Quoting a quote preserves the original post information (#8746)

Let's say post #2 quotes post number #1. If a user decides to quote the
quote in post #2, it should keep the information of post #1
("user_1, post: 1, topic: X"), instead of replacing with current post
info ("user_2, post: 2, topic: X").

app/assets/javascripts/discourse/components/quote-button.js.es6

@@ -1,7 +1,7 @@
 import { scheduleOnce } from "@ember/runloop";
 import Component from "@ember/component";
 import discourseDebounce from "discourse/lib/debounce";
-import { selectedText } from "discourse/lib/utilities";
+import { selectedText, selectedElement } from "discourse/lib/utilities";
 
 export default Component.extend({
   classNames: ["quote-button"],
@@ -48,8 +48,28 @@ export default Component.extend({
       }
     }
 
+    let opts;
+    for (
+      let element = selectedElement();
+      element && element.tagName !== "ARTICLE";
+      element = element.parentElement
+    ) {
+      if (element.tagName === "ASIDE" && element.classList.contains("quote")) {
+        opts = {
+          username:
+            element.dataset.username ||
+            element
+              .querySelector(".title")
+              .textContent.trim()
+              .replace(/:$/, ""),
+          post: element.dataset.post,
+          topic: element.dataset.topic
+        };
+      }
+    }
+
     const _selectedText = selectedText();
-    quoteState.selected(postId, _selectedText);
+    quoteState.selected(postId, _selectedText, opts);
     this.set("visible", quoteState.buffer.length > 0);
 
     // avoid hard loops in quote selection unconditionally
@@ -165,8 +185,8 @@ export default Component.extend({
   },
 
   click() {
-    const { postId, buffer } = this.quoteState;
+    const { postId, buffer, opts } = this.quoteState;
-    this.attrs.selectText(postId, buffer).then(() => this._hideButton());
+    this.attrs.selectText(postId, buffer, opts).then(() => this._hideButton());
     return false;
   }
 });

app/assets/javascripts/discourse/controllers/topic.js.es6

@@ -266,7 +266,7 @@ export default Controller.extend(bufferedProperty("model"), {
       this.send("showFeatureTopic");
     },
 
-    selectText(postId, buffer) {
+    selectText(postId, buffer, opts) {
       const loadedPost = this.get("model.postStream").findLoadedPost(postId);
       const promise = loadedPost
         ? Promise.resolve(loadedPost)
@@ -275,7 +275,7 @@ export default Controller.extend(bufferedProperty("model"), {
       return promise.then(post => {
         const composer = this.composer;
         const viewOpen = composer.get("model.viewOpen");
-        const quotedText = Quote.build(post, buffer);
+        const quotedText = Quote.build(post, buffer, opts);
 
         // If we can't create a post, delegate to reply as new topic
         if (!viewOpen && !this.get("model.details.can_create_post")) {

app/assets/javascripts/discourse/lib/quote-state.js.es6

@@ -3,13 +3,15 @@ export default class QuoteState {
     this.clear();
   }
 
-  selected(postId, buffer) {
+  selected(postId, buffer, opts) {
     this.postId = postId;
     this.buffer = buffer;
+    this.opts = opts;
   }
 
   clear() {
     this.buffer = "";
     this.postId = null;
+    this.opts = null;
   }
 }

app/assets/javascripts/discourse/lib/quote.js.es6

@@ -8,6 +8,7 @@ export default {
     }
 
     if (!contents) contents = "";
+    if (!opts) opts = {};
 
     const sansQuotes = contents.replace(this.REGEXP, "").trim();
     if (sansQuotes.length === 0) {
@@ -26,9 +27,9 @@ export default {
       stripped.replace(/\W/g, "") === contents.replace(/\W/g, "");
 
     const params = [
-      post.get("username"),
+      opts.username || post.username,
-      `post:${post.get("post_number")}`,
+      `post:${opts.post || post.post_number}`,
-      `topic:${post.get("topic_id")}`
+      `topic:${opts.topic || post.topic_id}`
     ];
 
     opts = opts || {};

app/assets/javascripts/discourse/lib/utilities.js.es6

@@ -143,6 +143,13 @@ export function selectedText() {
   return toMarkdown($div.html());
 }
 
+export function selectedElement() {
+  const selection = window.getSelection();
+  if (selection.rangeCount > 0) {
+    return selection.getRangeAt(0).startContainer.parentElement;
+  }
+}
+
 // Determine the row and col of the caret in an element
 export function caretRowCol(el) {
   var cp = caretPosition(el);

app/assets/javascripts/pretty-text/engines/discourse-markdown/quotes.js.es6

@@ -73,6 +73,10 @@ const rule = {
       token.attrs.push(["class", "quote no-group"]);
     }
 
+    if (username) {
+      token.attrs.push(["data-username", username]);
+    }
+
     if (postNumber) {
       token.attrs.push(["data-post", postNumber]);
     }

app/jobs/regular/update_username.rb

@@ -164,6 +164,8 @@ module Jobs
 
         username_replaced = false
 
+        aside["data-username"] = @new_username if aside["data-username"] == @old_username
+
         div.children.each do |child|
           if child.text?
             content = child.content

spec/components/pretty_text_spec.rb

@@ -33,7 +33,7 @@ describe PrettyText do
 
         topic = Fabricate(:topic, title: "this is a test topic :slight_smile:")
         expected = <<~HTML
-          <aside class="quote no-group" data-post="2" data-topic="#{topic.id}">
+          <aside class="quote no-group" data-username="EvilTrout" data-post="2" data-topic="#{topic.id}">
           <div class="title">
           <div class="quote-controls"></div>
           <a href="http://test.localhost/t/this-is-a-test-topic/#{topic.id}/2">This is a test topic <img src="/images/emoji/twitter/slight_smile.png?v=#{Emoji::EMOJI_VERSION}" title="slight_smile" alt="slight_smile" class="emoji"></a>
@@ -158,7 +158,7 @@ describe PrettyText do
         topic = Fabricate(:topic, title: "this is topic with secret category", category: category)
 
         expected = <<~HTML
-          <aside class="quote no-group" data-post="3" data-topic="#{topic.id}">
+          <aside class="quote no-group" data-username="maja" data-post="3" data-topic="#{topic.id}">
           <div class="title">
           <div class="quote-controls"></div>
           <a href="http://test.localhost/t/#{topic.id}/3">#{I18n.t("on_another_topic")}</a>
@@ -181,7 +181,7 @@ describe PrettyText do
           [/quote]
         MD
         html = <<~HTML
-          <aside class="quote no-group" data-post="123" data-topic="456" data-full="true">
+          <aside class="quote no-group" data-username="#{user.username}" data-post="123" data-topic="456" data-full="true">
           <div class="title">
           <div class="quote-controls"></div>
           <img alt width="20" height="20" src="//test.localhost/uploads/default/avatars/42d/57c/46ce7ee487/40.png" class="avatar"> #{user.username}:</div>
@@ -203,7 +203,7 @@ describe PrettyText do
           [/quote]
         MD
         html = <<~HTML
-          <aside class="quote no-group" data-post="123" data-topic="456" data-full="true">
+          <aside class="quote no-group" data-username="#{user.username}" data-post="123" data-topic="456" data-full="true">
           <div class="title">
           <div class="quote-controls"></div>
           <img alt width="20" height="20" src="//test.localhost/uploads/default/avatars/42d/57c/46ce7ee487/40.png" class="avatar"> #{user.username}:</div>
@@ -224,7 +224,7 @@ describe PrettyText do
         MD
 
         html = <<~HTML
-          <aside class="quote no-group" data-post="555" data-topic="666">
+          <aside class="quote no-group" data-username="#{user.username}" data-post="555" data-topic="666">
           <div class="title">
           <div class="quote-controls"></div>
           <img alt width="20" height="20" src="//test.localhost/uploads/default/avatars/42d/57c/46ce7ee487/40.png" class="avatar"> #{user.username}:</div>
@@ -251,7 +251,7 @@ describe PrettyText do
 
         topic = Fabricate(:topic, title: "this is a test topic")
         expected = <<~HTML
-          <aside class="quote group-#{group.name}" data-post="2" data-topic="#{topic.id}">
+          <aside class="quote group-#{group.name}" data-username="#{user.username}" data-post="2" data-topic="#{topic.id}">
           <div class="title">
           <div class="quote-controls"></div>
           <img alt width="20" height="20" src="//test.localhost/uploads/default/avatars/42d/57c/46ce7ee487/40.png" class="avatar"><a href="http://test.localhost/t/this-is-a-test-topic/#{topic.id}/2">This is a test topic</a>

spec/services/username_changer_spec.rb

@@ -359,7 +359,7 @@ describe UsernameChanger do
 
           expect(post.cooked).to match_html(<<~HTML)
             <p>Lorem ipsum</p>
-            <aside class="quote no-group" data-post="1" data-topic="#{quoted_post.topic.id}">
+            <aside class="quote no-group" data-username="bar" data-post="1" data-topic="#{quoted_post.topic.id}">
             <div class="title">
             <div class="quote-controls"></div>
             <img alt='' width="20" height="20" src="#{avatar_url}" class="avatar"> bar:</div>
@@ -367,7 +367,7 @@ describe UsernameChanger do
             <p>quoted post</p>
             </blockquote>
             </aside>
-            <aside class="quote no-group">
+            <aside class="quote no-group" data-username="bar">
             <div class="title">
             <div class="quote-controls"></div>
             <img alt='' width="20" height="20" src="#{avatar_url}" class="avatar"> bar:</div>
@@ -375,7 +375,7 @@ describe UsernameChanger do
             <p>quoted post</p>
             </blockquote>
             </aside>
-            <aside class="quote no-group" data-post="1" data-topic="#{quoted_post.topic.id}">
+            <aside class="quote no-group" data-username="bar" data-post="1" data-topic="#{quoted_post.topic.id}">
             <div class="title">
             <div class="quote-controls"></div>
             <img alt='' width="20" height="20" src="#{avatar_url}" class="avatar"> bar:</div>
@@ -410,7 +410,7 @@ describe UsernameChanger do
           let(:expected_cooked) do
             <<~HTML
               <p>Lorem ipsum</p>
-              <aside class="quote no-group" data-post="1" data-topic="#{quoted_post.topic.id}">
+              <aside class="quote no-group" data-username="bar" data-post="1" data-topic="#{quoted_post.topic.id}">
               <div class="title">
               <div class="quote-controls"></div>
               <img alt='' width="20" height="20" src="#{avatar_url}" class="avatar"> bar:</div>

test/javascripts/acceptance/topic-test.js.es6

@@ -315,3 +315,21 @@ QUnit.test("View Hidden Replies", async assert => {
 
   assert.equal(find(".gap").length, 0, "it hides gap");
 });
+
+QUnit.test("Quoting a quote keeps the original poster name", async assert => {
+  await visit("/t/internationalization-localization/280");
+
+  const selection = window.getSelection();
+  const range = document.createRange();
+  range.selectNodeContents($("#post_5 blockquote")[0]);
+  selection.removeAllRanges();
+  selection.addRange(range);
+
+  await click(".quote-button");
+
+  assert.ok(
+    find(".d-editor-input")
+      .val()
+      .indexOf('quote="codinghorror said, post:3, topic:280"') !== -1
+  );
+});

test/javascripts/lib/pretty-text-test.js.es6

@@ -395,7 +395,7 @@ QUnit.test("Quotes", assert => {
   assert.cookedOptions(
     '[quote="eviltrout, post: 1"]\na quote\n\nsecond line\n\nthird line\n[/quote]',
     { topicId: 2 },
-    `<aside class=\"quote no-group\" data-post=\"1\">
+    `<aside class=\"quote no-group\" data-username=\"eviltrout\" data-post=\"1\">
 <div class=\"title\">
 <div class=\"quote-controls\"></div>
  eviltrout:</div>
@@ -411,7 +411,7 @@ QUnit.test("Quotes", assert => {
   assert.cookedOptions(
     '[quote="bob, post:1"]\nmy quote\n[/quote]',
     { topicId: 2, lookupAvatar: function() {} },
-    `<aside class=\"quote no-group\" data-post=\"1\">
+    `<aside class=\"quote no-group\" data-username=\"bob\" data-post=\"1\">
 <div class=\"title\">
 <div class=\"quote-controls\"></div>
  bob:</div>
@@ -440,7 +440,7 @@ QUnit.test("Quotes", assert => {
   assert.cookedOptions(
     `[quote="bob, post:1, topic:1"]\ntest quote\n[/quote]`,
     { lookupPrimaryUserGroupByPostNumber: () => "aUserGroup" },
-    `<aside class="quote group-aUserGroup" data-post="1" data-topic="1">
+    `<aside class="quote group-aUserGroup" data-username="bob" data-post="1" data-topic="1">
 <div class="title">
 <div class="quote-controls"></div>
  bob:</div>
@@ -1190,7 +1190,7 @@ QUnit.test("quotes", assert => {
 QUnit.test("quote formatting", assert => {
   assert.cooked(
     '[quote="EvilTrout, post:123, topic:456, full:true"]\n[sam]\n[/quote]',
-    `<aside class=\"quote no-group\" data-post=\"123\" data-topic=\"456\" data-full=\"true\">
+    `<aside class=\"quote no-group\" data-username=\"EvilTrout\" data-post=\"123\" data-topic=\"456\" data-full=\"true\">
 <div class=\"title\">
 <div class=\"quote-controls\"></div>
  EvilTrout:</div>
@@ -1203,7 +1203,7 @@ QUnit.test("quote formatting", assert => {
 
   assert.cooked(
     '[quote="eviltrout, post:1, topic:1"]\nabc\n[/quote]',
-    `<aside class=\"quote no-group\" data-post=\"1\" data-topic=\"1\">
+    `<aside class=\"quote no-group\" data-username=\"eviltrout\" data-post=\"1\" data-topic=\"1\">
 <div class=\"title\">
 <div class=\"quote-controls\"></div>
  eviltrout:</div>
@@ -1216,7 +1216,7 @@ QUnit.test("quote formatting", assert => {
 
   assert.cooked(
     '[quote="eviltrout, post:1, topic:1"]\nabc\n[/quote]\nhello',
-    `<aside class=\"quote no-group\" data-post=\"1\" data-topic=\"1\">
+    `<aside class=\"quote no-group\" data-username=\"eviltrout\" data-post=\"1\" data-topic=\"1\">
 <div class=\"title\">
 <div class=\"quote-controls\"></div>
  eviltrout:</div>
@@ -1230,12 +1230,12 @@ QUnit.test("quote formatting", assert => {
 
   assert.cooked(
     '[quote="Alice, post:1, topic:1"]\n[quote="Bob, post:2, topic:1"]\n[/quote]\n[/quote]',
-    `<aside class=\"quote no-group\" data-post=\"1\" data-topic=\"1\">
+    `<aside class=\"quote no-group\" data-username=\"Alice\" data-post=\"1\" data-topic=\"1\">
 <div class=\"title\">
 <div class=\"quote-controls\"></div>
  Alice:</div>
 <blockquote>
-<aside class=\"quote no-group\" data-post=\"2\" data-topic=\"1\">
+<aside class=\"quote no-group\" data-username=\"Bob\" data-post=\"2\" data-topic=\"1\">
 <div class=\"title\">
 <div class=\"quote-controls\"></div>
  Bob:</div>
@@ -1249,7 +1249,7 @@ QUnit.test("quote formatting", assert => {
   assert.cooked(
     '[quote="Alice, post:1, topic:1"]\n[quote="Bob, post:2, topic:1"]\n[/quote]',
     `<p>[quote=&quot;Alice, post:1, topic:1&quot;]</p>
-<aside class=\"quote no-group\" data-post=\"2\" data-topic=\"1\">
+<aside class=\"quote no-group\" data-username=\"Bob\" data-post=\"2\" data-topic=\"1\">
 <div class=\"title\">
 <div class=\"quote-controls\"></div>
  Bob:</div>
@@ -1261,7 +1261,7 @@ QUnit.test("quote formatting", assert => {
 
   assert.cooked(
     "[quote=\"Alice, post:1, topic:1\"]\n```javascript\nvar foo ='foo';\nvar bar = 'bar';\n```\n[/quote]",
-    `<aside class=\"quote no-group\" data-post=\"1\" data-topic=\"1\">
+    `<aside class=\"quote no-group\" data-username=\"Alice\" data-post=\"1\" data-topic=\"1\">
 <div class=\"title\">
 <div class=\"quote-controls\"></div>
  Alice:</div>
@@ -1276,7 +1276,7 @@ var bar = 'bar';
 
   assert.cooked(
     "[quote=\"Alice, post:1, topic:1\"]\n\n```javascript\nvar foo ='foo';\nvar bar = 'bar';\n```\n[/quote]",
-    `<aside class=\"quote no-group\" data-post=\"1\" data-topic=\"1\">
+    `<aside class=\"quote no-group\" data-username=\"Alice\" data-post=\"1\" data-topic=\"1\">
 <div class=\"title\">
 <div class=\"quote-controls\"></div>
  Alice:</div>
@@ -1296,7 +1296,7 @@ QUnit.test("quotes with trailing formatting", assert => {
   );
   assert.equal(
     result,
-    `<aside class=\"quote no-group\" data-post=\"123\" data-topic=\"456\" data-full=\"true\">
+    `<aside class=\"quote no-group\" data-username=\"EvilTrout\" data-post=\"123\" data-topic=\"456\" data-full=\"true\">
 <div class=\"title\">
 <div class=\"quote-controls\"></div>
  EvilTrout:</div>