From 429cf656e78ae1858c29a446cccd386542ae2867 Mon Sep 17 00:00:00 2001
From: Bianca Nenciu <nbianca@users.noreply.github.com>
Date: Thu, 21 Nov 2024 05:11:51 +0200
Subject: [PATCH] FIX: Use FinalDestination::HTTP to push notifications
 (#29858)

Sometimes `Jobs::PushNotification` gets stuck, probably because of the
network call. This commit replaces `Excon` with `FinalDestination::HTTP`
which is safer.
---
 app/jobs/regular/push_notification.rb | 27 +++++++++++----------
 spec/jobs/push_notification_spec.rb   | 34 ++++++++++++++++++++++-----
 2 files changed, 43 insertions(+), 18 deletions(-)

diff --git a/app/jobs/regular/push_notification.rb b/app/jobs/regular/push_notification.rb
index 8db0c47f345..2494927cfb3 100644
--- a/app/jobs/regular/push_notification.rb
+++ b/app/jobs/regular/push_notification.rb
@@ -28,19 +28,22 @@ module Jobs
 
           next if push_url.blank?
 
-          begin
-            result =
-              Excon.post(
-                push_url,
-                body: payload.merge(notifications: notifications).to_json,
-                headers: {
-                  "Content-Type" => "application/json",
-                  "Accept" => "application/json",
-                },
-              )
+          uri = URI.parse(push_url)
 
-            if result.status != 200
-              # we failed to push a notification ... log it
+          http = FinalDestination::HTTP.new(uri.host, uri.port)
+          http.use_ssl = true
+
+          request =
+            FinalDestination::HTTP::Post.new(
+              uri.request_uri,
+              { "Content-Type" => "application/json" },
+            )
+          request.body = payload.merge(notifications: notifications).to_json
+
+          begin
+            response = http.request(request)
+
+            if response.code.to_i != 200
               Rails.logger.warn(
                 "Failed to push a notification to #{push_url} Status: #{result.status}: #{result.status_line}",
               )
diff --git a/spec/jobs/push_notification_spec.rb b/spec/jobs/push_notification_spec.rb
index e081fd677c5..a1f1648c8b6 100644
--- a/spec/jobs/push_notification_spec.rb
+++ b/spec/jobs/push_notification_spec.rb
@@ -17,23 +17,45 @@ RSpec.describe Jobs::PushNotification do
     }
   end
 
+  let!(:request) do
+    stub_request(:post, "https://test.localhost:80/").with(
+      body: {
+        "secret_key" => SiteSetting.push_api_secret_key,
+        "url" => "http://test.localhost",
+        "title" => "Discourse",
+        "description" => "",
+        "notifications" => [
+          {
+            "notification_type" => 1,
+            "excerpt" => "Hello you",
+            "url" => "http://test.localhost/t/#{post.topic_id}/#{post.post_number}",
+            "client_id" => user.id,
+          },
+        ],
+      }.to_json,
+      headers: {
+        "Content-Type" => "application/json",
+      },
+    ).to_return(status: 200, body: "", headers: {})
+  end
+
   before { SiteSetting.push_notification_time_window_mins = 5 }
 
   context "with valid user" do
     it "does not send push notification when user is online" do
       user.update!(last_seen_at: 1.minute.ago)
 
-      Excon.expects(:post).never
-
       Jobs::PushNotification.new.execute(data)
+
+      expect(request).not_to have_been_requested
     end
 
     it "sends push notification when user is offline" do
       user.update!(last_seen_at: 10.minutes.ago)
 
-      Excon.expects(:post).once
-
       Jobs::PushNotification.new.execute(data)
+
+      expect(request).to have_been_requested.once
     end
   end
 
@@ -41,9 +63,9 @@ RSpec.describe Jobs::PushNotification do
     it "does not send push notification" do
       data["user_id"] = -999
 
-      Excon.expects(:post).never
-
       Jobs::PushNotification.new.execute(data)
+
+      expect(request).not_to have_been_requested
     end
   end
 end