diff --git a/Gemfile b/Gemfile index cbb730e262..96506d54b5 100644 --- a/Gemfile +++ b/Gemfile @@ -105,7 +105,7 @@ gem "pg" gem "mini_sql" gem "pry-rails", require: false gem "pry-byebug", require: false -gem "rtlcss_wrapper", require: false +gem "rtlcss", require: false gem "rake" gem "thor", require: false diff --git a/Gemfile.lock b/Gemfile.lock index 64d256d821..c9c0ba4eff 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -408,7 +408,7 @@ GEM json-schema (>= 2.2, < 4.0) railties (>= 3.1, < 7.1) rspec-core (>= 2.14) - rtlcss_wrapper (0.1.0) + rtlcss (0.2.0) mini_racer (~> 0.6.3) rubocop (1.44.1) json (~> 2.3) @@ -638,7 +638,7 @@ DEPENDENCIES rspec-rails rss rswag-specs - rtlcss_wrapper + rtlcss rubocop-discourse ruby-prof ruby-readability diff --git a/app/assets/javascripts/discourse-common/package.json b/app/assets/javascripts/discourse-common/package.json index 6dc33fd7cf..8146a41841 100644 --- a/app/assets/javascripts/discourse-common/package.json +++ b/app/assets/javascripts/discourse-common/package.json @@ -20,7 +20,7 @@ "@uppy/core": "^3.0.4", "@uppy/drop-target": "^2.0.1", "@uppy/utils": "^5.1.1", - "@uppy/xhr-upload": "^3.0.4", + "@uppy/xhr-upload": "^3.1.0", "ember-auto-import": "^2.6.0", "ember-cli-babel": "^7.26.10", "ember-cli-htmlbars": "^6.1.1", diff --git a/app/assets/javascripts/discourse/package.json b/app/assets/javascripts/discourse/package.json index 018aca4469..4d12e8a4cc 100644 --- a/app/assets/javascripts/discourse/package.json +++ b/app/assets/javascripts/discourse/package.json @@ -34,7 +34,7 @@ "@uppy/core": "^3.0.4", "@uppy/drop-target": "^2.0.1", "@uppy/utils": "^5.1.1", - "@uppy/xhr-upload": "^3.0.4", + "@uppy/xhr-upload": "^3.1.0", "a11y-dialog": "7.5.2", "admin": "1.0.0", "babel-import-util": "^1.3.0", diff --git a/app/assets/javascripts/discourse/tests/acceptance/topic-post-decorate-cooked-test.js b/app/assets/javascripts/discourse/tests/acceptance/topic-post-decorate-cooked-test.js index 61ccbe8025..2bc38d5993 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/topic-post-decorate-cooked-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/topic-post-decorate-cooked-test.js @@ -23,30 +23,34 @@ acceptance("Acceptance | decorateCookedElement", function () { DemoComponent ); - withPluginApi(0, (api) => { - api.decorateCookedElement((cooked, helper) => { - if (helper.getModel().post_number !== 1) { - return; - } - cooked.innerHTML = - "
Some existing content
"; + withPluginApi( + 0, + (api) => { + api.decorateCookedElement((cooked, helper) => { + if (helper.getModel().post_number !== 1) { + return; + } + cooked.innerHTML = + "
Some existing content
"; - // Create new wrapper element and append - cooked.appendChild( + // Create new wrapper element and append + cooked.appendChild( + helper.renderGlimmer( + "div.glimmer-wrapper", + hbs`<@data.component />`, + { component: DemoComponent } + ) + ); + + // Append to existing element helper.renderGlimmer( - "div.glimmer-wrapper", - hbs`<@data.component />`, - { component: DemoComponent } - ) - ); - - // Append to existing element - helper.renderGlimmer( - cooked.querySelector(".existing-wrapper"), - hbs` with more content from glimmer` - ); - }); - }); + cooked.querySelector(".existing-wrapper"), + hbs` with more content from glimmer` + ); + }); + }, + { id: "render-glimmer-test" } + ); await visit("/t/internationalization-localization/280"); diff --git a/app/assets/javascripts/yarn.lock b/app/assets/javascripts/yarn.lock index f85db172bf..70539046e4 100644 --- a/app/assets/javascripts/yarn.lock +++ b/app/assets/javascripts/yarn.lock @@ -1613,20 +1613,20 @@ resolved "https://registry.yarnpkg.com/@uppy/store-default/-/store-default-3.0.2.tgz#870724c45a2f671d625123cb4a412e3bfae935d9" integrity sha512-kIQMCjXui6tjF1E9xGo4YHkvt71McXkU0FStrcQuBrRXuOhb+NcuWh3sMh3KryivVNgT6w5Odrlw2FUFkl9cqA== -"@uppy/utils@^5.0.2", "@uppy/utils@^5.1.1": - version "5.1.1" - resolved "https://registry.yarnpkg.com/@uppy/utils/-/utils-5.1.1.tgz#9597e8696e17d71413672bd56eb082c7410514a3" - integrity sha512-uoI+PcIVQboky0ZbN4PQeK1seZnnJocomzeK7blId9HKJ6QNgZLf2ibk2CQuQxrOuNsWhgrhs5uLO5Si0oM0Yw== +"@uppy/utils@^5.0.2", "@uppy/utils@^5.1.1", "@uppy/utils@^5.1.2": + version "5.1.2" + resolved "https://registry.yarnpkg.com/@uppy/utils/-/utils-5.1.2.tgz#136e4a2d3fc7222c6d19e0111fdd56bb2449be60" + integrity sha512-a/QSxcYeK1SSOjyyohi0oSwLDMJ9SSWwDELDii1WOKiJwmtt5O6pGCKHTnvrr6RwbiedngG0dZi3koYOG+MdoA== dependencies: lodash.throttle "^4.1.1" -"@uppy/xhr-upload@^3.0.4": - version "3.0.4" - resolved "https://registry.yarnpkg.com/@uppy/xhr-upload/-/xhr-upload-3.0.4.tgz#219a92c832bee1f089992958d27ec71dbe9d9d7d" - integrity sha512-uJ1oxcwEaSLnrexvi6Lp57hV3z3DsovgVmYIVwg+z/EnrRcL32wNRE7FcIr8Mk9e1jdMiFYlk6cQmiP2dZep8A== +"@uppy/xhr-upload@^3.0.4", "@uppy/xhr-upload@^3.1.0": + version "3.1.0" + resolved "https://registry.yarnpkg.com/@uppy/xhr-upload/-/xhr-upload-3.1.0.tgz#a6a2d11329ac8a8745ff15535c156082ebc41e3a" + integrity sha512-v0OsrB74w9RcFU8VMFElxMuGaSFHBuAlkbK30zHEY2YEKUsCfkWMdUbpaWNc1KWERH66Fv57IsA3AbK2HTjWdw== dependencies: - "@uppy/companion-client" "^3.0.2" - "@uppy/utils" "^5.0.2" + "@uppy/companion-client" "^3.1.1" + "@uppy/utils" "^5.1.2" nanoid "^4.0.0" "@webassemblyjs/ast@1.11.1": diff --git a/app/assets/stylesheets/common/base/discourse.scss b/app/assets/stylesheets/common/base/discourse.scss index b8782c233d..e875d3accd 100644 --- a/app/assets/stylesheets/common/base/discourse.scss +++ b/app/assets/stylesheets/common/base/discourse.scss @@ -67,6 +67,7 @@ body { background-attachment: fixed; background-size: cover; min-height: 100%; + box-sizing: border-box; @include clearfix; } diff --git a/lib/stylesheet/compiler.rb b/lib/stylesheet/compiler.rb index 536249ae6d..43018e2705 100644 --- a/lib/stylesheet/compiler.rb +++ b/lib/stylesheet/compiler.rb @@ -66,8 +66,8 @@ module Stylesheet result = engine.render if options[:rtl] - require "rtlcss_wrapper" - [RtlcssWrapper.flip_css(result), nil] + require "rtlcss" + [Rtlcss.flip_css(result), nil] else source_map = engine.source_map source_map.force_encoding("UTF-8") diff --git a/lib/topic_query.rb b/lib/topic_query.rb index cbfbab113f..abdb52d42d 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -507,21 +507,7 @@ class TopicQuery whisperer: @user&.whisperer?, ).order("CASE WHEN topics.user_id = tu.user_id THEN 1 ELSE 2 END") - if @user - # micro optimisation so we don't load up all of user stats which we do not need - unread_at = - DB.query_single("select first_unread_at from user_stats where user_id = ?", @user.id).first - - if max_age = options[:max_age] - max_age_date = max_age.days.ago - unread_at ||= max_age_date - unread_at = unread_at > max_age_date ? unread_at : max_age_date - end - - # perf note, in the past we tried doing this in a subquery but performance was - # terrible, also tried with a join and it was bad - result = result.where("topics.updated_at >= ?", unread_at) - end + result = apply_max_age_limit(result, options) self.class.results_filter_callbacks.each do |filter_callback| result = filter_callback.call(:unread, result, @user, options) @@ -548,6 +534,28 @@ class TopicQuery suggested_ordering(result, options) end + def new_and_unread_results(options = {}) + base = default_results(options.reverse_merge(unordered: true)) + + new_results = + TopicQuery.new_filter( + base, + treat_as_new_topic_start_date: @user.user_option.treat_as_new_topic_start_date, + ) + new_results = remove_muted(new_results, @user, options) + new_results = remove_dismissed(new_results, @user) + + unread_results = + apply_max_age_limit(TopicQuery.unread_filter(base, whisperer: @user&.whisperer?), options) + + base.joins_values.concat(new_results.joins_values, unread_results.joins_values) + base.joins_values.uniq! + results = base.merge(new_results.or(unread_results)) + + results = results.order("CASE WHEN topics.user_id = tu.user_id THEN 1 ELSE 2 END") + suggested_ordering(results, options) + end + protected def per_page_setting @@ -1167,4 +1175,23 @@ class TopicQuery col_name = whisperer ? "highest_staff_post_number" : "highest_post_number" list.where("tu.last_read_post_number IS NULL OR tu.last_read_post_number < topics.#{col_name}") end + + def apply_max_age_limit(results, options) + if @user + # micro optimisation so we don't load up all of user stats which we do not need + unread_at = + DB.query_single("select first_unread_at from user_stats where user_id = ?", @user.id).first + + if max_age = options[:max_age] + max_age_date = max_age.days.ago + unread_at ||= max_age_date + unread_at = unread_at > max_age_date ? unread_at : max_age_date + end + + # perf note, in the past we tried doing this in a subquery but performance was + # terrible, also tried with a join and it was bad + results = results.where("topics.updated_at >= ?", unread_at) + end + results + end end diff --git a/plugins/chat/assets/javascripts/discourse/components/channels-list.js b/plugins/chat/assets/javascripts/discourse/components/channels-list.js index 9cac0ff735..bb1b660ebc 100644 --- a/plugins/chat/assets/javascripts/discourse/components/channels-list.js +++ b/plugins/chat/assets/javascripts/discourse/components/channels-list.js @@ -84,12 +84,20 @@ export default class ChannelsList extends Component { @action storeScrollPosition() { + if (this.chatStateManager.isDrawerActive) { + return; + } + const scrollTop = document.querySelector(".channels-list")?.scrollTop || 0; this.session.channelsListPosition = scrollTop; } @bind _applyScrollPosition() { + if (this.chatStateManager.isDrawerActive) { + return; + } + const position = this.chatStateManager.isFullPageActive ? this.session.channelsListPosition || 0 : 0; diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message-info.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-message-info.hbs index efea5081cd..2520c2186b 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message-info.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message-info.hbs @@ -1,7 +1,16 @@ -
+
{{#if @message.chat_webhook_event}} {{#if @message.chat_webhook_event.username}} - + {{@message.chat_webhook_event.username}} {{/if}} @@ -12,7 +21,11 @@ {{else}} {{this.name}} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message-info.js b/plugins/chat/assets/javascripts/discourse/components/chat-message-info.js index 9bb31d6f4c..c33af19bca 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message-info.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message-info.js @@ -1,57 +1,27 @@ -import { computed } from "@ember/object"; -import Component from "@ember/component"; +import Component from "@glimmer/component"; import { prioritizeNameInUx } from "discourse/lib/settings"; +import { inject as service } from "@ember/service"; +import { bind } from "discourse-common/utils/decorators"; export default class ChatMessageInfo extends Component { - tagName = ""; - message = null; - details = null; + @service siteSettings; - didInsertElement() { - this._super(...arguments); - this.message.user?.trackStatus?.(); + @bind + trackStatus() { + this.#user?.trackStatus?.(); } - willDestroyElement() { - this._super(...arguments); - this.message.user?.stopTrackingStatus?.(); + @bind + stopTrackingStatus() { + this.#user?.stopTrackingStatus?.(); } - @computed("message.user") - get name() { - return this.prioritizeName - ? this.message.user.name - : this.message.user.username; - } - - @computed("message.reviewable_id", "message.user_flag_status") - get isFlagged() { - return this.message?.reviewable_id || this.message?.user_flag_status === 0; - } - - @computed("message.user.name") - get prioritizeName() { - return ( - this.siteSettings.display_name_on_posts && - prioritizeNameInUx(this.message?.user?.name) - ); - } - - @computed("message.user.status") - get showStatus() { - return !!this.message.user?.status; - } - - @computed("message.user") get usernameClasses() { - const user = this.message?.user; - + const user = this.#user; const classes = this.prioritizeName ? ["is-full-name"] : ["is-username"]; - if (!user) { return classes; } - if (user.staff) { classes.push("is-staff"); } @@ -64,7 +34,44 @@ export default class ChatMessageInfo extends Component { if (user.groupModerator) { classes.push("is-category-moderator"); } - + if (user.new_user) { + classes.push("is-new-user"); + } + if (user.primary_group_name) { + classes.push("group--" + user.primary_group_name); + } return classes.join(" "); } + + get name() { + return this.prioritizeName + ? this.#user?.get("name") + : this.#user?.get("username"); + } + + get isFlagged() { + return ( + this.#message?.get("reviewable_id") || + this.#message?.get("user_flag_status") === 0 + ); + } + + get prioritizeName() { + return ( + this.siteSettings.display_name_on_posts && + prioritizeNameInUx(this.#user?.get("name")) + ); + } + + get showStatus() { + return !!this.#user?.get("status"); + } + + get #user() { + return this.#message?.get("user"); + } + + get #message() { + return this.args.message; + } } diff --git a/plugins/chat/assets/stylesheets/common/common.scss b/plugins/chat/assets/stylesheets/common/common.scss index 7cc3ae8d39..a36e52e46c 100644 --- a/plugins/chat/assets/stylesheets/common/common.scss +++ b/plugins/chat/assets/stylesheets/common/common.scss @@ -606,7 +606,8 @@ html.has-full-page-chat { #main-outlet-wrapper { // restrict the row height, including when virtual keyboard is open grid-template-rows: calc( - var(--chat-vh, 1vh) * 100 - var(--header-offset) + var(--chat-vh, 1vh) * 100 - var(--header-offset, 0px) - + var(--footer-nav-height, 0px) ); .sidebar-wrapper { // prevents sidebar from overflowing behind the virtual keyboard @@ -614,16 +615,6 @@ html.has-full-page-chat { } } - // iPad webview - .footer-nav-ipad { - #main-outlet-wrapper { - // restrict the row height, including when virtual keyboard is open - grid-template-rows: calc( - var(--chat-vh, 1vh) * 100 - calc(var(--header-offset)) - ); - } - } - .full-page-chat, .chat-live-pane, #main-outlet { diff --git a/plugins/chat/assets/stylesheets/mobile/mobile.scss b/plugins/chat/assets/stylesheets/mobile/mobile.scss index 8e54337969..107c8629dc 100644 --- a/plugins/chat/assets/stylesheets/mobile/mobile.scss +++ b/plugins/chat/assets/stylesheets/mobile/mobile.scss @@ -92,13 +92,6 @@ html.has-full-page-chat { } } -html.has-full-page-chat body { - #main-outlet-wrapper { - // restricts the height of the page - grid-template-rows: calc(var(--chat-vh, 1vh) * 100 - var(--header-offset)); - } -} - .chat-message-separator { margin-left: 0; } diff --git a/plugins/chat/lib/chat_message_mentions.rb b/plugins/chat/lib/chat_message_mentions.rb new file mode 100644 index 0000000000..84516466b0 --- /dev/null +++ b/plugins/chat/lib/chat_message_mentions.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +class Chat::ChatMessageMentions + def initialize(message) + @message = message + + mentions = parse_mentions(message) + group_mentions = parse_group_mentions(message) + + @has_global_mention = mentions.include?("@all") + @has_here_mention = mentions.include?("@here") + @parsed_direct_mentions = normalize(mentions) + @parsed_group_mentions = normalize(group_mentions) + end + + attr_accessor :has_global_mention, + :has_here_mention, + :parsed_direct_mentions, + :parsed_group_mentions + + def global_mentions + return User.none unless @has_global_mention + channel_members.where.not(username_lower: @parsed_direct_mentions) + end + + def direct_mentions + chat_users.where(username_lower: @parsed_direct_mentions) + end + + def group_mentions + chat_users.includes(:groups).joins(:groups).where(groups: mentionable_groups) + end + + def here_mentions + return User.none unless @has_here_mention + + channel_members + .where("last_seen_at > ?", 5.minutes.ago) + .where.not(username_lower: @parsed_direct_mentions) + end + + def mentionable_groups + @mentionable_groups ||= + Group.mentionable(@message.user, include_public: false).where(id: visible_groups.map(&:id)) + end + + def visible_groups + @visible_groups ||= + Group.where("LOWER(name) IN (?)", @parsed_group_mentions).visible_groups(@message.user) + end + + private + + def channel_members + chat_users.where( + user_chat_channel_memberships: { + following: true, + chat_channel_id: @message.chat_channel.id, + }, + ) + end + + def chat_users + User + .includes(:user_chat_channel_memberships, :group_users) + .distinct + .joins("LEFT OUTER JOIN user_chat_channel_memberships uccm ON uccm.user_id = users.id") + .joins(:user_option) + .real + .not_suspended + .where(user_options: { chat_enabled: true }) + .where.not(username_lower: @message.user.username.downcase) + end + + def parse_mentions(message) + Nokogiri::HTML5.fragment(message.cooked).css(".mention").map(&:text) + end + + def parse_group_mentions(message) + Nokogiri::HTML5.fragment(message.cooked).css(".mention-group").map(&:text) + end + + def normalize(mentions) + mentions.reduce([]) do |memo, mention| + %w[@here @all].include?(mention.downcase) ? memo : (memo << mention[1..-1].downcase) + end + end +end diff --git a/plugins/chat/lib/chat_notifier.rb b/plugins/chat/lib/chat_notifier.rb index 4719b96801..7887cfade9 100644 --- a/plugins/chat/lib/chat_notifier.rb +++ b/plugins/chat/lib/chat_notifier.rb @@ -60,6 +60,7 @@ class Chat::ChatNotifier @timestamp = timestamp @chat_channel = @chat_message.chat_channel @user = @chat_message.user + @mentions = Chat::ChatMessageMentions.new(chat_message) end ### Public API @@ -108,11 +109,12 @@ class Chat::ChatNotifier private def list_users_to_notify - direct_mentions_count = direct_mentions_from_cooked.length - group_mentions_count = group_name_mentions.length + mentions_count = + @mentions.parsed_direct_mentions.length + @mentions.parsed_group_mentions.length + mentions_count += 1 if @mentions.has_global_mention + mentions_count += 1 if @mentions.has_here_mention - skip_notifications = - (direct_mentions_count + group_mentions_count) > SiteSetting.max_mentions_per_chat_message + skip_notifications = mentions_count > SiteSetting.max_mentions_per_chat_message {}.tap do |to_notify| # The order of these methods is the precedence @@ -131,48 +133,13 @@ class Chat::ChatNotifier end end - def chat_users - User - .includes(:user_chat_channel_memberships, :group_users) - .distinct - .joins("LEFT OUTER JOIN user_chat_channel_memberships uccm ON uccm.user_id = users.id") - .joins(:user_option) - .real - .not_suspended - .where(user_options: { chat_enabled: true }) - .where.not(username_lower: @user.username.downcase) - end - - def rest_of_the_channel - chat_users.where( - user_chat_channel_memberships: { - following: true, - chat_channel_id: @chat_channel.id, - }, - ) - end - - def members_accepting_channel_wide_notifications - rest_of_the_channel.where(user_options: { ignore_channel_wide_mention: [false, nil] }) - end - - def direct_mentions_from_cooked - @direct_mentions_from_cooked ||= - Nokogiri::HTML5.fragment(@chat_message.cooked).css(".mention").map(&:text) - end - - def normalized_mentions(mentions) - mentions.reduce([]) do |memo, mention| - %w[@here @all].include?(mention.downcase) ? memo : (memo << mention[1..-1].downcase) - end - end - def expand_global_mention(to_notify, already_covered_ids, skip) - typed_global_mention = direct_mentions_from_cooked.include?("@all") + has_all_mention = @mentions.has_global_mention - if typed_global_mention && @chat_channel.allow_channel_wide_mentions && !skip - to_notify[:global_mentions] = members_accepting_channel_wide_notifications - .where.not(username_lower: normalized_mentions(direct_mentions_from_cooked)) + if has_all_mention && @chat_channel.allow_channel_wide_mentions && !skip + to_notify[:global_mentions] = @mentions + .global_mentions + .where(user_options: { ignore_channel_wide_mention: [false, nil] }) .where.not(id: already_covered_ids) .pluck(:id) @@ -183,12 +150,12 @@ class Chat::ChatNotifier end def expand_here_mention(to_notify, already_covered_ids, skip) - typed_here_mention = direct_mentions_from_cooked.include?("@here") + has_here_mention = @mentions.has_here_mention - if typed_here_mention && @chat_channel.allow_channel_wide_mentions && !skip - to_notify[:here_mentions] = members_accepting_channel_wide_notifications - .where("last_seen_at > ?", 5.minutes.ago) - .where.not(username_lower: normalized_mentions(direct_mentions_from_cooked)) + if has_here_mention && @chat_channel.allow_channel_wide_mentions && !skip + to_notify[:here_mentions] = @mentions + .here_mentions + .where(user_options: { ignore_channel_wide_mention: [false, nil] }) .where.not(id: already_covered_ids) .pluck(:id) @@ -225,10 +192,7 @@ class Chat::ChatNotifier if skip direct_mentions = [] else - direct_mentions = - chat_users - .where(username_lower: normalized_mentions(direct_mentions_from_cooked)) - .where.not(id: already_covered_ids) + direct_mentions = @mentions.direct_mentions.where.not(id: already_covered_ids) end grouped = group_users_to_notify(direct_mentions) @@ -239,48 +203,31 @@ class Chat::ChatNotifier already_covered_ids.concat(to_notify[:direct_mentions]) end - def group_name_mentions - @group_mentions_from_cooked ||= - normalized_mentions( - Nokogiri::HTML5.fragment(@chat_message.cooked).css(".mention-group").map(&:text), - ) - end - - def visible_groups - @visible_groups ||= Group.where("LOWER(name) IN (?)", group_name_mentions).visible_groups(@user) - end - def expand_group_mentions(to_notify, already_covered_ids, skip) - return [] if skip || visible_groups.empty? + return [] if skip || @mentions.visible_groups.empty? - mentionable_groups = - Group.mentionable(@user, include_public: false).where(id: visible_groups.map(&:id)) - - mentions_disabled = visible_groups - mentionable_groups + reached_by_group = + @mentions + .group_mentions + .where("user_count <= ?", SiteSetting.max_users_notified_per_group_mention) + .where.not(id: already_covered_ids) too_many_members, mentionable = - mentionable_groups.partition do |group| + @mentions.mentionable_groups.partition do |group| group.user_count > SiteSetting.max_users_notified_per_group_mention end + mentions_disabled = @mentions.visible_groups - @mentions.mentionable_groups to_notify[:group_mentions_disabled] = mentions_disabled to_notify[:too_many_members] = too_many_members - mentionable.each { |g| to_notify[g.name.downcase] = [] } - reached_by_group = - chat_users - .includes(:groups) - .joins(:groups) - .where(groups: mentionable) - .where.not(id: already_covered_ids) - grouped = group_users_to_notify(reached_by_group) - grouped[:already_participating].each do |user| # When a user is a member of multiple mentioned groups, # the most far to the left should take precedence. - ordered_group_names = group_name_mentions & mentionable.map { |mg| mg.name.downcase } + ordered_group_names = + @mentions.parsed_group_mentions & mentionable.map { |mg| mg.name.downcase } user_group_names = user.groups.map { |ug| ug.name.downcase } group_name = ordered_group_names.detect { |gn| user_group_names.include?(gn) } diff --git a/plugins/chat/plugin.rb b/plugins/chat/plugin.rb index c5b2f5c625..8a4792cb65 100644 --- a/plugins/chat/plugin.rb +++ b/plugins/chat/plugin.rb @@ -178,6 +178,7 @@ after_initialize do load File.expand_path("../lib/chat_message_updater.rb", __FILE__) load File.expand_path("../lib/chat_message_rate_limiter.rb", __FILE__) load File.expand_path("../lib/chat_message_reactor.rb", __FILE__) + load File.expand_path("../lib/chat_message_mentions.rb", __FILE__) load File.expand_path("../lib/chat_notifier.rb", __FILE__) load File.expand_path("../lib/chat_seeder.rb", __FILE__) load File.expand_path("../lib/chat_statistics.rb", __FILE__) diff --git a/plugins/chat/spec/lib/chat_message_mentions_spec.rb b/plugins/chat/spec/lib/chat_message_mentions_spec.rb new file mode 100644 index 0000000000..7ff25c873d --- /dev/null +++ b/plugins/chat/spec/lib/chat_message_mentions_spec.rb @@ -0,0 +1,154 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe Chat::ChatMessageMentions do + fab!(:channel_member_1) { Fabricate(:user) } + fab!(:channel_member_2) { Fabricate(:user) } + fab!(:channel_member_3) { Fabricate(:user) } + fab!(:not_a_channel_member) { Fabricate(:user) } + fab!(:chat_channel) { Fabricate(:chat_channel) } + + before do + chat_channel.add(channel_member_1) + chat_channel.add(channel_member_2) + chat_channel.add(channel_member_3) + end + + describe "#global_mentions" do + it "returns all members of the channel" do + message = create_message("mentioning @all") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.global_mentions.pluck(:username) + + expect(result).to contain_exactly( + channel_member_1.username, + channel_member_2.username, + channel_member_3.username, + ) + end + + it "doesn't include users that were also mentioned directly" do + message = create_message("mentioning @all and @#{channel_member_1.username}") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.global_mentions.pluck(:username) + + expect(result).to contain_exactly(channel_member_2.username, channel_member_3.username) + end + + it "returns an empty list if there are no global mentions" do + message = create_message("not mentioning anybody") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.global_mentions.pluck(:username) + + expect(result).to be_empty + end + end + + describe "#here_mentions" do + before do + freeze_time + channel_member_1.update(last_seen_at: 2.minutes.ago) + channel_member_2.update(last_seen_at: 2.minutes.ago) + channel_member_3.update(last_seen_at: 5.minutes.ago) + end + + it "returns all members of the channel who were online in the last 5 minutes" do + message = create_message("mentioning @here") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.here_mentions.pluck(:username) + + expect(result).to contain_exactly(channel_member_1.username, channel_member_2.username) + end + + it "doesn't include users that were also mentioned directly" do + message = create_message("mentioning @here and @#{channel_member_1.username}") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.here_mentions.pluck(:username) + + expect(result).to contain_exactly(channel_member_2.username) + end + + it "returns an empty list if there are no here mentions" do + message = create_message("not mentioning anybody") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.here_mentions.pluck(:username) + + expect(result).to be_empty + end + end + + describe "#direct_mentions" do + it "returns users who were mentioned directly" do + message = + create_message("mentioning @#{channel_member_1.username} and @#{channel_member_2.username}") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.direct_mentions.pluck(:username) + + expect(result).to contain_exactly(channel_member_1.username, channel_member_2.username) + end + + it "returns a mentioned user even if he's not a member of the channel" do + message = create_message("mentioning @#{not_a_channel_member.username}") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.direct_mentions.pluck(:username) + + expect(result).to contain_exactly(not_a_channel_member.username) + end + + it "returns an empty list if no one was mentioned directly" do + message = create_message("not mentioning anybody") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.direct_mentions.pluck(:username) + + expect(result).to be_empty + end + end + + describe "#group_mentions" do + fab!(:group1) { Fabricate(:group, mentionable_level: Group::ALIAS_LEVELS[:everyone]) } + fab!(:group_member_1) { Fabricate(:user, group_ids: [group1.id]) } + fab!(:group_member_2) { Fabricate(:user, group_ids: [group1.id]) } + fab!(:group_member_3) { Fabricate(:user, group_ids: [group1.id]) } + + before do + chat_channel.add(group_member_1) + chat_channel.add(group_member_2) + end + + it "returns members of a mentioned group even if some of them is not members of the channel" do + message = create_message("mentioning @#{group1.name}") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.group_mentions.pluck(:username) + + expect(result).to contain_exactly( + group_member_1.username, + group_member_2.username, + group_member_3.username, + ) + end + + it "returns an empty list if no group was mentioned" do + message = create_message("not mentioning anybody") + + mentions = Chat::ChatMessageMentions.new(message) + result = mentions.group_mentions.pluck(:username) + + expect(result).to be_empty + end + end + + def create_message(text) + Fabricate(:chat_message, chat_channel: chat_channel, message: text) + end +end diff --git a/plugins/chat/test/javascripts/components/chat-message-info-test.js b/plugins/chat/test/javascripts/components/chat-message-info-test.js index 4fea2ebd07..989656e803 100644 --- a/plugins/chat/test/javascripts/components/chat-message-info-test.js +++ b/plugins/chat/test/javascripts/components/chat-message-info-test.js @@ -4,14 +4,17 @@ import hbs from "htmlbars-inline-precompile"; import { exists, query } from "discourse/tests/helpers/qunit-helpers"; import I18n from "I18n"; import { module, test } from "qunit"; -import User from "discourse/models/user"; import { render } from "@ember/test-helpers"; +import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; module("Discourse Chat | Component | chat-message-info", function (hooks) { setupRenderingTest(hooks); test("chat_webhook_event", async function (assert) { - this.set("message", { chat_webhook_event: { username: "discobot" } }); + this.set( + "message", + ChatMessage.create({ chat_webhook_event: { username: "discobot" } }) + ); await render(hbs``); @@ -26,7 +29,7 @@ module("Discourse Chat | Component | chat-message-info", function (hooks) { }); test("user", async function (assert) { - this.set("message", { user: { username: "discobot" } }); + this.set("message", ChatMessage.create({ user: { username: "discobot" } })); await render(hbs``); @@ -37,10 +40,13 @@ module("Discourse Chat | Component | chat-message-info", function (hooks) { }); test("date", async function (assert) { - this.set("message", { - user: { username: "discobot" }, - created_at: moment(), - }); + this.set( + "message", + ChatMessage.create({ + user: { username: "discobot" }, + created_at: moment(), + }) + ); await render(hbs``); @@ -48,13 +54,16 @@ module("Discourse Chat | Component | chat-message-info", function (hooks) { }); test("bookmark (with reminder)", async function (assert) { - this.set("message", { - user: { username: "discobot" }, - bookmark: Bookmark.create({ - reminder_at: moment(), - name: "some name", - }), - }); + this.set( + "message", + ChatMessage.create({ + user: { username: "discobot" }, + bookmark: Bookmark.create({ + reminder_at: moment(), + name: "some name", + }), + }) + ); await render(hbs``); @@ -64,12 +73,15 @@ module("Discourse Chat | Component | chat-message-info", function (hooks) { }); test("bookmark (no reminder)", async function (assert) { - this.set("message", { - user: { username: "discobot" }, - bookmark: Bookmark.create({ - name: "some name", - }), - }); + this.set( + "message", + ChatMessage.create({ + user: { username: "discobot" }, + bookmark: Bookmark.create({ + name: "some name", + }), + }) + ); await render(hbs``); @@ -78,7 +90,7 @@ module("Discourse Chat | Component | chat-message-info", function (hooks) { test("user status", async function (assert) { const status = { description: "off to dentist", emoji: "tooth" }; - this.set("message", { user: User.create({ status }) }); + this.set("message", ChatMessage.create({ user: { status } })); await render(hbs``); @@ -86,10 +98,13 @@ module("Discourse Chat | Component | chat-message-info", function (hooks) { }); test("reviewable", async function (assert) { - this.set("message", { - user: { username: "discobot" }, - user_flag_status: 0, - }); + this.set( + "message", + ChatMessage.create({ + user: { username: "discobot" }, + user_flag_status: 0, + }) + ); await render(hbs``); @@ -98,14 +113,57 @@ module("Discourse Chat | Component | chat-message-info", function (hooks) { I18n.t("chat.you_flagged") ); - this.set("message", { - user: { username: "discobot" }, - reviewable_id: 1, - }); + this.set( + "message", + ChatMessage.create({ + user: { username: "discobot" }, + reviewable_id: 1, + }) + ); assert.strictEqual( query(".chat-message-info__flag a .svg-icon-title").title, I18n.t("chat.flagged") ); }); + + test("with username classes", async function (assert) { + this.set( + "message", + ChatMessage.create({ + user: { + username: "discobot", + admin: true, + moderator: true, + groupModerator: true, + new_user: true, + primary_group_name: "foo", + }, + }) + ); + + await render(hbs``); + + assert.dom(".chat-message-info__username.is-staff").exists(); + assert.dom(".chat-message-info__username.is-admin").exists(); + assert.dom(".chat-message-info__username.is-moderator").exists(); + assert.dom(".chat-message-info__username.is-category-moderator ").exists(); + assert.dom(".chat-message-info__username.is-new-user").exists(); + assert.dom(".chat-message-info__username.group--foo").exists(); + }); + + test("without username classes", async function (assert) { + this.set("message", ChatMessage.create({ user: { username: "discobot" } })); + + await render(hbs``); + + assert.dom(".chat-message-info__username.is-staff").doesNotExist(); + assert.dom(".chat-message-info__username.is-admin").doesNotExist(); + assert.dom(".chat-message-info__username.is-moderator").doesNotExist(); + assert + .dom(".chat-message-info__username.is-category-moderator ") + .doesNotExist(); + assert.dom(".chat-message-info__username.is-new-user").doesNotExist(); + assert.dom(".chat-message-info__username.group--foo").doesNotExist(); + }); }); diff --git a/spec/lib/topic_query_spec.rb b/spec/lib/topic_query_spec.rb index 261a4cbb7c..520ee27e5a 100644 --- a/spec/lib/topic_query_spec.rb +++ b/spec/lib/topic_query_spec.rb @@ -1731,4 +1731,146 @@ RSpec.describe TopicQuery do end end end + + describe "#new_and_unread_results" do + fab!(:unread_topic) { Fabricate(:post).topic } + fab!(:new_topic) { Fabricate(:post).topic } + fab!(:read_topic) { Fabricate(:post).topic } + + before do + unread_post = Fabricate(:post, topic: unread_topic) + read_post = Fabricate(:post, topic: read_topic) + + TopicUser.change( + user.id, + unread_topic.id, + notification_level: TopicUser.notification_levels[:tracking], + ) + TopicUser.change( + user.id, + read_topic.id, + notification_level: TopicUser.notification_levels[:tracking], + ) + TopicUser.update_last_read(user, unread_topic.id, unread_post.post_number - 1, 1, 1) + TopicUser.update_last_read(user, read_topic.id, read_post.post_number, 1, 1) + end + + it "includes unread and new topics for the user" do + expect(TopicQuery.new(user).new_and_unread_results.pluck(:id)).to contain_exactly( + unread_topic.id, + new_topic.id, + ) + end + + it "doesn't include deleted topics" do + unread_topic.trash! + expect(TopicQuery.new(user).new_and_unread_results.pluck(:id)).to contain_exactly( + new_topic.id, + ) + end + + it "doesn't include muted topics with unread posts" do + TopicUser.change( + user.id, + unread_topic.id, + notification_level: TopicUser.notification_levels[:muted], + ) + expect(TopicQuery.new(user).new_and_unread_results.pluck(:id)).to contain_exactly( + new_topic.id, + ) + end + + it "doesn't include muted new topics" do + TopicUser.change( + user.id, + new_topic.id, + notification_level: TopicUser.notification_levels[:muted], + ) + expect(TopicQuery.new(user).new_and_unread_results.pluck(:id)).to contain_exactly( + unread_topic.id, + ) + end + + it "doesn't include new topics in muted category" do + CategoryUser.create!( + user_id: user.id, + category_id: new_topic.category.id, + notification_level: CategoryUser.notification_levels[:muted], + ) + expect(TopicQuery.new(user).new_and_unread_results.pluck(:id)).to contain_exactly( + unread_topic.id, + ) + end + + it "includes unread and trakced topics even if they're in a muted category" do + new_topic.update!(category: Fabricate(:category)) + CategoryUser.create!( + user_id: user.id, + category_id: unread_topic.category.id, + notification_level: CategoryUser.notification_levels[:muted], + ) + expect(TopicQuery.new(user).new_and_unread_results.pluck(:id)).to contain_exactly( + unread_topic.id, + new_topic.id, + ) + end + + it "doesn't include new topics that have a muted tag(s)" do + SiteSetting.tagging_enabled = true + + tag = Fabricate(:tag) + new_topic.tags << tag + new_topic.save! + + TagUser.create!( + tag_id: tag.id, + user_id: user.id, + notification_level: NotificationLevels.all[:muted], + ) + expect(TopicQuery.new(user).new_and_unread_results.pluck(:id)).to contain_exactly( + unread_topic.id, + ) + end + + it "includes unread and tracked topics even if they have a muted tag(s)" do + SiteSetting.tagging_enabled = true + + tag = Fabricate(:tag) + unread_topic.tags << tag + unread_topic.save! + + TagUser.create!( + tag_id: tag.id, + user_id: user.id, + notification_level: NotificationLevels.all[:muted], + ) + expect(TopicQuery.new(user).new_and_unread_results.pluck(:id)).to contain_exactly( + unread_topic.id, + new_topic.id, + ) + end + + it "doesn't include topics in restricted categories that user cannot access" do + category = Fabricate(:category_with_definition) + group = Fabricate(:group) + category.set_permissions(group => :full) + category.save! + + unread_topic.update!(category: category) + new_topic.update!(category: category) + + expect(TopicQuery.new(user).new_and_unread_results.pluck(:id)).to be_blank + end + + it "doesn't include dismissed topics" do + DismissedTopicUser.create!( + user_id: user.id, + topic_id: new_topic.id, + created_at: Time.zone.now, + ) + expect(TopicQuery.new(user).new_and_unread_results.pluck(:id)).to contain_exactly( + unread_topic.id, + ) + end + end end