#2041: Migrate IVC CHAMPVA email to VANotify::V2::QueueEmailJob#27783
#2041: Migrate IVC CHAMPVA email to VANotify::V2::QueueEmailJob#27783sarahwcodes wants to merge 8 commits intomasterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Migrates IVC CHAMPVA email sending to optionally use VANotify::V2::QueueEmailJob (via Flipper) to improve handling of personalization data in Sidekiq.
Changes:
- Add
va_notify_v2_ivc_champva_emailfeature flag to control V1 (VANotify::EmailJob) vs V2 (VANotify::V2::QueueEmailJob) behavior. - Refactor
IvcChampva::Email#send_emailto route sending through a helper that selects V1/V2 based on the flag. - Expand service specs to cover both Flipper states (and error handling) for enqueueing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| modules/ivc_champva/app/services/ivc_champva/email.rb | Adds Flipper-gated routing to V2 queueing and extracts enqueue logic into a helper method. |
| modules/ivc_champva/spec/services/email_spec.rb | Updates specs to validate behavior for Flipper off/on and job enqueue parameters. |
| config/features.yml | Introduces the va_notify_v2_ivc_champva_email feature definition. |
| let(:expected_template_id) { Settings.vanotify.services.ivc_champva.template_id.form_10_10d_email } | ||
| let(:expected_personalisation) do | ||
| data.slice(:first_name, :last_name, :file_count, :pega_status, :date_submitted, :form_uuid) | ||
| end |
There was a problem hiding this comment.
expected_personalisation is built with data.slice(...), but the service under test builds personalisation via %i[...].index_with { |k| data[k] }, which intentionally includes keys even when they’re missing from data (nil values). There are real call sites that omit first_name/last_name (e.g., the Pega alert payload builders), so this spec won’t catch regressions where the nil keys are dropped. Update the expectation to mirror the index_with behavior and add a case covering a payload missing optional keys.
|
@sarahwcodes small merge conflict. |
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
Related issue(s)
Testing done
Confirm email sends correctly prior to turning flipper on:
Screenshots
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?