DEV: Create a chat_mention record when self mentioning (#21438)
In the past, we create a `chat_mention` records only when we wanted to notify a user about a mention. Since we don't send notifications when a user mentioning himself, we didn't create a `chat_mention` records in those cases. Now we use `chat_mentions` records in other scenarios too, so when a user is mentioning himself we want to: 1. Create a `chat_mention` record for that mention 2. Do not create a notification for that mention
This commit is contained in:
parent
f494e54128
commit
2703f2311a
|
@ -132,6 +132,7 @@ module Chat
|
||||||
.global_mentions
|
.global_mentions
|
||||||
.not_suspended
|
.not_suspended
|
||||||
.where(user_options: { ignore_channel_wide_mention: [false, nil] })
|
.where(user_options: { ignore_channel_wide_mention: [false, nil] })
|
||||||
|
.where.not(username_lower: @user.username_lower)
|
||||||
.where.not(id: already_covered_ids)
|
.where.not(id: already_covered_ids)
|
||||||
.pluck(:id)
|
.pluck(:id)
|
||||||
|
|
||||||
|
@ -150,6 +151,7 @@ module Chat
|
||||||
.here_mentions
|
.here_mentions
|
||||||
.not_suspended
|
.not_suspended
|
||||||
.where(user_options: { ignore_channel_wide_mention: [false, nil] })
|
.where(user_options: { ignore_channel_wide_mention: [false, nil] })
|
||||||
|
.where.not(username_lower: @user.username_lower)
|
||||||
.where.not(id: already_covered_ids)
|
.where.not(id: already_covered_ids)
|
||||||
.pluck(:id)
|
.pluck(:id)
|
||||||
|
|
||||||
|
@ -187,9 +189,12 @@ module Chat
|
||||||
direct_mentions = []
|
direct_mentions = []
|
||||||
else
|
else
|
||||||
direct_mentions =
|
direct_mentions =
|
||||||
@chat_message.parsed_mentions.direct_mentions.not_suspended.where.not(
|
@chat_message
|
||||||
id: already_covered_ids,
|
.parsed_mentions
|
||||||
)
|
.direct_mentions
|
||||||
|
.not_suspended
|
||||||
|
.where.not(username_lower: @user.username_lower)
|
||||||
|
.where.not(id: already_covered_ids)
|
||||||
end
|
end
|
||||||
|
|
||||||
grouped = group_users_to_notify(direct_mentions)
|
grouped = group_users_to_notify(direct_mentions)
|
||||||
|
@ -209,6 +214,7 @@ module Chat
|
||||||
.group_mentions
|
.group_mentions
|
||||||
.not_suspended
|
.not_suspended
|
||||||
.where("user_count <= ?", SiteSetting.max_users_notified_per_group_mention)
|
.where("user_count <= ?", SiteSetting.max_users_notified_per_group_mention)
|
||||||
|
.where.not(username_lower: @user.username_lower)
|
||||||
.where.not(id: already_covered_ids)
|
.where.not(id: already_covered_ids)
|
||||||
|
|
||||||
too_many_members, mentionable =
|
too_many_members, mentionable =
|
||||||
|
|
|
@ -91,7 +91,6 @@ module Chat
|
||||||
.joins(:user_option)
|
.joins(:user_option)
|
||||||
.real
|
.real
|
||||||
.where(user_options: { chat_enabled: true })
|
.where(user_options: { chat_enabled: true })
|
||||||
.where.not(username_lower: @message.user.username.downcase)
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def parse_mentions(message)
|
def parse_mentions(message)
|
||||||
|
|
|
@ -146,9 +146,10 @@ describe Chat::MessageCreator do
|
||||||
"this is a @#{user1.username} message with @system @mentions @#{user2.username} and @#{user3.username}",
|
"this is a @#{user1.username} message with @system @mentions @#{user2.username} and @#{user3.username}",
|
||||||
).chat_message
|
).chat_message
|
||||||
|
|
||||||
# a mention for the user himself wasn't created
|
# a mention for the user himself was created, but a notification wasn't
|
||||||
user1_mention = user1.chat_mentions.where(chat_message: message).first
|
user1_mention = user1.chat_mentions.where(chat_message: message).first
|
||||||
expect(user1_mention).to be_nil
|
expect(user1_mention).to be_present
|
||||||
|
expect(user1_mention.notification).to be_nil
|
||||||
|
|
||||||
system_user_mention = Discourse.system_user.chat_mentions.where(chat_message: message).first
|
system_user_mention = Discourse.system_user.chat_mentions.where(chat_message: message).first
|
||||||
expect(system_user_mention).to be_nil
|
expect(system_user_mention).to be_nil
|
||||||
|
@ -172,59 +173,94 @@ describe Chat::MessageCreator do
|
||||||
}.to change { user2.chat_mentions.count }.by(1)
|
}.to change { user2.chat_mentions.count }.by(1)
|
||||||
end
|
end
|
||||||
|
|
||||||
it "notifies @all properly" do
|
context "when mentioning @all" do
|
||||||
expect {
|
it "creates a chat mention record for every user" do
|
||||||
described_class.create(chat_channel: public_chat_channel, user: user1, content: "@all")
|
expect {
|
||||||
}.to change { Chat::Mention.count }.by(4)
|
described_class.create(chat_channel: public_chat_channel, user: user1, content: "@all")
|
||||||
|
}.to change { Chat::Mention.count }.by(5)
|
||||||
|
|
||||||
Chat::UserChatChannelMembership.where(
|
Chat::UserChatChannelMembership.where(
|
||||||
user: user2,
|
user: user2,
|
||||||
chat_channel: public_chat_channel,
|
|
||||||
).update_all(following: false)
|
|
||||||
expect {
|
|
||||||
described_class.create(
|
|
||||||
chat_channel: public_chat_channel,
|
chat_channel: public_chat_channel,
|
||||||
user: user1,
|
).update_all(following: false)
|
||||||
content: "again! @all",
|
|
||||||
)
|
expect {
|
||||||
}.to change { Chat::Mention.count }.by(3)
|
described_class.create(
|
||||||
|
chat_channel: public_chat_channel,
|
||||||
|
user: user1,
|
||||||
|
content: "again! @all",
|
||||||
|
)
|
||||||
|
}.to change { Chat::Mention.count }.by(4)
|
||||||
|
end
|
||||||
|
|
||||||
|
it "does not create a notification for acting user" do
|
||||||
|
message =
|
||||||
|
described_class.create(
|
||||||
|
chat_channel: public_chat_channel,
|
||||||
|
user: user1,
|
||||||
|
content: "@all",
|
||||||
|
).chat_message
|
||||||
|
|
||||||
|
mention = user1.chat_mentions.where(chat_message: message).first
|
||||||
|
expect(mention).to be_present
|
||||||
|
expect(mention.notification).to be_blank
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
it "notifies @here properly" do
|
context "when mentioning @here" do
|
||||||
admin1.update(last_seen_at: 1.year.ago)
|
it "creates a chat mention record for every active user" do
|
||||||
admin2.update(last_seen_at: 1.year.ago)
|
admin1.update(last_seen_at: 1.year.ago)
|
||||||
user1.update(last_seen_at: Time.now)
|
admin2.update(last_seen_at: 1.year.ago)
|
||||||
user2.update(last_seen_at: Time.now)
|
|
||||||
user3.update(last_seen_at: Time.now)
|
|
||||||
expect {
|
|
||||||
described_class.create(chat_channel: public_chat_channel, user: user1, content: "@here")
|
|
||||||
}.to change { Chat::Mention.count }.by(2)
|
|
||||||
end
|
|
||||||
|
|
||||||
it "doesn't sent double notifications when '@here' is mentioned" do
|
user1.update(last_seen_at: Time.now)
|
||||||
user2.update(last_seen_at: Time.now)
|
user2.update(last_seen_at: Time.now)
|
||||||
expect {
|
user3.update(last_seen_at: Time.now)
|
||||||
described_class.create(
|
|
||||||
chat_channel: public_chat_channel,
|
|
||||||
user: user1,
|
|
||||||
content: "@here @#{user2.username}",
|
|
||||||
)
|
|
||||||
}.to change { user2.chat_mentions.count }.by(1)
|
|
||||||
end
|
|
||||||
|
|
||||||
it "notifies @here plus other mentions" do
|
expect {
|
||||||
admin1.update(last_seen_at: Time.now)
|
described_class.create(chat_channel: public_chat_channel, user: user1, content: "@here")
|
||||||
admin2.update(last_seen_at: 1.year.ago)
|
}.to change { Chat::Mention.count }.by(3)
|
||||||
user1.update(last_seen_at: 1.year.ago)
|
end
|
||||||
user2.update(last_seen_at: 1.year.ago)
|
|
||||||
user3.update(last_seen_at: 1.year.ago)
|
it "does not create a notification for acting user" do
|
||||||
expect {
|
user1.update(last_seen_at: Time.now)
|
||||||
described_class.create(
|
|
||||||
chat_channel: public_chat_channel,
|
message =
|
||||||
user: user1,
|
described_class.create(
|
||||||
content: "@here plus @#{user3.username}",
|
chat_channel: public_chat_channel,
|
||||||
)
|
user: user1,
|
||||||
}.to change { user3.chat_mentions.count }.by(1)
|
content: "@here",
|
||||||
|
).chat_message
|
||||||
|
|
||||||
|
mention = user1.chat_mentions.where(chat_message: message).first
|
||||||
|
expect(mention).to be_present
|
||||||
|
expect(mention.notification).to be_blank
|
||||||
|
end
|
||||||
|
|
||||||
|
it "doesn't send double notifications" do
|
||||||
|
user2.update(last_seen_at: Time.now)
|
||||||
|
expect {
|
||||||
|
described_class.create(
|
||||||
|
chat_channel: public_chat_channel,
|
||||||
|
user: user1,
|
||||||
|
content: "@here @#{user2.username}",
|
||||||
|
)
|
||||||
|
}.to change { user2.chat_mentions.count }.by(1)
|
||||||
|
end
|
||||||
|
|
||||||
|
it "notifies @here plus other mentions" do
|
||||||
|
admin1.update(last_seen_at: Time.now)
|
||||||
|
admin2.update(last_seen_at: 1.year.ago)
|
||||||
|
user1.update(last_seen_at: 1.year.ago)
|
||||||
|
user2.update(last_seen_at: 1.year.ago)
|
||||||
|
user3.update(last_seen_at: 1.year.ago)
|
||||||
|
expect {
|
||||||
|
described_class.create(
|
||||||
|
chat_channel: public_chat_channel,
|
||||||
|
user: user1,
|
||||||
|
content: "@here plus @#{user3.username}",
|
||||||
|
)
|
||||||
|
}.to change { user3.chat_mentions.count }.by(1)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
it "doesn't create mention notifications for users without a membership record" do
|
it "doesn't create mention notifications for users without a membership record" do
|
||||||
|
@ -371,6 +407,19 @@ describe Chat::MessageCreator do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "creates a chat_mention record without notification when self mentioning" do
|
||||||
|
message =
|
||||||
|
described_class.create(
|
||||||
|
chat_channel: direct_message_channel,
|
||||||
|
user: user1,
|
||||||
|
content: "hello @#{user1.username}",
|
||||||
|
).chat_message
|
||||||
|
|
||||||
|
mention = user1.chat_mentions.where(chat_message: message).first
|
||||||
|
expect(mention).to be_present
|
||||||
|
expect(mention.notification).to be_nil
|
||||||
|
end
|
||||||
|
|
||||||
context "when ignore_channel_wide_mention is enabled" do
|
context "when ignore_channel_wide_mention is enabled" do
|
||||||
before { user2.user_option.update(ignore_channel_wide_mention: true) }
|
before { user2.user_option.update(ignore_channel_wide_mention: true) }
|
||||||
|
|
||||||
|
@ -887,6 +936,19 @@ describe Chat::MessageCreator do
|
||||||
)
|
)
|
||||||
}.not_to change { Chat::Mention.count }
|
}.not_to change { Chat::Mention.count }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "creates a chat mention without notification for acting user" do
|
||||||
|
message =
|
||||||
|
described_class.create(
|
||||||
|
chat_channel: public_chat_channel,
|
||||||
|
user: user1,
|
||||||
|
content: "@#{user_group.name}",
|
||||||
|
).chat_message
|
||||||
|
|
||||||
|
mention = user1.chat_mentions.where(chat_message: message).first
|
||||||
|
expect(mention).to be_present
|
||||||
|
expect(mention.notification).to be_blank
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "push notifications" do
|
describe "push notifications" do
|
||||||
|
|
|
@ -213,6 +213,21 @@ describe Chat::MessageUpdater do
|
||||||
expect(mention.notification).to be_nil
|
expect(mention.notification).to be_nil
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "creates a chat_mention record without notification when self mentioning" do
|
||||||
|
chat_message = create_chat_message(user1, "I will mention myself soon", public_chat_channel)
|
||||||
|
new_content = "hello @#{user1.username}"
|
||||||
|
|
||||||
|
described_class.update(
|
||||||
|
guardian: guardian,
|
||||||
|
chat_message: chat_message,
|
||||||
|
new_content: new_content,
|
||||||
|
)
|
||||||
|
|
||||||
|
mention = user1.chat_mentions.where(chat_message: chat_message).first
|
||||||
|
expect(mention).to be_present
|
||||||
|
expect(mention.notification).to be_nil
|
||||||
|
end
|
||||||
|
|
||||||
context "when updating a mentioned user" do
|
context "when updating a mentioned user" do
|
||||||
it "updates the mention record" do
|
it "updates the mention record" do
|
||||||
chat_message = create_chat_message(user1, "ping @#{user2.username}", public_chat_channel)
|
chat_message = create_chat_message(user1, "ping @#{user2.username}", public_chat_channel)
|
||||||
|
|
|
@ -95,6 +95,21 @@ RSpec.describe Chat::ParsedMentions do
|
||||||
expect(result).to contain_exactly(channel_member_1.username, channel_member_2.username)
|
expect(result).to contain_exactly(channel_member_1.username, channel_member_2.username)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "returns a user when self-mentioning" do
|
||||||
|
message =
|
||||||
|
Fabricate(
|
||||||
|
:chat_message,
|
||||||
|
chat_channel: chat_channel,
|
||||||
|
message: "Hey @#{channel_member_1.username}",
|
||||||
|
user: channel_member_1,
|
||||||
|
)
|
||||||
|
|
||||||
|
mentions = described_class.new(message)
|
||||||
|
|
||||||
|
result = mentions.direct_mentions.pluck(:username)
|
||||||
|
expect(result).to contain_exactly(channel_member_1.username)
|
||||||
|
end
|
||||||
|
|
||||||
it "returns a mentioned user even if he's not a member of the channel" do
|
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}")
|
message = create_message("mentioning @#{not_a_channel_member.username}")
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue