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

Champva 94284 pega reporting api connection #19951

Merged
merged 4 commits into from
Dec 31, 2024

Conversation

stevelong00
Copy link
Contributor

@stevelong00 stevelong00 commented Dec 18, 2024

Summary

  • This work is behind a feature toggle (flipper): NO
  • Created a client for the Pega API with some tests to make use of it.
  • We will need to retrieve data from this API in upcoming tickets. With this ticket we have implemented a simple API client and some test code as a proof of concept.
  • I work for the IVC Forms team and we own this module.

Related issue(s)

Testing done

  • New code is covered by unit tests
  • Prior to this change there was no client for the Pega API. With this change, there is a client that is only used by unit test code.
  • In addition to running the test suite, verification can be performed by:
    • Enable the 'hit the production endpoint' test
    • Configure the API key in your local environment as described in the comments
    • Run this test locally
    • Verify the response using a debugger

Screenshots

None

What areas of the site does it impact?

None, this code is only used by unit tests so far.

Acceptance criteria

  • I added unit tests and integration tests for each feature (if applicable).
  • No error nor warning in the console.
  • Events are being sent to the appropriate logging solution
  • Documentation has been updated (link to documentation)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Feature/bug has a monitor built into Datadog (if applicable)
  • If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected
  • I added a screenshot of the developed feature

Requested Feedback

None

@stevelong00 stevelong00 changed the title Champva 94284 paga reporting api connection Champva 94284 pega reporting api connection Dec 19, 2024
@michaelclement
Copy link
Contributor

michaelclement commented Dec 23, 2024

@stevelong00 after troubleshooting with Cindy we found that the headers were automatically getting converted to have an uppercase first letter. Cindy updated her python lambda to grab the headers using appropriate casing and I was able to use your existing code to pull a report in the Argo staging console.

michaelclement
michaelclement previously approved these changes Dec 26, 2024
Comment on lines 403 to 404
prefill: true
pega_api:
api_key: fake_api_key
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we want that prefill line in here (for when users have save-in-progress'd data).

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I've added the production API key to the AWS param store under this path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I wasn't sure if that prefill line was a placeholder or if it was doing something.

Copy link
Contributor

@michaelclement michaelclement left a comment

Choose a reason for hiding this comment

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

Looks great!

Comment on lines +9 to +11
def base_path
'https://bt41mfpkj5.execute-api.us-gov-west-1.amazonaws.com/prod/'
end
Copy link
Contributor

@michaelclement michaelclement Dec 31, 2024

Choose a reason for hiding this comment

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

Not a blocker, but we may want to move this into our AWS param store at some point in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking about that, but this is how paths are being stored elsewhere in similar code so I decided to err on the side of consistency.

@stevelong00 stevelong00 merged commit 6c089e7 into master Dec 31, 2024
39 of 41 checks passed
@stevelong00 stevelong00 deleted the CHAMPVA-94284-paga-reporting-api-connection branch December 31, 2024 18:20
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.

4 participants