Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR #4 for silent failure work - Add lighthouse files and changes for uploading a document #20453

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

pmclaren19
Copy link
Contributor

@pmclaren19 pmclaren19 commented Jan 24, 2025

Summary

This pr is the third pr to merge some of the work from #20105. The first pr that included the db changes was previously merged #20318. The second pr that added the model, factory, constants, helpers and vcr cassettes was also previously merged #20346. The third pr added evss files and changes for

This pr does the following:

  • Updated codeowners
  • Updated lib/lighthouse/benefits_documents/service.rb so that we add to the evidence_submission table when the ff cst_send_evidence_submission_failure_emails is enabled. Also use the helpers and constants.
  • Deleted app/sidekiq/lighthouse/document_upload.rb and replaced it with app/sidekiq/lighthouse/evidence_submissions/document_upload.rb
  • Added app/sidekiq/lighthouse/evidence_submissions/document_upload.rb to replace app/sidekiq/lighthouse/document_upload.rb so that it is in its own folder. Also updated the evidence_submission table when the ff cst_send_evidence_submission_failure_emails is enabled. Also use the helpers and constants. Updated logic to pass in personalization with an obscured file name.
  • Updated app/sidekiq/lighthouse/failure_notification.rb to expect a personalization object instead of first_name, filename, date_submitted, and date_failed
  • Updated modules/mobile/spec/requests/mobile/v0/claim/documents_spec.rb moved variables around, added user_account so tests passed, and updated logic so that it had the flipper cst_send_evidence_submission_failure_emails enabled and disabled for all tests.
  • Updated spec/lib/lighthouse/benefits_documents/service_spec.rb
  • Updated spec/sidekiq/lighthouse/evidence_submissions/document_upload_spec.rb
  • Updated spec/sidekiq/lighthouse/failure_notification_spec.rb

Related issue(s)

Testing done

  • New code is covered by unit tests
    Describe what the old behavior was prior to the change
  • The code didn't add or update records in the evidence_submissions table

Information for how to add va-notify to settings.local.yaml -> here

How to test that feature flag works when ffs are disabled and we upload a file successfully

  • Make sure benefits_documents_use_lighthouse is enabled
  • Make sure cst_synchronous_evidence_uploads is disabled
  • cst_send_evidence_failure_emails can be disabled or enabled
  • Make sure cst_send_evidence_submission_failure_emails is disabled
  • Locally make sure your settings.local.yml has a vanotify section
  • Update lib/lighthouse/benefits_claims/service.rb with an icn of a user in staging EX: @icn = '1012830712V627751' # icn for user 19
  • Update document_upload and failure_notification so that sidekiq retries are set to 0 for testing locally
  • Update lib/lighthouse/benefits_documents/configuration.rb to the participantId of the staging user that you are using ( you can get this from argo or ask someone with argo access to get this for you) EX: participantId: 600_073_191,
  • Run vets-api and vets-website locally and upload a file
  • Should see that no records were added/updated for evidence_submission table

How to test that feature flag works when ffs are disabled and we upload a file and get an error, causing an email to be sent

  • Make sure benefits_documents_use_lighthouse is enabled
  • Make sure cst_synchronous_evidence_uploads is disabled
  • Make sure cst_send_evidence_failure_emails is enabled
  • Make sure cst_send_evidence_submission_failure_emails is disabled
  • Locally make sure your settings.local.yml has a vanotify section
  • Update lib/lighthouse/benefits_claims/service.rb with an icn of a user in staging EX: @icn = '1012830712V627751' # icn for user 19
  • Update document_upload and failure_notification so that sidekiq retries are set to 0 for testing locally
  • Update failure_notification so that in notify_client.send_email() we replace recipient_identifier: { id_value: icn, id_type: 'ICN' } with email_address: 'YOUR_EMAIL',
  • Update lib/lighthouse/benefits_documents/configuration.rb to the participantId is not being set to a specific staging user (only needed if you were testing previous scenario) --> this is how we will generate an error
  • Run vets-api and vets-website locally and upload a file
  • Should see that no records were added/updated for evidence_submission table and failure notification was called and an email was sent to you for a failed document upload

How to test that feature flag works when ffs are enabled and we upload a file successfully

  • Make sure benefits_documents_use_lighthouse is enabled
  • Make sure cst_synchronous_evidence_uploads is disabled
  • cst_send_evidence_failure_emails can be disabled or enabled
  • Make sure cst_send_evidence_submission_failure_emails is enabled
  • Locally make sure your settings.local.yml has a vanotify section
  • Update lib/lighthouse/benefits_claims/service.rb with an icn of a user in staging EX: @icn = '1012830712V627751' # icn for user 19
  • Update document_upload and failure_notification so that sidekiq retries are set to 0 for testing locally
  • Update lib/lighthouse/benefits_documents/configuration.rb to the participantId of the staging user that you are using ( you can get this from argo or ask someone with argo access to get this for you) EX: participantId: 600_073_191,
  • Run vets-api and vets-website locally and upload a file
  • Should see that 1 record was added/updated for evidence_submission table

