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

Lighthouse BRD passthrough wrapper for front end. #10728

Merged
merged 0 commits into from
Oct 5, 2022
Merged

Lighthouse BRD passthrough wrapper for front end. #10728

merged 0 commits into from
Oct 5, 2022

Conversation

kylesoskin
Copy link
Contributor

@kylesoskin kylesoskin commented Sep 16, 2022 •
edited
Loading

Description of change

This PR adds a generic API method that is essentially just a passthrough for any component of the front-end to hit/use the Lighthouse Benefits Data Reference API

It is just a pass-through, just hit the same path with the same args as the data you want, but with a different base url

IE

LH BRD End-point
https://sandbox-api.va.gov/services/benefits-reference-data/v1/contention-types?page=1&pageSize=2
To get that data from the front-end
${vets-api}/v0/benefits_reference_data/contention-types?page=1&pageSize=2

This is being made primarily for benefits team to make use of in forms, BUT ANYONE CAN USE THIS for brd data.

Original issue(s)

department-of-veterans-affairs/va.gov-team#37771

Related

Things to know about this PR

  • Are there additions to a settings.yml file? Do they vary by environment? (Yes, for now, but want to see if there is a general api key already in there for opendata)(there is, Im using lighthouse.api_key that is already in there)
  • Is there a feature flag? What is it? (no)
  • Is there some Sentry logging that was added? What alerts are relevant? (no)
  • Are there any Prometheus metrics being collected? What Grafana dashboard were they added do? (no)
  • Are there Swagger docs that were updated? (yes, bare-bones one)
  • Is there any PII concerns or questions? (no)

TODO:

  • Check for generic/general api key already in settings.yml so I dont have to add a specific one for this service
  • Add swagger docs
  • Add tests
  • Use different Error handler that the base one. I want to return better messaging, the current base error handling is not sufficient
  • Change from v0 to v1 to align with LH BRD version? Maybe?

@va-vfs-bot va-vfs-bot temporarily deployed to ksoskin/BRD_generic_end_point/main/main September 16, 2022 23:59 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to ksoskin/BRD_generic_end_point/main/main September 17, 2022 00:12 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to ksoskin/BRD_generic_end_point/main/main September 19, 2022 16:03 Inactive
@github-actions
Copy link

github-actions bot commented Sep 19, 2022 •
edited
Loading

1 Warning
⚠️ This PR changes 233 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/controllers/v0/apidocs_controller.rb (+1/-0)

  • app/controllers/v0/benefits_reference_data_controller.rb (+19/-0)

  • config/routes.rb (+1/-0)

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

  • lib/lighthouse/benefits_reference_data/configuration.rb (+77/-0)

  • lib/lighthouse/benefits_reference_data/service.rb (+43/-0)

  • lib/lighthouse/benefits_reference_data/service_exception.rb (+24/-0)

  • spec/controllers/v0/benefits_reference_data_controller_spec.rb (+51/-0)

  • spec/requests/swagger_spec.rb (+13/-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/

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

Generated by 🚫 Danger

@kylesoskin kylesoskin marked this pull request as ready for review September 19, 2022 19:56
@kylesoskin kylesoskin requested a review from a team September 19, 2022 19:56
@kylesoskin kylesoskin requested review from a team as code owners September 19, 2022 19:56
@va-vfs-bot va-vfs-bot temporarily deployed to ksoskin/BRD_generic_end_point/main/main September 19, 2022 23:03 Inactive
LindseySaari
LindseySaari previously approved these changes Sep 20, 2022
View reviewed changes
Copy link
Contributor

@LindseySaari LindseySaari left a comment

Choose a reason for hiding this comment

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

👍

@kylesoskin
Copy link
Contributor Author

I am on paternity leave for 2 weeks, this change requires additional changes in the DevOps repo before being merged. So no one merge this yet!! I will when I get back.

It can be sent back into draft state if needed. I planned to merge and do prd required DevOps changes today but baby came sooner than expected.

@kylesoskin
Copy link
Contributor Author

kylesoskin commented Oct 1, 2022 •
edited
Loading

TODO later: Support multiple api versions
TODO later: Maybe change from benefits_reference_data -> benefits-reference-data ?

@va-vfs-bot va-vfs-bot temporarily deployed to ksoskin/BRD_generic_end_point/main/main October 5, 2022 15:33 Inactive
@kylesoskin kylesoskin mentioned this pull request Oct 5, 2022
Closed
2 tasks
@kylesoskin kylesoskin merged commit 8ad1a61 into master Oct 5, 2022
@kylesoskin kylesoskin deleted the ksoskin/BRD_generic_end_point branch October 5, 2022 18:03
s-caso added a commit that referenced this pull request Oct 6, 2022
@s-caso @joeniquette @kylesoskin
add keys and values to parameters for toe submission (#10840)
bf5e32e 
* add keys and values to parameters for toe submission

* rubocop fixes

* rubocop fix

* [47821] Add question text for 5655 additional comments field (#10842)

* Add max 400 character length to additional comments

* Push to 450

* Specify question text for additional comments overflow

* new int metadata file (#10843)

see this: https://app.zenhub.com/workspaces/identity-5f5bab705a94c9001ba33734/issues/department-of-veterans-affairs/va.gov-team/47861

* Lighthouse BRD passthrough wrapper for front end. (#10728)

* initial commit of BRD passthrough wrapper for front end.

* rubocop'ing

* Moving these from their own folder into an existing lighthouse folder

* Wrapping this logging, it always returns a generic "service unavailable" otherwise, which is misleading, since the service is available.

Wrapping allows for 403,404 errors to come through. any other errors will be raises the same as they would have been otherwise.

* Laying out config a bit differently

* Adding base settings.yml vals

* Adding bare-bones swagger spec

* Adding some tests

* Checking this a little deeper

* Rubocop

* Rubocop whitespace fix

* Making less verbose desc

* Github tests are failing saying this is untested, adding?

* Figuring out what this test wants I think.

* Missing slash

* Typo fix

* This is actually an array, not an object.

* Remove stray ap

* Copy/paste fix

* add keys and values to parameters for toe submission

* rubocop fixes

* rubocop fix

* update modified_keys variable in submissino service to match vets-service

* removing wrapping hash braces

* upadte params naming

Co-authored-by: Scott James <[email protected]>
Co-authored-by: Joe Niquette <[email protected]>
Co-authored-by: Kyle Soskin <[email protected]>
@kylesoskin
Copy link
Contributor Author

TODO (maintenance task/improvements): being tracked in ticket https://app.zenhub.com/workspaces/benefits-team-1-6138d7b57a2631001a4b7562/issues/department-of-veterans-affairs/va.gov-team/37771

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LindseySaari LindseySaari LindseySaari approved these changes

Assignees
No one assigned
Labels
console-services-review
Projects
Console Services Team
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kylesoskin @LindseySaari @va-vfs-bot