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

Bdex/57064 migrate post intent to file to lighthouse #12753

Merged

Conversation

mchae-nava
Copy link
Contributor

@mchae-nava mchae-nava commented May 22, 2023

Summary

  • Implements create_inent_to_file (post) functions for the Lighthouse ITF provider, in addition to some unit tests
  • Flipper flag: 'disability_compensation_lighthouse_intent_to_file_provider'

Related issue(s)

57064

Testing done

  • Unit Tests
  • Rails console

What areas of the site does it impact?

526 Backend; Ideally impact is not noticeable

Acceptance criteria

  • Form 526: no changes to the endpoint functionality
  • Maintain test coverage
  • Documentation of specific steps taken for code complete

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?

@@ -5,21 +5,30 @@
module V0
class BenefitsClaimsController < ApplicationController
def index
claims = service.get_claims
claims = service.get_claims(settings.access_token.client_id, settings.access_token.rsa_key)
Copy link
Contributor

Choose a reason for hiding this comment

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

We removed these arguments on get_claims and get_claim on Friday - so we can avoid changes to Claim Status Tool Team's code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah thanks for noting. I think some of the diffs here are because this branch is downstream from yours; if we removed it, I'll either update with yours, or master after yours merges in

Copy link
Contributor

Choose a reason for hiding this comment

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

hey mark, i think what eddie was trying to say here is that we don't need to change this file at all. we can remove the arguments that pertain to the lighthouse credentials in this file.

Copy link
Contributor

@aurora-a-k-a-lightning aurora-a-k-a-lightning left a comment

Choose a reason for hiding this comment

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

just a few thoughts and a little clean up 🤔

Copy link
Contributor

@aurora-a-k-a-lightning aurora-a-k-a-lightning left a comment

Choose a reason for hiding this comment

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

another note: look at IntentToFilesController#submit settings configuration.
should match #index's pattern

Copy link
Contributor

@aurora-a-k-a-lightning aurora-a-k-a-lightning left a comment

Choose a reason for hiding this comment

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

i think we can remove some code that touches other team's code 😄

@@ -5,21 +5,30 @@
module V0
class BenefitsClaimsController < ApplicationController
def index
claims = service.get_claims
claims = service.get_claims(settings.access_token.client_id, settings.access_token.rsa_key)
Copy link
Contributor

Choose a reason for hiding this comment

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

hey mark, i think what eddie was trying to say here is that we don't need to change this file at all. we can remove the arguments that pertain to the lighthouse credentials in this file.

app/controllers/v0/benefits_claims_controller.rb Outdated Show resolved Hide resolved
app/controllers/v0/benefits_claims_controller.rb Outdated Show resolved Hide resolved
@va-vsp-bot va-vsp-bot requested a deployment to bdex/57064-migrate-post-intent_to_file-to_lighthouse/main/main May 26, 2023 18:37 In progress
@va-vsp-bot va-vsp-bot requested a deployment to bdex/57064-migrate-post-intent_to_file-to_lighthouse/main/main May 26, 2023 19:37 In progress
@va-vsp-bot va-vsp-bot requested a deployment to bdex/57064-migrate-post-intent_to_file-to_lighthouse/main/main May 26, 2023 19:56 In progress
@va-vsp-bot va-vsp-bot requested a deployment to bdex/57064-migrate-post-intent_to_file-to_lighthouse/main/main May 26, 2023 19:57 In progress
Copy link
Contributor

@aurora-a-k-a-lightning aurora-a-k-a-lightning left a comment

Choose a reason for hiding this comment

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

thank you!!!
:awesome" work ^_^

Copy link
Contributor

@ryan-mcneil ryan-mcneil left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@mchae-nava mchae-nava merged commit d7192ab into master May 26, 2023
@mchae-nava mchae-nava deleted the bdex/57064-migrate-post-intent_to_file-to_lighthouse branch May 26, 2023 20:38
ryan-mcneil pushed a commit that referenced this pull request Dec 11, 2023
* implement GET request for Lighthouse intent-to-file

* modify BenefitsClaims::Service to request intent to file data

* extend BenefitsService::Configuration to handle different consumer/client needs

* modify existing BenefitsClaims index and show operations to handle new configuration

* implement GET request for Lighthouse intent-to-file

* modify BenefitsClaims::Service to request intent to file data

* extend BenefitsService::Configuration to handle different consumer/client needs

* modify existing BenefitsClaims index and show operations to handle new configuration

* add parameters to fix evss method, move transform to private method, rm unneeded comment

* add error handling and request specs

* fix a few request specs

* fix some tests to accommodate provider pattern

* fix rubocop errors, add tests to lighthouse itf provider

* Merge branch 'bdex/57063/migrate-get-intent_to_file-to-lighthouse' of https://github.com/department-of-veterans-affairs/vets-api into bdex/57063/migrate-get-intent_to_file-to-lighthouse

* Implement service ITF post

* Add TODO for swagger spec

* implement GET request for Lighthouse intent-to-file

* modify BenefitsClaims::Service to request intent to file data

* extend BenefitsService::Configuration to handle different consumer/client needs

* modify existing BenefitsClaims index and show operations to handle new configuration

* add parameters to fix evss method, move transform to private method, rm unneeded comment

* add error handling and request specs

* fix a few request specs

* fix some tests to accommodate provider pattern

* fix rubocop errors, add tests to lighthouse itf provider

* a little cleanup, fixing errors in lighthouse service, config, and spec

* Revert "Add TODO for swagger spec"

This reverts commit e92bd8b7e2e92dd29820722568ace32109ce5436.

* minor tweak for Rubocop and require needed files

* Correct stomped swagger_spec

* Remove committed debug statements; Update TODOs to be more indexable

* Update parameters of evss provider

* Add template automated tests

* Correct improper identifier in EVSS ITF provider

* Correct LH provider parameters and tests

* Update evss itf spec

* Update LH ITF specs; Attempt VCR continuous integration; Add mock responses

* Remove some TODOs

* Add missing param to service spec

* Some cleanup and missed comments

* Change create to supply ssn on function call instead of initialize; Simpliy current_user interface

* Regenerate VCR cassettes

* Add some future TODO considerations; Fix expects to match data structure

* Fix missed parameter update

* Linting auto-fixes

* Revert changes to benefits_claims_controller

* Commit removing credentials from intent_to_files_controller

* Remove missed comments and unused var assignment

---------

Co-authored-by: Eddie Glenn <eddieglenn@navapbc.com>
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.

6 participants