diff --git a/config/features.yml b/config/features.yml index 2a61f03bbd7..4b5d90e0546 100644 --- a/config/features.yml +++ b/config/features.yml @@ -3058,6 +3058,18 @@ features: description: >- If enabled, BurialFormsController will use VANotify::V2::QueueEmailJob instead of VANotify::EmailJob for confirmation emails + va_notify_v2_simple_forms_email: + actor_type: user + description: >- + If enabled, SimpleFormsApi::Notification::Email will use + VANotify::V2::QueueEmailJob instead of VANotify::EmailJob for both + immediate sends (#send_email_now) and scheduled sends (#enqueue_email) + va_notify_v2_simple_forms_upload_email: + actor_type: user + description: >- + If enabled, SimpleFormsApi::Notification::FormUploadEmail will use + VANotify::V2::QueueEmailJob instead of VANotify::EmailJob for both + immediate sends (#send_email_now) and scheduled sends (#enqueue_email) va_notify_delivery_status_update_job: actor_type: user description: If enabled, VANotify::DelieveryStatusUpdateJob will be used to query VANotify::Notifications diff --git a/modules/simple_forms_api/app/services/simple_forms_api/notification/email.rb b/modules/simple_forms_api/app/services/simple_forms_api/notification/email.rb index 8ecdd509d76..a8b6246362d 100644 --- a/modules/simple_forms_api/app/services/simple_forms_api/notification/email.rb +++ b/modules/simple_forms_api/app/services/simple_forms_api/notification/email.rb @@ -111,13 +111,25 @@ def enqueue_email(at, template_id) end def async_job_with_form_data(email, at, template_id) - VANotify::EmailJob.perform_at( - at, - email, - template_id, - get_personalization, - *email_args - ) + if Flipper.enabled?(:va_notify_v2_simple_forms_email) + callback_options = email_args.last + VANotify::V2::QueueEmailJob.enqueue_at( + at, + email, + template_id, + get_personalization, + 'Settings.vanotify.services.va_gov.api_key', + callback_options + ) + else + VANotify::EmailJob.perform_at( + at, + email, + template_id, + get_personalization, + *email_args + ) + end end def async_job_with_user_account(user_account, at, template_id) @@ -137,8 +149,18 @@ def async_job_with_user_account(user_account, at, template_id) def send_email_now(template_id) email_address = resolve_notification_email || user&.email personalization = get_personalization + return unless email_address && personalization - if email_address && personalization + if Flipper.enabled?(:va_notify_v2_simple_forms_email) + callback_options = email_args.last + VANotify::V2::QueueEmailJob.enqueue( + email_address, + template_id, + personalization, + 'Settings.vanotify.services.va_gov.api_key', + callback_options + ) + else VANotify::EmailJob.perform_async( email_address, template_id, diff --git a/modules/simple_forms_api/app/services/simple_forms_api/notification/form_upload_email.rb b/modules/simple_forms_api/app/services/simple_forms_api/notification/form_upload_email.rb index 2471b541136..d10760be139 100644 --- a/modules/simple_forms_api/app/services/simple_forms_api/notification/form_upload_email.rb +++ b/modules/simple_forms_api/app/services/simple_forms_api/notification/form_upload_email.rb @@ -102,22 +102,43 @@ def check_if_form_is_supported(config) end def send_email_now - VANotify::EmailJob.perform_async( - form_data['email'], - template_id, - get_personalization, - *email_args - ) + if Flipper.enabled?(:va_notify_v2_simple_forms_upload_email) + VANotify::V2::QueueEmailJob.enqueue( + form_data['email'], + template_id, + get_personalization, + 'Settings.vanotify.services.va_gov.api_key', + { callback_metadata: { notification_type:, form_number:, confirmation_number:, statsd_tags: } } + ) + else + VANotify::EmailJob.perform_async( + form_data['email'], + template_id, + get_personalization, + *email_args + ) + end end def enqueue_email(at) - VANotify::EmailJob.perform_at( - at, - form_data['email'], - template_id, - get_personalization, - *email_args - ) + if Flipper.enabled?(:va_notify_v2_simple_forms_upload_email) + VANotify::V2::QueueEmailJob.enqueue_at( + at, + form_data['email'], + template_id, + get_personalization, + 'Settings.vanotify.services.va_gov.api_key', + { callback_metadata: { notification_type:, form_number:, confirmation_number:, statsd_tags: } } + ) + else + VANotify::EmailJob.perform_at( + at, + form_data['email'], + template_id, + get_personalization, + *email_args + ) + end end def email_args diff --git a/modules/simple_forms_api/spec/requests/simple_forms_api/v1/simple_forms_spec.rb b/modules/simple_forms_api/spec/requests/simple_forms_api/v1/simple_forms_spec.rb index 5110a124fc7..ebfb02f59ed 100644 --- a/modules/simple_forms_api/spec/requests/simple_forms_api/v1/simple_forms_spec.rb +++ b/modules/simple_forms_api/spec/requests/simple_forms_api/v1/simple_forms_spec.rb @@ -867,6 +867,7 @@ before do sign_in(user) allow(Flipper).to receive(:enabled?).with(:simple_forms_email_delivery_callback).and_return(true) + allow(Flipper).to receive(:enabled?).with(:va_notify_v2_simple_forms_email).and_return(false) end describe '21_4142' do diff --git a/modules/simple_forms_api/spec/services/notification/email_spec.rb b/modules/simple_forms_api/spec/services/notification/email_spec.rb index ba8e8d19d38..50f69ff168f 100644 --- a/modules/simple_forms_api/spec/services/notification/email_spec.rb +++ b/modules/simple_forms_api/spec/services/notification/email_spec.rb @@ -15,6 +15,10 @@ describe SimpleFormsApi::Notification::Email do let(:lighthouse_updated_at) { Time.current } + before do + allow(Flipper).to receive(:enabled?).with(:va_notify_v2_simple_forms_email).and_return(false) + end + %i[confirmation error received].each do |notification_type| describe '#initialize' do context 'when all required arguments are passed in' do @@ -220,6 +224,7 @@ context 'flipper is on' do before do allow(Flipper).to receive(:enabled?).and_return true + allow(Flipper).to receive(:enabled?).with(:va_notify_v2_simple_forms_email).and_return(false) end context 'fetching the template id' do @@ -393,6 +398,70 @@ end end + context 'when va_notify_v2_simple_forms_email is enabled' do + before do + allow(Flipper).to receive(:enabled?).with(:form21_10210_confirmation_email).and_return(true) + allow(Flipper).to receive(:enabled?).with(:va_notify_v2_simple_forms_email).and_return(true) + allow(Flipper).to receive(:enabled?).with(:simple_forms_email_delivery_callback).and_return(false) + allow(VANotify::V2::QueueEmailJob).to receive(:enqueue) + end + + it 'sends email via V2 QueueEmailJob' do + subject = described_class.new(config, notification_type:) + + subject.send + + expect(VANotify::V2::QueueEmailJob).to have_received(:enqueue).with( + a_string_matching(/@/), + a_string_matching(/\S+/), + a_hash_including('confirmation_number'), + 'Settings.vanotify.services.va_gov.api_key', + a_hash_including(:callback_metadata) + ) + end + + it 'does not call V1 EmailJob' do + allow(VANotify::EmailJob).to receive(:perform_async) + subject = described_class.new(config, notification_type:) + + subject.send + + expect(VANotify::EmailJob).not_to have_received(:perform_async) + end + end + + context 'when va_notify_v2_simple_forms_email is disabled' do + before do + allow(Flipper).to receive(:enabled?).with(:form21_10210_confirmation_email).and_return(true) + allow(Flipper).to receive(:enabled?).with(:va_notify_v2_simple_forms_email).and_return(false) + allow(Flipper).to receive(:enabled?).with(:simple_forms_email_delivery_callback).and_return(false) + allow(VANotify::EmailJob).to receive(:perform_async) + end + + it 'sends email via V1 EmailJob' do + subject = described_class.new(config, notification_type:) + + subject.send + + expect(VANotify::EmailJob).to have_received(:perform_async).with( + a_string_matching(/@/), + a_string_matching(/\S+/), + a_hash_including('confirmation_number'), + a_string_matching(/\S+/), + a_hash_including(:callback_metadata) + ) + end + + it 'does not call V2 QueueEmailJob' do + allow(VANotify::V2::QueueEmailJob).to receive(:enqueue) + subject = described_class.new(config, notification_type:) + + subject.send + + expect(VANotify::V2::QueueEmailJob).not_to have_received(:enqueue) + end + end + context 'send at time is specified' do context 'user_account is passed in' do let(:confirmation_number) { 'confirmation_number' } @@ -431,21 +500,56 @@ end context 'user and user_account are not passed in' do - it 'sends the email at the specified time' do - time = double - allow(VANotify::EmailJob).to receive(:perform_at) - subject = described_class.new(config, notification_type:) + context 'when va_notify_v2_simple_forms_email is disabled' do + it 'sends the email at the specified time via V1' do + time = double + allow(VANotify::EmailJob).to receive(:perform_at) + subject = described_class.new(config, notification_type:) - subject.send(at: time) + subject.send(at: time) - expect(VANotify::EmailJob).to have_received(:perform_at).with( - time, - a_string_matching(/@/), - a_string_matching(/\S+/), - a_hash_including('confirmation_number'), - a_string_matching(/\S+/), - a_hash_including(:callback_metadata) - ) + expect(VANotify::EmailJob).to have_received(:perform_at).with( + time, + a_string_matching(/@/), + a_string_matching(/\S+/), + a_hash_including('confirmation_number'), + a_string_matching(/\S+/), + a_hash_including(:callback_metadata) + ) + end + end + + context 'when va_notify_v2_simple_forms_email is enabled' do + before do + allow(Flipper).to receive(:enabled?).with(:va_notify_v2_simple_forms_email).and_return(true) + allow(VANotify::V2::QueueEmailJob).to receive(:enqueue_at) + end + + it 'sends the email at the specified time via V2' do + time = double + subject = described_class.new(config, notification_type:) + + subject.send(at: time) + + expect(VANotify::V2::QueueEmailJob).to have_received(:enqueue_at).with( + time, + a_string_matching(/@/), + a_string_matching(/\S+/), + a_hash_including('confirmation_number'), + 'Settings.vanotify.services.va_gov.api_key', + a_hash_including(:callback_metadata) + ) + end + + it 'does not call V1 EmailJob' do + time = double + allow(VANotify::EmailJob).to receive(:perform_at) + subject = described_class.new(config, notification_type:) + + subject.send(at: time) + + expect(VANotify::EmailJob).not_to have_received(:perform_at) + end end end end diff --git a/modules/simple_forms_api/spec/services/notification/form_upload_email_spec.rb b/modules/simple_forms_api/spec/services/notification/form_upload_email_spec.rb index 7419b95e5a1..2bdf34ba37a 100644 --- a/modules/simple_forms_api/spec/services/notification/form_upload_email_spec.rb +++ b/modules/simple_forms_api/spec/services/notification/form_upload_email_spec.rb @@ -21,6 +21,10 @@ } end + before do + allow(Flipper).to receive(:enabled?).with(:va_notify_v2_simple_forms_upload_email).and_return(false) + end + describe '#initialize' do context 'when all required arguments are passed in' do let(:config) do @@ -185,20 +189,54 @@ end context 'send at time is specified' do - it 'sends the email at the specified time' do - time = double - allow(VANotify::EmailJob).to receive(:perform_at) - - subject = described_class.new(config, notification_type: :confirmation) - subject.send(at: time) + context 'when va_notify_v2_simple_forms_upload_email is disabled' do + it 'sends the email at the specified time via V1' do + time = double + allow(VANotify::EmailJob).to receive(:perform_at) + + subject = described_class.new(config, notification_type: :confirmation) + subject.send(at: time) + + expect(VANotify::EmailJob).to have_received(:perform_at).with( + time, + email, + template_id, + expected_personalization, + *email_args + ) + end + end - expect(VANotify::EmailJob).to have_received(:perform_at).with( - time, - email, - template_id, - expected_personalization, - *email_args - ) + context 'when va_notify_v2_simple_forms_upload_email is enabled' do + before do + allow(Flipper).to receive(:enabled?).with(:va_notify_v2_simple_forms_upload_email).and_return(true) + allow(VANotify::V2::QueueEmailJob).to receive(:enqueue_at) + end + + it 'sends the email at the specified time via V2' do + time = double + subject = described_class.new(config, notification_type: :confirmation) + subject.send(at: time) + + expect(VANotify::V2::QueueEmailJob).to have_received(:enqueue_at).with( + time, + email, + template_id, + expected_personalization, + 'Settings.vanotify.services.va_gov.api_key', + { callback_metadata: { notification_type: :confirmation, form_number:, + confirmation_number:, statsd_tags: } } + ) + end + + it 'does not call V1 EmailJob' do + time = double + allow(VANotify::EmailJob).to receive(:perform_at) + subject = described_class.new(config, notification_type: :confirmation) + subject.send(at: time) + + expect(VANotify::EmailJob).not_to have_received(:perform_at) + end end end end @@ -250,5 +288,64 @@ it_behaves_like 'sends confirmation email' end + + context 'when va_notify_v2_simple_forms_upload_email is enabled' do + let(:form_number) { '21-0779' } + + before do + allow(Flipper).to receive(:enabled?).with(:va_notify_v2_simple_forms_upload_email).and_return(true) + allow(VANotify::V2::QueueEmailJob).to receive(:enqueue) + end + + it 'sends email via V2 QueueEmailJob' do + subject = described_class.new(config, notification_type: :confirmation) + subject.send + + expect(VANotify::V2::QueueEmailJob).to have_received(:enqueue).with( + email, + template_id, + expected_personalization, + 'Settings.vanotify.services.va_gov.api_key', + { callback_metadata: { notification_type: :confirmation, form_number:, confirmation_number:, statsd_tags: } } + ) + end + + it 'does not call V1 EmailJob' do + allow(VANotify::EmailJob).to receive(:perform_async) + subject = described_class.new(config, notification_type: :confirmation) + subject.send + + expect(VANotify::EmailJob).not_to have_received(:perform_async) + end + end + + context 'when va_notify_v2_simple_forms_upload_email is disabled' do + let(:form_number) { '21-0779' } + + before do + allow(Flipper).to receive(:enabled?).with(:va_notify_v2_simple_forms_upload_email).and_return(false) + allow(VANotify::EmailJob).to receive(:perform_async) + end + + it 'sends email via V1 EmailJob' do + subject = described_class.new(config, notification_type: :confirmation) + subject.send + + expect(VANotify::EmailJob).to have_received(:perform_async).with( + email, + template_id, + expected_personalization, + *email_args + ) + end + + it 'does not call V2 QueueEmailJob' do + allow(VANotify::V2::QueueEmailJob).to receive(:enqueue) + subject = described_class.new(config, notification_type: :confirmation) + subject.send + + expect(VANotify::V2::QueueEmailJob).not_to have_received(:enqueue) + end + end end end diff --git a/modules/va_notify/app/sidekiq/va_notify/v2/queue_email_job.rb b/modules/va_notify/app/sidekiq/va_notify/v2/queue_email_job.rb index cc7fbdd27e4..a3ec8a4b8e4 100644 --- a/modules/va_notify/app/sidekiq/va_notify/v2/queue_email_job.rb +++ b/modules/va_notify/app/sidekiq/va_notify/v2/queue_email_job.rb @@ -52,6 +52,17 @@ def self.enqueue(email, template_id, personalisation, api_key_path, callback_opt perform_async(template_id, key, api_key_path, callback_options) end + # rubocop:disable Metrics/ParameterLists + def self.enqueue_at(at, email, template_id, personalisation, api_key_path, callback_options = {}) + # rubocop:enable Metrics/ParameterLists + unless api_key_path.start_with?('Settings.') + raise ArgumentError, "API key path must start with 'Settings.': #{api_key_path}" + end + + key = Sidekiq::AttrPackage.create(email:, personalisation:) + perform_at(at, template_id, key, api_key_path, callback_options) + end + private def fetch_attrs(attr_package_key, template_id = nil) diff --git a/modules/va_notify/spec/sidekiq/v2/queue_email_job_spec.rb b/modules/va_notify/spec/sidekiq/v2/queue_email_job_spec.rb index 9da60b1df40..2687dcf9d43 100644 --- a/modules/va_notify/spec/sidekiq/v2/queue_email_job_spec.rb +++ b/modules/va_notify/spec/sidekiq/v2/queue_email_job_spec.rb @@ -80,6 +80,59 @@ end end + describe '.enqueue_at' do + let(:at_time) { 1.hour.from_now } + + it 'creates an AttrPackage and calls perform_at with correct arguments' do + expect(Sidekiq::AttrPackage).to receive(:create) + .with(email:, personalisation:) + .and_return(key) + + expect(described_class).to receive(:perform_at) + .with(at_time, template_id, key, api_key_path, callback_options) + + described_class.enqueue_at(at_time, email, template_id, personalisation, api_key_path, callback_options) + end + + it 'defaults callback_options to an empty hash' do + expect(Sidekiq::AttrPackage).to receive(:create) + .with(email:, personalisation:) + .and_return(key) + + expect(described_class).to receive(:perform_at) + .with(at_time, template_id, key, api_key_path, {}) + + described_class.enqueue_at(at_time, email, template_id, personalisation, api_key_path) + end + + it 'raises Sidekiq::AttrPackageError when AttrPackage creation fails' do + allow(Sidekiq::AttrPackage).to receive(:create) + .and_raise(Sidekiq::AttrPackageError.new('create', 'Connection refused')) + + expect do + described_class.enqueue_at(at_time, email, template_id, personalisation, api_key_path, callback_options) + end.to raise_error(Sidekiq::AttrPackageError, /Connection refused/) + end + + context 'when api_key_path does not start with Settings.' do + it 'raises ArgumentError' do + expect do + described_class.enqueue_at(at_time, email, template_id, personalisation, 'invalid_path', callback_options) + end.to raise_error(ArgumentError, "API key path must start with 'Settings.': invalid_path") + end + + it 'does not create an AttrPackage' do + expect(Sidekiq::AttrPackage).not_to receive(:create) + + begin + described_class.enqueue_at(at_time, email, template_id, personalisation, 'invalid_path', callback_options) + rescue ArgumentError + nil + end + end + end + end + context 'when errors occur' do before do allow(Sidekiq::AttrPackage).to receive(:delete).with(key)