vets-api icon indicating copy to clipboard operation
vets-api copied to clipboard

[Document Upload Failure] Veteran-uploaded document failure notification email

Open NB28VT opened this issue 10 months ago • 6 comments

Summary

Note this won't go live until DataDog monitors and VA Notify template are complete and VA notify service settings are configured

  • *This work is behind a feature toggle (flipper): YES, form526_send_document_upload_failure_notification
  • (Summarize the changes that have been made to the platform)

Adds a new VA Notify Mailer job, DisabilityCompensationForm::Form526DocumentUploadFailureEmail, which is triggered when we fail to upload to EVSS a document the veteran attached to their Form 526 application. Instead of the document failing to reach EVSS and the veteran not knowing about it, this will send them an email, with an obscured version of the filename (for PII protection), and instructions on how to physically mail a copy of the document instead.

What that means in practice:

  1. When veterans fill out a Form 526 they can optionally attach documents such as doctor's notes to support their application.
  2. We save these files in our S3 storage until the veteran is ready to submit their full Form 526 application.
  3. Once the veteran decides to submit their application, we send the Form 526 itself to EVSS.
  4. After that succeeds, we do a number of follow up actions, including looping through all of the veteran uploaded documents from step 1 and running EVSS::DisabilityCompensationForm:SubmitUploads to upload each to EVSS. Note: Despite the name, EVSS::DisabilityCompensationForm:SubmitUploads actually submits a single document the veteran uploaded, we run one job for each document
  5. As with many of our other jobs, EVSS::DisabilityCompensationForm:SubmitUploads may fail to successfully submit the document to EVSS, in which case it will retry until all of its retries are exhausted.
  6. At this point, we've given up on trying to upload the document to EVSS, and will instead send the veteran this new email instead:
Screenshot 2024-04-16 at 10 44 59 AM

Additional changes worth noting:

  • Convenience methods added for parsing Form526Submission form JSON for things like veteran email address and formatted timestamps

  • Convenience method for obscuring filenames

  • Basic setup for a new service with VA Notify; they created a new service for us so we could use their callback system in the future

  • (Which team do you work for, does your team own the maintenance of this component?)

I am on the Disability Benefits team. We plan to monitor the performance of this mailer in DataDog in a ticket linked below.

  • (If introducing a flipper, what is the success criteria being targeted?)

We are planning to treat this Flipper as a full on and off switch, rather than rolling things out in a "canary launch" that would only trigger the mailer for a specific percentage of users we slowly increase. This is because catatstrophic upload failures which never succeed on retries are rare, and it would likely take us a while to see a representative set . We will be monitoring the performance of this solution in DataDog after release and can turn off the flipper if necessary.

Related issue(s)

Testing done

  • [X] 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: X 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?

    Testing plan is TBD need to figure out how we would do this in staging but we likely just need to confirm this works end to end and then monitor the emails that go out once this is live

What areas of the site does it impact?

Form526 Submission flow only

Acceptance criteria

  • [X] I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • [ ] No error nor warning in the console.
  • [x] Events are being sent to the appropriate logging solution
  • [ ] Documentation has been updated (link to documentation)
  • [X] 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 or Grafana (if applicable) TBD
  • [ ] 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

There are a few areas of note for feedback I'll add my own comments to call them out

NB28VT avatar Apr 16 '24 14:04 NB28VT

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file: spec/models/supporting_evidence_attachment_spec.rb

va-vsp-bot avatar Apr 16 '24 14:04 va-vsp-bot

1 Warning
:warning: This PR changes 455 LoC (not counting whitespace/newlines).

In order to ensure each PR receives the proper attention it deserves, we recommend not exceeding
200. Expect some delays getting reviews.

File Summary

Files

  • .github/CODEOWNERS (+3/-0)

  • app/models/form526_submission.rb (+9/-0)

  • app/models/supporting_evidence_attachment.rb (+20/-0)

  • app/sidekiq/evss/disability_compensation_form/form526_document_upload_failure_email.rb (+106/-0)

  • app/sidekiq/evss/disability_compensation_form/submit_uploads.rb (+11/-0)

  • config/features.yml (+4/-0)

  • config/settings.yml (+4/-0)

  • spec/models/supporting_evidence_attachment_spec.rb (+121/-0)

  • spec/sidekiq/evss/disability_compensation_form/form526_document_upload_failure_email_spec.rb (+125/-0)

  • spec/sidekiq/evss/disability_compensation_form/submit_uploads_spec.rb (+52/-0)

    Note: We exclude files matching the following when considering PR size:

    *.csv, *.json, *.tsv, *.txt, Gemfile.lock, app/swagger, modules/mobile/docs, spec/fixtures/, spec/support/vcr_cassettes/, modules/mobile/spec/support/vcr_cassettes/, db/seeds, modules/vaos/app/docs, modules/meb_api/app/docs, modules/appeals_api/app/swagger/, *.bru
    

Big PRs are difficult to review, often become stale, and cause delays.

Generated by :no_entry_sign: Danger

github-actions[bot] avatar Apr 16 '24 14:04 github-actions[bot]

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file: app/models/saved_claim/disability_compensation.rb

va-vsp-bot avatar Apr 25 '24 21:04 va-vsp-bot

FYI the failing coverage report is a known issue in master the platform is looking into

NB28VT avatar May 01 '24 20:05 NB28VT

FYI the failing coverage report is a known issue in master the platform is looking into

@NB28VT pull in master to your branch and it will resolve this ^ issue

stevenjcumming avatar May 03 '24 01:05 stevenjcumming

@RachalCassity this should be ready for re-review I've addressed the changes in CODEOWNERS, thanks!

NB28VT avatar May 13 '24 14:05 NB28VT