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

[CST] #93324 - Create webhook endpoints for VA Notify to call to update EvidenceSubmission.va_notify_status #20494

Draft
wants to merge 50 commits into
base: master
Choose a base branch
from

Conversation

samcoforma
Copy link
Contributor

Note: Delete the description statements, complete each step. None are optional, but can be justified as to why they cannot be completed as written. Provide known gaps to testing that may raise the risk of merging to production.

Summary

  • This work is behind a feature toggle (flipper): YES/NO
  • (Summarize the changes that have been made to the platform)
  • (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?)
  • (If introducing a flipper, what is the success criteria being targeted?)

Related issue(s)

  • Link to ticket created in va.gov-team repo OR screenshot of Jira ticket if your team uses Jira
  • Link to previous change of the code/bug (if applicable)
  • Link to epic if not included in ticket

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

Note: Optional

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?

samcoforma and others added 30 commits October 4, 2024 10:22
* Created migration file for new evidence submissions table
* Created initial model for evidence submissions

* updated the CODEOWNERS file for the new evidence submission model
…19033)

* added job_class to evidence_submissions table

* adding recording to EvidenceSubmissions and updated unit tests for Lighthouse and EVSS document uploads

* updated fields that get logged into EvidenceSubmission

* updated UserAccount association for EvidenceSubmissions to be a reference and updated unit tests

* switched from using the build method to new on EvidenceSubmission object initialization

* fixed UserAccount stubbing in legacy_documents_spec.rb to fix failing test

* fixed linting errors

* fixed mobile and regular documents_spec requests tests

* fixed linting error

* removed unneeded comment from migration file
…ng document (#19552)

* working files may be in broken state

* finished unit testing for DocumentUpload

* added testing around raising errors by document_upload, and cleaned up comments

* removed unnecessary comment from worker_service.rb

* moved EvidenceSubmission record creation to DocumentUpload, and returned to using perform_async instead of perform_in

* added blank line at the end of constants.rb

* Updated incorrect error for when a claim id is not present
* Created migration file for new evidence submissions table
* Created initial model for evidence submissions

* updated the CODEOWNERS file for the new evidence submission model
…19033)

* added job_class to evidence_submissions table

* adding recording to EvidenceSubmissions and updated unit tests for Lighthouse and EVSS document uploads

* updated fields that get logged into EvidenceSubmission

* updated UserAccount association for EvidenceSubmissions to be a reference and updated unit tests

* switched from using the build method to new on EvidenceSubmission object initialization

* fixed UserAccount stubbing in legacy_documents_spec.rb to fix failing test

* fixed linting errors

* fixed mobile and regular documents_spec requests tests

* fixed linting error

* removed unneeded comment from migration file
…ng document (#19552)

* working files may be in broken state

* finished unit testing for DocumentUpload

* added testing around raising errors by document_upload, and cleaned up comments

* removed unnecessary comment from worker_service.rb

* moved EvidenceSubmission record creation to DocumentUpload, and returned to using perform_async instead of perform_in

* added blank line at the end of constants.rb

* Updated incorrect error for when a claim id is not present
* added initial evidence submission polling job file and polling service file

* added some more more evidence submission polling related files

* started fleshing out 'perform' in evidence_submission_document_upload_polling_job.rb, added status scopes to EvidenceSubmission model, and started fleshing out update_documents_status_service.rb

* added TODO notes as comments

* made some minor changes

* added new Constants.rb file

* writing tests

* wrote unit tests, and implemented acknowledgment_date, failed_date, and the status updater

* worked on processing timeout and added delete_date

* Created factories for EvidenceSubmission records and updated unit test

* Updated upload_status_updater.rb and wrote unit tests for it

* fixed unit test for spec/sidekiq/lighthouse/evidence_submission_document_upload_polling_job_spec.rb

* commented logging code that we're not using yet

* fixed delete_date model to use datetime instead of date

* fixed upload_status_updater_spec.rb and started working on update_documents_status_service_spec.rb

* updated update_documents_status_service, tests and moved test file to the correct folder

* updated test to have more expects

* update tests to use be_within

* update tests to use be_within

* updated spec/lib/lighthouse/benefits_documents/upload_status_updater_spec.rb

* added extra parentheses around .utc

* restored accidentally deleted error handler function in evidence_submission_document_upload_polling_job

* updated CODEOWNERS file

* removed extra sub-folder called factories from spec/factories/lighthouse/benefits_documents/factories/evidence_submission.rb

* fixed failing test

* fixed merge conflicts

* cleaned up .github/CODEOWNERS

* removed unneeded comment from migration file

---------

Co-authored-by: Peri McLaren <[email protected]>
#19956)

* added back changes from previous pr and started writing tests in document upload files

* add tests

* update tests

* remove byebug

* added codeowners and removed files that were deleted

* add tests

* updates

* added tests and factories

* add test changes

* updated tests

* remove change

* updates

* update failure notification email job

* fixed factory

* add frozen_string_literal: true

* remove comma from codeowners file

* update failure_notification_email_job.rb and test to include lighthouse

* remove and from codeowners file

* update documents_spec.rb to replace Lighthouse::DocumentUpload with Lighthouse::BenefitsDocuments::DocumentUpload

* update service_spec.rb

* updated class

* changed the name of the job

* fix linter issues with line length

* update folder name from benefits_documents to evidence_submissions

* resolved linting issues

* update spacing
… and added back failure_notification and tests
pmclaren19 and others added 19 commits January 8, 2025 10:16
…e factory, move evidence submission creation on lighthouse document upload
…evidence submission there, updated doc upload files to update the evidence submission, fixed tests
@va-vfs-bot va-vfs-bot temporarily deployed to 93324-CST-Create-webhook-endpoints-for-VA-Notify-to-call-to-update-the-Email-status-on-the-table/main/main January 28, 2025 17:37 Inactive
@samcoforma samcoforma self-assigned this Feb 11, 2025
Copy link

1 Error
🚫

Modified files in db/migrate or db/schema.rb changes should be the only files checked into this PR.

File Summary

DB File(s)

  • db/migrate/20250103152522_create_evidence_submissions.rb

App File(s)

  • .github/CODEOWNERS
  • app/sidekiq/evss/document_upload.rb
  • app/sidekiq/lighthouse/evidence_submission_document_upload_polling_job.rb
  • app/sidekiq/lighthouse/failure_notification.rb
  • lib/lighthouse/benefits_documents/va_notify_email_status_callback.rb
  • spec/lib/lighthouse/benefits_documents/va_notify_email_status_callback_spec.rb
  • spec/sidekiq/lighthouse/evidence_submission_document_upload_polling_job_spec.rb

Application code must always be backwards compatible with the DB,
both before and after migrations have been run. For more info:

1 Warning
⚠️ This PR changes 274 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 (+1/-1)

  • app/sidekiq/evss/document_upload.rb (+3/-0)

  • app/sidekiq/lighthouse/evidence_submission_document_upload_polling_job.rb (+45/-0)

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

  • db/migrate/20250103152522_create_evidence_submissions.rb (+23/-0)

  • lib/lighthouse/benefits_documents/va_notify_email_status_callback.rb (+53/-0)

  • spec/lib/lighthouse/benefits_documents/va_notify_email_status_callback_spec.rb (+79/-0)

  • spec/sidekiq/lighthouse/evidence_submission_document_upload_polling_job_spec.rb (+67/-0)

    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

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.

4 participants