Issue 834: Connect frontend to real API endpoints for download & upload #1061
Conversation
…834-upload-download-connect-real-endpoints
…d-download-connect-real-endpoints
…834-upload-download-connect-real-endpoints
jtwillis92
changed the base branch from
raft-tdp-main
to
epics/89/issues/416/download-files-frontend
…834-upload-download-connect-real-endpoints
jtwillis92
changed the title
…834-upload-download-connect-real-endpoints
…r SET_FILE_LIST to correctly rename API fields to frontend expected state variable names
…834-upload-download-connect-real-endpoints
…IST reducer to fix a bug where reports would show for the wrong years
| @@ -152,7 +152,7 @@ class Common(Configuration): | |||
| } | |||
|
|
|||
| # General | |||
| APPEND_SLASH = False | |||
| APPEND_SLASH = True | |||
There was a problem hiding this comment.
This prevents a 404 error from being raised when the trailing slash isn't included at the end of an API URL. Instead it will redirect to add the slash that is needed. Note however that this redirect causes a CORS preflight error on the frontend so we still need to ensure that the trailing slash is present in all UI code.
| @@ -300,6 +300,9 @@ class Common(Configuration): | |||
| "rest_framework.authentication.SessionAuthentication", | |||
| "rest_framework.authentication.TokenAuthentication", | |||
| ), | |||
| "DEFAULT_FILTER_BACKENDS": [ | |||
| "django_filters.rest_framework.DjangoFilterBackend", | |||
There was a problem hiding this comment.
This package was already installed but previously unused due to this setting being missing and no view defining a filter backend.
https://www.django-rest-framework.org/api-guide/filtering/#setting-filter-backends
| """Class metadata linking to the ReportFile and fields accepted.""" | ||
|
|
||
| model = ReportFile | ||
| fields = ['stt', 'quarter', 'year'] |
There was a problem hiding this comment.
This FilterSet allows query parameters to be supplied to the list endpoint which allows the frontend to retrieve only the reports needed for the current selections. Example API call:
GET /v1/reports?stt=10&quarter=Q1&year=2021
| """Filters that can be applied to GET requests as query parameters.""" | ||
|
|
||
| # Override the generated definition for the STT field so we can require it. | ||
| stt = filters.NumberFilter(field_name='stt_id', required=True) |
There was a problem hiding this comment.
Note that this is required when these filters are active. This is important to prevent users from being able to view data for STTs they do not have access to. When this is supplied in the query parameters the permission classes are able to confirm the user has the appropriate access level to see reports for the given STT. Without this being required reports from all STTs can be retrieved by omitting the parameter.
|
|
||
| def filter_queryset(self, queryset): | ||
| """Only apply filters to the list action.""" | ||
| if self.action != 'list': |
There was a problem hiding this comment.
The filters are only relevant to the "list" view (GET /v1/reports) so we disable them for other actions within this viewset otherwise an error will be raised from the STT filter being required. All other actions operate on a single object (POST new report, GET a single report by ID, Download a report by ID)
| # sp we will merge all together and just do one check | ||
| request_parameters = ChainMap( | ||
| view.kwargs, | ||
| request.query_params, |
There was a problem hiding this comment.
To make permissions work for the STT filter on the list endpoint we need to also include the query parameters supplied to the request here.
| @@ -21,6 +20,8 @@ function UploadReport({ handleCancel, header, stt }) { | |||
| // The logged in user in our Redux state | |||
| const user = useSelector((state) => state.auth.user) | |||
|
|
|||
| // TODO: Move this to Redux state so we can modify this value outside of | |||
There was a problem hiding this comment.
Note to self: if I don't get to this open a follow on ticket.
jtwillis92
added
QASP Review
and removed
a11y-review
ADPennington
added
Deploy with CircleCI-qasp
jtwillis92
removed
the
Deploy with CircleCI-qasp
ADPennington
added
the
Deploy with CircleCI-qasp
@ttran-hub this is ready for gov a11y review! Please note that this work will close out the #89 epic. in addition to @reitermb's a11y review for this PR (above), additional a11y reviews related to the download work include: |
ADPennington
added
Deploy with CircleCI-qasp
Download Section # buttons pass A11ly test. Double check "Download Section 3" button, it's not activate file download during test on Chrome and Firefox. cc @iamjolly UPDATE: @jtwillis92, recheck the "Download Section 3" button, seems to resolve after logout and re-login |
There was a problem hiding this comment.
thanks @jtwillis92 👍🏾 @abottoms-coder this is ready to merge
Deliverable 1: Accepted Features
Performance Standard(s): At the beginning of each sprint, the Product Owner and development team will collaborate to define a set of user stories to be completed during the sprint. Acceptance criteria for each story will also be defined. The development team will deliver code and functionality to satisfy these user stories.
Acceptable Quality Level: Delivered code meets the acceptance criteria for each user story. Incomplete stories will be assessed and considered for inclusion in the next sprint.
- Look up the acceptance criteria in the related issue; paste ACs below in checklist format.
- Check against the criteria:
- Frontend uploads hit the actual API endpoint
- Frontend downloads from the actual API endpoint
beyond scope of this PR, but noticed during testing that most recent version is not served to download. this will be addressed as part of Uploads: Implement Versioning for Data Files #1007
As Product Owner, @lfrohlich will decide if ACs are met.
Deliverable 2: Tested Code
Performance Standard(s): Code delivered under the order must have substantial test code coverage. Version-controlled HHS GitHub repository of code that comprises products that will remain in the government domain.
Acceptable Quality Level: Minimum of 90% test coverage of all code. All areas of code are meaningfully tested.
- Are all areas of code introduced in this PR meaningfully tested?
- If this PR introduces backend code changes, are they meaningfully tested?
- If this PR introduces frontend code changes, are they meaningfully tested?
- Are code coverage minimums met?
- Frontend coverage: ___ (see https://github.com/raft-tech/TANF-app README coverage dashboard)
- Backend coverage: ___ (see https://github.com/raft-tech/TANF-app README coverage dashboard)
Deliverable 3: Properly Styled Code
Performance Standard(s): GSA 18F Front- End Guide
Acceptable Quality Level: 0 linting errors and 0 warnings
- Are backend code style checks passing on CircleCI?
- Are frontend code style checks passing on CircleCI?
Deliverable 4: Accessible
Performance Standard(s): Web Content Accessibility Guidelines 2.1 AA standards
Acceptable Quality Level: 0 errors reported using an automated scanner and 0 errors reported in manual testing
- Does this PR complete the epic?
- Are links included to any other gov-approved PRs associated with epic?
- Does PR include documentation for Raft's a11y review?
Issue 834: Connect frontend to real API endpoints for download & upload #1061 (comment) - Did automated and manual testing with @iamjolly and @ttran-hub using Accessibility Insights reveal any errors introduced in this PR?
Issue 834: Connect frontend to real API endpoints for download & upload #1061 (comment)
couple of follow-on tickets #1107 and #1108 created for more a11y improvements
Deliverable 5: Deployed
Performance Standard(s): Code must successfully build and deploy into the staging environment.
Acceptable Quality Level: Successful build with a single command
- Was the code successfully deployed via automated CircleCI process to development on Cloud.gov?
Deliverable 6: Documented
Performance Standard(s): Summary of user stories completed every two weeks. All dependencies are listed and the licenses are documented. Major functionality in the software/source code is documented, including system diagram. Individual methods are documented inline in a format that permits the use of tools such as JSDoc. All non-inherited 800-53 system security controls are documented in the Open Control or OSCAL format and HHS Section 508 Product Assessment Template (PAT) are updated as appropriate.
Acceptable Quality Level: Combination of manual review and automated testing, if available
- If this PR introduces backend code, is that code documented both inline and overall?
- If this PR introduces frontend code, is that code documented both inline and overall?
Deliverable 7: Secure
Performance Standard(s): Open Web Application Security Project (OWASP) Application Security Verification Standard 3.0
Acceptable Quality Level: Code submitted must be free of medium- and high-level static and dynamic security vulnerabilities
- Does the OWASP Scan pass on CircleCI?
- Do manual code review and manual testing detect any security issues?
no new issues detected
ADPennington
added
Ready to Merge
and removed
Deploy with CircleCI-qasp
andrew-jameson
deleted the
feat/834-upload-download-connect-real-endpoints
branch
Summary of Changes
Utilizes the API endpoints for file download and upload in the frontend instead of mocked endpoints in Mirage.
Additionally, adds filtering capabilities to the list endpoint (
/v1/reports) and expands upon the permissions classes to ensure that STT users cannot see ReportFiles for STTs they do not have access to.Closes #834
How to Test
As a Data Prepper
As an OFA Admin
Deliverable 1: Accepted Features
As facilitator/product manager, @kniz-raft will decide if ACs are met from Raft's perspective.
Deliverable 2: Tested Code
Deliverable 3: Properly Styled Code
Deliverable 4: Accessible
Deliverable 5: Deployed
Deliverable 6: Documented
Deliverable 7: Secure