VANotify: 2026 migrate hca to queue email job#27595
Conversation
…failure emails with feature toggle
…ts-api into mm/2026-migrate-hca-to-queue-email-job
MarchandMD
changed the title
…ack metadata directly in enqueue and perform_async methods
…ts-api into mm/2026-migrate-hca-to-queue-email-job
There was a problem hiding this comment.
Pull request overview
This PR migrates HealthCareApplication#send_failure_email to optionally use VANotify::V2::QueueEmailJob behind a new Flipper flag, following the VANotify team’s phased migration approach.
Changes:
- Added
va_notify_v2_health_care_application_model_send_failure_emailfeature flag. - Updated
HealthCareApplication#send_failure_emailto enqueue viaVANotify::V2::QueueEmailJobwhen the flag is enabled, otherwise continue usingVANotify::EmailJob. - Added/updated model specs to cover both flag-enabled and flag-disabled behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
app/models/health_care_application.rb |
Adds feature-flagged switch between V1 EmailJob and V2 QueueEmailJob for failure emails. |
spec/models/health_care_application_spec.rb |
Adds spec coverage for the new flag on/off behavior and V2 enqueue usage. |
config/features.yml |
Registers the new Flipper feature flag with description. |
| if Flipper.enabled?(:va_notify_v2_health_care_application_model_send_failure_email) | ||
| VANotify::V2::QueueEmailJob.enqueue( | ||
| email, template_id, personalisation, | ||
| 'Settings.vanotify.services.health_apps_1010.api_key', metadata | ||
| ) |
There was a problem hiding this comment.
send_failure_email hard-codes the API key path string ('Settings.vanotify.services.health_apps_1010.api_key') inline for the V2 job. Elsewhere the codebase uses an API_KEY_PATH constant for this (e.g., app/sidekiq/hca/ezr_submission_job.rb and app/sidekiq/form1010cg/submission_job.rb), which avoids typos and makes future refactors safer. Consider defining a constant on HealthCareApplication (or reusing an existing constant) and using it for the V2 enqueue call (and potentially the V1 path too).
| first_name = parsed_form.dig('veteranFullName', 'first') | ||
| template_id = Settings.vanotify.services.health_apps_1010.template_id.form1010_ez_failure_email | ||
| api_key = Settings.vanotify.services.health_apps_1010.api_key | ||
| personalisation = { 'salutation' => first_name ? "Dear #{first_name}," : '' } | ||
| metadata = { callback_metadata: { notification_type: 'error', form_number: FORM_ID, statsd_tags: DD_ZSF_TAGS } } | ||
|
|
||
| salutation = first_name ? "Dear #{first_name}," : '' | ||
| metadata = | ||
| { | ||
| callback_metadata: { | ||
| notification_type: 'error', | ||
| form_number: FORM_ID, | ||
| statsd_tags: DD_ZSF_TAGS | ||
| } | ||
| } | ||
| if Flipper.enabled?(:va_notify_v2_health_care_application_model_send_failure_email) | ||
| VANotify::V2::QueueEmailJob.enqueue( | ||
| email, template_id, personalisation, | ||
| 'Settings.vanotify.services.health_apps_1010.api_key', metadata | ||
| ) | ||
| else | ||
| api_key = Settings.vanotify.services.health_apps_1010.api_key | ||
| VANotify::EmailJob.perform_async(email, template_id, personalisation, api_key, metadata) | ||
| end |
There was a problem hiding this comment.
This method reads Settings values (template_id and, when the flag is off, api_key). In this repo Settings values can come through Parameter Store with unexpected types due to env_parse_values (e.g., integers/strings), so it’s safer to explicitly coerce/validate these before using them (e.g., ensure they’re non-blank Strings and fail fast/log clearly if not). This will help avoid silent misconfiguration causing notification failures at runtime.
| it 'sends a failure email using V2 QueueEmailJob' do | ||
| subject | ||
| expect(VANotify::V2::QueueEmailJob).to have_received(:enqueue).with( | ||
| email_address, | ||
| template_id, | ||
| { 'salutation' => "Dear #{health_care_application.parsed_form['veteranFullName']['first']}," }, | ||
| 'Settings.vanotify.services.health_apps_1010.api_key', | ||
| callback_metadata | ||
| ) | ||
| end | ||
|
|
||
| it 'sends a failure email to the email address provided on the form' do | ||
| subject | ||
| expect(VANotify::EmailJob).to have_received(:perform_async).with(*template_params) | ||
| end | ||
| it 'increments statsd' do | ||
| expect { subject }.to trigger_statsd_increment("#{statsd_key_prefix}.submission_failure_email_sent") | ||
| end | ||
|
|
||
| it 'logs error if email job throws error' do | ||
| allow(VANotify::EmailJob).to receive(:perform_async).and_raise(standard_error) | ||
| allow(Rails.logger).to receive(:error) | ||
| expect(Rails.logger).to receive(:error).with( | ||
| '[10-10EZ] - Failure sending Submission Failure Email', | ||
| { exception: standard_error } | ||
| ) | ||
| expect(Rails.logger).to receive(:info).with( | ||
| '[10-10EZ] - HCA total failure', | ||
| { | ||
| first_initial: 'F', | ||
| middle_initial: 'M', | ||
| last_initial: 'Z' | ||
| } | ||
| ) | ||
| expect { subject }.not_to raise_error | ||
| end | ||
| it 'logs error if email job throws error' do | ||
| allow(VANotify::V2::QueueEmailJob).to receive(:enqueue).and_raise(standard_error) | ||
| allow(Rails.logger).to receive(:error) | ||
| expect(Rails.logger).to receive(:error).with( | ||
| '[10-10EZ] - Failure sending Submission Failure Email', | ||
| { exception: standard_error } | ||
| ) | ||
| expect { subject }.not_to raise_error | ||
| end | ||
|
|
||
| it 'increments statsd' do | ||
| expect { subject }.to trigger_statsd_increment("#{statsd_key_prefix}.submission_failure_email_sent") | ||
| context 'without first name' do | ||
| subject do | ||
| health_care_application.parsed_form['veteranFullName'] = nil | ||
| super() | ||
| end | ||
|
|
||
| it 'sends a failure email with empty salutation' do | ||
| subject | ||
| expect(VANotify::V2::QueueEmailJob).to have_received(:enqueue).with( | ||
| email_address, | ||
| template_id, | ||
| { 'salutation' => '' }, | ||
| 'Settings.vanotify.services.health_apps_1010.api_key', | ||
| callback_metadata | ||
| ) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| context 'without first name' do | ||
| subject do | ||
| health_care_application.parsed_form['veteranFullName'] = nil | ||
| super() | ||
| context 'when va_notify_v2_health_care_application_model_send_failure_email is disabled' do | ||
| before do | ||
| allow(Flipper).to receive(:enabled?) | ||
| .with(:va_notify_v2_health_care_application_model_send_failure_email).and_return(false) | ||
| end | ||
|
|
||
| let(:template_params_no_name) do | ||
| [ | ||
| let(:api_key) { Settings.vanotify.services.health_apps_1010.api_key } | ||
|
|
||
| it 'sends a failure email using V1 EmailJob' do | ||
| subject | ||
| expect(VANotify::EmailJob).to have_received(:perform_async).with( | ||
| email_address, | ||
| template_id, | ||
| { | ||
| 'salutation' => '' | ||
| }, | ||
| { 'salutation' => "Dear #{health_care_application.parsed_form['veteranFullName']['first']}," }, | ||
| api_key, | ||
| callback_metadata | ||
| ] | ||
| ) |
There was a problem hiding this comment.
The enabled/disabled contexts assert the expected job is called, but they don’t assert the other job is not called. Adding negative expectations (V2 enabled => VANotify::EmailJob not called; V2 disabled => VANotify::V2::QueueEmailJob not called) would better protect against regressions that accidentally send duplicate emails during the migration.
Keep your PR as a Draft until it's ready for Platform review. A PR is ready for Platform review when it has a teammate approval and tests, linting, and settings checks pass CI. See these tips on how to avoid common delays in getting your PR merged.
Summary
before this PR:
app/models/health_care_application.rbwas using VANotify::EmailJob within#send_failure_emailcorrectly.after this PR:
A new feature flag has been introduced; when the feature flag is NOT enabled, perform the VANotify::EmailJob as normal, with no changes; When the feature flag IS enabled, use the new VANotify::V2::QueueEmailJob#enqueue.
There's also new specs for the feature flag being enabled/disabled
The VANotify team is currently migrating all instances of VANotify::EmailJob to use the new VANotify::V2::QueueEmailJob, using this pattern of migrating with flippers
VANotify! 🎉
Related issue(s)
https://va.ghe.com/orgs/software/projects/185/views/2?filterQuery=label%3AStrike&pane=issue&itemId=382981&issue=software%7Cvanotify-team%7C2026 (parent epic is included in ticket)
Testing done
bundle exec rspec spec/models/health_care_application_spec.rb:738Screenshots
Note: Optional
What areas of the site does it impact?
(Describe what parts of the site are impacted andifcode touched other areas)
Acceptance criteria
Requested Feedback
(OPTIONAL)What should the reviewers know in addition to the above. Is there anything specific you wish the reviewer to assist with. Do you have any concerns with this PR, why?