From 2eb2f273a86109706271e3b7c98a9d2cdd9669f7 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Wed, 9 May 2018 08:14:14 +0800
Subject: [PATCH] Refactor of `PushSubscriptionPusher`.

---
 app/services/push_notification_pusher.rb      | 16 +++++----
 .../push_notification_controller_spec.rb      | 36 +++++++------------
 2 files changed, 23 insertions(+), 29 deletions(-)

diff --git a/app/services/push_notification_pusher.rb b/app/services/push_notification_pusher.rb
index cc7a9cb8978..204e3f668be 100644
--- a/app/services/push_notification_pusher.rb
+++ b/app/services/push_notification_pusher.rb
@@ -33,13 +33,17 @@ class PushNotificationPusher
   end
 
   def self.subscribe(user, subscription, send_confirmation)
-    subscriptions = PushSubscription.where(user: user, data: subscription.to_json)
-    if subscriptions.length > 1
+    data = subscription.to_json
+    subscriptions = PushSubscription.where(user: user, data: data)
+    subscriptions_count = subscriptions.count
+
+    if subscriptions_count > 1
       subscriptions.destroy_all
-      PushSubscription.create user: user, data: subscription.to_json
-    elsif subscriptions.length == 0
-      PushSubscription.create user: user, data: subscription.to_json
+      PushSubscription.create!(user: user, data: data)
+    elsif subscriptions_count == 0
+      PushSubscription.create!(user: user, data: data)
     end
+
     if send_confirmation == "true"
       message = {
         title: I18n.t("discourse_push_notifications.popup.confirm_title",
@@ -55,7 +59,7 @@ class PushNotificationPusher
   end
 
   def self.unsubscribe(user, subscription)
-    PushSubscription.find_by(user: user, data: subscription.to_json)&.destroy
+    PushSubscription.find_by(user: user, data: subscription.to_json)&.destroy!
   end
 
   protected
diff --git a/spec/requests/push_notification_controller_spec.rb b/spec/requests/push_notification_controller_spec.rb
index 5c0f9dc03cb..42f12e3a073 100644
--- a/spec/requests/push_notification_controller_spec.rb
+++ b/spec/requests/push_notification_controller_spec.rb
@@ -61,29 +61,19 @@ describe PushNotificationController do
     end
 
     it "should not create duplicate subscriptions" do
-      post '/push_notifications/subscribe.json', params: {
-             username: user.username,
-             subscription: {
-               endpoint: "endpoint",
-               keys: {
-                 p256dh: "256dh",
-                 auth: "auth"
-               }
-             },
-             send_confirmation: false
-           }
-
-      post '/push_notifications/subscribe.json', params: {
-             username: user.username,
-             subscription: {
-               endpoint: "endpoint",
-               keys: {
-                 p256dh: "256dh",
-                 auth: "auth"
-               }
-             },
-             send_confirmation: false
-           }
+      2.times do
+        post '/push_notifications/subscribe.json', params: {
+           username: user.username,
+           subscription: {
+             endpoint: "endpoint",
+             keys: {
+               p256dh: "256dh",
+               auth: "auth"
+             }
+           },
+           send_confirmation: false
+         }
+      end
 
       expect(response.status).to eq(200)
       expect(user.push_subscriptions.count).to eq(1)