discourse/app/services/post_alerter.rb
Martin Brennan 87684f7c5e
FEATURE: Use group SMTP job and mailer instead of UserNotifications change (#13489)
This PR backtracks a fair bit on this one https://github.com/discourse/discourse/pull/13220/files.

Instead of sending the group SMTP email for each user via `UserNotifications`, we are changing to send only one email with the existing `Jobs::GroupSmtpEmail` job and `GroupSmtpMailer`. We are changing this job and mailer along with `PostAlerter` to make the first topic allowed user the `to_address` for the email and any other `topic_allowed_users` to be the CC address on the email. This is to cut down on emails sent via SMTP, which is subject to daily limits from providers such as Gmail. We log these details in the `EmailLog` table now.

In addition to this, we have changed `PostAlerter` to no longer rely on incoming email email addresses for sending the `GroupSmtpEmail` job. This was unreliable as a user's email could have changed in the meantime. Also it was a little overcomplicated to use the incoming email records -- it is far simpler to reason about to just use topic allowed users.

This also adds a fix to include cc_addresses in the EmailLog.addressed_to_user scope.
2021-06-28 08:55:13 +10:00

774 lines
26 KiB
Ruby

# frozen_string_literal: true
class PostAlerter
USER_BATCH_SIZE = 100
def self.post_created(post, opts = {})
PostAlerter.new(opts).after_save_post(post, true)
post
end
def self.post_edited(post, opts = {})
PostAlerter.new(opts).after_save_post(post, false)
post
end
def initialize(default_opts = {})
@default_opts = default_opts
end
def not_allowed?(user, post)
user.blank? ||
user.bot? ||
user.id == post.user_id
end
def all_allowed_users(post)
@all_allowed_users ||= post.topic.all_allowed_users.reject { |u| not_allowed?(u, post) }
end
def allowed_users(post)
@allowed_users ||= post.topic.allowed_users.reject { |u| not_allowed?(u, post) }
end
def allowed_group_users(post)
@allowed_group_users ||= post.topic.allowed_group_users.reject { |u| not_allowed?(u, post) }
end
def directly_targeted_users(post)
allowed_users(post) - allowed_group_users(post)
end
def indirectly_targeted_users(post)
allowed_group_users(post)
end
def only_allowed_users(users, post)
return users unless post.topic.private_message?
users.select { |u| all_allowed_users(post).include?(u) }
end
def notify_about_reply?(post)
# small actions can be whispers in this case they will have an action code
# we never want to notify on this
post.post_type == Post.types[:regular] ||
(post.post_type == Post.types[:whisper] && post.action_code.nil?)
end
def after_save_post(post, new_record = false)
notified = [post.user, post.last_editor].uniq
# mentions (users/groups)
mentioned_groups, mentioned_users = extract_mentions(post)
if mentioned_groups || mentioned_users
mentioned_opts = {}
editor = post.last_editor
if post.last_editor_id != post.user_id
# Mention comes from an edit by someone else, so notification should say who added the mention.
mentioned_opts = { user_id: editor.id, original_username: editor.username, display_username: editor.username }
end
if mentioned_users
mentioned_users = only_allowed_users(mentioned_users, post)
notified += notify_users(mentioned_users - notified, :mentioned, post, mentioned_opts)
end
expand_group_mentions(mentioned_groups, post) do |group, users|
users = only_allowed_users(users, post)
notified += notify_users(users - notified, :group_mentioned, post, mentioned_opts.merge(group: group))
end
end
# replies
reply_to_user = post.reply_notification_target
if new_record && reply_to_user && !notified.include?(reply_to_user) && notify_about_reply?(post)
notified += notify_non_pm_users(reply_to_user, :replied, post)
end
# quotes
quoted_users = extract_quoted_users(post)
notified += notify_non_pm_users(quoted_users - notified, :quoted, post)
# linked
linked_users = extract_linked_users(post)
notified += notify_non_pm_users(linked_users - notified, :linked, post)
# private messages
if new_record
if post.topic.private_message?
notify_pm_users(post, reply_to_user, notified)
elsif notify_about_reply?(post)
notify_post_users(post, notified, new_record: new_record)
end
end
sync_group_mentions(post, mentioned_groups)
if new_record && post.post_number == 1
topic = post.topic
if topic.present?
watchers = category_watchers(topic) + tag_watchers(topic) + group_watchers(topic)
notify_first_post_watchers(post, watchers)
end
end
end
def group_watchers(topic)
GroupUser.where(
group_id: topic.allowed_groups.pluck(:group_id),
notification_level: GroupUser.notification_levels[:watching_first_post]
).pluck(:user_id)
end
def tag_watchers(topic)
topic.tag_users
.where(notification_level: TagUser.notification_levels[:watching_first_post])
.pluck(:user_id)
end
def category_watchers(topic)
topic.category_users
.where(notification_level: CategoryUser.notification_levels[:watching_first_post])
.pluck(:user_id)
end
def notify_first_post_watchers(post, user_ids)
return if user_ids.blank?
user_ids.uniq!
warn_if_not_sidekiq
# Don't notify the OP
user_ids -= [post.user_id]
users = User.where(id: user_ids)
DiscourseEvent.trigger(:before_create_notifications_for_users, users, post)
each_user_in_batches(users) do |user|
create_notification(user, Notification.types[:watching_first_post], post)
end
end
def sync_group_mentions(post, mentioned_groups)
GroupMention.where(post_id: post.id).destroy_all
return if mentioned_groups.blank?
now = Time.zone.now
# insert_all instead of insert_all! since multiple post_alert jobs might be
# running concurrently
GroupMention.insert_all(
mentioned_groups.map do |group|
{
post_id: post.id,
group_id: group.id,
created_at: now,
updated_at: now,
}
end
)
end
def unread_posts(user, topic)
Post.secured(Guardian.new(user))
.where('post_number > COALESCE((
SELECT last_read_post_number FROM topic_users tu
WHERE tu.user_id = ? AND tu.topic_id = ? ),0)',
user.id, topic.id)
.where('reply_to_user_id = :user_id
OR exists(SELECT 1 from topic_users tu
WHERE tu.user_id = :user_id AND
tu.topic_id = :topic_id AND
notification_level = :topic_level)
OR exists(SELECT 1 from category_users cu
WHERE cu.user_id = :user_id AND
cu.category_id = :category_id AND
notification_level = :category_level)
OR exists(SELECT 1 from tag_users tu
WHERE tu.user_id = :user_id AND
tu.tag_id IN (SELECT tag_id FROM topic_tags WHERE topic_id = :topic_id) AND
notification_level = :tag_level)',
user_id: user.id,
topic_id: topic.id,
category_id: topic.category_id,
topic_level: TopicUser.notification_levels[:watching],
category_level: CategoryUser.notification_levels[:watching],
tag_level: TagUser.notification_levels[:watching]
)
.where(topic_id: topic.id)
end
def first_unread_post(user, topic)
unread_posts(user, topic).order('post_number').first
end
def unread_count(user, topic)
unread_posts(user, topic).count
end
def destroy_notifications(user, types, topic)
return if user.blank?
return unless Guardian.new(user).can_see?(topic)
User.transaction do
user.notifications.where(
notification_type: types,
topic_id: topic.id
).destroy_all
# Reload so notification counts sync up correctly
user.reload
end
end
NOTIFIABLE_TYPES = [
:mentioned,
:replied,
:quoted,
:posted,
:linked,
:private_message,
:group_mentioned,
:watching_first_post,
:event_reminder,
:event_invitation
].map { |t| Notification.types[t] }
def group_stats(topic)
sql = <<~SQL
SELECT COUNT(*) FROM topics t
JOIN topic_allowed_groups g ON g.group_id = :group_id AND g.topic_id = t.id
LEFT JOIN group_archived_messages a ON a.topic_id = t.id AND a.group_id = g.group_id
WHERE a.id IS NULL AND t.deleted_at is NULL AND t.archetype = 'private_message'
SQL
topic.allowed_groups.map do |g|
{
group_id: g.id,
group_name: g.name,
inbox_count: DB.query_single(sql, group_id: g.id).first.to_i
}
end
end
def notify_group_summary(user, post)
@group_stats ||= {}
stats = (@group_stats[post.topic_id] ||= group_stats(post.topic))
return unless stats
group_id = post.topic
.topic_allowed_groups
.where(group_id: user.groups.pluck(:id))
.pluck_first(:group_id)
stat = stats.find { |s| s[:group_id] == group_id }
return unless stat && stat[:inbox_count] > 0
notification_type = Notification.types[:group_message_summary]
DistributedMutex.synchronize("group_message_notify_#{user.id}") do
Notification.where(notification_type: notification_type, user_id: user.id).each do |n|
n.destroy if n.data_hash[:group_id] == stat[:group_id]
end
Notification.create(
notification_type: notification_type,
user_id: user.id,
data: {
group_id: stat[:group_id],
group_name: stat[:group_name],
inbox_count: stat[:inbox_count],
username: user.username_lower
}.to_json
)
end
# TODO decide if it makes sense to also publish a desktop notification
end
def should_notify_edit?(notification, post, opts)
notification.created_at < 1.day.ago ||
notification.data_hash["display_username"] != (opts[:display_username].presence || post.user.username)
end
def should_notify_like?(user, notification)
return true if user.user_option.like_notification_frequency == UserOption.like_notification_frequency_type[:always]
return true if user.user_option.like_notification_frequency == UserOption.like_notification_frequency_type[:first_time_and_daily] && notification.created_at < 1.day.ago
false
end
def should_notify_previous?(user, post, notification, opts)
case notification.notification_type
when Notification.types[:edited] then should_notify_edit?(notification, post, opts)
when Notification.types[:liked] then should_notify_like?(user, notification)
else false
end
end
COLLAPSED_NOTIFICATION_TYPES ||= [
Notification.types[:replied],
Notification.types[:posted],
Notification.types[:private_message],
]
def create_notification(user, type, post, opts = {})
opts = @default_opts.merge(opts)
DiscourseEvent.trigger(:before_create_notification, user, type, post, opts)
return if user.blank? || user.bot? || post.blank?
return if (topic = post.topic).blank?
is_liked = type == Notification.types[:liked]
return if is_liked && user.user_option.like_notification_frequency == UserOption.like_notification_frequency_type[:never]
# Make sure the user can see the post
return unless Guardian.new(user).can_see?(post)
return if user.staged? && topic.category&.mailinglist_mirror?
notifier_id = opts[:user_id] || post.user_id # xxxxx look at revision history
# apply muting here
return if notifier_id && MutedUser.where(user_id: user.id, muted_user_id: notifier_id)
.joins(:muted_user)
.where('NOT admin AND NOT moderator')
.exists?
# apply ignored here
return if notifier_id && IgnoredUser.where(user_id: user.id, ignored_user_id: notifier_id)
.joins(:ignored_user)
.where('NOT admin AND NOT moderator')
.exists?
# skip if muted on the topic
return if TopicUser.where(
topic: topic,
user: user,
notification_level: TopicUser.notification_levels[:muted]
).exists?
# skip if muted on the group
if group = opts[:group]
return if GroupUser.where(
group_id: opts[:group_id],
user_id: user.id,
notification_level: TopicUser.notification_levels[:muted]
).exists?
end
existing_notifications = user.notifications
.order("notifications.id DESC")
.where(
topic_id: post.topic_id,
post_number: post.post_number
).limit(10)
# Don't notify the same user about the same type of notification on the same post
existing_notification_of_same_type = existing_notifications.find { |n| n.notification_type == type }
if existing_notification_of_same_type && !should_notify_previous?(user, post, existing_notification_of_same_type, opts)
return
end
# linked, quoted, mentioned may be suppressed if you already have a reply notification
if type == Notification.types[:quoted] || type == Notification.types[:linked] || type == Notification.types[:mentioned]
return if existing_notifications.find { |n| n.notification_type == Notification.types[:replied] }
end
notification_data = {}
if is_liked &&
existing_notification_of_same_type &&
existing_notification_of_same_type.created_at > 1.day.ago &&
(
user.user_option.like_notification_frequency ==
UserOption.like_notification_frequency_type[:always]
)
data = existing_notification_of_same_type.data_hash
notification_data["username2"] = data["display_username"]
notification_data["count"] = (data["count"] || 1).to_i + 1
# don't use destroy so we don't trigger a notification count refresh
Notification.where(id: existing_notification_of_same_type.id).destroy_all
end
collapsed = false
if COLLAPSED_NOTIFICATION_TYPES.include?(type)
destroy_notifications(user, COLLAPSED_NOTIFICATION_TYPES, topic)
collapsed = true
end
original_post = post
original_username = opts[:display_username].presence || post.username
if collapsed
post = first_unread_post(user, topic) || post
count = unread_count(user, topic)
if count > 1
I18n.with_locale(user.effective_locale) do
opts[:display_username] = I18n.t('embed.replies', count: count)
end
end
end
UserActionManager.notification_created(original_post, user, type, opts[:acting_user_id])
topic_title = topic.title
# when sending a private message email, keep the original title
if topic.private_message? && modifications = post.revisions.map(&:modifications)
if first_title_modification = modifications.find { |m| m.has_key?("title") }
topic_title = first_title_modification["title"][0]
end
end
notification_data.merge!(topic_title: topic_title,
original_post_id: original_post.id,
original_post_type: original_post.post_type,
original_username: original_username,
revision_number: opts[:revision_number],
display_username: opts[:display_username] || post.user.username)
if group = opts[:group]
notification_data[:group_id] = group.id
notification_data[:group_name] = group.name
end
if opts[:skip_send_email_to]&.include?(user.email)
skip_send_email = true
elsif original_post.via_email && (incoming_email = original_post.incoming_email)
skip_send_email =
incoming_email.to_addresses_split.include?(user.email) ||
incoming_email.cc_addresses_split.include?(user.email)
else
skip_send_email = opts[:skip_send_email]
end
# Create the notification
created = user.notifications.create!(
notification_type: type,
topic_id: post.topic_id,
post_number: post.post_number,
post_action_id: opts[:post_action_id],
data: notification_data.to_json,
skip_send_email: skip_send_email
)
if created.id && existing_notifications.empty? && NOTIFIABLE_TYPES.include?(type) && !user.suspended?
create_notification_alert(user: user, post: original_post, notification_type: type, username: original_username)
end
created.id ? created : nil
end
def create_notification_alert(user:, post:, notification_type:, excerpt: nil, username: nil)
if post_url = post.url
payload = {
notification_type: notification_type,
post_number: post.post_number,
topic_title: post.topic.title,
topic_id: post.topic.id,
excerpt: excerpt || post.excerpt(400, text_entities: true, strip_links: true, remap_emoji: true),
username: username || post.username,
post_url: post_url
}
DiscourseEvent.trigger(:pre_notification_alert, user, payload)
MessageBus.publish("/notification-alert/#{user.id}", payload, user_ids: [user.id])
push_notification(user, payload)
DiscourseEvent.trigger(:post_notification_alert, user, payload)
end
end
def push_notification(user, payload)
return if user.do_not_disturb?
if user.push_subscriptions.exists?
Jobs.enqueue(:send_push_notification, user_id: user.id, payload: payload)
end
if SiteSetting.allow_user_api_key_scopes.split("|").include?("push") && SiteSetting.allowed_user_api_push_urls.present?
clients = user.user_api_keys
.joins(:scopes)
.where("user_api_key_scopes.name IN ('push', 'notifications')")
.where("push_url IS NOT NULL AND push_url <> ''")
.where("position(push_url IN ?) > 0", SiteSetting.allowed_user_api_push_urls)
.where("revoked_at IS NULL")
.order(client_id: :asc)
.pluck(:client_id, :push_url)
if clients.length > 0
Jobs.enqueue(:push_notification, clients: clients, payload: payload, user_id: user.id)
end
end
end
def expand_group_mentions(groups, post)
return unless post.user && groups
Group.mentionable(post.user, include_public: false).where(id: groups.map(&:id)).each do |group|
next if group.user_count >= SiteSetting.max_users_notified_per_group_mention
yield group, group.users
end
end
# TODO: Move to post-analyzer?
def extract_mentions(post)
mentions = post.raw_mentions
return if mentions.blank?
groups = Group.where('LOWER(name) IN (?)', mentions)
mentions -= groups.map(&:name).map(&:downcase)
groups = nil if groups.empty?
if mentions.present?
users = User.where(username_lower: mentions).where.not(id: post.user_id)
users = nil if users.empty?
end
[groups, users]
end
# TODO: Move to post-analyzer?
# Returns a list of users who were quoted in the post
def extract_quoted_users(post)
post.raw.scan(/\[quote=\"([^,]+),.+\"\]/).uniq.map do |m|
User.find_by("username_lower = :username AND id != :id", username: m.first.strip.downcase, id: post.user_id)
end.compact
end
def extract_linked_users(post)
users = post.topic_links.where(reflection: false).map do |link|
linked_post = link.link_post
if !linked_post && topic = link.link_topic
linked_post = topic.posts.find_by(post_number: 1)
end
(linked_post && post.user_id != linked_post.user_id && linked_post.user) || nil
end.compact
DiscourseEvent.trigger(:after_extract_linked_users, users, post)
users
end
# Notify a bunch of users
def notify_non_pm_users(users, type, post, opts = {})
return [] if post.topic&.private_message?
notify_users(users, type, post, opts)
end
def notify_users(users, type, post, opts = {})
users = [users] unless users.is_a?(Array)
users.reject!(&:staged?) if post.topic&.private_message?
warn_if_not_sidekiq
DiscourseEvent.trigger(:before_create_notifications_for_users, users, post)
users.each do |u|
create_notification(u, Notification.types[type], post, opts)
end
users
end
def notify_pm_users(post, reply_to_user, notified)
return unless post.topic
warn_if_not_sidekiq
# To simplify things and to avoid IMAP double sync issues, and to cut down
# on emails sent via SMTP, any topic_allowed_users (except those who are
# not_allowed?) for a group that has SMTP enabled will have their notification
# email combined into one and sent via a single group SMTP email with CC addresses.
emails_to_skip_send = email_using_group_smtp_if_configured(post)
# We create notifications for all directly_targeted_users and email those
# who do _not_ have their email addresses in the emails_to_skip_send array
# (which will include all topic allowed users' email addresses if group SMTP
# is enabled).
users = directly_targeted_users(post).reject { |u| notified.include?(u) }
DiscourseEvent.trigger(:before_create_notifications_for_users, users, post)
users.each do |user|
notification_level = TopicUser.get(post.topic, user)&.notification_level
if reply_to_user == user || notification_level == TopicUser.notification_levels[:watching] || user.staged?
create_notification(user, Notification.types[:private_message], post, skip_send_email_to: emails_to_skip_send)
end
end
# Users that are part of all mentioned groups. Emails sent by this notification
# flow will not be sent via group SMTP if it is enabled.
users = indirectly_targeted_users(post).reject { |u| notified.include?(u) }
DiscourseEvent.trigger(:before_create_notifications_for_users, users, post)
users.each do |user|
case TopicUser.get(post.topic, user)&.notification_level
when TopicUser.notification_levels[:watching]
# only create a notification when watching the group
create_notification(user, Notification.types[:private_message], post, skip_send_email_to: emails_to_skip_send)
when TopicUser.notification_levels[:tracking]
notify_group_summary(user, post)
end
end
end
def group_notifying_via_smtp(post)
return nil if !SiteSetting.enable_smtp || post.post_type != Post.types[:regular]
post.topic.allowed_groups.where(smtp_enabled: true).first
end
def email_using_group_smtp_if_configured(post)
emails_to_skip_send = []
group = group_notifying_via_smtp(post)
return emails_to_skip_send if group.blank?
to_address = nil
cc_addresses = []
# We need to use topic_allowed_users here instead of directly_targeted_users
# because we want to make sure the to_address goes to the OP of the topic.
topic_allowed_users_by_age = post.topic.topic_allowed_users.includes(:user).order(:created_at).reject do |tau|
not_allowed?(tau.user, post)
end
return emails_to_skip_send if topic_allowed_users_by_age.empty?
# This should usually be the OP of the topic, unless they are the one
# replying by email (they are excluded by not_allowed? then)
to_address = topic_allowed_users_by_age.first.user.email
cc_addresses = topic_allowed_users_by_age[1..-1].map { |tau| tau.user.email }
email_addresses = [to_address, cc_addresses].flatten
# If any of these email addresses were cc address on the
# incoming email for the target post, do not send them emails (they
# already have been notified by the CC on the email)
if post.incoming_email.present?
cc_addresses = cc_addresses - post.incoming_email.cc_addresses_split
# If the to address is one of the recently added CC addresses, then we
# need to bail early, because otherwise we are sending a notification
# email to the user who was just added by CC. In this case the OP probably
# replied and CC'd some people, and they are the only other topic users.
return if post.incoming_email.cc_addresses_split.include?(to_address)
end
# Send a single email using group SMTP settings to cut down on the
# number of emails sent via SMTP, also to replicate how support systems
# and group inboxes generally work in other systems.
Jobs.enqueue(
:group_smtp_email,
group_id: group.id,
post_id: post.id,
email: to_address,
cc_emails: cc_addresses
)
# Add the group's email_username into the array, because it is used for
# skip_send_email_to in the case of user private message notifications
# (we do not want the group to be sent any emails from here because it
# will make another email for IMAP to pick up in the group's mailbox)
emails_to_skip_send = email_addresses.dup if email_addresses.any?
emails_to_skip_send << group.email_username
emails_to_skip_send.uniq
end
def notify_post_users(post, notified, group_ids: nil, include_topic_watchers: true, include_category_watchers: true, include_tag_watchers: true, new_record: false)
return unless post.topic
warn_if_not_sidekiq
condition = +<<~SQL
users.id IN (
SELECT id FROM users WHERE false
/*topic*/
/*category*/
/*tags*/
)
SQL
if include_topic_watchers
condition.sub! "/*topic*/", <<~SQL
UNION
SELECT user_id
FROM topic_users
WHERE notification_level = :watching
AND topic_id = :topic_id
SQL
end
if include_category_watchers
condition.sub! "/*category*/", <<~SQL
UNION
SELECT cu.user_id
FROM category_users cu
LEFT JOIN topic_users tu ON tu.user_id = cu.user_id
AND tu.topic_id = :topic_id
WHERE cu.notification_level = :watching
AND cu.category_id = :category_id
AND (tu.user_id IS NULL OR tu.notification_level = :watching)
SQL
end
tag_ids = post.topic.topic_tags.pluck('topic_tags.tag_id')
if include_tag_watchers && tag_ids.present?
condition.sub! "/*tags*/", <<~SQL
UNION
SELECT tag_users.user_id
FROM tag_users
LEFT JOIN topic_users tu ON tu.user_id = tag_users.user_id
AND tu.topic_id = :topic_id
WHERE tag_users.notification_level = :watching
AND tag_users.tag_id IN (:tag_ids)
AND (tu.user_id IS NULL OR tu.notification_level = :watching)
SQL
end
notify = User.where(condition,
watching: TopicUser.notification_levels[:watching],
topic_id: post.topic_id,
category_id: post.topic.category_id,
tag_ids: tag_ids
)
if group_ids.present?
notify = notify.joins(:group_users).where("group_users.group_id IN (?)", group_ids)
end
if post.topic.private_message?
notify = notify.where(staged: false).staff
end
exclude_user_ids = notified.map(&:id)
notify = notify.where("users.id NOT IN (?)", exclude_user_ids) if exclude_user_ids.present?
DiscourseEvent.trigger(:before_create_notifications_for_users, notify, post)
already_seen_user_ids = Set.new TopicUser.where(topic_id: post.topic.id).where("highest_seen_post_number >= ?", post.post_number).pluck(:user_id)
each_user_in_batches(notify) do |user|
notification_type = !new_record && already_seen_user_ids.include?(user.id) ? Notification.types[:edited] : Notification.types[:posted]
opts = {}
opts[:display_username] = post.last_editor.username if notification_type == Notification.types[:edited]
create_notification(user, notification_type, post, opts)
end
end
def warn_if_not_sidekiq
Rails.logger.warn("PostAlerter.#{caller_locations(1, 1)[0].label} was called outside of sidekiq") unless Sidekiq.server?
end
private
def each_user_in_batches(users)
# This is race-condition-safe, unlike #find_in_batches
users.pluck(:id).each_slice(USER_BATCH_SIZE) do |user_ids_batch|
User.where(id: user_ids_batch).each { |user| yield(user) }
end
end
end