Skip to content
This repository is currently being migrated. It's locked while the migration is in progress.

Decision Letter SMS: Create new LetterReadySMSJob sidekiq job#26323

Merged
sethdarragile6 merged 7 commits intomasterfrom
bmt3/130913_create_letterreadysms_job
Feb 9, 2026
Merged

Decision Letter SMS: Create new LetterReadySMSJob sidekiq job#26323
sethdarragile6 merged 7 commits intomasterfrom
bmt3/130913_create_letterreadysms_job

Conversation

@sethdarragile6
Copy link
Copy Markdown
Contributor

@sethdarragile6 sethdarragile6 commented Feb 3, 2026

This is the first implementation task for enabling SMS notifications for Decision Letters. In subsequent stories, we will connect it up to the existing LetterReady infrastructure (see epic for further details)

The changes add:

  • New LetterReadySmsJob Sidekiq job
  • SMS retry constant and VA Notify configuration (sender ID, template ID)
  • Specs for the new job

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

  • This work is behind a feature toggle (flipper): NO
  • (Summarize the changes that have been made to the platform)
    • New LetterReadySmsJob Sidekiq job
    • SMS retry constant and VA Notify configuration (sender ID, template ID)
    • Specs for the new job
  • (If bug, how to reproduce)
  • (What is the solution, why is this the solution?)
  • (Which team do you work for, does your team own the maintenance of this component?)
    Benefits Management Tools Team 3
  • (If introducing a flipper, what is the success criteria being targeted?)

Related issue(s)

Testing done

  • New code is covered by unit tests
  • Describe what the old behavior was prior to the change
  • Describe the steps required to verify your changes are working as expected. Exclusively stating 'Specs run' is NOT acceptable as appropriate testing
  • If this work is behind a flipper:
    • Tests need to be written for both the flipper on and flipper off scenarios. Docs.
    • What is the testing plan for rolling out the feature?

Screenshots

Tested from my local environment using User 57 from Staging. Note: content of message not yet finalized/approved

image

What areas of the site does it impact?

(Describe what parts of the site are impacted andifcode touched other areas)

Acceptance criteria

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No error nor warning in the console.
  • Events are being sent to the appropriate logging solution
  • Documentation has been updated (link to documentation)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Feature/bug has a monitor built into Datadog (if applicable)
  • If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected
  • I added a screenshot of the developed feature

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?

…tter ready SMS notifications

The changes add:
- New LetterReadySmsJob Sidekiq job with PII protection via AttrPackage
- SMS retry constant and VA Notify configuration (sender ID, template ID)
- Comprehensive specs for the new job

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@sethdarragile6 sethdarragile6 self-assigned this Feb 3, 2026
@va-vsp-bot
Copy link
Copy Markdown
Collaborator

⚠️ The following Parameter Store values are invalid. Please make sure the values are correct and exist in AWS Parameter Store before merging:

  • /dsva-vagov/vets-api/dev/env_vars/vanotify/services/benefits_management_tools/template_id/decision_letter_ready_sms
  • /dsva-vagov/vets-api/prod/env_vars/vanotify/services/benefits_management_tools/template_id/decision_letter_ready_sms
  • /dsva-vagov/vets-api/sandbox/env_vars/vanotify/services/benefits_management_tools/template_id/decision_letter_ready_sms

1 similar comment
@va-vsp-bot
Copy link
Copy Markdown
Collaborator

⚠️ The following Parameter Store values are invalid. Please make sure the values are correct and exist in AWS Parameter Store before merging:

  • /dsva-vagov/vets-api/dev/env_vars/vanotify/services/benefits_management_tools/template_id/decision_letter_ready_sms
  • /dsva-vagov/vets-api/prod/env_vars/vanotify/services/benefits_management_tools/template_id/decision_letter_ready_sms
  • /dsva-vagov/vets-api/sandbox/env_vars/vanotify/services/benefits_management_tools/template_id/decision_letter_ready_sms

@va-vsp-bot
Copy link
Copy Markdown
Collaborator

👍 All Parameter Store values in this PR are valid

@va-vsp-bot
Copy link
Copy Markdown
Collaborator

👍 All Parameter Store values in this PR are valid

Comment thread app/sidekiq/event_bus_gateway/letter_ready_sms_job.rb
lianafleming
lianafleming previously approved these changes Feb 5, 2026
@sethdarragile6 sethdarragile6 marked this pull request as ready for review February 5, 2026 18:45
Copilot AI review requested due to automatic review settings February 5, 2026 18:45
@sethdarragile6 sethdarragile6 requested review from a team as code owners February 5, 2026 18:45
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces SMS notification capability for Decision Letters by creating a new Sidekiq job LetterReadySmsJob that sends SMS notifications via VA Notify when decision letters are ready for veterans.

Changes:

  • New LetterReadySmsJob Sidekiq job with retry logic, PII protection via Redis cache, and comprehensive error handling
  • Configuration additions for VA Notify SMS (sender ID and template ID) across settings files
  • New constant SIDEKIQ_RETRY_COUNT_FIRST_SMS for SMS-specific retry count
  • Comprehensive test coverage including success cases, error handling, PII protection, and retry behavior

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
app/sidekiq/event_bus_gateway/letter_ready_sms_job.rb New Sidekiq job implementing SMS notification logic with retry configuration and PII handling
app/sidekiq/event_bus_gateway/constants.rb Added SIDEKIQ_RETRY_COUNT_FIRST_SMS constant for SMS retry configuration
config/settings.yml Added SMS sender ID and template ID configuration from environment variables
config/settings/development.yml Added fake SMS configuration values for development environment
config/settings/test.yml Added fake SMS configuration values for test environment
spec/sidekiq/event_bus_gateway/letter_ready_sms_job_spec.rb Comprehensive test suite covering job functionality, error handling, and edge cases

Comment thread app/sidekiq/event_bus_gateway/constants.rb
Comment thread app/sidekiq/event_bus_gateway/letter_ready_sms_job.rb Outdated
Comment thread app/sidekiq/event_bus_gateway/letter_ready_sms_job.rb
Comment thread config/settings.yml
Comment thread config/settings.yml
Comment thread app/sidekiq/event_bus_gateway/letter_ready_sms_job.rb
@james-taggart-kind
Copy link
Copy Markdown
Contributor

What is this?

Part of the multichannel notification system the notifies veterans when they have a letter ready (benefits letters, claims decisions, etc.). This job focuses on SMS, but there are other jobs that focus on email and push notifications.

Why does this exist?

We want to send veterans and their families custom SMS messages they can trust to let them know their letter is ready.

How did we do it?

  • Created a new job for SMS
  • Retrieves info from BGS and MPI
  • Personalizes the message
  • Tracks the path of the request
  • Includes validation
  • Handles exceptions
  • High test coverage

rjohnson2011
rjohnson2011 previously approved these changes Feb 5, 2026
…ckageError rescue block

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@va-vsp-bot
Copy link
Copy Markdown
Collaborator

👍 All Parameter Store values in this PR are valid

@va-vsp-bot
Copy link
Copy Markdown
Collaborator

👍 All Parameter Store values in this PR are valid

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@va-vsp-bot
Copy link
Copy Markdown
Collaborator

👍 All Parameter Store values in this PR are valid

…nses in AttrPackage tests

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@va-vsp-bot
Copy link
Copy Markdown
Collaborator

👍 All Parameter Store values in this PR are valid

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 5, 2026

⚠️ Warning: Low Coverage Report

Coverage: 90.5%

Please add or expand tests to improve coverage before merging.

View detailed coverage: Open test run → Scroll to Artifacts → Download Coverage Report → Open index.html

@sethdarragile6 sethdarragile6 enabled auto-merge (squash) February 6, 2026 00:01
@sethdarragile6
Copy link
Copy Markdown
Contributor Author

@rjohnson2011 would you mind re-approving when you get a chance? My apologies, I forgot to run Copilot before taking it out of draft and I tweaked a few things. I also added some unit tests per a teammates suggestion. Thanks 🙏

@sethdarragile6 sethdarragile6 merged commit a530b43 into master Feb 9, 2026
43 of 45 checks passed
@sethdarragile6 sethdarragile6 deleted the bmt3/130913_create_letterreadysms_job branch February 9, 2026 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants