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

[81760] Adding special error case to ToU provisioner response when user cannot be provisioned #16731

Conversation

bosawt
Copy link
Contributor

@bosawt bosawt commented May 13, 2024

Summary

  • This PR adds an error case to the Provisioner class for Terms of User Controller that raises an error when the user cannot be provisioned

Related issue(s)

Testing done

  • Forced response from update provisioning to ensure that cerner_provisioning was false in response, and then ensured that ToU Controller update_provisioning and accept_and_provision returned an error with the message Account not Provisioned

What areas of the site does it impact?

  • Terms of Use, Authentication

Acceptance criteria

  • Default update_provisioning mock should have the CernerProvisioned: false field to create this error
  • Call terms_of_use_agreements/update_provisioning and terms_of_use_agreements/:version/accept_and_provision
  • Confirm error with the message Account not provisioned

@bosawt bosawt requested a review from a team as a code owner May 13, 2024 21:44
@va-vfs-bot va-vfs-bot temporarily deployed to 81760_update_provisioning_error_response_user_cannot_be_provisioned/main/main May 13, 2024 23:29 Inactive
Copy link
Contributor

@rileyanderson rileyanderson left a comment

Choose a reason for hiding this comment

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

Do we want to change the log here? Maybe the intent is to log that the call itself was successful but it is kinda confusing how we log update provisioning success when provisioning fails. these are the logs we get for a failed provisioning:

Rails -- [MAP][SignUp][Service] update provisioning success, icn: 1008596379V859838
Rails -- [TermsOfUse] [Provisioner] update_provisioning error -- { :icn => "1008596379V859838", :response => { :agreement_signed => true, :opt_out => false, :cerner_provisioned => false, :bypass_eligible => true } }
Rails -- [TermsOfUseAgreementsController] update_provisioning error: Account not Provisioned -- { :icn => "1008596379V859838" }```

Copy link
Contributor

@rileyanderson rileyanderson left a comment

Choose a reason for hiding this comment

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

update_provisioning looks good. I get a 422 when provisioning fails:
Screenshot 2024-05-14 at 9 45 21 AM

accept_and_provision should be good once that error is rescued

@bosawt
Copy link
Contributor Author

bosawt commented May 14, 2024

Do we want to change the log here? Maybe the intent is to log that the call itself was successful but it is kinda confusing how we log update provisioning success when provisioning fails. these are the logs we get for a failed provisioning:

Rails -- [MAP][SignUp][Service] update provisioning success, icn: 1008596379V859838
Rails -- [TermsOfUse] [Provisioner] update_provisioning error -- { :icn => "1008596379V859838", :response => { :agreement_signed => true, :opt_out => false, :cerner_provisioned => false, :bypass_eligible => true } }
Rails -- [TermsOfUseAgreementsController] update_provisioning error: Account not Provisioned -- { :icn => "1008596379V859838" }```

I think we still want to say 'success' but maybe give more context, like the json array with tou status, provision status, etc. Cause it is technically a successful call to that endpoint, it's just the response makes us error

@bosawt bosawt force-pushed the 81760_update_provisioning_error_response_user_cannot_be_provisioned branch from e62eb74 to 250f8db Compare May 14, 2024 21:49
@bosawt bosawt requested a review from rileyanderson May 14, 2024 21:49
@bosawt
Copy link
Contributor Author

bosawt commented May 14, 2024

Do we want to change the log here? Maybe the intent is to log that the call itself was successful but it is kinda confusing how we log update provisioning success when provisioning fails. these are the logs we get for a failed provisioning:

Rails -- [MAP][SignUp][Service] update provisioning success, icn: 1008596379V859838
Rails -- [TermsOfUse] [Provisioner] update_provisioning error -- { :icn => "1008596379V859838", :response => { :agreement_signed => true, :opt_out => false, :cerner_provisioned => false, :bypass_eligible => true } }
Rails -- [TermsOfUseAgreementsController] update_provisioning error: Account not Provisioned -- { :icn => "1008596379V859838" }```

I updated the log to still say update provisioning success, but now it returns the update provisioning response so should be easier to see what's going on there

Copy link
Contributor

@rileyanderson rileyanderson left a comment

Choose a reason for hiding this comment

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

Looks good

@bosawt bosawt merged commit cff6907 into master May 15, 2024
19 checks passed
@bosawt bosawt deleted the 81760_update_provisioning_error_response_user_cannot_be_provisioned branch May 15, 2024 23:05
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.

None yet

3 participants