From a76d864c5136b7a52160b5394cac5f98b746ea43 Mon Sep 17 00:00:00 2001 From: Andrei Prigorshnev Date: Tue, 6 Dec 2022 19:10:36 +0400 Subject: [PATCH] FEATURE: Show live user status on inline mentions on posts (#18683) Note that we don't have a database table and a model for post mentions yet, and I decided to implement it without adding one to avoid heavy data migrations. Still, we may want to add such a model later, that would be convenient, we have such a model for mentions in chat. Note that status appears on all mentions on all posts in a topic except of the case when you just posted a new post, and it appeared on the bottom of the topic. On such posts, status won't be shown immediately for now (you'll need to reload the page to see the status). I'll take care of it in one of the following PRs. --- .../discourse/app/models/post-stream.js | 10 +- .../discourse/app/widgets/post-cooked.js | 89 +++++++++- .../acceptance/post-inline-mentions-test.js | 160 ++++++++++++++++++ app/models/post.rb | 4 + app/models/post_analyzer.rb | 13 +- app/serializers/post_serializer.rb | 15 +- app/serializers/web_hook_post_serializer.rb | 1 + lib/pretty_text.rb | 14 ++ lib/topic_view.rb | 24 ++- .../schemas/json/topic_create_response.json | 5 + spec/requests/topics_controller_spec.rb | 85 ++++++++++ 11 files changed, 400 insertions(+), 20 deletions(-) create mode 100644 app/assets/javascripts/discourse/tests/acceptance/post-inline-mentions-test.js diff --git a/app/assets/javascripts/discourse/app/models/post-stream.js b/app/assets/javascripts/discourse/app/models/post-stream.js index 674ea63cf33..295061e3e01 100644 --- a/app/assets/javascripts/discourse/app/models/post-stream.js +++ b/app/assets/javascripts/discourse/app/models/post-stream.js @@ -608,7 +608,7 @@ export default RestModel.extend({ }, prependPost(post) { - this._initUserModel(post); + this._initUserModels(post); const stored = this.storePost(post); if (stored) { const posts = this.posts; @@ -619,7 +619,7 @@ export default RestModel.extend({ }, appendPost(post) { - this._initUserModel(post); + this._initUserModels(post); const stored = this.storePost(post); if (stored) { const posts = this.posts; @@ -1259,7 +1259,7 @@ export default RestModel.extend({ } }, - _initUserModel(post) { + _initUserModels(post) { post.user = User.create({ id: post.user_id, username: post.username, @@ -1268,6 +1268,10 @@ export default RestModel.extend({ if (post.user_status) { post.user.status = post.user_status; } + + if (post.mentioned_users) { + post.mentioned_users = post.mentioned_users.map((u) => User.create(u)); + } }, _checkIfShouldShowRevisions() { diff --git a/app/assets/javascripts/discourse/app/widgets/post-cooked.js b/app/assets/javascripts/discourse/app/widgets/post-cooked.js index 62941eaf607..ae19ed8756e 100644 --- a/app/assets/javascripts/discourse/app/widgets/post-cooked.js +++ b/app/assets/javascripts/discourse/app/widgets/post-cooked.js @@ -4,10 +4,13 @@ import { ajax } from "discourse/lib/ajax"; import highlightSearch from "discourse/lib/highlight-search"; import { iconHTML } from "discourse-common/lib/icon-library"; import { isValidLink } from "discourse/lib/click-track"; -import { number } from "discourse/lib/formatter"; +import { number, until } from "discourse/lib/formatter"; import { spinnerHTML } from "discourse/helpers/loading-spinner"; import { escape } from "pretty-text/sanitizer"; import domFromString from "discourse-common/lib/dom-from-string"; +import getURL from "discourse-common/lib/get-url"; +import { emojiUnescape } from "discourse/lib/text"; +import { escapeExpression } from "discourse/lib/utilities"; let _beforeAdoptDecorators = []; let _afterAdoptDecorators = []; @@ -57,16 +60,25 @@ export default class PostCooked { init() { this.originalQuoteContents = null; + // todo should be a better way of detecting if it is composer preview + this._isInComposerPreview = !this.decoratorHelper; + const cookedDiv = this._computeCooked(); + this.cookedDiv = cookedDiv; this._insertQuoteControls(cookedDiv); this._showLinkCounts(cookedDiv); this._applySearchHighlight(cookedDiv); + this._initUserStatusToMentions(); this._decorateAndAdopt(cookedDiv); return cookedDiv; } + destroy() { + this._stopTrackingMentionedUsersStatus(); + } + _decorateAndAdopt(cooked) { _beforeAdoptDecorators.forEach((d) => d(cooked, this.decoratorHelper)); @@ -200,7 +212,7 @@ export default class PostCooked { try { const result = await ajax(`/posts/by_number/${topicId}/${postId}`); - const post = this.decoratorHelper.getModel(); + const post = this._post(); const quotedPosts = post.quoted || {}; quotedPosts[result.id] = result; post.set("quoted", quotedPosts); @@ -361,6 +373,79 @@ export default class PostCooked { return cookedDiv; } + + _initUserStatusToMentions() { + if (!this._isInComposerPreview) { + this._trackMentionedUsersStatus(); + this._rerenderUserStatusOnMentions(); + } + } + + _rerenderUserStatusOnMentions() { + this._post()?.mentioned_users?.forEach((user) => + this._rerenderUserStatusOnMention(this.cookedDiv, user) + ); + } + + _rerenderUserStatusOnMention(postElement, user) { + const href = getURL(`/u/${user.username.toLowerCase()}`); + const mentions = postElement.querySelectorAll(`a.mention[href="${href}"]`); + + mentions.forEach((mention) => { + this._updateUserStatus(mention, user.status); + }); + } + + _updateUserStatus(mention, status) { + this._removeUserStatus(mention); + if (status) { + this._insertUserStatus(mention, status); + } + } + + _insertUserStatus(mention, status) { + const emoji = escapeExpression(`:${status.emoji}:`); + const statusHtml = emojiUnescape(emoji, { + class: "user-status", + title: this._userStatusTitle(status), + }); + mention.insertAdjacentHTML("beforeend", statusHtml); + } + + _removeUserStatus(mention) { + mention.querySelector("img.user-status")?.remove(); + } + + _userStatusTitle(status) { + if (!status.ends_at) { + return status.description; + } + + const until_ = until( + status.ends_at, + this.currentUser.timezone, + this.currentUser.locale + ); + return escapeExpression(`${status.description} ${until_}`); + } + + _trackMentionedUsersStatus() { + this._post()?.mentioned_users?.forEach((user) => { + user.trackStatus(); + user.on("status-changed", this, "_rerenderUserStatusOnMentions"); + }); + } + + _stopTrackingMentionedUsersStatus() { + this._post()?.mentioned_users?.forEach((user) => { + user.stopTrackingStatus(); + user.off("status-changed", this, "_rerenderUserStatusOnMentions"); + }); + } + + _post() { + return this.decoratorHelper?.getModel?.(); + } } PostCooked.prototype.type = "Widget"; diff --git a/app/assets/javascripts/discourse/tests/acceptance/post-inline-mentions-test.js b/app/assets/javascripts/discourse/tests/acceptance/post-inline-mentions-test.js new file mode 100644 index 00000000000..a6f7035d51e --- /dev/null +++ b/app/assets/javascripts/discourse/tests/acceptance/post-inline-mentions-test.js @@ -0,0 +1,160 @@ +import { + acceptance, + exists, + publishToMessageBus, + query, +} from "discourse/tests/helpers/qunit-helpers"; +import { visit } from "@ember/test-helpers"; +import { test } from "qunit"; +import { cloneJSON } from "discourse-common/lib/object"; +import topicFixtures from "../fixtures/topic"; +import pretender, { response } from "discourse/tests/helpers/create-pretender"; + +acceptance("Post inline mentions test", function (needs) { + needs.user(); + + const topicId = 130; + const mentionedUserId = 1; + const status = { + description: "Surfing", + emoji: "surfing_man", + ends_at: null, + }; + + function topicWithoutUserStatus() { + const topic = cloneJSON(topicFixtures[`/t/${topicId}.json`]); + const firstPost = topic.post_stream.posts[0]; + firstPost.cooked = + '

I am mentioning @user1 again.

'; + firstPost.mentioned_users = [ + { + id: mentionedUserId, + username: "user1", + avatar_template: "/letter_avatar_proxy/v4/letter/a/bbce88/{size}.png", + }, + ]; + return topic; + } + + function topicWithUserStatus() { + const topic = topicWithoutUserStatus(); + topic.post_stream.posts[0].mentioned_users[0].status = status; + return topic; + } + + test("shows user status on inline mentions", async function (assert) { + pretender.get(`/t/${topicId}.json`, () => { + return response(topicWithUserStatus()); + }); + + await visit(`/t/lorem-ipsum-dolor-sit-amet/${topicId}`); + + assert.ok( + exists(".topic-post .cooked .mention .user-status"), + "user status is shown" + ); + const statusElement = query(".topic-post .cooked .mention .user-status"); + assert.equal( + statusElement.title, + status.description, + "status description is correct" + ); + assert.ok( + statusElement.src.includes(status.emoji), + "status emoji is correct" + ); + }); + + test("inserts user status on message bus message", async function (assert) { + pretender.get(`/t/${topicId}.json`, () => { + return response(topicWithoutUserStatus()); + }); + await visit(`/t/lorem-ipsum-dolor-sit-amet/${topicId}`); + + assert.notOk( + exists(".topic-post .cooked .mention .user-status"), + "user status isn't shown" + ); + + await publishToMessageBus("/user-status", { + [mentionedUserId]: { + description: status.description, + emoji: status.emoji, + }, + }); + + assert.ok( + exists(".topic-post .cooked .mention .user-status"), + "user status is shown" + ); + const statusElement = query(".topic-post .cooked .mention .user-status"); + assert.equal( + statusElement.title, + status.description, + "status description is correct" + ); + assert.ok( + statusElement.src.includes(status.emoji), + "status emoji is correct" + ); + }); + + test("updates user status on message bus message", async function (assert) { + pretender.get(`/t/${topicId}.json`, () => { + return response(topicWithUserStatus()); + }); + await visit(`/t/lorem-ipsum-dolor-sit-amet/${topicId}`); + + assert.ok( + exists(".topic-post .cooked .mention .user-status"), + "initial user status is shown" + ); + + const newStatus = { + description: "off to dentist", + emoji: "tooth", + }; + await publishToMessageBus("/user-status", { + [mentionedUserId]: { + description: newStatus.description, + emoji: newStatus.emoji, + }, + }); + + assert.ok( + exists(".topic-post .cooked .mention .user-status"), + "updated user status is shown" + ); + const statusElement = query(".topic-post .cooked .mention .user-status"); + assert.equal( + statusElement.title, + newStatus.description, + "updated status description is correct" + ); + assert.ok( + statusElement.src.includes(newStatus.emoji), + "updated status emoji is correct" + ); + }); + + test("removes user status on message bus message", async function (assert) { + pretender.get(`/t/${topicId}.json`, () => { + return response(topicWithUserStatus()); + }); + await visit(`/t/lorem-ipsum-dolor-sit-amet/${topicId}`); + + assert.ok( + exists(".topic-post .cooked .mention .user-status"), + "initial user status is shown" + ); + + await publishToMessageBus("/user-status", { + [mentionedUserId]: null, + }); + + assert.notOk( + exists(".topic-post .cooked .mention .user-status"), + "updated user has disappeared" + ); + }); +}); diff --git a/app/models/post.rb b/app/models/post.rb index 39aab5700d8..7d1f5514145 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1128,6 +1128,10 @@ class Post < ActiveRecord::Base end end + def mentions + PrettyText.extract_mentions(Nokogiri::HTML5.fragment(cooked)) + end + private def parse_quote_into_arguments(quote) diff --git a/app/models/post_analyzer.rb b/app/models/post_analyzer.rb index c049f80c394..bbd5bdd7f3e 100644 --- a/app/models/post_analyzer.rb +++ b/app/models/post_analyzer.rb @@ -78,18 +78,7 @@ class PostAnalyzer def raw_mentions return [] if @raw.blank? return @raw_mentions if @raw_mentions.present? - - raw_mentions = cooked_stripped.css('.mention, .mention-group').map do |e| - if name = e.inner_text - name = name[1..-1] - name = User.normalize_username(name) - name - end - end - - raw_mentions.compact! - raw_mentions.uniq! - @raw_mentions = raw_mentions + @raw_mentions = PrettyText.extract_mentions(cooked_stripped) end # from rack ... compat with ruby 2.2 diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index 8f0bebf4c59..6ec5f549f21 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -88,7 +88,8 @@ class PostSerializer < BasicPostSerializer :reviewable_score_count, :reviewable_score_pending_count, :user_suspended, - :user_status + :user_status, + :mentioned_users def initialize(object, opts) super(object, opts) @@ -560,6 +561,17 @@ class PostSerializer < BasicPostSerializer UserStatusSerializer.new(object.user&.user_status, root: false) end + def mentioned_users + if @topic_view && (mentions = @topic_view.mentions[object.id]) + return mentions + .map { |username| @topic_view.mentioned_users[username] } + .compact + .map { |user| BasicUserWithStatusSerializer.new(user, root: false) } + end + + [] + end + private def can_review_topic? @@ -590,5 +602,4 @@ private def post_actions @post_actions ||= (@topic_view&.all_post_actions || {})[object.id] end - end diff --git a/app/serializers/web_hook_post_serializer.rb b/app/serializers/web_hook_post_serializer.rb index da7dd3bbd0a..beaa6738319 100644 --- a/app/serializers/web_hook_post_serializer.rb +++ b/app/serializers/web_hook_post_serializer.rb @@ -32,6 +32,7 @@ class WebHookPostSerializer < PostSerializer flair_bg_color flair_color notice + mentioned_users }.each do |attr| define_method("include_#{attr}?") do false diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index 49effdc9309..d8cb0cf94dd 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -445,6 +445,20 @@ module PrettyText links end + def self.extract_mentions(cooked) + mentions = cooked.css('.mention, .mention-group').map do |e| + if (name = e.inner_text) + name = name[1..-1] + name = User.normalize_username(name) + name + end + end + + mentions.compact! + mentions.uniq! + mentions + end + def self.excerpt(html, max_length, options = {}) # TODO: properly fix this HACK in ExcerptParser without introducing XSS doc = Nokogiri::HTML5.fragment(html) diff --git a/lib/topic_view.rb b/lib/topic_view.rb index b242d035031..c6c8b8f49f4 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -34,7 +34,9 @@ class TopicView :queued_posts_enabled, :personal_message, :can_review_topic, - :page + :page, + :mentioned_users, + :mentions ) alias queued_posts_enabled? queued_posts_enabled @@ -145,6 +147,9 @@ class TopicView end end + parse_mentions + load_mentioned_users + TopicView.preload(self) @draft_key = @topic.draft_key @@ -670,6 +675,23 @@ class TopicView @topic.published_page end + def parse_mentions + @mentions = @posts + .to_h { |p| [p.id, p.mentions] } + .reject { |_, v| v.empty? } + end + + def load_mentioned_users + usernames = @mentions.values.flatten.uniq + mentioned_users = User.where(username: usernames) + + if SiteSetting.enable_user_status + mentioned_users = mentioned_users.includes(:user_status) + end + + @mentioned_users = mentioned_users.to_h { |u| [u.username, u] } + end + protected def read_posts_set diff --git a/spec/requests/api/schemas/json/topic_create_response.json b/spec/requests/api/schemas/json/topic_create_response.json index 8af2978141d..c933497183e 100644 --- a/spec/requests/api/schemas/json/topic_create_response.json +++ b/spec/requests/api/schemas/json/topic_create_response.json @@ -200,6 +200,11 @@ }, "reviewable_score_pending_count": { "type": "integer" + }, + "mentioned_users": { + "type": "array", + "items": { + } } }, "required": [ diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index fe79743a80b..6f086d212b2 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -2684,6 +2684,91 @@ RSpec.describe TopicsController do expect(response.status).to eq(200) end + context "with mentions" do + fab!(:post) { Fabricate(:post, user: post_author1) } + fab!(:topic) { post.topic } + fab!(:post2) { Fabricate( + :post, + user: post_author2, + topic: topic, + raw: "I am mentioning @#{post_author1.username}." + ) } + + it "returns mentions" do + get "/t/#{topic.slug}/#{topic.id}.json" + + expect(response.status).to eq(200) + + json = response.parsed_body + expect(json["post_stream"]["posts"][1]["mentioned_users"].length).to be(1) + + mentioned_user = json["post_stream"]["posts"][1]["mentioned_users"][0] + expect(mentioned_user["id"]).to be(post_author1.id) + expect(mentioned_user["name"]).to eq(post_author1.name) + expect(mentioned_user["username"]).to eq(post_author1.username) + end + + it "doesn't return status on mentions by default" do + post_author1.set_status!("off to dentist", "tooth") + + get "/t/#{topic.slug}/#{topic.id}.json" + + expect(response.status).to eq(200) + + json = response.parsed_body + expect(json["post_stream"]["posts"][1]["mentioned_users"].length).to be(1) + status = json["post_stream"]["posts"][1]["mentioned_users"][0]["status"] + expect(status).to be_nil + end + + it "returns mentions with status if user status is enabled" do + SiteSetting.enable_user_status = true + post_author1.set_status!("off to dentist", "tooth") + + get "/t/#{topic.slug}/#{topic.id}.json" + + expect(response.status).to eq(200) + + json = response.parsed_body + expect(json["post_stream"]["posts"][1]["mentioned_users"].length).to be(1) + + status = json["post_stream"]["posts"][1]["mentioned_users"][0]["status"] + expect(status).to be_present + expect(status["emoji"]).to eq(post_author1.user_status.emoji) + expect(status["description"]).to eq(post_author1.user_status.description) + end + + it "returns an empty list of mentioned users if there is no mentions in a post" do + Fabricate( + :post, + user: post_author2, + topic: topic, + raw: "Post without mentions.") + + get "/t/#{topic.slug}/#{topic.id}.json" + + expect(response.status).to eq(200) + + json = response.parsed_body + expect(json["post_stream"]["posts"][2]["mentioned_users"].length).to be(0) + end + + it "returns an empty list of mentioned users if an unexisting user was mentioned" do + Fabricate( + :post, + user: post_author2, + topic: topic, + raw: "Mentioning an @unexisting_user.") + + get "/t/#{topic.slug}/#{topic.id}.json" + + expect(response.status).to eq(200) + + json = response.parsed_body + expect(json["post_stream"]["posts"][2]["mentioned_users"].length).to be(0) + end + end + describe "has_escaped_fragment?" do context "when the SiteSetting is disabled" do it "uses the application layout even with an escaped fragment param" do