Bmt3/126967 notification model update (#25503)

* BMT3/121448 - add basic client and push service

BMT3/121448 - add start of VANotify new push client and send_push method

* BMT3/121448 - rubocop fixes

* BMT3/121448 - update push notification service

* BMT3/121448 - VaNotify service use UuidValidator, spec fixes

* BMT3/121448 - replace sentry logging with shared logging

* BMT3/121448 - rubocop fixes

* BMT3/121448 - add feature flag around VaNotify Client creation

* BMT3/121448 - bad merge revert

* BMT3/121448 - pr review updates

* BMT3/121448 - missing PR comment review updates

* BMT3/121448 - api key parsing fix

* BMT3/121452 - add push notification model

* BMT3/121452 - removing EventBusGatewayPushNotification model and adding to subsequent PR

* BMT3/121452 - new event bus push sidekiq jobs

* BMT3/121452 - add more error logging

* BMT3/121452 - file renaming due to Zeitwerk error

* BMT3/121452 - consistent skipped email and push logging

* BMT3/125679 - allow for percentage rollout of push notifications

* BMT3/124679 - check for bad data prior to feature flag

* BMT3/126967 - Event bus notification schema update

* BMT3/126967 - revert logging updates

* BMT3/126967 - merge conflict fix

* BMT3/126967 - revert model updates

* BMT3/126967 - model updates

* BMT3/126967 - spec updates

Author: lianafleming

app/models/event_bus_gateway_notification.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class EventBusGatewayNotification < ApplicationRecord
-  belongs_to :user_account
+  belongs_to :user_account, optional: true
 
   validates :va_notify_id, presence: true
   validates :template_id, presence: true

app/models/event_bus_gateway_push_notification.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class EventBusGatewayPushNotification < ApplicationRecord
-  belongs_to :user_account
+  belongs_to :user_account, optional: true
 
   validates :template_id, presence: true
 end

app/sidekiq/event_bus_gateway/letter_ready_push_job.rb

@@ -61,10 +61,24 @@ def send_push_notification(icn, template_id)
         personalisation: {}
       )
 
-      EventBusGatewayPushNotification.create!(
+      create_push_notification_record(template_id, icn)
+    end
+
+    def create_push_notification_record(template_id, icn)
+      notification = EventBusGatewayPushNotification.create(
         user_account: user_account(icn),
         template_id:
       )
+
+      return if notification.persisted?
+
+      ::Rails.logger.warn(
+        'LetterReadyPushJob notification record failed to save',
+        {
+          errors: notification.errors.full_messages,
+          template_id:
+        }
+      )
     end
 
     def notify_client

spec/sidekiq/event_bus_gateway/letter_ready_email_job_spec.rb

@@ -157,6 +157,32 @@
         end.not_to raise_error
       end
     end
+
+    context 'when user_account is nil' do
+      before do
+        allow_any_instance_of(described_class).to receive(:user_account).and_return(nil)
+      end
+
+      it 'successfully creates notification record without user_account' do
+        job_instance = subject.new
+
+        expect do
+          job_instance.send(:send_email_notification, participant_id, template_id, first_name, mpi_profile.icn)
+        end.to change(EventBusGatewayNotification, :count).by(1)
+
+        notification = EventBusGatewayNotification.last
+        expect(notification.user_account).to be_nil
+        expect(notification.template_id).to eq(template_id)
+        expect(notification.va_notify_id).to eq(notification_id)
+      end
+
+      it 'still sends email successfully' do
+        job_instance = subject.new
+
+        expect(va_notify_service).to receive(:send_email).with(expected_email_args)
+        job_instance.send(:send_email_notification, participant_id, template_id, first_name, mpi_profile.icn)
+      end
+    end
   end
 
   describe 'PII protection with AttrPackage' do

spec/sidekiq/event_bus_gateway/letter_ready_push_job_spec.rb

@@ -99,6 +99,31 @@
         job_instance.send(:send_push_notification, mpi_profile.icn, template_id)
       end.to change(EventBusGatewayPushNotification, :count).by(1)
     end
+
+    context 'when user_account is nil' do
+      before do
+        allow_any_instance_of(described_class).to receive(:user_account).and_return(nil)
+      end
+
+      it 'successfully creates notification record without user_account' do
+        job_instance = subject.new
+
+        expect do
+          job_instance.send(:send_push_notification, mpi_profile.icn, template_id)
+        end.to change(EventBusGatewayPushNotification, :count).by(1)
+
+        notification = EventBusGatewayPushNotification.last
+        expect(notification.user_account).to be_nil
+        expect(notification.template_id).to eq(template_id)
+      end
+
+      it 'still sends push notification successfully' do
+        job_instance = subject.new
+
+        expect(va_notify_service).to receive(:send_push).with(expected_push_args)
+        job_instance.send(:send_push_notification, mpi_profile.icn, template_id)
+      end
+    end
   end
 
   describe 'PII protection with AttrPackage' do
@@ -258,19 +283,36 @@
     end
 
     context 'when notification creation fails' do
-      let(:creation_error) { ActiveRecord::RecordInvalid.new }
+      let(:invalid_notification) do
+        instance_double(EventBusGatewayPushNotification,
+                        persisted?: false,
+                        errors: double(full_messages: ['Template must exist']))
+      end
 
       before do
-        allow(EventBusGatewayPushNotification).to receive(:create!).and_raise(creation_error)
+        allow(EventBusGatewayPushNotification).to receive(:create).and_return(invalid_notification)
+        allow(Rails.logger).to receive(:warn)
       end
 
-      it 'records notification send failure and re-raises error' do
-        expect_any_instance_of(described_class)
-          .to receive(:record_notification_send_failure)
-          .with(creation_error, 'Push')
+      it 'logs a warning with error details' do
+        expect(Rails.logger).to receive(:warn).with(
+          'LetterReadyPushJob notification record failed to save',
+          {
+            errors: ['Template must exist'],
+            template_id:
+          }
+        )
 
-        expect { subject.new.perform(participant_id, template_id) }
-          .to raise_error(creation_error)
+        subject.new.perform(participant_id, template_id)
+      end
+
+      it 'still sends the push notification successfully' do
+        expect(va_notify_service).to receive(:send_push)
+        subject.new.perform(participant_id, template_id)
+      end
+
+      it 'does not raise an error' do
+        expect { subject.new.perform(participant_id, template_id) }.not_to raise_error
       end
     end
   end