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 API Key Generation Page #2218

Merged
merged 18 commits into from
Jan 31, 2024
Merged

Add API Key Generation Page #2218

merged 18 commits into from
Jan 31, 2024

Conversation

macrael
Copy link
Contributor

@macrael macrael commented Jan 30, 2024

Summary

This PR adds a route "dev/api-access" that allows any user to generate an API key that will allow access to the app for 90 days.

Also adds a Cypress test for api key access

Related issues

https://jiraent.cms.gov/browse/MCR-3833

Screenshots

Screenshot 2024-01-29 at 4 35 10 PM

Test cases covered

Wrote a new cypress test that generates an API key and calls an API

<h2>Resources</h2>
<ul>
<li>
The MC-Review{' '}
Copy link
Contributor

Choose a reason for hiding this comment

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

use &nbsp; not jsx with empty strings to ensure accurate spaces visually - this is in a few places

Copy link
Contributor Author

Choose a reason for hiding this comment

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

huh those were all added by the linter

@@ -136,6 +141,7 @@ if (ldClientId === undefined) {
authMode={authMode}
apolloClient={apolloClient}
s3Client={s3Client}
apiURL={apiURL}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why string this variable through as a prop everywhere ? I think its not needed, it is not used across the application logic ( whereasauthMode is )

I suggest we just access this process.env directly in the AppAccess.tsx component and remove the code changes in App/AppBody/AppRoutes entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, I can do that. Agreed this is annoying to prop drill so deep. Maybe we could have some kind of config context to read from? Anytime we read env vars out side of index.tsx (or apollo_gql.ts on the back end) it smells a little, making it a little harder to test things cleanly

Copy link
Contributor

@pearl-truss pearl-truss left a comment

Choose a reason for hiding this comment

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

This looks great! I left a few comments and a design suggestion would be to display both buttons side by side. When they're both on screen it looks a little off with the current placement to me

Screenshot 2024-01-30 at 4 26 49 PM

<ul>
<li>
<code>
Authorization: Bearer eyJhbGciOiJIU...
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 it could be helpful to include a complete sample cURL request here

@@ -0,0 +1,75 @@
describe('thirdPartyAPIAccess', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's great to have cypress test for this!

@macrael macrael merged commit 5078efd into main Jan 31, 2024
27 checks passed
@macrael macrael deleted the wml-api-key-page branch January 31, 2024 22:55
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.

3 participants