Skip to content

Lg 7463 va mock proofer#6941

Merged
gangelo merged 3 commits intomainfrom
lg-7463-va-mock-proofer
Sep 16, 2022
Merged

Lg 7463 va mock proofer#6941
gangelo merged 3 commits intomainfrom
lg-7463-va-mock-proofer

Conversation

@holytoastr
Copy link
Contributor

@holytoastr holytoastr commented Sep 12, 2022

Provides a basic mock API call for environments where we cannot access the VA API staging environment

@holytoastr holytoastr added the inherited proofing Pull Requests for the Inherited Proofing feature label Sep 12, 2022
@holytoastr holytoastr self-assigned this Sep 12, 2022
@holytoastr holytoastr force-pushed the lg-7463-va-mock-proofer branch 2 times, most recently from fde7ae2 to 3e82a9a Compare September 12, 2022 21:00
Why:
Inherited proofing team needs a way to mock api calls to the va api for testing

changelog: Internal, Inherited Proofing, Adding a mock VA API
@holytoastr holytoastr force-pushed the lg-7463-va-mock-proofer branch from 3e82a9a to 2c1a3c3 Compare September 15, 2022 12:59
Copy link
Contributor

@gangelo gangelo left a comment

Choose a reason for hiding this comment

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

@holytoastr open to whether or not we should include the new object the VA is returning, but my inclination is that we should include it :(

Rather than around the VA API.
@gangelo gangelo self-assigned this Sep 15, 2022
@jscodefix
Copy link
Contributor

In my opinion, there is no reason to validate the auth_code (as being mocked-auth-code-for-testing); why do we care what the auth_code is, just accept anything and return the values. @rnagilla-gsa ... I'll defer to your opinion.

@gangelo
Copy link
Contributor

gangelo commented Sep 16, 2022

In my opinion, there is no reason to validate the auth_code (as being mocked-auth-code-for-testing); why do we care what the auth_code is, just accept anything and return the values. @rnagilla-gsa ... I'll defer to your opinion.

Hi @jscodefix,

That is a good question.

Yes, I did think of just checking if blank?; we at least care about that since this proofer will get called if the Inherited Proofer FF is turned off, in which case the "real" codez should at least pass something; if this is not the case, it will at least eliminate that surprise.

Yes, so, the only time we should ever be calling this proofer is when the Inherited Proofer FF is turned off, in which case the only auth code we should ever be passing to the proofer is 'mocked-auth-code-for-testing' because we'd be using the sinatra app instead.

If the IP FF flag is turned on and we're using the sinatra app (locally or on int), we need to make sure the sinatra app is passing 'mocked-auth-code-for-testing' because we'd be calling the VA staging endpoint which only (currently) recognizes that auth code. I assume you're testing for this in the sinatra app, but there's nothing wrong with a little redundancy.

The only solid reason I can think of removing that guard (and just checking if blank?), is if the VA decides to provide additional mock auth codes that might (for example), force an error of some kind, return different mhv_data, etc. In this case, the sinatra app would provide multiple auth codes, and we'd have to remove/refactor that guard.

We can remove that guard, I just like the peace of mind knowing that the auth code can make its way from the sinatra app all the way to the proofer without surprises.

CC @rnagilla-gsa, @holytoastr

@gangelo gangelo requested a review from jscodefix September 16, 2022 14:03
@gangelo gangelo marked this pull request as draft September 16, 2022 14:05
@gangelo gangelo marked this pull request as ready for review September 16, 2022 14:05
@gangelo gangelo requested review from gangelo and removed request for gangelo September 16, 2022 14:06
@gangelo gangelo merged commit 4982f37 into main Sep 16, 2022
@gangelo gangelo deleted the lg-7463-va-mock-proofer branch September 16, 2022 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

inherited proofing Pull Requests for the Inherited Proofing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants