From 3d4ae604b8e66d98d0222a05931d6c0e04e04b12 Mon Sep 17 00:00:00 2001
From: Sorin Davidoi <sorin.davidoi@gmail.com>
Date: Tue, 18 Jul 2017 16:25:40 +0200
Subject: [PATCH] fix(push-subscriptions): Refactor how Sidekiq jobs are
 handled (#4226)

---
 app/models/web/push_subscription.rb         |  8 +++++---
 app/services/notify_service.rb              |  6 +++++-
 app/workers/web_push_notification_worker.rb | 22 +++++++++------------
 3 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/app/models/web/push_subscription.rb b/app/models/web/push_subscription.rb
index 4440706a69..baf6a1ece1 100644
--- a/app/models/web/push_subscription.rb
+++ b/app/models/web/push_subscription.rb
@@ -12,6 +12,9 @@
 #  updated_at :datetime         not null
 #
 
+require 'webpush'
+require_relative '../../models/setting'
+
 class Web::PushSubscription < ApplicationRecord
   include RoutingHelper
   include StreamEntriesHelper
@@ -37,7 +40,6 @@ class Web::PushSubscription < ApplicationRecord
     nsfw = notification.target_status.nil? || notification.target_status.spoiler_text.empty? ? nil : notification.target_status.spoiler_text
 
     # TODO: Make sure that the payload does not exceed 4KB - Webpush::PayloadTooLarge
-    # TODO: Queue the requests - Webpush::TooManyRequests
     Webpush.payload_send(
       message: JSON.generate(
         title: title,
@@ -59,7 +61,7 @@ class Web::PushSubscription < ApplicationRecord
       p256dh: key_p256dh,
       auth: key_auth,
       vapid: {
-        # subject: "mailto:#{Setting.site_contact_email}",
+        subject: "mailto:#{Setting.site_contact_email}",
         private_key: Rails.configuration.x.vapid_private_key,
         public_key: Rails.configuration.x.vapid_public_key,
       },
@@ -166,7 +168,7 @@ class Web::PushSubscription < ApplicationRecord
       p256dh: key_p256dh,
       auth: key_auth,
       vapid: {
-        # subject: "mailto:#{Setting.site_contact_email}",
+        subject: "mailto:#{Setting.site_contact_email}",
         private_key: Rails.configuration.x.vapid_private_key,
         public_key: Rails.configuration.x.vapid_public_key,
       },
diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb
index 0ab61b634c..c7d8ad50a5 100644
--- a/app/services/notify_service.rb
+++ b/app/services/notify_service.rb
@@ -65,7 +65,11 @@ class NotifyService < BaseService
   end
 
   def send_push_notifications
-    WebPushNotificationWorker.perform_async(@recipient.id, @notification.id)
+    sessions_with_subscriptions_ids = @recipient.user.session_activations.where.not(web_push_subscription: nil).pluck(:id)
+
+    WebPushNotificationWorker.push_bulk(sessions_with_subscriptions_ids) do |session_activation_id|
+      [session_activation_id, @notification.id]
+    end
   end
 
   def send_email
diff --git a/app/workers/web_push_notification_worker.rb b/app/workers/web_push_notification_worker.rb
index e8f1d72bd4..da4043ddbd 100644
--- a/app/workers/web_push_notification_worker.rb
+++ b/app/workers/web_push_notification_worker.rb
@@ -5,22 +5,18 @@ class WebPushNotificationWorker
 
   sidekiq_options backtrace: true
 
-  def perform(recipient_id, notification_id)
-    recipient = Account.find(recipient_id)
+  def perform(session_activation_id, notification_id)
+    session_activation = SessionActivation.find(session_activation_id)
     notification = Notification.find(notification_id)
 
-    sessions_with_subscriptions = recipient.user.session_activations.where.not(web_push_subscription: nil)
+    begin
+      session_activation.web_push_subscription.push(notification)
+    rescue Webpush::InvalidSubscription, Webpush::ExpiredSubscription => e
+      # Subscription expiration is not currently implemented in any browser
+      session_activation.web_push_subscription.destroy!
+      session_activation.update!(web_push_subscription: nil)
 
-    sessions_with_subscriptions.each do |session|
-      begin
-        session.web_push_subscription.push(notification)
-      rescue Webpush::InvalidSubscription, Webpush::ExpiredSubscription
-        # Subscription expiration is not currently implemented in any browser
-        session.web_push_subscription.destroy!
-        session.update!(web_push_subscription: nil)
-      rescue Webpush::PayloadTooLarge => e
-        Rails.logger.error(e)
-      end
+      raise e
     end
   end
 end