-
Notifications
You must be signed in to change notification settings - Fork 4
5390 feedback reports admin UI #5486
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
base: develop
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #5486 +/- ##
=========================================
Coverage 92.00% 92.01%
=========================================
Files 471 474 +3
Lines 20162 20269 +107
Branches 439 460 +21
=========================================
+ Hits 18550 18650 +100
- Misses 1531 1534 +3
- Partials 81 85 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
tdrs-frontend/src/components/FeedbackReports/FeedbackReports.jsx
Outdated
Show resolved
Hide resolved
tdrs-frontend/src/components/FeedbackReports/FeedbackReportsHistory.jsx
Outdated
Show resolved
Hide resolved
tdrs-frontend/src/components/FeedbackReports/FeedbackReportsHistory.jsx
Outdated
Show resolved
Hide resolved
jtimpe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work!
tdrs-frontend/src/components/FeedbackReports/FeedbackReportsHistory.jsx
Outdated
Show resolved
Hide resolved
| /** | ||
| * Fetches the upload history from the backend | ||
| */ | ||
| const fetchUploadHistory = useCallback(async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wondering what happens if the number of uploads are more than for example 50? I understand this might never happen but it might make sense to add pagination maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is pagination from the backend by default. Currently only 32 records are returned from the backend. So if more than 32 items are present, then the table will display only those items in pages of 5 because of the PaginatedComponent.
It looks like the SubmissionHistory tables turn backend pagination off and filter on the frontend only. Wouldn't be a good idea here though, since this table doesn't have any subfilters it just shows the entire ReportSource table.
@elipe17 @jtimpe What do y'all think? Do we need to support traversing every stored uploaded report in this table? If so, a better solution than the PaginatedComponent may be needed. Maybe an load the next 32 items when you click the last page.
Or we could use an elipsis to signify more records that could be loaded. Load them when that is clicked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UPDATE: In office hours we discussed writing a follow on ticket to add a year selector which should be good enough. I will also write another ticket for implementing actual pagination in the future.
|
hi @mattcoleanderson if select Feedback Reports in the menu, I'm taken to the home page |
When a report is submitted, the STT is determined by the folder structure of the zip file. For example if the following folder is submitted: The STT will be parsed from the 3rd directory:
This is something we've attempted to do with TANF and FRA uploads before and due to various issues, have settled with not clearing the selection after upload for those submission pages. I think we should do the same for this one.
So the "successful" banner is to indicate that your report was successfully uploaded, not that it was successfully parsed. This directory has various zip files that will fail or pass for various reasons that you can submit, as well as a READ.me that explains why they fail and what their folder structure is: https://github.com/raft-tech/TANF-app/tree/5417-feeback-reports-stt-ui/tdrs-backend/tdpservice/reports/test/data It should help you understand the proper tree structure for the zip file |
|
Couple notes transferring here from our DM -- but all looks good given your answers and my testing! Let's title case the status so that they're consistent with other pages ie: |
|
Second the above. Maybe more of a broader question than for just this PR, but @mattcoleanderson do you happen to know when we made the swap from always-enabled submit buttons with error text (if the user attempts to submit without a file) to the current implementation with disabled state buttons? I'm blanking on whether we had any cross-functional discussion on that. I think at the very least we might need to tweak the behavior of uswds disabled buttons if we keep that to make them play a little nicer with screenreaders. |


Summary of Changes
Provide a brief summary of changes
Pull request closes #5390
How to Test
List the steps to test the PR
These steps are generic, please adjust as necessary.
Deliverables
More details on how deliverables herein are assessed included here.
Deliverable 1: Accepted Features
Checklist of ACs:
lfrohlichand/oradpenningtonconfirmed that ACs are met.Deliverable 2: Tested Code
CodeCov Reportcomment in PR)CodeCov Reportcomment in PR)Deliverable 3: Properly Styled Code
Deliverable 4: Accessible
iamjollyandttran-hubusing Accessibility Insights reveal any errors introduced in this PR?Deliverable 5: Deployed
Deliverable 6: Documented
Deliverable 7: Secure
Deliverable 8: User Research
Research product(s) clearly articulate(s):