From 7b7e1717f2c1579ae7de9d344a7e7cbc63f8fffc Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Wed, 22 Jan 2020 16:10:23 +0200 Subject: [PATCH] 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"). --- .../discourse/components/quote-button.js.es6 | 28 ++++++++++++++++--- .../discourse/controllers/topic.js.es6 | 4 +-- .../discourse/lib/quote-state.js.es6 | 4 ++- .../javascripts/discourse/lib/quote.js.es6 | 7 +++-- .../discourse/lib/utilities.js.es6 | 7 +++++ .../engines/discourse-markdown/quotes.js.es6 | 4 +++ app/jobs/regular/update_username.rb | 2 ++ spec/components/pretty_text_spec.rb | 12 ++++---- spec/services/username_changer_spec.rb | 8 +++--- test/javascripts/acceptance/topic-test.js.es6 | 18 ++++++++++++ test/javascripts/lib/pretty-text-test.js.es6 | 24 ++++++++-------- 11 files changed, 86 insertions(+), 32 deletions(-) diff --git a/app/assets/javascripts/discourse/components/quote-button.js.es6 b/app/assets/javascripts/discourse/components/quote-button.js.es6 index 718f68c98ad..742be7865a2 100644 --- a/app/assets/javascripts/discourse/components/quote-button.js.es6 +++ b/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; - this.attrs.selectText(postId, buffer).then(() => this._hideButton()); + const { postId, buffer, opts } = this.quoteState; + this.attrs.selectText(postId, buffer, opts).then(() => this._hideButton()); return false; } }); diff --git a/app/assets/javascripts/discourse/controllers/topic.js.es6 b/app/assets/javascripts/discourse/controllers/topic.js.es6 index cc0725a697e..c474fa50fa7 100644 --- a/app/assets/javascripts/discourse/controllers/topic.js.es6 +++ b/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")) { diff --git a/app/assets/javascripts/discourse/lib/quote-state.js.es6 b/app/assets/javascripts/discourse/lib/quote-state.js.es6 index 56e28438876..63f7337aec0 100644 --- a/app/assets/javascripts/discourse/lib/quote-state.js.es6 +++ b/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; } } diff --git a/app/assets/javascripts/discourse/lib/quote.js.es6 b/app/assets/javascripts/discourse/lib/quote.js.es6 index 568cf987c8f..7a39c6da302 100644 --- a/app/assets/javascripts/discourse/lib/quote.js.es6 +++ b/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"), - `post:${post.get("post_number")}`, - `topic:${post.get("topic_id")}` + opts.username || post.username, + `post:${opts.post || post.post_number}`, + `topic:${opts.topic || post.topic_id}` ]; opts = opts || {}; diff --git a/app/assets/javascripts/discourse/lib/utilities.js.es6 b/app/assets/javascripts/discourse/lib/utilities.js.es6 index ff43ee32a18..ce91d7c8dc2 100644 --- a/app/assets/javascripts/discourse/lib/utilities.js.es6 +++ b/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); diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown/quotes.js.es6 b/app/assets/javascripts/pretty-text/engines/discourse-markdown/quotes.js.es6 index 6c2860577ae..c66122f7f4b 100644 --- a/app/assets/javascripts/pretty-text/engines/discourse-markdown/quotes.js.es6 +++ b/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]); } diff --git a/app/jobs/regular/update_username.rb b/app/jobs/regular/update_username.rb index 26657124e34..a92cf82c6e0 100644 --- a/app/jobs/regular/update_username.rb +++ b/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 diff --git a/spec/components/pretty_text_spec.rb b/spec/components/pretty_text_spec.rb index 8f2c16e57f3..11da34414cc 100644 --- a/spec/components/pretty_text_spec.rb +++ b/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 -