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

Add endpoint for EVSS #4370

Merged
merged 11 commits into from
Jun 15, 2020
Merged

Add endpoint for EVSS #4370

merged 11 commits into from
Jun 15, 2020

Conversation

annaswims
Copy link
Contributor

@annaswims annaswims commented Jun 8, 2020

Description of change

For form526 BDD we need to add separation location, we can get that data from the EVSS reference data endpoint intakesites.

Previously, there were two other endpoints that were calling EVSS reference data endpoints through an old/unused authentication scheme so I stripped that out. That means we no longer need the sockisify gem.

Original issue

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

Things to know about this PR

@va-vfs-bot va-vfs-bot temporarily deployed to aec/9735_intake_sites/master June 9, 2020 01:59 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to aec/9735_intake_sites/master June 9, 2020 05:19 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to aec/9735_intake_sites/master June 9, 2020 05:25 Inactive
@annaswims annaswims force-pushed the aec/9735_intake_sites branch from 75ea1c6 to fa105ea Compare June 10, 2020 01:30
@@ -173,7 +173,6 @@ class ApidocsController < ApplicationController
Swagger::Schemas::ConnectedApplications,
Swagger::Schemas::Dependents,
Swagger::Schemas::DismissedStatus,
Swagger::Schemas::Form526::RatedDisabilities,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

duplicate

@va-vfs-bot
Copy link

va-vfs-bot commented Jun 10, 2020

1 Warning
⚠️ You changed 292 LoC. This exceeds our desired maximum of 250.

File Summary

Included Files

  • Gemfile (+0/-1 )
  • app/controllers/v0/apidocs_controller.rb (+1/-1 )
  • app/controllers/v0/disability_compensation_forms_controller.rb (+5/-0 )
  • app/serializers/evss_separation_location_serializer.rb (+5/-0 )
  • config/initializers/faraday_socks.rb (+0/-21 )
  • config/routes.rb (+1/-0 )
  • config/settings.yml (+1/-7 )
  • lib/evss/aws_configuration.rb (+0/-45 )
  • lib/evss/jwt.rb (+0/-45 )
  • lib/evss/reference_data/configuration.rb (+4/-4 )
  • lib/evss/reference_data/intake_sites_response.rb (+25/-0 )
  • lib/evss/reference_data/service.rb (+5/-9 )
  • spec/lib/evss/aws_configuration_spec.rb (+0/-11 )
  • spec/lib/evss/jwt_spec.rb (+0/-48 )
  • spec/lib/evss/reference_data/configuration_spec.rb (+1/-1 )
  • spec/lib/evss/reference_data/service_spec.rb (+19/-1 )
  • spec/request/address_request_spec.rb (+2/-14 )
  • spec/request/swagger_spec.rb (+15/-0 )

Exclusions

  • Gemfile.lock (+0/-1 )
  • app/swagger/requests/disability_compensation_form.rb (+27/-0 )
  • app/swagger/schemas/form526/separation_locations.rb (+22/-0 )
  • spec/support/vcr_cassettes/evss/reference_data/401_malformed.yml (+0/-55 )
  • spec/support/vcr_cassettes/evss/reference_data/countries.yml (+53/-38 )
  • spec/support/vcr_cassettes/evss/reference_data/get_intake_sites.yml (+1047/-0 )
  • spec/support/vcr_cassettes/evss/reference_data/states.yml (+123/-36 )

Note: We exclude the following files when considering PR size

["Gemfile.lock", ".json", "spec/fixtures/", ".txt", "spec/support/vcr_cassettes/", "app/swagger"]

Big PRs are difficult to review and often become stale. Consider breaking this PR up into smaller ones.<

Generated by 🚫 Danger

@@ -194,9 +195,6 @@ evss:
cert_path: ~
key_path: ~
root_ca: ~
jwt:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unused

@@ -444,10 +442,6 @@ betamocks:
cache_dir: /cache
services_config: config/betamocks/services_config.yml

faraday_socks_proxy:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unused - TODO: check if I need to remove from devops repo.

@annaswims annaswims requested a review from omgitsbillryan June 11, 2020 18:58
@annaswims annaswims requested a review from SarahJaine June 11, 2020 20:16
@annaswims annaswims marked this pull request as ready for review June 11, 2020 21:07
@annaswims annaswims requested review from a team as code owners June 11, 2020 21:07
@va-vfs-bot va-vfs-bot temporarily deployed to aec/9735_intake_sites/master June 11, 2020 21:21 Inactive
@annaswims
Copy link
Contributor Author

this pr will fix the EVSS connection in review instances https://github.com/department-of-veterans-affairs/devops/pull/7107

Copy link
Contributor

@erluti erluti left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -517,6 +517,13 @@
end
end

it 'supports getting separation_locations' do
expect(subject).to validate(:get, '/v0/disability_compensation_form/separation_locations', 401)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SarahJaine What I missed earlier when we were talking was the headers argument. The 401 response is unauthenticated.

@annaswims annaswims merged commit e6e72c1 into master Jun 15, 2020
@annaswims annaswims deleted the aec/9735_intake_sites branch June 15, 2020 20:34
Copy link
Contributor

@SarahJaine SarahJaine left a comment

Choose a reason for hiding this comment

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

For posterity, Anna and I review the PR in a video chat this morning. We decided on some future tickets but overall this looks great to me. Way to to Anna! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants