Queue a different notify_mentioned job per mention type

This commit is contained in:
Roman Rizzi 2022-12-29 12:55:06 -03:00
parent 17cebd0454
commit 9b9d162cc2
No known key found for this signature in database
GPG Key ID: 64024A71CE7330D3
3 changed files with 100 additions and 56 deletions

View File

@ -12,10 +12,21 @@ module Jobs
return
end
mention_type = args[:mention_type]&.to_sym
return if (
mention_type.blank? ||
(
!Chat::ChatNotifier::STATIC_MENTION_TYPES.include?(mention_type) &&
!Group.where("LOWER(name) = ?", mention_type).exists?
)
)
@creator = @chat_message.user
@chat_channel = @chat_message.chat_channel
user_ids_to_notify = args[:to_notify_ids_map] || {}
user_ids_to_notify.each { |mention_type, ids| process_mentions(ids, mention_type.to_sym) }
user_ids_to_notify = args[:user_ids].to_a
process_mentions(user_ids_to_notify, mention_type)
end
private

View File

@ -27,6 +27,11 @@
# The ignore/mute filtering is also applied via the ChatNotifyWatching job,
# which prevents desktop / push notifications being sent.
class Chat::ChatNotifier
DIRECT_MENTIONS = :direct_mentions
HERE_MENTIONS = :here_mentions
GLOBAL_MENTIONS = :global_mentions
STATIC_MENTION_TYPES = [DIRECT_MENTIONS, HERE_MENTIONS, GLOBAL_MENTIONS]
class << self
def push_notification_tag(type, chat_channel_id)
"#{Discourse.current_hostname}-chat-#{type}-#{chat_channel_id}"
@ -64,14 +69,17 @@ class Chat::ChatNotifier
to_notify = list_users_to_notify
notify_creator_of_inaccessible_mentions(to_notify)
notify_mentioned_users(to_notify)
to_notify.each do |mention_type, user_ids|
notify_mentioned_users(mention_type, user_ids)
end
global_mentions = []
global_mentions << "all" if typed_global_mention?
global_mentions << "here" if typed_here_mention?
notify_watching_users(
to_notify[:direct_mentions],
to_notify[DIRECT_MENTIONS],
global_mentions,
to_notify[:mentioned_group_ids]
)
@ -84,7 +92,10 @@ class Chat::ChatNotifier
purge_outdated_mentions(to_notify)
notify_creator_of_inaccessible_mentions(to_notify)
notify_mentioned_users(to_notify)
to_notify.each do |mention_type, user_ids|
notify_mentioned_users(mention_type, user_ids)
end
to_notify
end
@ -130,10 +141,8 @@ class Chat::ChatNotifier
end
def chat_users
users =
User.includes(:user_chat_channel_memberships, :group_users)
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)
@ -145,7 +154,7 @@ class Chat::ChatNotifier
def channel_members
chat_users.where(
user_chat_channel_memberships: {
uccm: {
following: true,
chat_channel_id: @chat_channel.id,
},
@ -189,19 +198,19 @@ class Chat::ChatNotifier
.where("last_seen_at < ?", 5.minutes.ago)
end
to_notify[:global_mentions] = global_mentions.pluck(:id)
to_notify[GLOBAL_MENTIONS] = global_mentions.pluck(:id)
else
to_notify[:global_mentions] = []
to_notify[GLOBAL_MENTIONS] = []
end
end
def expand_here_mention(to_notify, skip)
if typed_here_mention? && @chat_channel.allow_channel_wide_mentions && !skip
to_notify[:here_mentions] = channel_wide_mentions(to_notify[:mentioned_group_ids])
to_notify[HERE_MENTIONS] = channel_wide_mentions(to_notify[:mentioned_group_ids])
.where("last_seen_at > ?", 5.minutes.ago)
.pluck(:id)
else
to_notify[:here_mentions] = []
to_notify[HERE_MENTIONS] = []
end
end
@ -239,7 +248,7 @@ class Chat::ChatNotifier
grouped = group_users_to_notify(direct_mentions)
to_notify[:direct_mentions] = grouped[:already_participating].map(&:id)
to_notify[DIRECT_MENTIONS] = grouped[:already_participating].map(&:id)
to_notify[:welcome_to_join] = grouped[:welcome_to_join]
to_notify[:unreachable] = grouped[:unreachable]
end
@ -331,12 +340,13 @@ class Chat::ChatNotifier
end
end
def notify_mentioned_users(to_notify)
def notify_mentioned_users(mention_type, user_ids)
Jobs.enqueue(
:chat_notify_mentioned,
{
chat_message_id: @chat_message.id,
to_notify_ids_map: to_notify.as_json,
user_ids: user_ids,
mention_type: mention_type,
timestamp: @timestamp.iso8601(6),
},
)

View File

@ -7,6 +7,8 @@ describe Jobs::ChatNotifyMentioned do
fab!(:user_2) { Fabricate(:user) }
fab!(:public_channel) { Fabricate(:category_channel) }
let(:user_ids) { [user_2.id] }
before do
Group.refresh_automatic_groups!
user_1.reload
@ -28,31 +30,34 @@ describe Jobs::ChatNotifyMentioned do
def track_desktop_notification(
user: user_2,
message:,
to_notify_ids_map:
user_ids:,
mention_type:
)
MessageBus
.track_publish("/chat/notification-alert/#{user.id}") do
subject.execute(
chat_message_id: message.id,
timestamp: message.created_at,
to_notify_ids_map: to_notify_ids_map
user_ids: user_ids,
mention_type: mention_type
)
end
.first
end
def track_core_notification(user: user_2, message:, to_notify_ids_map:)
def track_core_notification(user: user_2, message:, user_ids:, mention_type:)
subject.execute(
chat_message_id: message.id,
timestamp: message.created_at,
to_notify_ids_map: to_notify_ids_map,
user_ids: user_ids,
mention_type: mention_type
)
Notification.where(user: user, notification_type: Notification.types[:chat_mention]).last
end
describe "scenarios where we should skip sending notifications" do
let(:to_notify_ids_map) { { here_mentions: [user_2.id] } }
let(:mention_type) { Chat::ChatNotifier::HERE_MENTIONS }
it "does nothing if there is a newer version of the message" do
message = create_chat_message
@ -61,7 +66,7 @@ describe Jobs::ChatNotifyMentioned do
PostAlerter.expects(:push_notification).never
desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
expect(desktop_notification).to be_nil
created_notification =
@ -79,7 +84,7 @@ describe Jobs::ChatNotifyMentioned do
PostAlerter.expects(:push_notification).never
desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
expect(desktop_notification).to be_nil
created_notification =
@ -95,7 +100,7 @@ describe Jobs::ChatNotifyMentioned do
PostAlerter.expects(:push_notification).never
desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
expect(desktop_notification).to be_nil
created_notification =
@ -110,7 +115,7 @@ describe Jobs::ChatNotifyMentioned do
PostAlerter.expects(:push_notification).never
desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
expect(desktop_notification).to be_nil
created_notification =
@ -127,7 +132,7 @@ describe Jobs::ChatNotifyMentioned do
PostAlerter.expects(:push_notification).once
desktop_notification =
track_desktop_notification(message: message_2, to_notify_ids_map: to_notify_ids_map)
track_desktop_notification(message: message_2, user_ids: user_ids, mention_type: mention_type)
expect(desktop_notification).to be_present
end
@ -135,14 +140,14 @@ describe Jobs::ChatNotifyMentioned do
it "does nothing if user is not participating in a private channel" do
user_3 = Fabricate(:user)
@chat_group.add(user_3)
to_notify_map = { direct_mentions: [user_3.id] }
message = create_chat_message(channel: @personal_chat_channel)
PostAlerter.expects(:push_notification).never
desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_map)
track_desktop_notification(
message: message, user_ids: [user_3.id], mention_type: Chat::ChatNotifier::HERE_MENTIONS
)
expect(desktop_notification).to be_nil
created_notification =
@ -157,7 +162,7 @@ describe Jobs::ChatNotifyMentioned do
)
desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
expect(desktop_notification).to be_nil
end
@ -173,7 +178,8 @@ describe Jobs::ChatNotifyMentioned do
subject.execute(
chat_message_id: message.id,
timestamp: message.created_at,
to_notify_ids_map: to_notify_ids_map,
user_ids: user_ids,
mention_type: mention_type
)
end
@ -185,7 +191,7 @@ describe Jobs::ChatNotifyMentioned do
)
desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
expect(desktop_notification).to be_nil
end
@ -202,10 +208,25 @@ describe Jobs::ChatNotifyMentioned do
subject.execute(
chat_message_id: message.id,
timestamp: message.created_at,
to_notify_ids_map: to_notify_ids_map,
user_ids: user_ids,
mention_type: mention_type
)
end
it "does nothing when the mention type is invalid" do
message = create_chat_message
PostAlerter.expects(:push_notification).never
desktop_notification =
track_desktop_notification(message: message, user_ids: user_ids, mention_type: "invalid")
expect(desktop_notification).to be_nil
created_notification =
Notification.where(user: user_2, notification_type: Notification.types[:chat_mention]).last
expect(created_notification).to be_nil
end
context "when the user is muting the message sender" do
it "does not send notifications to the user who is muting the acting user" do
Fabricate(:muted_user, user: user_2, muted_user: user_1)
@ -214,7 +235,7 @@ describe Jobs::ChatNotifyMentioned do
PostAlerter.expects(:push_notification).never
desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
expect(desktop_notification).to be_nil
created_notification =
@ -229,7 +250,7 @@ describe Jobs::ChatNotifyMentioned do
PostAlerter.expects(:push_notification).never
desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
expect(desktop_notification).to be_nil
created_notification =
@ -246,7 +267,7 @@ describe Jobs::ChatNotifyMentioned do
message = create_chat_message
desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
expect(desktop_notification).to be_present
expect(desktop_notification.data[:notification_type]).to eq(Notification.types[:chat_mention])
@ -279,7 +300,8 @@ describe Jobs::ChatNotifyMentioned do
subject.execute(
chat_message_id: message.id,
timestamp: message.created_at,
to_notify_ids_map: to_notify_ids_map,
user_ids: user_ids,
mention_type: mention_type
)
end
@ -287,7 +309,7 @@ describe Jobs::ChatNotifyMentioned do
message = create_chat_message
created_notification =
track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_core_notification(message: message, user_ids: user_ids, mention_type: mention_type)
expect(created_notification).to be_present
expect(created_notification.high_priority).to eq(true)
@ -315,7 +337,8 @@ describe Jobs::ChatNotifyMentioned do
subject.execute(
chat_message_id: message.id,
timestamp: message.created_at,
to_notify_ids_map: to_notify_ids_map,
user_ids: user_ids,
mention_type: mention_type
)
end.first
@ -327,7 +350,7 @@ describe Jobs::ChatNotifyMentioned do
describe "#execute" do
describe "global mention notifications" do
let(:to_notify_ids_map) { { global_mentions: [user_2.id] } }
let(:mention_type) { Chat::ChatNotifier::GLOBAL_MENTIONS }
let(:payload_translated_title) do
I18n.t(
@ -344,7 +367,7 @@ describe Jobs::ChatNotifyMentioned do
message = create_chat_message
created_notification =
track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_core_notification(message: message, user_ids: user_ids, mention_type: mention_type)
data_hash = created_notification.data_hash
@ -355,7 +378,7 @@ describe Jobs::ChatNotifyMentioned do
message = create_chat_message
desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
expect(desktop_notification.data[:translated_title]).to eq(payload_translated_title)
end
@ -365,7 +388,7 @@ describe Jobs::ChatNotifyMentioned do
message = create_chat_message(channel: @personal_chat_channel)
desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
expected_title =
I18n.t(
@ -380,7 +403,7 @@ describe Jobs::ChatNotifyMentioned do
end
describe "here mention notifications" do
let(:to_notify_ids_map) { { here_mentions: [user_2.id] } }
let(:mention_type) { Chat::ChatNotifier::HERE_MENTIONS }
let(:payload_translated_title) do
I18n.t(
@ -397,7 +420,7 @@ describe Jobs::ChatNotifyMentioned do
message = create_chat_message
created_notification =
track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_core_notification(message: message, user_ids: user_ids, mention_type: mention_type)
data_hash = created_notification.data_hash
expect(data_hash[:identifier]).to eq("here")
@ -407,7 +430,7 @@ describe Jobs::ChatNotifyMentioned do
message = create_chat_message
desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
expect(desktop_notification.data[:translated_title]).to eq(payload_translated_title)
end
@ -417,7 +440,7 @@ describe Jobs::ChatNotifyMentioned do
message = create_chat_message(channel: @personal_chat_channel)
desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
expected_title =
I18n.t(
@ -432,7 +455,7 @@ describe Jobs::ChatNotifyMentioned do
end
describe "direct mention notifications" do
let(:to_notify_ids_map) { { direct_mentions: [user_2.id] } }
let(:mention_type) { Chat::ChatNotifier::DIRECT_MENTIONS }
let(:payload_translated_title) do
I18n.t(
@ -449,7 +472,7 @@ describe Jobs::ChatNotifyMentioned do
message = create_chat_message
created_notification =
track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_core_notification(message: message, user_ids: user_ids, mention_type: mention_type)
data_hash = created_notification.data_hash
expect(data_hash[:identifier]).to be_nil
@ -459,7 +482,7 @@ describe Jobs::ChatNotifyMentioned do
message = create_chat_message
desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
expect(desktop_notification.data[:translated_title]).to eq(payload_translated_title)
end
@ -469,7 +492,7 @@ describe Jobs::ChatNotifyMentioned do
message = create_chat_message(channel: @personal_chat_channel)
desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
expected_title =
I18n.t(
@ -484,7 +507,7 @@ describe Jobs::ChatNotifyMentioned do
end
describe "group mentions" do
let(:to_notify_ids_map) { { @chat_group.name.to_sym => [user_2.id] } }
let(:mention_type) { @chat_group.name.to_sym }
let(:payload_translated_title) do
I18n.t(
@ -501,7 +524,7 @@ describe Jobs::ChatNotifyMentioned do
message = create_chat_message
created_notification =
track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_core_notification(message: message, user_ids: user_ids, mention_type: mention_type)
data_hash = created_notification.data_hash
expect(data_hash[:identifier]).to eq(@chat_group.name)
@ -512,7 +535,7 @@ describe Jobs::ChatNotifyMentioned do
message = create_chat_message
desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
expect(desktop_notification.data[:translated_title]).to eq(payload_translated_title)
end
@ -522,7 +545,7 @@ describe Jobs::ChatNotifyMentioned do
message = create_chat_message(channel: @personal_chat_channel)
desktop_notification =
track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
track_desktop_notification(message: message, user_ids: user_ids, mention_type: mention_type)
expected_title =
I18n.t(