diff --git a/plugins/chat/app/jobs/regular/chat/notify_mentioned.rb b/plugins/chat/app/jobs/regular/chat/notify_mentioned.rb index 3388d17973a..bfbd925ff5d 100644 --- a/plugins/chat/app/jobs/regular/chat/notify_mentioned.rb +++ b/plugins/chat/app/jobs/regular/chat/notify_mentioned.rb @@ -122,7 +122,7 @@ module Jobs read: is_read, ) - mention.update!(notification: notification) + mention.notifications << notification end def send_notifications(membership, mention_type) diff --git a/plugins/chat/app/models/chat/mention.rb b/plugins/chat/app/models/chat/mention.rb index ab3bbee9925..9e89dd23b3e 100644 --- a/plugins/chat/app/models/chat/mention.rb +++ b/plugins/chat/app/models/chat/mention.rb @@ -3,10 +3,14 @@ module Chat class Mention < ActiveRecord::Base self.table_name = "chat_mentions" + self.ignored_columns = ["notification_id"] belongs_to :user belongs_to :chat_message, class_name: "Chat::Message" - belongs_to :notification, dependent: :destroy + has_many :mention_notifications, + class_name: "Chat::MentionNotification", + foreign_key: :chat_mention_id + has_many :notifications, through: :mention_notifications, dependent: :destroy end end diff --git a/plugins/chat/app/models/chat/mention_notification.rb b/plugins/chat/app/models/chat/mention_notification.rb new file mode 100644 index 00000000000..62c3a254341 --- /dev/null +++ b/plugins/chat/app/models/chat/mention_notification.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Chat + class MentionNotification < ActiveRecord::Base + self.table_name = "chat_mention_notifications" + + belongs_to :chat_mention + belongs_to :notification, dependent: :destroy + end +end + +# == Schema Information +# +# Table name: chat_mention_notifications +# +# chat_mention_id :integer not null +# notification_id :integer not null +# +# Indexes +# +# index_chat_mention_notifications_on_chat_mention_id (chat_mention_id) +# index_chat_mention_notifications_on_notification_id (notification_id) UNIQUE +# diff --git a/plugins/chat/app/services/chat/action/mark_mentions_read.rb b/plugins/chat/app/services/chat/action/mark_mentions_read.rb index 8d0207d63bb..f6c3f1c2a4d 100644 --- a/plugins/chat/app/services/chat/action/mark_mentions_read.rb +++ b/plugins/chat/app/services/chat/action/mark_mentions_read.rb @@ -17,7 +17,12 @@ module Chat .where(notification_type: Notification.types[:chat_mention]) .where(user: user) .where(read: false) - .joins("INNER JOIN chat_mentions ON chat_mentions.notification_id = notifications.id") + .joins( + "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", + ) .joins("INNER JOIN chat_messages ON chat_mentions.chat_message_id = chat_messages.id") .where("chat_messages.chat_channel_id IN (?)", channel_ids) .then do |notifications| diff --git a/plugins/chat/app/services/chat/trash_message.rb b/plugins/chat/app/services/chat/trash_message.rb index 2d3b6d7a6c5..fa06c1e68ae 100644 --- a/plugins/chat/app/services/chat/trash_message.rb +++ b/plugins/chat/app/services/chat/trash_message.rb @@ -55,9 +55,13 @@ module Chat end def destroy_notifications(message:, **) - ids = Chat::Mention.where(chat_message: message).pluck(:notification_id) - Notification.where(id: ids).destroy_all - Chat::Mention.where(chat_message: message).update_all(notification_id: nil) + Notification.where( + id: + Chat::Mention + .where(chat_message: message) + .joins(:notifications) + .select("notifications.id"), + ).destroy_all end def update_tracking_state(message:, **) diff --git a/plugins/chat/db/migrate/20231214180000_add_chat_mention_notifications.rb b/plugins/chat/db/migrate/20231214180000_add_chat_mention_notifications.rb new file mode 100644 index 00000000000..6683a0441cb --- /dev/null +++ b/plugins/chat/db/migrate/20231214180000_add_chat_mention_notifications.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddChatMentionNotifications < ActiveRecord::Migration[7.0] + def change + create_table :chat_mention_notifications, id: false do |t| + t.integer :chat_mention_id, null: false + t.integer :notification_id, null: false + end + + add_index :chat_mention_notifications, %i[chat_mention_id] + add_index :chat_mention_notifications, %i[notification_id], unique: true + end +end diff --git a/plugins/chat/db/migrate/20231214180001_update_relationship_between_chat_mentions_and_notifications.rb b/plugins/chat/db/migrate/20231214180001_update_relationship_between_chat_mentions_and_notifications.rb new file mode 100644 index 00000000000..c90359201fa --- /dev/null +++ b/plugins/chat/db/migrate/20231214180001_update_relationship_between_chat_mentions_and_notifications.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class UpdateRelationshipBetweenChatMentionsAndNotifications < ActiveRecord::Migration[7.0] + disable_ddl_transaction! + BATCH_SIZE = 5000 + + def up + begin + inserted_count = DB.exec(<<~SQL, batch_size: BATCH_SIZE) + INSERT INTO chat_mention_notifications(chat_mention_id, notification_id) + SELECT cm.id, cm.notification_id + FROM chat_mentions cm + LEFT JOIN chat_mention_notifications cmn ON cmn.chat_mention_id = cm.id + WHERE cm.notification_id IS NOT NULL and cmn.chat_mention_id IS NULL + LIMIT :batch_size; + SQL + end while inserted_count > 0 + end + + def down + end +end diff --git a/plugins/chat/db/post_migrate/20231214180002_update_relationship_between_chat_mentions_and_notifications_post_migrate.rb b/plugins/chat/db/post_migrate/20231214180002_update_relationship_between_chat_mentions_and_notifications_post_migrate.rb new file mode 100644 index 00000000000..858f51557e2 --- /dev/null +++ b/plugins/chat/db/post_migrate/20231214180002_update_relationship_between_chat_mentions_and_notifications_post_migrate.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class UpdateRelationshipBetweenChatMentionsAndNotificationsPostMigrate < ActiveRecord::Migration[ + 7.0 +] + disable_ddl_transaction! + BATCH_SIZE = 5000 + + def up + begin + inserted_count = DB.exec(<<~SQL, batch_size: BATCH_SIZE) + INSERT INTO chat_mention_notifications(chat_mention_id, notification_id) + SELECT cm.id, cm.notification_id + FROM chat_mentions cm + LEFT JOIN chat_mention_notifications cmn ON cmn.chat_mention_id = cm.id + WHERE cm.notification_id IS NOT NULL and cmn.chat_mention_id IS NULL + LIMIT :batch_size; + SQL + end while inserted_count > 0 + end + + def down + end +end diff --git a/plugins/chat/lib/chat/notifier.rb b/plugins/chat/lib/chat/notifier.rb index 45cfbcc1535..fae9241230c 100644 --- a/plugins/chat/lib/chat/notifier.rb +++ b/plugins/chat/lib/chat/notifier.rb @@ -85,7 +85,8 @@ module Chat already_notified_user_ids = Chat::Mention .where(chat_message: @chat_message) - .where.not(notification: nil) + .left_outer_joins(:notifications) + .where.not(notifications: { id: nil }) .pluck(:user_id) to_notify, inaccessible, all_mentioned_user_ids = list_users_to_notify diff --git a/plugins/chat/lib/chat/user_notifications_extension.rb b/plugins/chat/lib/chat/user_notifications_extension.rb index 383997277c1..ec1907feec4 100644 --- a/plugins/chat/lib/chat/user_notifications_extension.rb +++ b/plugins/chat/lib/chat/user_notifications_extension.rb @@ -11,9 +11,8 @@ module Chat .joins(:user, :chat_channel) .where.not(user: user) .where("chat_messages.created_at > ?", 1.week.ago) - .joins( - "LEFT OUTER JOIN chat_mentions cm ON cm.chat_message_id = chat_messages.id AND cm.notification_id IS NOT NULL", - ) + .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( "INNER JOIN user_chat_channel_memberships uccm ON uccm.chat_channel_id = chat_channels.id", ) @@ -22,7 +21,7 @@ module Chat (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 ( - (cm.user_id = :user_id AND uccm.following IS true AND chat_channels.chatable_type = 'Category') OR + (cm.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') ) SQL diff --git a/plugins/chat/spec/jobs/regular/chat/channel_delete_spec.rb b/plugins/chat/spec/jobs/regular/chat/channel_delete_spec.rb index e8ea19cd1c9..6e48a4e0f92 100644 --- a/plugins/chat/spec/jobs/regular/chat/channel_delete_spec.rb +++ b/plugins/chat/spec/jobs/regular/chat/channel_delete_spec.rb @@ -26,7 +26,7 @@ describe Jobs::Chat::ChannelDelete do Chat::Mention.create( user: user2, chat_message: messages.sample, - notification: Fabricate(:notification), + notifications: [Fabricate(:notification)], ) @incoming_chat_webhook_id = Fabricate(:incoming_chat_webhook, chat_channel: chat_channel) diff --git a/plugins/chat/spec/jobs/regular/chat/notify_mentioned_spec.rb b/plugins/chat/spec/jobs/regular/chat/notify_mentioned_spec.rb index 2d02fe176d4..3cde7483515 100644 --- a/plugins/chat/spec/jobs/regular/chat/notify_mentioned_spec.rb +++ b/plugins/chat/spec/jobs/regular/chat/notify_mentioned_spec.rb @@ -282,10 +282,6 @@ describe Jobs::Chat::NotifyMentioned do expect(data_hash[:is_direct_message_channel]).to eq(false) expect(data_hash[:chat_channel_title]).to eq(expected_channel_title) expect(data_hash[:chat_channel_slug]).to eq(public_channel.slug) - - chat_mention = - Chat::Mention.where(notification: created_notification, user: user_2, chat_message: message) - expect(chat_mention).to be_present end end diff --git a/plugins/chat/spec/mailers/user_notifications_spec.rb b/plugins/chat/spec/mailers/user_notifications_spec.rb index 236d15b7d47..183e1618f00 100644 --- a/plugins/chat/spec/mailers/user_notifications_spec.rb +++ b/plugins/chat/spec/mailers/user_notifications_spec.rb @@ -257,7 +257,7 @@ describe UserNotifications do :chat_mention, user: user, chat_message: chat_message, - notification: notification, + notifications: [notification], ) end @@ -310,7 +310,7 @@ describe UserNotifications do :chat_mention, user: user, chat_message: another_chat_message, - notification: notification, + notifications: [notification], ) another_chat_channel.update!(last_message: another_chat_message) @@ -350,7 +350,7 @@ describe UserNotifications do :chat_mention, user: user, chat_message: another_chat_message, - notification: notification, + notifications: [notification], ) another_chat_channel.update!(last_message: another_chat_message) end @@ -379,7 +379,7 @@ describe UserNotifications do :chat_mention, user: user, chat_message: chat_message, - notification: notification, + notifications: [notification], ) end @@ -406,7 +406,7 @@ describe UserNotifications do :chat_mention, user: user, chat_message: chat_message, - notification: notification, + notifications: [notification], ) end @@ -513,7 +513,7 @@ describe UserNotifications do :chat_mention, user: user, chat_message: new_message, - notification: notification, + notifications: [notification], ) email = described_class.chat_summary(user, {}) @@ -642,7 +642,12 @@ describe UserNotifications do 2.times do msg = Fabricate(:chat_message, user: sender, chat_channel: channel) notification = Fabricate(:notification) - Fabricate(:chat_mention, user: user, chat_message: msg, notification: notification) + Fabricate( + :chat_mention, + user: user, + chat_message: msg, + notifications: [notification], + ) end email = described_class.chat_summary(user, {}) @@ -668,7 +673,7 @@ describe UserNotifications do :chat_mention, user: user, chat_message: msg, - notification: notification, + notifications: [notification], ) end @@ -695,7 +700,7 @@ describe UserNotifications do :chat_mention, user: user, chat_message: new_message, - notification: notification, + notifications: [notification], ) email = described_class.chat_summary(user, {}) diff --git a/plugins/chat/spec/models/chat/message_spec.rb b/plugins/chat/spec/models/chat/message_spec.rb index 1b88d55a9e2..1601529ec8b 100644 --- a/plugins/chat/spec/models/chat/message_spec.rb +++ b/plugins/chat/spec/models/chat/message_spec.rb @@ -449,8 +449,8 @@ describe Chat::Message do it "destroys chat_mention" do message_1 = Fabricate(:chat_message) - notification = Fabricate(:notification) - mention_1 = Fabricate(:chat_mention, chat_message: message_1, notification: notification) + notification = Fabricate(:notification, notification_type: Notification.types[:chat_mention]) + mention_1 = Fabricate(:chat_mention, chat_message: message_1, notifications: [notification]) message_1.reload.destroy! diff --git a/plugins/chat/spec/queries/chat/channel_unreads_query_spec.rb b/plugins/chat/spec/queries/chat/channel_unreads_query_spec.rb index 112b255c3b7..be57404be42 100644 --- a/plugins/chat/spec/queries/chat/channel_unreads_query_spec.rb +++ b/plugins/chat/spec/queries/chat/channel_unreads_query_spec.rb @@ -128,7 +128,11 @@ describe Chat::ChannelUnreadsQuery do user_id: current_user.id, data: { chat_message_id: message.id, chat_channel_id: channel.id }.to_json, ) - Chat::Mention.create!(notification: notification, user: current_user, chat_message: message) + Chat::Mention.create!( + notifications: [notification], + user: current_user, + chat_message: message, + ) end it "returns a correct unread mention" do diff --git a/plugins/chat/spec/requests/chat/api/reads_controller_spec.rb b/plugins/chat/spec/requests/chat/api/reads_controller_spec.rb index 84190706430..67b3b099b8b 100644 --- a/plugins/chat/spec/requests/chat/api/reads_controller_spec.rb +++ b/plugins/chat/spec/requests/chat/api/reads_controller_spec.rb @@ -199,7 +199,7 @@ RSpec.describe Chat::Api::ReadsController do }.to_json, ) .tap do |notification| - Chat::Mention.create!(user: user, chat_message: msg, notification: notification) + Chat::Mention.create!(user: user, chat_message: msg, notifications: [notification]) end end end diff --git a/plugins/chat/spec/requests/chat/api/thread_reads_controller_spec.rb b/plugins/chat/spec/requests/chat/api/thread_reads_controller_spec.rb index 4f1c0808c33..fe4bc4a7257 100644 --- a/plugins/chat/spec/requests/chat/api/thread_reads_controller_spec.rb +++ b/plugins/chat/spec/requests/chat/api/thread_reads_controller_spec.rb @@ -66,7 +66,7 @@ RSpec.describe Chat::Api::ThreadReadsController do }.to_json, ) .tap do |notification| - Chat::Mention.create!(user: user, chat_message: msg, notification: notification) + Chat::Mention.create!(user: user, chat_message: msg, notifications: [notification]) end end end diff --git a/plugins/chat/spec/services/chat/mark_all_user_channels_read_spec.rb b/plugins/chat/spec/services/chat/mark_all_user_channels_read_spec.rb index 0ba3fb698bd..549e489c177 100644 --- a/plugins/chat/spec/services/chat/mark_all_user_channels_read_spec.rb +++ b/plugins/chat/spec/services/chat/mark_all_user_channels_read_spec.rb @@ -85,12 +85,12 @@ RSpec.describe Chat::MarkAllUserChannelsRead do before do Chat::Mention.create!( - notification: notification_1, + notifications: [notification_1], user: current_user, chat_message: message_1, ) Chat::Mention.create!( - notification: notification_2, + notifications: [notification_2], user: current_user, chat_message: message_3, ) diff --git a/plugins/chat/spec/services/chat/trash_message_spec.rb b/plugins/chat/spec/services/chat/trash_message_spec.rb index 821662433ca..cb03dea8783 100644 --- a/plugins/chat/spec/services/chat/trash_message_spec.rb +++ b/plugins/chat/spec/services/chat/trash_message_spec.rb @@ -49,13 +49,13 @@ RSpec.describe Chat::TrashMessage do it "destroys notifications for mentions" do notification = Fabricate(:notification) - mention = Fabricate(:chat_mention, chat_message: message, notification: notification) + mention = Fabricate(:chat_mention, chat_message: message, notifications: [notification]) result mention = Chat::Mention.find_by(id: mention.id) expect(mention).to be_present - expect(mention.notification_id).to be_nil + expect(mention.notifications).to be_empty end it "publishes associated Discourse and MessageBus events" do diff --git a/plugins/chat/spec/services/chat/update_message_spec.rb b/plugins/chat/spec/services/chat/update_message_spec.rb index 69979936b9a..347675c8fd1 100644 --- a/plugins/chat/spec/services/chat/update_message_spec.rb +++ b/plugins/chat/spec/services/chat/update_message_spec.rb @@ -189,7 +189,7 @@ RSpec.describe Chat::UpdateMessage do ) mention = user3.chat_mentions.where(chat_message: message.id).first - expect(mention.notification).to be_present + expect(mention.notifications.length).to be(1) end it "doesn't create mentions for already mentioned users" do @@ -215,7 +215,7 @@ RSpec.describe Chat::UpdateMessage do ) mention = user_without_memberships.chat_mentions.where(chat_message: chat_message).first - expect(mention.notification).to be_nil + expect(mention.notifications).to be_empty end it "destroys mentions that should be removed" do @@ -271,7 +271,7 @@ RSpec.describe Chat::UpdateMessage do ) mention = admin1.chat_mentions.where(chat_message_id: message.id).first - expect(mention.notification).to be_nil + expect(mention.notifications).to be_empty end it "creates a chat_mention record without notification when self mentioning" do @@ -282,7 +282,7 @@ RSpec.describe Chat::UpdateMessage do mention = user1.chat_mentions.where(chat_message: chat_message).first expect(mention).to be_present - expect(mention.notification).to be_nil + expect(mention.notifications).to be_empty end it "adds mentioned user and their status to the message bus message" do diff --git a/plugins/chat/spec/services/chat/update_user_last_read_spec.rb b/plugins/chat/spec/services/chat/update_user_last_read_spec.rb index 9db2ee92fff..bf3195b40bb 100644 --- a/plugins/chat/spec/services/chat/update_user_last_read_spec.rb +++ b/plugins/chat/spec/services/chat/update_user_last_read_spec.rb @@ -85,7 +85,7 @@ RSpec.describe Chat::UpdateUserLastRead do before do Jobs.run_immediately! Chat::Mention.create!( - notification: notification, + notifications: [notification], user: current_user, chat_message: message_1, ) diff --git a/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb b/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb index c1bd66fcb03..c2bc3bf6a1f 100644 --- a/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb +++ b/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb @@ -62,12 +62,12 @@ RSpec.describe Chat::UpdateUserThreadLastRead do before do Jobs.run_immediately! Chat::Mention.create!( - notification: notification_1, + notifications: [notification_1], user: current_user, chat_message: Fabricate(:chat_message, chat_channel: channel, thread: thread), ) Chat::Mention.create!( - notification: notification_2, + notifications: [notification_2], user: current_user, chat_message: Fabricate(:chat_message, chat_channel: channel, thread: thread), )