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

diff --git a/app/controllers/settings/preferences_controller.rb b/app/controllers/settings/preferences_controller.rb
index c7492700cd6b7ba6a45e69954debbe8627d74a63..bfe651bc66755453317c43c24c65514dfdcba9ed 100644 (file)
--- 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 de054e403eeaa72d38e470913e15f253c950f03a..5fb7655a9b4837e48462a0fa70d1ec4903fd2a98 100644 (file)
--- 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 d19fe2c92ce223b3e102875fdff3d44a87bb1805..9da7b2639eed4a7629e5bf2c497c75b213fe7238 100644 (file)
--- 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 a90f17cfdde2ffcc71e1420591a5f4daa36e1251..d30b338765b9f400dfc5b952230eb9537d190dca 100644 (file)
--- 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 223e5d7407a5b21cc6cd91f031167f60e48a84bb..42754a85209a7ba4141241333908b097cd8c09f0 100644 (file)
--- a/app/views/settings/preferences/notifications/show.html.haml
+++ b/app/views/settings/preferences/notifications/show.html.haml
@@ -25,6 +25,9 @@
         = ff.input :pending_account, as: :boolean, wrapper: :with_label
         = 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

diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml
index b19b7891fda4c0993752e4e959d5ef56fc8fe80c..b784b1da7b5d1f78328b5446273c9824f25dd314 100644 (file)
--- a/config/locales/simple_form.en.yml
+++ b/config/locales/simple_form.en.yml
@@ -49,6 +49,7 @@ en:
         phrase: Will be matched regardless of casing in text or content warning of a post
         scopes: Which APIs the application will be allowed to access. If you select a top-level scope, you don't need to select individual ones.
         setting_aggregate_reblogs: Do not show new boosts for posts that have been recently boosted (only affects newly-received boosts)
+        setting_always_send_emails: Normally e-mail notifications won't be sent when you are actively using Mastodon
         setting_default_sensitive: Sensitive media is hidden by default and can be revealed with a click
         setting_display_media_default: Hide media marked as sensitive
         setting_display_media_hide_all: Always hide media
@@ -151,6 +152,7 @@ en:
         phrase: Keyword or phrase
         setting_advanced_layout: Enable advanced web interface
         setting_aggregate_reblogs: Group boosts in timelines
+        setting_always_send_emails: Always send e-mail notifications
         setting_auto_play_gif: Auto-play animated GIFs
         setting_boost_modal: Show confirmation dialog before boosting
         setting_crop_images: Crop images in non-expanded posts to 16x9

diff --git a/config/settings.yml b/config/settings.yml
index 06dd2b3f3e3a7172356220f6aa0023b442841059..eaa05071e112ff4d87a51a767674aeaa68d6c552 100644 (file)
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -38,16 +38,17 @@ defaults: &defaults
   trendable_by_default: false
   crop_images: true
   notification_emails:
-    follow: false
+    follow: true
     reblog: false
     favourite: false
-    mention: false
+    mention: true
     follow_request: true
     digest: true
     report: true
     pending_account: true
     trending_tag: true
     appeal: true
+  always_send_emails: false
   interactions:
     must_be_follower: false
     must_be_following: false