Change e-mail notifications to only be sent when recipient is offline (#17984)

* Change e-mail notifications to only be sent when recipient is offline

Change the default for follow and mention notifications back on

* Add preference to always send e-mail notifications

* Change wording

5 files changed, 53 insertions, 23 deletions

diff --git a/app/controllers/settings/preferences_controller.rb b/app/controllers/settings/preferences_controller.rb
index c7492700c..bfe651bc6 100644
--- a/app/controllers/settings/preferences_controller.rb
+++ b/app/controllers/settings/preferences_controller.rb
@@ -54,7 +54,8 @@ class Settings::PreferencesController < Settings::BaseController
       :setting_use_pending_items,
       :setting_trends,
       :setting_crop_images,
-      notification_emails: %i(follow follow_request reblog favourite mention digest report pending_account trending_tag),
+      :setting_always_send_emails,
+      notification_emails: %i(follow follow_request reblog favourite mention digest report pending_account trending_tag appeal),
       interactions: %i(must_be_follower must_be_following must_be_following_dm)
     )
   end
diff --git a/app/lib/user_settings_decorator.rb b/app/lib/user_settings_decorator.rb
index de054e403..5fb7655a9 100644
--- a/app/lib/user_settings_decorator.rb
+++ b/app/lib/user_settings_decorator.rb
@@ -38,6 +38,7 @@ class UserSettingsDecorator
     user.settings['use_pending_items']   = use_pending_items_preference if change?('setting_use_pending_items')
     user.settings['trends']              = trends_preference if change?('setting_trends')
     user.settings['crop_images']         = crop_images_preference if change?('setting_crop_images')
+    user.settings['always_send_emails']  = always_send_emails_preference if change?('setting_always_send_emails')
   end
 
   def merged_notification_emails
@@ -132,6 +133,10 @@ class UserSettingsDecorator
     boolean_cast_setting 'setting_crop_images'
   end
 
+  def always_send_emails_preference
+    boolean_cast_setting 'setting_always_send_emails'
+  end
+
   def boolean_cast_setting(key)
     ActiveModel::Type::Boolean.new.cast(settings[key])
   end
diff --git a/app/models/user.rb b/app/models/user.rb
index d19fe2c92..9da7b2639 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -130,7 +130,7 @@ class User < ApplicationRecord
            :reduce_motion, :system_font_ui, :noindex, :theme, :display_media,
            :expand_spoilers, :default_language, :aggregate_reblogs, :show_application,
            :advanced_layout, :use_blurhash, :use_pending_items, :trends, :crop_images,
-           :disable_swiping,
+           :disable_swiping, :always_send_emails,
            to: :settings, prefix: :setting, allow_nil: false
 
   attr_reader :invite_code
diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb
index a90f17cfd..d30b33876 100644
--- a/app/services/notify_service.rb
+++ b/app/services/notify_service.rb
@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class NotifyService < BaseService
+  include Redisable
+
   def call(recipient, type, activity)
     @recipient    = recipient
     @activity     = activity
@@ -8,10 +10,15 @@ class NotifyService < BaseService
 
     return if recipient.user.nil? || blocked?
 
-    create_notification!
+    @notification.save!
+
+    # It's possible the underlying activity has been deleted
+    # between the save call and now
+    return if @notification.activity.nil?
+
     push_notification!
     push_to_conversation! if direct_message?
-    send_email! if email_enabled?
+    send_email! if email_needed?
   rescue ActiveRecord::RecordInvalid
     nil
   end
@@ -92,8 +99,8 @@ class NotifyService < BaseService
   end
 
   def blocked?
-    blocked   = @recipient.suspended?                            # Skip if the recipient account is suspended anyway
-    blocked ||= from_self? && @notification.type != :poll        # Skip for interactions with self
+    blocked   = @recipient.suspended?
+    blocked ||= from_self? && @notification.type != :poll
 
     return blocked if message? && from_staff?
 
@@ -117,38 +124,52 @@ class NotifyService < BaseService
     end
   end
 
-  def create_notification!
-    @notification.save!
+  def push_notification!
+    push_to_streaming_api! if subscribed_to_streaming_api?
+    push_to_web_push_subscriptions!
   end
 
-  def push_notification!
-    return if @notification.activity.nil?
+  def push_to_streaming_api!
+    redis.publish("timeline:#{@recipient.id}:notifications", Oj.dump(event: :notification, payload: InlineRenderer.render(@notification, @recipient, :notification)))
+  end
 
-    Redis.current.publish("timeline:#{@recipient.id}:notifications", Oj.dump(event: :notification, payload: InlineRenderer.render(@notification, @recipient, :notification)))
-    send_push_notifications!
+  def subscribed_to_streaming_api?
+    redis.exists?("subscribed:timeline:#{@recipient.id}") || redis.exists?("subscribed:timeline:#{@recipient.id}:notifications")
   end
 
   def push_to_conversation!
-    return if @notification.activity.nil?
     AccountConversation.add_status(@recipient, @notification.target_status)
   end
 
-  def send_push_notifications!
-    subscriptions_ids = ::Web::PushSubscription.where(user_id: @recipient.user.id)
-                                               .select { |subscription| subscription.pushable?(@notification) }
-                                               .map(&:id)
+  def push_to_web_push_subscriptions!
+    ::Web::PushNotificationWorker.push_bulk(web_push_subscriptions.select { |subscription| subscription.pushable?(@notification) }) { |subscription| [subscription.id, @notification.id] }
+  end
 
-    ::Web::PushNotificationWorker.push_bulk(subscriptions_ids) do |subscription_id|
-      [subscription_id, @notification.id]
-    end
+  def web_push_subscriptions
+    @web_push_subscriptions ||= ::Web::PushSubscription.where(user_id: @recipient.user.id).to_a
+  end
+
+  def subscribed_to_web_push?
+    web_push_subscriptions.any?
   end
 
   def send_email!
-    return if @notification.activity.nil?
-    NotificationMailer.public_send(@notification.type, @recipient, @notification).deliver_later(wait: 2.minutes)
+    NotificationMailer.public_send(@notification.type, @recipient, @notification).deliver_later(wait: 2.minutes) if NotificationMailer.respond_to?(@notification.type)
+  end
+
+  def email_needed?
+    (!recipient_online? || always_send_emails?) && send_email_for_notification_type?
+  end
+
+  def recipient_online?
+    subscribed_to_streaming_api? || subscribed_to_web_push?
+  end
+
+  def always_send_emails?
+    @recipient.user.settings.always_send_emails
   end
 
-  def email_enabled?
+  def send_email_for_notification_type?
     @recipient.user.settings.notification_emails[@notification.type.to_s]
   end
 end
diff --git a/app/views/settings/preferences/notifications/show.html.haml b/app/views/settings/preferences/notifications/show.html.haml
index 223e5d740..42754a852 100644
--- a/app/views/settings/preferences/notifications/show.html.haml
+++ b/app/views/settings/preferences/notifications/show.html.haml
@@ -26,6 +26,9 @@
         = ff.input :trending_tag, as: :boolean, wrapper: :with_label
 
   .fields-group
+    = f.input :setting_always_send_emails, as: :boolean, wrapper: :with_label
+
+  .fields-group
     = f.simple_fields_for :notification_emails, hash_to_object(current_user.settings.notification_emails) do |ff|
       = ff.input :digest, as: :boolean, wrapper: :with_label