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

Feature/204 swagger UI page #76

Merged
merged 13 commits into from
Apr 26, 2023
Merged

Conversation

maxdiebold-erg
Copy link
Collaborator

Related Issues:

Main Changes:

  • Added Swagger API documentation for the profile /data routes and there corresponding /count routes.
    • I left the contact email blank for now. Also most parameters still need descriptions, since we're still waiting on glossary/tooltip descriptions from EPA.
    • The configuration file is stored with the S3 content, so the filepath shown in the UI would be strange if displayed:

swagger

If we want this JSON file to be publicly accessible, we may need to move it to the public folder; currently, the link to the file is hidden.

Steps To Test:

  1. Go to the API docs page: http://localhost:3000/api-docs.
  2. Confirm that all routes are properly documented, and that trying queries works as expected.

Copy link
Collaborator

@cschwinderg cschwinderg 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! I haven't looked to closely at the openapi file yet.

Can you add a button next to the Glossary and National Downloads buttons that opens the api documentation?

Are you getting these console errors/warnings on your end when you run the app?
image

@maxdiebold-erg
Copy link
Collaborator Author

I am getting those warnings, I'll try checking those repositories to see if anybody else is having those issues.

@maxdiebold-erg
Copy link
Collaborator Author

It looks like the warnings are due to the fact that the Autolinker package is generating source maps when compiling their TypeScript source code, but they aren't including the source code in the NPM package (as raised in gregjacobs/Autolinker.js#396 and discussed generally in facebook/create-react-app#11767). It doesn't look like there is a way to disable this warning for a particular package, but it would only become a problem when trying to debug Autolinker code.

Both swagger-ui and swagger-ui-react include remarkable as a dependency, which includes autolinker, so I think the only way to avoid the warning would be to keep the bundles in the public folder, like LEW does, but that option is a lot heavier.

@maxdiebold-erg
Copy link
Collaborator Author

@cschwinderg I added that button, and standardized the buttons on all pages (except for the Glossary button, since I'm not sure if we want the glossary to be functional on non-ATTAINS pages). The navigation section is starting to look a bit cluttered, especially on small screens, so we may want to switch to a dedicated sub-nav bar instead.

# Conflicts:
#	app/client/src/components/navButton.tsx
#	app/client/src/routes/home.tsx
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@cschwinderg
Copy link
Collaborator

Both swagger-ui and swagger-ui-react include remarkable as a dependency, which includes autolinker, so I think the only way to avoid the warning would be to keep the bundles in the public folder, like LEW does, but that option is a lot heavier.

We may need to keep the bundles in the public folder just to cut down on the noise in the console. We can discuss this lader though. I'll go ahead and merge this in as is.

@cschwinderg cschwinderg merged commit b8bdfa4 into develop Apr 26, 2023
@cschwinderg cschwinderg deleted the feature/204_swagger-ui-page branch April 26, 2023 20:49
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.

2 participants