diff --git a/app/services/notification/push_notification_service.rb b/app/services/notification/push_notification_service.rb index 3c7ed52e7..263800d4a 100644 --- a/app/services/notification/push_notification_service.rb +++ b/app/services/notification/push_notification_service.rb @@ -42,14 +42,12 @@ class Notification::PushNotificationService app_account_conversation_url(account_id: conversation.account_id, id: conversation.display_id) end - def send_browser_push?(subscription) + def can_send_browser_push?(subscription) VapidService.public_key && subscription.browser_push? end - def send_browser_push(subscription) - return unless send_browser_push?(subscription) - - WebPush.payload_send( + def browser_push_payload(subscription) + { message: JSON.generate(push_message), endpoint: subscription.subscription_attributes['endpoint'], p256dh: subscription.subscription_attributes['p256dh'], @@ -62,11 +60,22 @@ class Notification::PushNotificationService ssl_timeout: 5, open_timeout: 5, read_timeout: 5 - ) - rescue WebPush::ExpiredSubscription + } + end + + def send_browser_push(subscription) + return unless can_send_browser_push?(subscription) + + WebPush.payload_send(browser_push_payload(subscription)) + Rails.logger.info("Browser push sent to #{user.email} with title #{push_message[:title]}") + rescue WebPush::ExpiredSubscription, WebPush::InvalidSubscription, WebPush::Unauthorized => e + Rails.logger.info "WebPush subscription expired: #{e.message}" subscription.destroy! rescue Errno::ECONNRESET, Net::OpenTimeout, Net::ReadTimeout => e Rails.logger.error "WebPush operation error: #{e.message}" + rescue StandardError => e + ChatwootExceptionTracker.new(e, account: notification.account).capture_exception + true end def send_fcm_push(subscription) @@ -98,7 +107,11 @@ class Notification::PushNotificationService end def remove_subscription_if_error(subscription, response) - subscription.destroy! if JSON.parse(response[:body])['results']&.first&.keys&.include?('error') + if JSON.parse(response[:body])['results']&.first&.keys&.include?('error') + subscription.destroy! + else + Rails.logger.info("FCM push sent to #{user.email} with title #{push_message[:title]}") + end end def fcm_options(subscription) diff --git a/spec/factories/notification_subscriptions.rb b/spec/factories/notification_subscriptions.rb index 581c198a6..c886a12c0 100644 --- a/spec/factories/notification_subscriptions.rb +++ b/spec/factories/notification_subscriptions.rb @@ -6,5 +6,13 @@ FactoryBot.define do identifier { 'test' } subscription_type { 'browser_push' } subscription_attributes { { endpoint: 'test', auth: 'test' } } + + trait :browser_push do + subscription_type { 'browser_push' } + end + + trait :fcm do + subscription_type { 'fcm' } + end end end diff --git a/spec/services/notification/push_notification_service_spec.rb b/spec/services/notification/push_notification_service_spec.rb index bb8ed5c66..bba912715 100644 --- a/spec/services/notification/push_notification_service_spec.rb +++ b/spec/services/notification/push_notification_service_spec.rb @@ -7,33 +7,58 @@ describe Notification::PushNotificationService do let(:fcm_double) { instance_double(FCM) } let(:fcm_service_double) { instance_double(Notification::FcmService, fcm_client: fcm_double) } - before do - allow(WebPush).to receive(:payload_send).and_return(true) - allow(Notification::FcmService).to receive(:new).and_return(fcm_service_double) - allow(fcm_double).to receive(:send_v1).and_return({ body: { 'results': [] }.to_json }) - allow(GlobalConfigService).to receive(:load).with('FIREBASE_PROJECT_ID', nil).and_return('test_project_id') - allow(GlobalConfigService).to receive(:load).with('FIREBASE_CREDENTIALS', nil).and_return('test_credentials') - end - describe '#perform' do - it 'sends webpush notifications for webpush subscription' do - with_modified_env VAPID_PUBLIC_KEY: 'test' do - create(:notification_subscription, user: notification.user) + context 'when the push server returns success' do + before do + allow(WebPush).to receive(:payload_send).and_return(true) + allow(Rails.logger).to receive(:info) + allow(Notification::FcmService).to receive(:new).and_return(fcm_service_double) + allow(fcm_double).to receive(:send_v1).and_return({ body: { 'results': [] }.to_json }) + allow(GlobalConfigService).to receive(:load).with('FIREBASE_PROJECT_ID', nil).and_return('test_project_id') + allow(GlobalConfigService).to receive(:load).with('FIREBASE_CREDENTIALS', nil).and_return('test_credentials') + end - described_class.new(notification: notification).perform - expect(WebPush).to have_received(:payload_send) - expect(Notification::FcmService).not_to have_received(:new) + it 'sends webpush notifications for webpush subscription' do + with_modified_env VAPID_PUBLIC_KEY: 'test' do + create(:notification_subscription, user: notification.user) + + described_class.new(notification: notification).perform + expect(WebPush).to have_received(:payload_send) + expect(Notification::FcmService).not_to have_received(:new) + expect(Rails.logger).to have_received(:info).with("Browser push sent to #{user.email} with title #{notification.push_message_title}") + end + 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') + + 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 + 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') + context 'when the push server returns error' do + it 'sends webpush notifications for webpush subscription' do + with_modified_env VAPID_PUBLIC_KEY: 'test' do + mock_response = instance_double(Net::HTTPResponse, body: 'Subscription is invalid') + mock_host = 'fcm.googleapis.com' + + allow(WebPush).to receive(:payload_send).and_raise(WebPush::InvalidSubscription.new(mock_response, mock_host)) + allow(Rails.logger).to receive(:info) + + create(:notification_subscription, :browser_push, user: notification.user) + + expect(Rails.logger).to receive(:info) do |message| + expect(message).to include('WebPush subscription expired:') + 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) end end end