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.

Sign up for GitHub

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

Jump to bottom

[DBX-74810] Add flipper with LH BI api for 4142 #16483

Merged
merged 15 commits into from
Apr 30, 2024
Merged

[DBX-74810] Add flipper with LH BI api for 4142 #16483

merged 15 commits into from
Apr 30, 2024

Conversation

SamStuckey
Copy link
Contributor

@SamStuckey SamStuckey commented Apr 24, 2024 •
edited by SloopKoning
Loading

4142 Implement Migration from CentralMail to Benefits Intake

Summary

This solution is in response to issue 74810 which deals specifically with the way that form 21-4142 as part of an overall 21-526ez submission package is uploaded. Currently the upload process uses the CentralMail facility. With this new feature in place the upload will be through the Lighthouse intake route.

  • This work is behind a feature toggle (flipper): YES

disability_compensation_form4142_supplemental

Use Lighthouse API to submit supplemental Form 21-4142 from Form 526EZ submissions

Related issue(s)

Testing done

  • New code is covered by unit tests
  • With the flipper disabled, the old intake route for a form 4142 supplemental within a 21-526ez submission package is used.
  • with the flipper enabled, the new intake route via the Lighthouse API is used.

Acceptance criteria

  • I fixed and 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 or Grafana (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

@va-vfs-bot va-vfs-bot temporarily deployed to dbx/74810-migrate-4142-to-benefits-intake/main/main April 24, 2024 19:01 Inactive
@SamStuckey SamStuckey mentioned this pull request Apr 24, 2024
Closed
@tblackwe
Copy link
Contributor

@SloopKoning please let me know when linting issues are resolved, and spec tests are in. We can sync on the manual testing afterwards.

@va-vfs-bot va-vfs-bot temporarily deployed to dbx/74810-migrate-4142-to-benefits-intake/main/main April 24, 2024 20:51 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to dbx/74810-migrate-4142-to-benefits-intake/main/main April 25, 2024 13:36 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to dbx/74810-migrate-4142-to-benefits-intake/main/main April 25, 2024 14:11 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to dbx/74810-migrate-4142-to-benefits-intake/main/main April 25, 2024 14:17 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to dbx/74810-migrate-4142-to-benefits-intake/main/main April 25, 2024 16:29 Inactive
@SloopKoning
old spec has been updated to use flipper to isolate tests to each pos…
a15662c 
…sible submission route; however, found problem in subject code which needs a little intent research.
@va-vfs-bot va-vfs-bot temporarily deployed to dbx/74810-migrate-4142-to-benefits-intake/main/main April 25, 2024 20:15 Inactive
@SloopKoning
respored proper lineage for the CentralMailResponseError to a common …
9989699 
…exception; updated the associated job spec file to have two parts one for the central mail route and one for the lighthouse route; in the lighthouse section there are two tests that are skipped because there is no lighthouse specific vcr cassette available.
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 26, 2024 •
edited
Loading

1 Warning
⚠️ This PR changes 245 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

  • app/sidekiq/central_mail/submit_form4142_job.rb (+66/-11)

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

  • spec/sidekiq/central_mail/submit_form4142_job_spec.rb (+163/-1)

    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 🚫 Danger

@va-vfs-bot va-vfs-bot temporarily deployed to dbx/74810-migrate-4142-to-benefits-intake/main/main April 26, 2024 03:05 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to dbx/74810-migrate-4142-to-benefits-intake/main/main April 26, 2024 03:10 Inactive
tblackwe
tblackwe approved these changes Apr 26, 2024
View reviewed changes
Copy link
Contributor

@tblackwe tblackwe left a comment

Choose a reason for hiding this comment

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

Reviewed in call with Dewayne. Approved
manually tested in staging instance

@SloopKoning SloopKoning marked this pull request as ready for review April 26, 2024 15:07
@SloopKoning SloopKoning requested review from a team as code owners April 26, 2024 15:07
freeheeling
freeheeling reviewed Apr 26, 2024
View reviewed changes
app/sidekiq/central_mail/submit_form4142_job.rb
upload_url: @lighthouse_service.location,
file: { file: @pdf_path, file_name: @pdf_path.split('/').last },
metadata: generate_metadata.to_json,
attachments: [] # [ wipn8923 ] TODO: is this better than nil?
Copy link
Contributor

Choose a reason for hiding this comment

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

Will an answer to this question be revisited after deployment?

@SloopKoning SloopKoning merged commit fddead9 into master Apr 30, 2024
22 checks passed
@SloopKoning SloopKoning deleted the dbx/74810-migrate-4142-to-benefits-intake branch April 30, 2024 14:26
gabezurita added a commit that referenced this pull request Apr 30, 2024
@gabezurita
Merge branch 'master' into arf.80297/ARP-OGC-number
aedce15 
* master: (33 commits)
  [DBX-74810] Add flipper with LH BI api for 4142 (#16483)
  upadte param dig call (#16570)
  98 add code to poll average days (#16471)
  8147 claim model (#16525)
  Toxic Exposure: Save TE "Add Conditions" fields for future transformation (#16548)
  Revert "ClamAV in Containers (#15965)" (#16557)
  Dangerfile updates for db migration (#16511)
  API-35829-error_handling (#16522)
  add loggers to see what dd api call is doing in staging (#16539)
  8440: fix appointments contract schema (#16528)
  ClamAV in Containers (#15965)
  Protect against calling `.any?` on a non-array (#16547)
  BE | Ask VA Api: Refactor classes and specs (#16542)
  Added '10-10EZR' to the list of freeze schemas (#16478)
  Add prefill for 10-7959c (#16448)
  Bump shrine from 3.5.0 to 3.6.0 (#16532)
  change limit on place and location. update question number (#16537)
  Add words_to_remove for simple forms 21-0966 (#16477)
  updgrade sidekiq and super_diff (#16536)
  BE | Ask Va Api:  Update `Inquiries::Creator` `Correspondences::Creator` Error Handling (#16529)
  ...
@RachalCassity RachalCassity self-assigned this Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@freeheeling freeheeling freeheeling approved these changes

@RachalCassity RachalCassity RachalCassity approved these changes

@aurora-a-k-a-lightning aurora-a-k-a-lightning aurora-a-k-a-lightning approved these changes

@tblackwe tblackwe tblackwe approved these changes

Assignees

@RachalCassity RachalCassity

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@SamStuckey @tblackwe @aurora-a-k-a-lightning @RachalCassity @freeheeling @va-vfs-bot @SloopKoning