How to test that feature flag works when ffs are enabled and we upload a file and get an error, causing an email to be sent

  • Make sure benefits_documents_use_lighthouse is enabled
  • Make sure cst_synchronous_evidence_uploads is disabled
  • cst_send_evidence_failure_emails can be disabled or enabled
  • Make sure cst_send_evidence_submission_failure_emails is enabled
  • Locally make sure your settings.local.yml has a vanotify section
  • Update lib/lighthouse/benefits_claims/service.rb with an icn of a user in staging EX: @icn = '1012830712V627751' # icn for user 19
  • Update document_upload so that sidekiq retries are set to 0 for testing locally
  • Update lib/lighthouse/benefits_documents/configuration.rb to the participantId is not being set to a specific staging user (only needed if you were testing previous scenario) --> this is how we will generate an error
  • Run vets-api and vets-website locally and upload a file
  • Should see that 1 record was added/updated for evidence_submission table (NOTE: in our next pr we will have an email polling job sent to you for a failed document upload and in that job you will - Update a file so that where notify_client.send_email() is called we replace recipient_identifier: { id_value: icn, id_type: 'ICN' } with email_address: 'YOUR_EMAIL',)

Screenshots

Test that feature flag works when benefits_documents_use_lighthouse is enabled, cst_synchronous_evidence_uploads is disabled, cst_send_evidence_failure_emails is enabled or disabled, and cst_send_evidence_submission_failure_emails is disabled. When you upload a file, no evidence submission record is added and file is successfully uploaded:
Screenshot 2025-01-27 at 11 57 41 AM
Screenshot 2025-01-27 at 12 45 55 PM

Test that feature flag works when benefits_documents_use_lighthouse is enabled, cst_send_evidence_failure_emails is enabled, cst_synchronous_evidence_uploads is disabled, and cst_send_evidence_submission_failure_emails is disabled. When you upload a file and an error occurs, no evidence submission record is added and an email is sent for failed upload:
Screenshot 2025-01-27 at 2 26 27 PM
Screenshot 2025-01-27 at 2 26 13 PM

Test that feature flag works when benefits_documents_use_lighthouse is enabled, cst_synchronous_evidence_uploads is disabled, cst_send_evidence_failure_emails is disabled or enabled, and cst_send_evidence_submission_failure_emails is enabled. When you upload a file, no evidence submission record is added and file is successfully uploaded:
Screenshot 2025-01-27 at 1 36 17 PM
Screenshot 2025-01-27 at 1 36 45 PM

Test that feature flag works when benefits_documents_use_lighthouse is enabled, cst_send_evidence_failure_emails is enabled or disabled, cst_synchronous_evidence_uploads is disabled, and cst_send_evidence_submission_failure_emails is enabled. When you upload a file, an evidence submission record is added and updated (it will have a FAILED upload_status and a failed_date) and no email is sent for failed upload (NOTE: an email will eventually be sent when the polling job runs)
Screenshot 2025-01-27 at 2 38 31 PM
Screenshot 2025-01-27 at 2 36 58 PM

What areas of the site does it impact?

Claim Status Tool

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?

Copy link

github-actions bot commented Jan 24, 2025

1 Error
🚫 This PR changes 1275 LoC (not counting whitespace/newlines).

In order to ensure each PR receives the proper attention it deserves, those exceeding
500 will not be reviewed, nor will they be allowed to merge. Please break this PR up into
smaller ones.

If you have reason to believe that this PR should be granted an exception, please see the
Submitting pull requests for approval - FAQ.

File Summary

Files

  • .github/CODEOWNERS (+9/-1)

  • app/sidekiq/lighthouse/document_upload.rb (+0/-68)

  • app/sidekiq/lighthouse/evidence_submissions/document_upload.rb (+141/-0)

  • app/sidekiq/lighthouse/failure_notification.rb (+2/-2)

  • lib/lighthouse/benefits_documents/service.rb (+39/-14)

  • modules/mobile/spec/requests/mobile/v0/claim/documents_spec.rb (+490/-173)

  • spec/lib/lighthouse/benefits_documents/service_spec.rb (+54/-18)

  • spec/sidekiq/evss/document_upload_spec.rb (+1/-2)

  • spec/sidekiq/lighthouse/document_upload_spec.rb (+0/-58)

  • spec/sidekiq/lighthouse/evidence_submissions/document_upload_spec.rb (+197/-0)

  • spec/sidekiq/lighthouse/failure_notification_spec.rb (+4/-2)

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

    *.csv, *.json, *.tsv, *.txt, *.md, 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, *.pdf
    

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

Generated by 🚫 Danger

@va-vfs-bot va-vfs-bot temporarily deployed to 100037-4th-pr-for-silent-failure-work/main/main January 24, 2025 20:19 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 100037-4th-pr-for-silent-failure-work/main/main January 26, 2025 19:13 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 100037-4th-pr-for-silent-failure-work/main/main January 26, 2025 19:58 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 100037-4th-pr-for-silent-failure-work/main/main January 26, 2025 20:18 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 100037-4th-pr-for-silent-failure-work/main/main January 26, 2025 20:42 Inactive
@pmclaren19 pmclaren19 self-assigned this Jan 27, 2025
@va-vfs-bot va-vfs-bot temporarily deployed to 100037-4th-pr-for-silent-failure-work/main/main January 27, 2025 15:52 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 100037-4th-pr-for-silent-failure-work/main/main January 27, 2025 23:35 Inactive
samcoforma
samcoforma previously approved these changes Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants