DEV: Redesign chat mentions (#24752)

At the moment, when someone is mentioning a group, or using here or 
all mention, we create a chat_mention record per user. What we want 
instead is to have special kinds of mentions, so we can create only one 
chat_mention record in such cases. This PR implements that.

Note, that such mentions will still have N related notifications, one 
notification per a user. We don't expect we'll have performance 
problems on the notifications side, but if at some point we do, we 
should be able to solve them on the side of notifications 
(notifications are handled in jobs, also some little delays with 
the notifications are acceptable, so we can make sure notifications 
are properly queued, and that processing of every notification is 
fast enough to make delays small enough).

The preparation work for this PR was done in fbd24fa, where we make 
it possible for one mention to have several related notifications.

A pretty tricky part of this PR is schema and data migration, I've explained 
related details inline on the migration files.
This commit is contained in:
Andrei Prigorshnev 2024-01-17 15:24:01 +04:00 committed by GitHub
parent 6876c52857
commit 62f423da15
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
40 changed files with 613 additions and 157 deletions

View File

@ -145,13 +145,43 @@ module Jobs
memberships = get_memberships(user_ids) memberships = get_memberships(user_ids)
memberships.each do |membership| memberships.each do |membership|
mention = ::Chat::Mention.find_by(user: membership.user, chat_message: @chat_message) mention = find_mention(@chat_message, mention_type, membership.user.id)
if mention.present? if mention.present?
create_notification!(membership, mention, mention_type) create_notification!(membership, mention, mention_type)
send_notifications(membership, mention_type) send_notifications(membership, mention_type)
end end
end end
end end
def find_mention(chat_message, mention_type, user_id)
mention_klass = resolve_mention_klass(mention_type)
target_id = nil
if mention_klass == ::Chat::UserMention
target_id = user_id
elsif mention_klass == ::Chat::GroupMention
begin
target_id = Group.where("LOWER(name) = ?", "#{mention_type}").first.id
rescue => e
Discourse.warn_exception(e, message: "Mentioned group doesn't exist")
end
end
mention_klass.find_by(chat_message: chat_message, target_id: target_id)
end
def resolve_mention_klass(mention_type)
case mention_type
when :global_mentions
::Chat::AllMention
when :here_mentions
::Chat::HereMention
when :direct_mentions
::Chat::UserMention
else
::Chat::GroupMention
end
end
end end
end end
end end

View File

@ -0,0 +1,6 @@
# frozen_string_literal: true
module Chat
class AllMention < Mention
end
end

View File

@ -0,0 +1,7 @@
# frozen_string_literal: true
module Chat
class GroupMention < Mention
belongs_to :group, foreign_key: :target_id
end
end

View File

@ -0,0 +1,6 @@
# frozen_string_literal: true
module Chat
class HereMention < Mention
end
end

View File

@ -3,9 +3,8 @@
module Chat module Chat
class Mention < ActiveRecord::Base class Mention < ActiveRecord::Base
self.table_name = "chat_mentions" self.table_name = "chat_mentions"
self.ignored_columns = ["notification_id"] self.ignored_columns = %w[notification_id user_id]
belongs_to :user
belongs_to :chat_message, class_name: "Chat::Message" belongs_to :chat_message, class_name: "Chat::Message"
has_many :mention_notifications, has_many :mention_notifications,
class_name: "Chat::MentionNotification", class_name: "Chat::MentionNotification",
@ -20,12 +19,15 @@ end
# #
# id :bigint not null, primary key # id :bigint not null, primary key
# chat_message_id :integer not null # chat_message_id :integer not null
# user_id :integer not null # user_id :integer
# notification_id :integer not null # notification_id :integer not null
# target_id :integer
# type :integer not null
# created_at :datetime not null # created_at :datetime not null
# updated_at :datetime not null # updated_at :datetime not null
# #
# Indexes # Indexes
# #
# chat_mentions_index (chat_message_id,user_id,notification_id) UNIQUE # index_chat_mentions_on_chat_message_id (chat_message_id)
# index_chat_mentions_on_target_id (target_id)
# #

View File

@ -51,6 +51,22 @@ module Chat
dependent: :destroy, dependent: :destroy,
class_name: "Chat::Mention", class_name: "Chat::Mention",
foreign_key: :chat_message_id foreign_key: :chat_message_id
has_many :user_mentions,
dependent: :destroy,
class_name: "Chat::UserMention",
foreign_key: :chat_message_id
has_many :group_mentions,
dependent: :destroy,
class_name: "Chat::GroupMention",
foreign_key: :chat_message_id
has_one :all_mention,
dependent: :destroy,
class_name: "Chat::AllMention",
foreign_key: :chat_message_id
has_one :here_mention,
dependent: :destroy,
class_name: "Chat::HereMention",
foreign_key: :chat_message_id
scope :in_public_channel, scope :in_public_channel,
-> do -> do
@ -248,14 +264,10 @@ module Chat
end end
def upsert_mentions def upsert_mentions
mentioned_user_ids = parsed_mentions.all_mentioned_users_ids upsert_user_mentions
old_mentions = chat_mentions.pluck(:user_id) upsert_group_mentions
create_or_delete_all_mention
mentioned_user_ids_to_drop = old_mentions - mentioned_user_ids create_or_delete_here_mention
delete_mentions(mentioned_user_ids_to_drop)
mentioned_user_ids_to_add = mentioned_user_ids - old_mentions
insert_mentions(mentioned_user_ids_to_add)
end end
def in_thread? def in_thread?
@ -280,24 +292,16 @@ module Chat
private private
def delete_mentions(user_ids) def delete_mentions(mention_type, target_ids)
chat_mentions.where(user_id: user_ids).destroy_all chat_mentions.where(type: mention_type, target_id: target_ids).destroy_all
end end
def insert_mentions(user_ids) def insert_mentions(type, target_ids)
return if user_ids.empty? return if target_ids.empty?
now = Time.zone.now mentions =
mentions = [] target_ids.map do |target_id|
User { chat_message_id: self.id, target_id: target_id, type: type }
.where(id: user_ids)
.find_each do |user|
mentions << {
chat_message_id: self.id,
user_id: user.id,
created_at: now,
updated_at: now,
}
end end
Chat::Mention.insert_all(mentions) Chat::Mention.insert_all(mentions)
@ -314,6 +318,38 @@ module Chat
def ensure_last_editor_id def ensure_last_editor_id
self.last_editor_id ||= self.user_id self.last_editor_id ||= self.user_id
end end
def create_or_delete_all_mention
if !parsed_mentions.has_global_mention && all_mention.present?
all_mention.destroy!
association(:all_mention).reload
elsif parsed_mentions.has_global_mention && all_mention.blank?
build_all_mention.save!
end
end
def create_or_delete_here_mention
if !parsed_mentions.has_here_mention && here_mention.present?
here_mention.destroy!
association(:here_mention).reload
elsif parsed_mentions.has_here_mention && here_mention.blank?
build_here_mention.save!
end
end
def upsert_group_mentions
old_mentions = group_mentions.pluck(:target_id)
new_mentions = parsed_mentions.groups_to_mention.pluck(:id)
delete_mentions("Chat::GroupMention", old_mentions - new_mentions)
insert_mentions("Chat::GroupMention", new_mentions - old_mentions)
end
def upsert_user_mentions
old_mentions = user_mentions.pluck(:target_id)
new_mentions = parsed_mentions.direct_mentions.pluck(:id)
delete_mentions("Chat::UserMention", old_mentions - new_mentions)
insert_mentions("Chat::UserMention", new_mentions - old_mentions)
end
end end
end end

View File

@ -0,0 +1,7 @@
# frozen_string_literal: true
module Chat
class UserMention < Mention
belongs_to :user, foreign_key: :target_id
end
end

View File

@ -87,9 +87,9 @@ module Chat
if SiteSetting.enable_user_status if SiteSetting.enable_user_status
query = query.includes(user: :user_status) query = query.includes(user: :user_status)
query = query.includes(chat_mentions: { user: :user_status }) query = query.includes(user_mentions: { user: :user_status })
else else
query = query.includes(chat_mentions: :user) query = query.includes(user_mentions: :user)
end end
query query

View File

@ -36,7 +36,7 @@ module Chat
def mentioned_users def mentioned_users
object object
.chat_mentions .user_mentions
.limit(SiteSetting.max_mentions_per_chat_message) .limit(SiteSetting.max_mentions_per_chat_message)
.map(&:user) .map(&:user)
.compact .compact

View File

@ -17,7 +17,7 @@ module Chat
def mentioned_users def mentioned_users
object object
.chat_mentions .user_mentions
.limit(SiteSetting.max_mentions_per_chat_message) .limit(SiteSetting.max_mentions_per_chat_message)
.map(&:user) .map(&:user)
.compact .compact

View File

@ -0,0 +1,15 @@
# frozen_string_literal: true
class AddTypeAndTargetIdToChatMentions < ActiveRecord::Migration[7.0]
def up
add_column :chat_mentions, :type, :string, null: true
add_column :chat_mentions, :target_id, :integer, null: true
change_column_null :chat_mentions, :user_id, true
end
def down
change_column_null :chat_mentions, :user_id, false
remove_column :chat_mentions, :target_id
remove_column :chat_mentions, :type
end
end

View File

@ -0,0 +1,24 @@
# frozen_string_literal: true
class SetTypeAndTargetIdOnChatMentions < ActiveRecord::Migration[7.0]
disable_ddl_transaction!
BATCH_SIZE = 5000
def up
begin
updated_count = DB.exec(<<~SQL, batch_size: BATCH_SIZE)
WITH cte AS (SELECT id, user_id
FROM chat_mentions
WHERE type IS NULL AND target_id IS NULL
LIMIT :batch_size)
UPDATE chat_mentions
SET type = 'Chat::UserMention', target_id = cte.user_id
FROM cte
WHERE chat_mentions.id = cte.id;
SQL
end while updated_count > 0
end
def down
end
end

View File

@ -0,0 +1,23 @@
# frozen_string_literal: true
class AddAndRemoveIndexesOnChatMentions < ActiveRecord::Migration[7.0]
disable_ddl_transaction!
def up
remove_index :chat_mentions,
name: :chat_mentions_index,
algorithm: :concurrently,
if_exists: true
add_index :chat_mentions, %i[chat_message_id], algorithm: :concurrently
add_index :chat_mentions, %i[target_id], algorithm: :concurrently
end
def down
remove_index :chat_mentions, %i[target_id], algorithm: :concurrently, if_exists: true
remove_index :chat_mentions, %i[chat_message_id], algorithm: :concurrently, if_exists: true
add_index :chat_mentions,
%i[chat_message_id user_id notification_id],
unique: true,
name: "chat_mentions_index",
algorithm: :concurrently
end
end

View File

@ -0,0 +1,27 @@
# frozen_string_literal: true
class SetTypeAndTargetIdOnChatMentionsPostMigrate < ActiveRecord::Migration[7.0]
disable_ddl_transaction!
BATCH_SIZE = 5000
def up
# we're setting it again in post-migration
# in case some mentions have been created after we run
# this query the first time in the regular migration
begin
updated_count = DB.exec(<<~SQL, batch_size: BATCH_SIZE)
WITH cte AS (SELECT id, user_id
FROM chat_mentions
WHERE type IS NULL AND target_id IS NULL
LIMIT :batch_size)
UPDATE chat_mentions
SET type = 'Chat::UserMention', target_id = cte.user_id
FROM cte
WHERE chat_mentions.id = cte.id;
SQL
end while updated_count > 0
end
def down
end
end

View File

@ -0,0 +1,11 @@
# frozen_string_literal: true
class MakeTypeOnChatMentionsNonNullable < ActiveRecord::Migration[7.0]
def up
change_column_null :chat_mentions, :type, false
end
def down
change_column_null :chat_mentions, :type, true
end
end

View File

@ -0,0 +1,11 @@
# frozen_string_literal: true
module Chat
module GroupExtension
extend ActiveSupport::Concern
prepended do
has_many :chat_mentions, class_name: "Chat::GroupMention", foreign_key: "target_id"
end
end
end

View File

@ -52,13 +52,17 @@ module Chat
.joins("INNER JOIN chat_channels cc ON cc.id = uccm.chat_channel_id") .joins("INNER JOIN chat_channels cc ON cc.id = uccm.chat_channel_id")
.joins("INNER JOIN chat_messages c_msg ON c_msg.chat_channel_id = uccm.chat_channel_id") .joins("INNER JOIN chat_messages c_msg ON c_msg.chat_channel_id = uccm.chat_channel_id")
.joins("LEFT OUTER JOIN chat_mentions c_mentions ON c_mentions.chat_message_id = c_msg.id") .joins("LEFT OUTER JOIN chat_mentions c_mentions ON c_mentions.chat_message_id = c_msg.id")
.joins(
"LEFT OUTER JOIN chat_mention_notifications cmn ON cmn.chat_mention_id = c_mentions.id",
)
.joins("LEFT OUTER JOIN notifications n ON cmn.notification_id = n.id")
.where("c_msg.deleted_at IS NULL AND c_msg.user_id <> users.id") .where("c_msg.deleted_at IS NULL AND c_msg.user_id <> users.id")
.where("c_msg.created_at > ?", 1.week.ago) .where("c_msg.created_at > ?", 1.week.ago)
.where(<<~SQL) .where(<<~SQL)
(uccm.last_read_message_id IS NULL OR c_msg.id > uccm.last_read_message_id) AND (uccm.last_read_message_id IS NULL OR c_msg.id > uccm.last_read_message_id) AND
(uccm.last_unread_mention_when_emailed_id IS NULL OR c_msg.id > uccm.last_unread_mention_when_emailed_id) AND (uccm.last_unread_mention_when_emailed_id IS NULL OR c_msg.id > uccm.last_unread_mention_when_emailed_id) AND
( (
(uccm.user_id = c_mentions.user_id AND uccm.following IS true AND cc.chatable_type = 'Category') OR (uccm.user_id = n.user_id AND uccm.following IS true AND cc.chatable_type = 'Category') OR
(cc.chatable_type = 'DirectMessage') (cc.chatable_type = 'DirectMessage')
) )
SQL SQL

View File

@ -83,10 +83,15 @@ module Chat
def notify_edit def notify_edit
already_notified_user_ids = already_notified_user_ids =
Chat::Mention Notification
.where(chat_message: @chat_message) .where(notification_type: Notification.types[:chat_mention])
.left_outer_joins(:notifications) .joins(
.where.not(notifications: { id: nil }) "INNER JOIN chat_mention_notifications ON chat_mention_notifications.notification_id = notifications.id",
)
.joins(
"INNER JOIN chat_mentions ON chat_mentions.id = chat_mention_notifications.chat_mention_id",
)
.where("chat_mentions.chat_message_id = ?", @chat_message.id)
.pluck(:user_id) .pluck(:user_id)
to_notify, inaccessible, all_mentioned_user_ids = list_users_to_notify to_notify, inaccessible, all_mentioned_user_ids = list_users_to_notify

View File

@ -19,18 +19,6 @@ module Chat
:parsed_direct_mentions, :parsed_direct_mentions,
:parsed_group_mentions :parsed_group_mentions
def all_mentioned_users_ids
@all_mentioned_users_ids ||=
begin
user_ids = global_mentions.pluck(:id)
user_ids.concat(direct_mentions.pluck(:id))
user_ids.concat(group_mentions.pluck(:id))
user_ids.concat(here_mentions.pluck(:id))
user_ids.uniq!
user_ids
end
end
def count def count
@count ||= @count ||=
begin begin

View File

@ -12,7 +12,7 @@ module Chat
class_name: "Chat::UserChatThreadMembership", class_name: "Chat::UserChatThreadMembership",
dependent: :destroy dependent: :destroy
has_many :chat_message_reactions, class_name: "Chat::MessageReaction", dependent: :destroy has_many :chat_message_reactions, class_name: "Chat::MessageReaction", dependent: :destroy
has_many :chat_mentions, class_name: "Chat::Mention" has_many :chat_mentions, class_name: "Chat::UserMention", foreign_key: "target_id"
has_many :direct_message_users, class_name: "Chat::DirectMessageUser" has_many :direct_message_users, class_name: "Chat::DirectMessageUser"
has_many :direct_messages, through: :direct_message_users, class_name: "Chat::DirectMessage" has_many :direct_messages, through: :direct_message_users, class_name: "Chat::DirectMessage"
end end

View File

@ -13,6 +13,7 @@ module Chat
.where("chat_messages.created_at > ?", 1.week.ago) .where("chat_messages.created_at > ?", 1.week.ago)
.joins("LEFT OUTER JOIN chat_mentions cm ON cm.chat_message_id = chat_messages.id") .joins("LEFT OUTER JOIN chat_mentions cm ON cm.chat_message_id = chat_messages.id")
.joins("LEFT OUTER JOIN chat_mention_notifications cmn ON cmn.chat_mention_id = cm.id") .joins("LEFT OUTER JOIN chat_mention_notifications cmn ON cmn.chat_mention_id = cm.id")
.joins("LEFT OUTER JOIN notifications n ON cmn.notification_id = n.id")
.joins( .joins(
"INNER JOIN user_chat_channel_memberships uccm ON uccm.chat_channel_id = chat_channels.id", "INNER JOIN user_chat_channel_memberships uccm ON uccm.chat_channel_id = chat_channels.id",
) )
@ -21,7 +22,7 @@ module Chat
(uccm.last_read_message_id IS NULL OR chat_messages.id > uccm.last_read_message_id) AND (uccm.last_read_message_id IS NULL OR chat_messages.id > uccm.last_read_message_id) AND
(uccm.last_unread_mention_when_emailed_id IS NULL OR chat_messages.id > uccm.last_unread_mention_when_emailed_id) AND (uccm.last_unread_mention_when_emailed_id IS NULL OR chat_messages.id > uccm.last_unread_mention_when_emailed_id) AND
( (
(cm.user_id = :user_id AND cmn.notification_id IS NOT NULL AND uccm.following IS true AND chat_channels.chatable_type = 'Category') OR (n.user_id = :user_id AND cmn.notification_id IS NOT NULL AND uccm.following IS true AND chat_channels.chatable_type = 'Category') OR
(chat_channels.chatable_type = 'DirectMessage') (chat_channels.chatable_type = 'DirectMessage')
) )
SQL SQL

View File

@ -66,6 +66,7 @@ after_initialize do
Reviewable.prepend Chat::ReviewableExtension Reviewable.prepend Chat::ReviewableExtension
Bookmark.prepend Chat::BookmarkExtension Bookmark.prepend Chat::BookmarkExtension
User.prepend Chat::UserExtension User.prepend Chat::UserExtension
Group.prepend Chat::GroupExtension
Jobs::UserEmail.prepend Chat::UserEmailExtension Jobs::UserEmail.prepend Chat::UserEmailExtension
Plugin::Instance.prepend Chat::PluginInstanceExtension Plugin::Instance.prepend Chat::PluginInstanceExtension
Jobs::ExportCsvFile.class_eval { prepend Chat::MessagesExporter } Jobs::ExportCsvFile.class_eval { prepend Chat::MessagesExporter }

View File

@ -46,7 +46,12 @@ describe Chat::Mailer do
end end
describe "for chat mentions" do describe "for chat mentions" do
fab!(:mention) { Fabricate(:chat_mention, user: user_1, chat_message: chat_message) } fab!(:notification) do
Fabricate(:notification, notification_type: Notification.types[:chat_mention], user: user_1)
end
fab!(:mention) do
Fabricate(:user_chat_mention, chat_message: chat_message, notifications: [notification])
end
it "skips users without chat access" do it "skips users without chat access" do
chatters_group.remove(user_1) chatters_group.remove(user_1)
@ -157,7 +162,14 @@ describe Chat::Mailer do
last_unread_mention_when_emailed_id: chat_message.id, last_unread_mention_when_emailed_id: chat_message.id,
) )
unread_message = Fabricate(:chat_message, chat_channel: chat_channel, user: sender) unread_message = Fabricate(:chat_message, chat_channel: chat_channel, user: sender)
Fabricate(:chat_mention, user: user_1, chat_message: unread_message) notification_2 =
Fabricate(:notification, notification_type: Notification.types[:chat_mention], user: user_1)
Fabricate(
:user_chat_mention,
user: user_1,
chat_message: unread_message,
notifications: [notification_2],
)
described_class.send_unread_mentions_summary described_class.send_unread_mentions_summary
@ -183,7 +195,14 @@ describe Chat::Mailer do
last_read_message_id: nil, last_read_message_id: nil,
) )
new_message = Fabricate(:chat_message, chat_channel: chat_channel, user: sender) new_message = Fabricate(:chat_message, chat_channel: chat_channel, user: sender)
Fabricate(:chat_mention, user: user_2, chat_message: new_message) notification_2 =
Fabricate(:notification, notification_type: Notification.types[:chat_mention], user: user_2)
Fabricate(
:user_chat_mention,
user: user_2,
chat_message: new_message,
notifications: [notification_2],
)
described_class.send_unread_mentions_summary described_class.send_unread_mentions_summary
@ -222,7 +241,7 @@ describe Chat::Mailer do
) )
another_channel_message = Fabricate(:chat_message, chat_channel: chat_channel, user: sender) another_channel_message = Fabricate(:chat_message, chat_channel: chat_channel, user: sender)
Fabricate(:chat_mention, user: user_1, chat_message: another_channel_message) Fabricate(:user_chat_mention, user: user_1, chat_message: another_channel_message)
expect { described_class.send_unread_mentions_summary }.not_to change( expect { described_class.send_unread_mentions_summary }.not_to change(
Jobs::UserEmail.jobs, Jobs::UserEmail.jobs,
@ -234,7 +253,7 @@ describe Chat::Mailer do
another_channel = Fabricate(:category_channel) another_channel = Fabricate(:category_channel)
another_channel_message = another_channel_message =
Fabricate(:chat_message, chat_channel: another_channel, user: sender) Fabricate(:chat_message, chat_channel: another_channel, user: sender)
Fabricate(:chat_mention, user: user_1, chat_message: another_channel_message) Fabricate(:user_chat_mention, user: user_1, chat_message: another_channel_message)
another_channel_membership = another_channel_membership =
Fabricate( Fabricate(
:user_chat_channel_membership, :user_chat_channel_membership,
@ -264,7 +283,7 @@ describe Chat::Mailer do
end end
it "only queues the job once when the user has mentions and private messages" do it "only queues the job once when the user has mentions and private messages" do
Fabricate(:chat_mention, user: user_1, chat_message: chat_message) Fabricate(:user_chat_mention, user: user_1, chat_message: chat_message)
described_class.send_unread_mentions_summary described_class.send_unread_mentions_summary
@ -280,7 +299,7 @@ describe Chat::Mailer do
chat_channel: chat_channel, chat_channel: chat_channel,
last_read_message_id: chat_message.id, last_read_message_id: chat_message.id,
) )
Fabricate(:chat_mention, user: user_2, chat_message: chat_message) Fabricate(:user_chat_mention, user: user_2, chat_message: chat_message)
described_class.send_unread_mentions_summary described_class.send_unread_mentions_summary

View File

@ -115,7 +115,7 @@ Fabricator(:chat_message_with_service, class_name: "Chat::CreateMessage") do
end end
end end
Fabricator(:chat_mention, class_name: "Chat::Mention") do Fabricator(:user_chat_mention, class_name: "Chat::UserMention") do
transient read: false transient read: false
transient high_priority: true transient high_priority: true
transient identifier: :direct_mentions transient identifier: :direct_mentions
@ -124,6 +124,19 @@ Fabricator(:chat_mention, class_name: "Chat::Mention") do
chat_message { Fabricate(:chat_message) } chat_message { Fabricate(:chat_message) }
end end
Fabricator(:group_chat_mention, class_name: "Chat::GroupMention") do
chat_message { Fabricate(:chat_message) }
group { Fabricate(:group) }
end
Fabricator(:all_chat_mention, class_name: "Chat::AllMention") do
chat_message { Fabricate(:chat_message) }
end
Fabricator(:here_chat_mention, class_name: "Chat::HereMention") do
chat_message { Fabricate(:chat_message) }
end
Fabricator(:chat_message_reaction, class_name: "Chat::MessageReaction") do Fabricator(:chat_message_reaction, class_name: "Chat::MessageReaction") do
chat_message { Fabricate(:chat_message) } chat_message { Fabricate(:chat_message) }
user { Fabricate(:user) } user { Fabricate(:user) }

View File

@ -23,7 +23,7 @@ describe Jobs::Chat::ChannelDelete do
UploadReference.create(target: message, upload: upload) UploadReference.create(target: message, upload: upload)
end end
Chat::Mention.create( Chat::UserMention.create(
user: user2, user: user2,
chat_message: messages.sample, chat_message: messages.sample,
notifications: [Fabricate(:notification)], notifications: [Fabricate(:notification)],

View File

@ -44,7 +44,7 @@ describe Jobs::Chat::NotifyMentioned do
created_at: 10.minutes.ago, created_at: 10.minutes.ago,
thread: thread, thread: thread,
) )
Fabricate(:chat_mention, chat_message: message, user: mentioned_user) Fabricate(:user_chat_mention, chat_message: message, user: mentioned_user)
message message
end end
@ -226,6 +226,9 @@ describe Jobs::Chat::NotifyMentioned do
it "works for desktop notifications" do it "works for desktop notifications" do
message = create_chat_message message = create_chat_message
Fabricate(:all_chat_mention, chat_message: message)
Fabricate(:here_chat_mention, chat_message: message)
Fabricate(:group_chat_mention, group: @chat_group, chat_message: message)
desktop_notification = desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map) track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
@ -244,6 +247,9 @@ describe Jobs::Chat::NotifyMentioned do
it "works for push notifications" do it "works for push notifications" do
message = create_chat_message message = create_chat_message
Fabricate(:all_chat_mention, chat_message: message)
Fabricate(:here_chat_mention, chat_message: message)
Fabricate(:group_chat_mention, group: @chat_group, chat_message: message)
PostAlerter.expects(:push_notification).with( PostAlerter.expects(:push_notification).with(
user_2, user_2,
@ -266,6 +272,9 @@ describe Jobs::Chat::NotifyMentioned do
it "works for core notifications" do it "works for core notifications" do
message = create_chat_message message = create_chat_message
Fabricate(:all_chat_mention, chat_message: message)
Fabricate(:here_chat_mention, chat_message: message)
Fabricate(:group_chat_mention, group: @chat_group, chat_message: message)
created_notification = created_notification =
track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map) track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map)
@ -302,6 +311,7 @@ describe Jobs::Chat::NotifyMentioned do
it "includes global mention specific data to core notifications" do it "includes global mention specific data to core notifications" do
message = create_chat_message message = create_chat_message
Fabricate(:all_chat_mention, chat_message: message)
created_notification = created_notification =
track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map) track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map)
@ -313,6 +323,7 @@ describe Jobs::Chat::NotifyMentioned do
it "includes global mention specific data to desktop notifications" do it "includes global mention specific data to desktop notifications" do
message = create_chat_message message = create_chat_message
Fabricate(:all_chat_mention, chat_message: message)
desktop_notification = desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map) track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
@ -323,6 +334,7 @@ describe Jobs::Chat::NotifyMentioned do
context "with private channels" do context "with private channels" do
it "users a different translated title" do it "users a different translated title" do
message = create_chat_message(channel: @personal_chat_channel) message = create_chat_message(channel: @personal_chat_channel)
Fabricate(:all_chat_mention, chat_message: message)
desktop_notification = desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map) track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
@ -355,6 +367,7 @@ describe Jobs::Chat::NotifyMentioned do
it "includes here mention specific data to core notifications" do it "includes here mention specific data to core notifications" do
message = create_chat_message message = create_chat_message
Fabricate(:here_chat_mention, chat_message: message)
created_notification = created_notification =
track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map) track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map)
@ -365,6 +378,7 @@ describe Jobs::Chat::NotifyMentioned do
it "includes here mention specific data to desktop notifications" do it "includes here mention specific data to desktop notifications" do
message = create_chat_message message = create_chat_message
Fabricate(:here_chat_mention, chat_message: message)
desktop_notification = desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map) track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
@ -373,8 +387,9 @@ describe Jobs::Chat::NotifyMentioned do
end end
context "with private channels" do context "with private channels" do
it "users a different translated title" do it "uses a different translated title" do
message = create_chat_message(channel: @personal_chat_channel) message = create_chat_message(channel: @personal_chat_channel)
Fabricate(:here_chat_mention, chat_message: message)
desktop_notification = desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map) track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
@ -479,6 +494,7 @@ describe Jobs::Chat::NotifyMentioned do
it "includes here mention specific data to core notifications" do it "includes here mention specific data to core notifications" do
message = create_chat_message message = create_chat_message
Fabricate(:group_chat_mention, group: @chat_group, chat_message: message)
created_notification = created_notification =
track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map) track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map)
@ -490,6 +506,7 @@ describe Jobs::Chat::NotifyMentioned do
it "includes here mention specific data to desktop notifications" do it "includes here mention specific data to desktop notifications" do
message = create_chat_message message = create_chat_message
Fabricate(:group_chat_mention, group: @chat_group, chat_message: message)
desktop_notification = desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map) track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
@ -498,8 +515,9 @@ describe Jobs::Chat::NotifyMentioned do
end end
context "with private channels" do context "with private channels" do
it "users a different translated title" do it "uses a different translated title" do
message = create_chat_message(channel: @personal_chat_channel) message = create_chat_message(channel: @personal_chat_channel)
Fabricate(:group_chat_mention, group: @chat_group, chat_message: message)
desktop_notification = desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map) track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)

View File

@ -116,7 +116,7 @@ describe Chat::MessageMover do
it "updates references for reactions, uploads, revisions, mentions, etc." do it "updates references for reactions, uploads, revisions, mentions, etc." do
reaction = Fabricate(:chat_message_reaction, chat_message: message1) reaction = Fabricate(:chat_message_reaction, chat_message: message1)
upload = Fabricate(:upload_reference, target: message1) upload = Fabricate(:upload_reference, target: message1)
mention = Fabricate(:chat_mention, chat_message: message2, user: acting_user) mention = Fabricate(:user_chat_mention, chat_message: message2, user: acting_user)
revision = Fabricate(:chat_message_revision, chat_message: message3) revision = Fabricate(:chat_message_revision, chat_message: message3)
webhook_event = Fabricate(:chat_webhook_event, chat_message: message3) webhook_event = Fabricate(:chat_webhook_event, chat_message: message3)
move! move!

View File

@ -252,9 +252,9 @@ describe UserNotifications do
describe "email subject" do describe "email subject" do
context "with regular mentions" do context "with regular mentions" do
before do before do
notification = Fabricate(:notification) notification = Fabricate(:notification, user: user)
Fabricate( Fabricate(
:chat_mention, :user_chat_mention,
user: user, user: user,
chat_message: chat_message, chat_message: chat_message,
notifications: [notification], notifications: [notification],
@ -305,9 +305,9 @@ describe UserNotifications do
user: user, user: user,
last_read_message_id: another_chat_message.id - 2, last_read_message_id: another_chat_message.id - 2,
) )
notification = Fabricate(:notification) notification = Fabricate(:notification, user: user)
Fabricate( Fabricate(
:chat_mention, :user_chat_mention,
user: user, user: user,
chat_message: another_chat_message, chat_message: another_chat_message,
notifications: [notification], notifications: [notification],
@ -345,9 +345,9 @@ describe UserNotifications do
user: user, user: user,
last_read_message_id: another_chat_message.id - 2, last_read_message_id: another_chat_message.id - 2,
) )
notification = Fabricate(:notification) notification = Fabricate(:notification, user: user)
Fabricate( Fabricate(
:chat_mention, :user_chat_mention,
user: user, user: user,
chat_message: another_chat_message, chat_message: another_chat_message,
notifications: [notification], notifications: [notification],
@ -374,9 +374,9 @@ describe UserNotifications do
refresh_auto_groups refresh_auto_groups
channel = create_dm_channel(sender, [sender, user]) channel = create_dm_channel(sender, [sender, user])
Fabricate(:chat_message, user: sender, chat_channel: channel) Fabricate(:chat_message, user: sender, chat_channel: channel)
notification = Fabricate(:notification) notification = Fabricate(:notification, user: user)
Fabricate( Fabricate(
:chat_mention, :user_chat_mention,
user: user, user: user,
chat_message: chat_message, chat_message: chat_message,
notifications: [notification], notifications: [notification],
@ -401,9 +401,9 @@ describe UserNotifications do
describe "When there are mentions" do describe "When there are mentions" do
before do before do
notification = Fabricate(:notification) notification = Fabricate(:notification, user: user)
Fabricate( Fabricate(
:chat_mention, :user_chat_mention,
user: user, user: user,
chat_message: chat_message, chat_message: chat_message,
notifications: [notification], notifications: [notification],
@ -508,9 +508,9 @@ describe UserNotifications do
) )
new_message = Fabricate(:chat_message, user: sender, chat_channel: channel) new_message = Fabricate(:chat_message, user: sender, chat_channel: channel)
notification = Fabricate(:notification) notification = Fabricate(:notification, user: user)
Fabricate( Fabricate(
:chat_mention, :user_chat_mention,
user: user, user: user,
chat_message: new_message, chat_message: new_message,
notifications: [notification], notifications: [notification],
@ -652,9 +652,9 @@ describe UserNotifications do
it "includes a view more link " do it "includes a view more link " do
2.times do 2.times do
msg = Fabricate(:chat_message, user: sender, chat_channel: channel) msg = Fabricate(:chat_message, user: sender, chat_channel: channel)
notification = Fabricate(:notification) notification = Fabricate(:notification, user: user)
Fabricate( Fabricate(
:chat_mention, :user_chat_mention,
user: user, user: user,
chat_message: msg, chat_message: msg,
notifications: [notification], notifications: [notification],
@ -679,9 +679,9 @@ describe UserNotifications do
it "has only a link to view all messages" do it "has only a link to view all messages" do
2.times do 2.times do
msg = Fabricate(:chat_message, user: sender, chat_channel: channel) msg = Fabricate(:chat_message, user: sender, chat_channel: channel)
notification = Fabricate(:notification) notification = Fabricate(:notification, user: user)
Fabricate( Fabricate(
:chat_mention, :user_chat_mention,
user: user, user: user,
chat_message: msg, chat_message: msg,
notifications: [notification], notifications: [notification],
@ -706,9 +706,9 @@ describe UserNotifications do
new_message = new_message =
Fabricate(:chat_message, user: sender, chat_channel: channel, cooked: "New message") Fabricate(:chat_message, user: sender, chat_channel: channel, cooked: "New message")
notification = Fabricate(:notification) notification = Fabricate(:notification, user: user)
Fabricate( Fabricate(
:chat_mention, :user_chat_mention,
user: user, user: user,
chat_message: new_message, chat_message: new_message,
notifications: [notification], notifications: [notification],

View File

@ -450,7 +450,8 @@ describe Chat::Message do
it "destroys chat_mention" do it "destroys chat_mention" do
message_1 = Fabricate(:chat_message) message_1 = Fabricate(:chat_message)
notification = Fabricate(:notification, notification_type: Notification.types[:chat_mention]) notification = Fabricate(:notification, notification_type: Notification.types[:chat_mention])
mention_1 = Fabricate(:chat_mention, chat_message: message_1, notifications: [notification]) mention_1 =
Fabricate(:user_chat_mention, chat_message: message_1, notifications: [notification])
message_1.reload.destroy! message_1.reload.destroy!
@ -527,42 +528,162 @@ describe Chat::Message do
end end
describe "#upsert_mentions" do describe "#upsert_mentions" do
fab!(:user1) { Fabricate(:user) } context "with direct mentions" do
fab!(:user2) { Fabricate(:user) } fab!(:user1) { Fabricate(:user) }
fab!(:user3) { Fabricate(:user) } fab!(:user2) { Fabricate(:user) }
fab!(:user4) { Fabricate(:user) } fab!(:user3) { Fabricate(:user) }
fab!(:message) do fab!(:user4) { Fabricate(:user) }
Fabricate(:chat_message, message: "Hey @#{user1.username} and @#{user2.username}") fab!(:message) do
end Fabricate(:chat_message, message: "Hey @#{user1.username} and @#{user2.username}")
let(:already_mentioned) { [user1.id, user2.id] } end
let(:already_mentioned) { [user1.id, user2.id] }
it "creates newly added mentions" do it "creates newly added mentions" do
existing_mention_ids = message.chat_mentions.pluck(:id) existing_mention_ids = message.chat_mentions.pluck(:id)
update_message!( message.message = message.message + " @#{user3.username} @#{user4.username} "
message, message.cook
user: message.user,
text: message.message + " @#{user3.username} @#{user4.username} ",
)
expect(message.chat_mentions.pluck(:user_id)).to match_array( message.upsert_mentions
[user1.id, user2.id, user3.id, user4.id],
) expect(message.user_mentions.pluck(:target_id)).to match_array(
expect(message.chat_mentions.pluck(:id)).to include(*existing_mention_ids) # existing mentions weren't recreated [user1.id, user2.id, user3.id, user4.id],
)
expect(message.user_mentions.pluck(:id)).to include(*existing_mention_ids) # existing mentions weren't recreated
end
it "drops removed mentions" do
# user 2 is not mentioned anymore:
message.message = "Hey @#{user1.username}"
message.cook
message.upsert_mentions
expect(message.user_mentions.pluck(:target_id)).to contain_exactly(user1.id)
end
it "changes nothing if message mentions has not been changed" do
existing_mention_ids = message.chat_mentions.pluck(:id)
message.upsert_mentions
expect(message.user_mentions.pluck(:target_id)).to match_array(already_mentioned)
expect(message.user_mentions.pluck(:id)).to include(*existing_mention_ids) # the mentions weren't recreated
end
end end
it "drops removed mentions" do context "with group mentions" do
# user 2 is not mentioned anymore fab!(:group1) { Fabricate(:group, mentionable_level: Group::ALIAS_LEVELS[:everyone]) }
update_message!(message, user: message.user, text: "Hey @#{user1.username}") fab!(:group2) { Fabricate(:group, mentionable_level: Group::ALIAS_LEVELS[:everyone]) }
fab!(:group3) { Fabricate(:group, mentionable_level: Group::ALIAS_LEVELS[:everyone]) }
fab!(:group4) { Fabricate(:group, mentionable_level: Group::ALIAS_LEVELS[:everyone]) }
fab!(:message) do
Fabricate(:chat_message, message: "Hey @#{group1.name} and @#{group2.name}")
end
let(:already_mentioned) { [group1.id, group2.id] }
expect(message.chat_mentions.pluck(:user_id)).to contain_exactly(user1.id) it "creates newly added mentions" do
existing_mention_ids = message.chat_mentions.pluck(:id)
message.message = message.message + " @#{group3.name} @#{group4.name} "
message.cook
message.upsert_mentions
expect(message.group_mentions.pluck(:target_id)).to match_array(
[group1.id, group2.id, group3.id, group4.id],
)
expect(message.group_mentions.pluck(:id)).to include(*existing_mention_ids) # existing mentions weren't recreated
end
it "drops removed mentions" do
# group 2 is not mentioned anymore:
message.message = "Hey @#{group1.name}"
message.cook
message.upsert_mentions
expect(message.group_mentions.pluck(:target_id)).to contain_exactly(group1.id)
end
it "changes nothing if message mentions has not been changed" do
existing_mention_ids = message.chat_mentions.pluck(:id)
message.upsert_mentions
expect(message.group_mentions.pluck(:target_id)).to match_array(already_mentioned)
expect(message.group_mentions.pluck(:id)).to include(*existing_mention_ids) # the mentions weren't recreated
end
end end
it "changes nothing if passed mentions are identical to existing mentions" do context "with @here mentions" do
existing_mention_ids = message.chat_mentions.pluck(:id) fab!(:message) { Fabricate(:chat_message, message: "There are no mentions yet") }
update_message!(message, user: message.user, text: message.message)
expect(message.chat_mentions.pluck(:user_id)).to match_array(already_mentioned) it "creates @here mention" do
expect(message.chat_mentions.pluck(:id)).to include(*existing_mention_ids) # the mentions weren't recreated message.message = "Mentioning @here"
message.cook
message.upsert_mentions
expect(message.here_mention).to be_present
end
it "creates only one mention even if @here present more than once in a message" do
message.message = "Mentioning several times: @here @here @here"
message.cook
message.upsert_mentions
expect(message.here_mention).to be_present
expect(message.chat_mentions.count).to be(1)
end
it "drops @here mention when it's dropped from the message" do
message.message = "Mentioning @here"
message.cook
message.upsert_mentions
message.message = "No mentions now"
message.cook
message.upsert_mentions
expect(message.here_mention).to be_blank
end
end
context "with @all mentions" do
fab!(:message) { Fabricate(:chat_message, message: "There are no mentions yet") }
it "creates @all mention" do
message.message = "Mentioning @all"
message.cook
message.upsert_mentions
expect(message.all_mention).to be_present
end
it "creates only one mention even if @here present more than once in a message" do
message.message = "Mentioning several times: @all @all @all"
message.cook
message.upsert_mentions
expect(message.all_mention).to be_present
expect(message.chat_mentions.count).to be(1)
end
it "drops @here mention when it's dropped from the message" do
message.message = "Mentioning @all"
message.cook
message.upsert_mentions
message.message = "No mentions now"
message.cook
message.upsert_mentions
expect(message.all_mention).to be_blank
end
end end
end end

View File

@ -128,7 +128,7 @@ describe Chat::ChannelUnreadsQuery do
user_id: current_user.id, user_id: current_user.id,
data: { chat_message_id: message.id, chat_channel_id: channel.id }.to_json, data: { chat_message_id: message.id, chat_channel_id: channel.id }.to_json,
) )
Chat::Mention.create!( Chat::UserMention.create!(
notifications: [notification], notifications: [notification],
user: current_user, user: current_user,
chat_message: message, chat_message: message,

View File

@ -199,7 +199,7 @@ RSpec.describe Chat::Api::ReadsController do
}.to_json, }.to_json,
) )
.tap do |notification| .tap do |notification|
Chat::Mention.create!(user: user, chat_message: msg, notifications: [notification]) Chat::UserMention.create!(user: user, chat_message: msg, notifications: [notification])
end end
end end
end end

View File

@ -66,7 +66,7 @@ RSpec.describe Chat::Api::ThreadReadsController do
}.to_json, }.to_json,
) )
.tap do |notification| .tap do |notification|
Chat::Mention.create!(user: user, chat_message: msg, notifications: [notification]) Chat::UserMention.create!(user: user, chat_message: msg, notifications: [notification])
end end
end end
end end

View File

@ -14,7 +14,7 @@ describe Chat::MessageSerializer do
describe "#mentioned_users" do describe "#mentioned_users" do
it "is limited by max_mentions_per_chat_message setting" do it "is limited by max_mentions_per_chat_message setting" do
Fabricate.times(2, :chat_mention, chat_message: message_1) Fabricate.times(2, :user_chat_mention, chat_message: message_1)
SiteSetting.max_mentions_per_chat_message = 1 SiteSetting.max_mentions_per_chat_message = 1
expect(serializer.as_json[:mentioned_users].length).to eq(1) expect(serializer.as_json[:mentioned_users].length).to eq(1)
@ -228,7 +228,7 @@ describe Chat::MessageSerializer do
message: message:
"here should be a mention, but since we're fabricating objects it doesn't matter", "here should be a mention, but since we're fabricating objects it doesn't matter",
) )
Fabricate(:chat_mention, chat_message: message, user: mentioned_user) Fabricate(:user_chat_mention, chat_message: message, user: mentioned_user)
mentioned_user.destroy! mentioned_user.destroy!
message.reload message.reload

View File

@ -12,7 +12,7 @@ describe Chat::ThreadOriginalMessageSerializer do
describe "#mentioned_users" do describe "#mentioned_users" do
it "is limited by max_mentions_per_chat_message setting" do it "is limited by max_mentions_per_chat_message setting" do
Fabricate.times(2, :chat_mention, chat_message: message_1) Fabricate.times(2, :user_chat_mention, chat_message: message_1)
SiteSetting.max_mentions_per_chat_message = 1 SiteSetting.max_mentions_per_chat_message = 1
expect(serializer.as_json[:mentioned_users].length).to eq(1) expect(serializer.as_json[:mentioned_users].length).to eq(1)

View File

@ -84,12 +84,12 @@ RSpec.describe Chat::MarkAllUserChannelsRead do
let(:messages) { MessageBus.track_publish { result } } let(:messages) { MessageBus.track_publish { result } }
before do before do
Chat::Mention.create!( Chat::UserMention.create!(
notifications: [notification_1], notifications: [notification_1],
user: current_user, user: current_user,
chat_message: message_1, chat_message: message_1,
) )
Chat::Mention.create!( Chat::UserMention.create!(
notifications: [notification_2], notifications: [notification_2],
user: current_user, user: current_user,
chat_message: message_3, chat_message: message_3,

View File

@ -49,7 +49,8 @@ RSpec.describe Chat::TrashMessage do
it "destroys notifications for mentions" do it "destroys notifications for mentions" do
notification = Fabricate(:notification) notification = Fabricate(:notification)
mention = Fabricate(:chat_mention, chat_message: message, notifications: [notification]) mention =
Fabricate(:user_chat_mention, chat_message: message, notifications: [notification])
result result

View File

@ -369,46 +369,126 @@ RSpec.describe Chat::UpdateMessage do
end end
describe "with group mentions" do describe "with group mentions" do
it "creates group mentions on update" do fab!(:group_1) do
Fabricate(
:public_group,
users: [user1, user2],
mentionable_level: Group::ALIAS_LEVELS[:everyone],
)
end
fab!(:group_2) do
Fabricate(
:public_group,
users: [user3, user4],
mentionable_level: Group::ALIAS_LEVELS[:everyone],
)
end
it "creates a mention record when a group was mentioned on message update" do
chat_message = create_chat_message(user1, "ping nobody", public_chat_channel) chat_message = create_chat_message(user1, "ping nobody", public_chat_channel)
expect {
described_class.call(
guardian: guardian,
message_id: chat_message.id,
message: "ping @#{admin_group.name}",
)
}.to change { Chat::Mention.where(chat_message: chat_message).count }.by(2)
expect(admin1.chat_mentions.where(chat_message: chat_message)).to be_present described_class.call(
expect(admin2.chat_mentions.where(chat_message: chat_message)).to be_present guardian: guardian,
message_id: chat_message.id,
message: "ping @#{group_1.name}",
)
expect(group_1.chat_mentions.where(chat_message: chat_message).count).to be(1)
end end
it "doesn't duplicate mentions when the user is already direct mentioned and then group mentioned" do it "updates mention records when another group was mentioned on message update" do
chat_message = create_chat_message(user1, "ping @#{admin2.username}", public_chat_channel) chat_message = create_chat_message(user1, "ping @#{group_1.name}", public_chat_channel)
expect {
described_class.call( expect(chat_message.group_mentions.map(&:target_id)).to contain_exactly(group_1.id)
guardian: guardian,
message_id: chat_message.id, described_class.call(
message: "ping @#{admin_group.name} @#{admin2.username}", guardian: guardian,
) message_id: chat_message.id,
}.to change { admin1.chat_mentions.count }.by(1).and not_change { message: "ping @#{group_2.name}",
admin2.chat_mentions.count )
}
expect(chat_message.reload.group_mentions.map(&:target_id)).to contain_exactly(group_2.id)
end end
it "deletes old mentions when group mention is removed" do it "deletes a mention record when a group mention was removed on message update" do
chat_message = create_chat_message(user1, "ping @#{group_1.name}", public_chat_channel)
described_class.call(
guardian: guardian,
message_id: chat_message.id,
message: "ping nobody anymore!",
)
expect(group_1.chat_mentions.where(chat_message: chat_message).count).to be(0)
end
it "doesn't notify the second time users that has already been notified when creating the message" do
group_user = Fabricate(:user)
Fabricate(
:user_chat_channel_membership,
chat_channel: public_chat_channel,
user: group_user,
)
group =
Fabricate(
:public_group,
users: [group_user],
mentionable_level: Group::ALIAS_LEVELS[:everyone],
)
chat_message = chat_message =
create_chat_message(user1, "ping @#{admin_group.name}", public_chat_channel) create_chat_message(user1, "Mentioning @#{group.name}", public_chat_channel)
expect { expect(group_user.notifications.count).to be(1)
described_class.call( notification_id = group_user.notifications.first.id
guardian: guardian,
message_id: chat_message.id,
message: "ping nobody anymore!",
)
}.to change { Chat::Mention.where(chat_message: chat_message).count }.by(-2)
expect(admin1.chat_mentions.where(chat_message: chat_message)).not_to be_present described_class.call(
expect(admin2.chat_mentions.where(chat_message: chat_message)).not_to be_present guardian: guardian,
message_id: chat_message.id,
message: "Update the message and still mention the same group @#{group.name}",
)
expect(group_user.notifications.count).to be(1) # no new notifications has been created
expect(group_user.notifications.first.id).to be(notification_id) # the existing notification hasn't been recreated
end
end
describe "with @here mentions" do
it "doesn't notify the second time users that has already been notified when creating the message" do
user = Fabricate(:user)
Fabricate(:user_chat_channel_membership, chat_channel: public_chat_channel, user: user)
user.update!(last_seen_at: 4.minutes.ago)
chat_message = create_chat_message(user1, "Mentioning @here", public_chat_channel)
expect(user.notifications.count).to be(1)
notification_id = user.notifications.first.id
described_class.call(
guardian: guardian,
message_id: chat_message.id,
message: "Update the message and still mention @here",
)
expect(user.notifications.count).to be(1) # no new notifications have been created
expect(user.notifications.first.id).to be(notification_id) # the existing notification haven't been recreated
end
end
describe "with @all mentions" do
it "doesn't notify the second time users that has already been notified when creating the message" do
user = Fabricate(:user)
Fabricate(:user_chat_channel_membership, chat_channel: public_chat_channel, user: user)
chat_message = create_chat_message(user1, "Mentioning @all", public_chat_channel)
notification_id = user.notifications.first.id
described_class.call(
guardian: guardian,
message_id: chat_message.id,
message: "Update the message and still mention @all",
)
expect(user.notifications.count).to be(1) # no new notifications have been created
expect(user.notifications.first.id).to be(notification_id) # the existing notification haven't been recreated
end end
end end
end end

View File

@ -84,7 +84,7 @@ RSpec.describe Chat::UpdateUserLastRead do
before do before do
Jobs.run_immediately! Jobs.run_immediately!
Chat::Mention.create!( Chat::UserMention.create!(
notifications: [notification], notifications: [notification],
user: current_user, user: current_user,
chat_message: message_1, chat_message: message_1,

View File

@ -61,12 +61,12 @@ RSpec.describe Chat::UpdateUserThreadLastRead do
before do before do
Jobs.run_immediately! Jobs.run_immediately!
Chat::Mention.create!( Chat::UserMention.create!(
notifications: [notification_1], notifications: [notification_1],
user: current_user, user: current_user,
chat_message: Fabricate(:chat_message, chat_channel: channel, thread: thread), chat_message: Fabricate(:chat_message, chat_channel: channel, thread: thread),
) )
Chat::Mention.create!( Chat::UserMention.create!(
notifications: [notification_2], notifications: [notification_2],
user: current_user, user: current_user,
chat_message: Fabricate(:chat_message, chat_channel: channel, thread: thread), chat_message: Fabricate(:chat_message, chat_channel: channel, thread: thread),

View File

@ -202,8 +202,8 @@ RSpec.describe "Chat channel", type: :system do
SiteSetting.enable_user_status = true SiteSetting.enable_user_status = true
current_user.set_status!("off to dentist", "tooth") current_user.set_status!("off to dentist", "tooth")
other_user.set_status!("surfing", "surfing_man") other_user.set_status!("surfing", "surfing_man")
Fabricate(:chat_mention, user: current_user, chat_message: message) Fabricate(:user_chat_mention, user: current_user, chat_message: message)
Fabricate(:chat_mention, user: other_user, chat_message: message) Fabricate(:user_chat_mention, user: other_user, chat_message: message)
chat_page.visit_channel(channel_1) chat_page.visit_channel(channel_1)