From 8b1a033846765d85abf2a542df6d8fdc1d2ea700 Mon Sep 17 00:00:00 2001 From: Kunthawat Greethong Date: Sat, 15 Aug 2026 16:04:25 +0700 Subject: [PATCH] [verified] Remove Chatwoot Hub push relay --- .env.example | 5 +- .../notification/push_notification_service.rb | 19 ++--- .../notification/push_test_service.rb | 23 +----- .../push_notification_service_spec.rb | 32 ++++++-- .../notification/push_test_service_spec.rb | 77 +++++++++++++++++++ 5 files changed, 112 insertions(+), 44 deletions(-) create mode 100644 spec/services/notification/push_test_service_spec.rb diff --git a/.env.example b/.env.example index 8f09c9218..d106405e6 100644 --- a/.env.example +++ b/.env.example @@ -279,9 +279,8 @@ ANDROID_SHA256_CERT_FINGERPRINT=AC:73:8E:DE:EB:56:EA:CC:10:87:02:A7:65:37:7B:38: # meant to be used in github codespaces # WEBPACKER_DEV_SERVER_PUBLIC= -# If you want to use official mobile app, -# the notifications would be relayed via a Chatwoot server -ENABLE_PUSH_RELAY_SERVER=true +# Configure organization-owned VAPID/FCM credentials above for push delivery. +# Leave them unset to disable push notifications. # Stripe API key STRIPE_SECRET_KEY= diff --git a/app/services/notification/push_notification_service.rb b/app/services/notification/push_notification_service.rb index 90f835ecb..c230e5b89 100644 --- a/app/services/notification/push_notification_service.rb +++ b/app/services/notification/push_notification_service.rb @@ -9,7 +9,6 @@ class Notification::PushNotificationService notification_subscriptions.each do |subscription| send_browser_push(subscription) send_fcm_push(subscription) - send_push_via_chatwoot_hub(subscription) end end @@ -88,9 +87,13 @@ class Notification::PushNotificationService end def send_fcm_push(subscription) - return unless firebase_credentials_present? return unless subscription.fcm? + unless firebase_credentials_present? + Rails.logger.warn('FCM push skipped: Firebase credentials are not configured; no remote push relay is used') + return + end + fcm_service = Notification::FcmService.new( GlobalConfigService.load('FIREBASE_PROJECT_ID', nil), GlobalConfigService.load('FIREBASE_CREDENTIALS', nil) ) @@ -99,22 +102,10 @@ class Notification::PushNotificationService remove_subscription_if_error(subscription, response) end - def send_push_via_chatwoot_hub(subscription) - return if firebase_credentials_present? - return unless chatwoot_hub_enabled? - return unless subscription.fcm? - - ChatwootHub.send_push(fcm_options(subscription)) - end - def firebase_credentials_present? GlobalConfigService.load('FIREBASE_PROJECT_ID', nil) && GlobalConfigService.load('FIREBASE_CREDENTIALS', nil) end - def chatwoot_hub_enabled? - ActiveModel::Type::Boolean.new.cast(ENV.fetch('ENABLE_PUSH_RELAY_SERVER', true)) - end - def remove_subscription_if_error(subscription, response) if JSON.parse(response[:body])['results']&.first&.keys&.include?('error') subscription.destroy! diff --git a/app/services/notification/push_test_service.rb b/app/services/notification/push_test_service.rb index 4b2bec177..ba106783a 100644 --- a/app/services/notification/push_test_service.rb +++ b/app/services/notification/push_test_service.rb @@ -50,13 +50,11 @@ class Notification::PushTestService end def test_fcm(subscription) - if firebase_credentials_present? - test_fcm_direct(subscription) - elsif chatwoot_hub_enabled? - test_fcm_via_hub(subscription) - else - result(subscription, 'fcm', :skipped, 'No Firebase credentials and push relay disabled') + unless firebase_credentials_present? + return result(subscription, 'fcm', :skipped, 'Firebase credentials are not configured; no remote push relay is used') end + + test_fcm_direct(subscription) end def test_fcm_direct(subscription) @@ -72,23 +70,10 @@ class Notification::PushTestService result(subscription, 'fcm', :failure, "#{e.class.name}: #{e.message}") end - def test_fcm_via_hub(subscription) - response = ChatwootHub.send_push_with_response(fcm_options(subscription)) - result(subscription, 'fcm_via_hub', :success, "HTTP #{response.code} — #{response.body}") - rescue RestClient::ExceptionWithResponse => e - result(subscription, 'fcm_via_hub', :failure, "HTTP #{e.response&.code} — #{e.response&.body}") - rescue StandardError => e - result(subscription, 'fcm_via_hub', :failure, "#{e.class.name}: #{e.message}") - end - def firebase_credentials_present? GlobalConfigService.load('FIREBASE_PROJECT_ID', nil) && GlobalConfigService.load('FIREBASE_CREDENTIALS', nil) end - def chatwoot_hub_enabled? - ActiveModel::Type::Boolean.new.cast(ENV.fetch('ENABLE_PUSH_RELAY_SERVER', true)) - end - def browser_push_payload(subscription) { message: JSON.generate( diff --git a/spec/services/notification/push_notification_service_spec.rb b/spec/services/notification/push_notification_service_spec.rb index bba912715..9f3e3eb5a 100644 --- a/spec/services/notification/push_notification_service_spec.rb +++ b/spec/services/notification/push_notification_service_spec.rb @@ -30,19 +30,35 @@ describe Notification::PushNotificationService do end it 'sends a fcm notification for firebase subscription' do - with_modified_env ENABLE_PUSH_RELAY_SERVER: 'false' do - create(:notification_subscription, user: notification.user, subscription_type: 'fcm') + create(:notification_subscription, user: notification.user, subscription_type: 'fcm') - described_class.new(notification: notification).perform - expect(Notification::FcmService).to have_received(:new) - expect(fcm_double).to have_received(:send_v1) - expect(WebPush).not_to have_received(:payload_send) - expect(Rails.logger).to have_received(:info).with("FCM push sent to #{user.email} with title #{notification.push_message_title}") - end + described_class.new(notification: notification).perform + expect(Notification::FcmService).to have_received(:new) + expect(fcm_double).to have_received(:send_v1) + expect(WebPush).not_to have_received(:payload_send) + expect(Rails.logger).to have_received(:info).with("FCM push sent to #{user.email} with title #{notification.push_message_title}") end end end + context 'when Firebase credentials are not configured' do + before do + allow(Rails.logger).to receive(:warn) + allow(GlobalConfigService).to receive(:load).with('FIREBASE_PROJECT_ID', nil).and_return(nil) + allow(GlobalConfigService).to receive(:load).with('FIREBASE_CREDENTIALS', nil).and_return(nil) + end + + it 'skips FCM without using the Chatwoot Hub relay' do + create(:notification_subscription, user: notification.user, subscription_type: 'fcm') + + described_class.new(notification: notification).perform + + expect(Rails.logger).to have_received(:warn).with( + 'FCM push skipped: Firebase credentials are not configured; no remote push relay is used' + ) + end + end + context 'when the push server returns error' do it 'sends webpush notifications for webpush subscription' do with_modified_env VAPID_PUBLIC_KEY: 'test' do diff --git a/spec/services/notification/push_test_service_spec.rb b/spec/services/notification/push_test_service_spec.rb new file mode 100644 index 000000000..efcf0a66e --- /dev/null +++ b/spec/services/notification/push_test_service_spec.rb @@ -0,0 +1,77 @@ +require 'rails_helper' + +describe Notification::PushTestService do + let!(:user) { create(:user) } + let!(:subscription) do + create( + :notification_subscription, + :fcm, + user: user, + identifier: 'fcm-test', + subscription_attributes: { 'push_token' => 'test-token', 'device_id' => 'device-123456' } + ) + end + let(:fcm_double) { instance_double(FCM) } + let(:fcm_service_double) { instance_double(Notification::FcmService, fcm_client: fcm_double) } + + describe '#perform' do + context 'when Firebase credentials are not configured' do + before do + allow(GlobalConfigService).to receive(:load).and_return(nil) + allow(GlobalConfigService).to receive(:load).with('FIREBASE_PROJECT_ID', nil).and_return(nil) + allow(GlobalConfigService).to receive(:load).with('FIREBASE_CREDENTIALS', nil).and_return(nil) + end + + it 'skips FCM without using the Chatwoot Hub relay' do + result = described_class.new(user: user, subscription_ids: [subscription.id]).perform.first + + expect(result).to include( + type: 'fcm', + status: :skipped, + message: 'Firebase credentials are not configured; no remote push relay is used' + ) + end + end + + context 'when Firebase credentials are configured' do + before do + allow(GlobalConfigService).to receive(:load).and_return(nil) + allow(Notification::FcmService).to receive(:new).and_return(fcm_service_double) + allow(fcm_double).to receive(:send_v1).and_return(status_code: 200, body: '{}') + allow(GlobalConfigService).to receive(:load).with('FIREBASE_PROJECT_ID', nil).and_return('test-project') + allow(GlobalConfigService).to receive(:load).with('FIREBASE_CREDENTIALS', nil).and_return('test-credentials') + end + + it 'sends FCM directly without using the Chatwoot Hub relay' do + result = described_class.new(user: user, subscription_ids: [subscription.id]).perform.first + + expect(Notification::FcmService).to have_received(:new) + expect(fcm_double).to have_received(:send_v1) + expect(result[:status]).to eq(:success) + end + end + + it 'keeps browser WebPush available' do + browser_subscription = create( + :notification_subscription, + :browser_push, + user: user, + identifier: 'browser-test', + subscription_attributes: { + 'endpoint' => 'https://push.example.test/endpoint', + 'p256dh' => 'test-p256dh', + 'auth' => 'test-auth' + } + ) + allow(VapidService).to receive(:public_key).and_return('test-public-key') + allow(VapidService).to receive(:private_key).and_return('test-private-key') + allow(WebPush).to receive(:payload_send).and_return(true) + + result = described_class.new(user: user, subscription_ids: [browser_subscription.id]).perform.first + + expect(WebPush).to have_received(:payload_send) + expect(result[:type]).to eq('browser_push') + expect(result[:status]).to eq(:success) + end + end +end