-
Notifications
You must be signed in to change notification settings - Fork 32
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
feat: Add download csv button to people management #1369
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1369 +/- ##
=======================================
Coverage 85.79% 85.79%
=======================================
Files 609 610 +1
Lines 13384 13429 +45
Branches 2780 2784 +4
=======================================
+ Hits 11483 11522 +39
- Misses 1831 1837 +6
Partials 70 70 ☔ View full report in Codecov by Sentry. |
b15b211
to
2524094
Compare
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.
Just a couple things but looks good!
const expectedHeaders = ['Name', 'Email', 'Joined Organization', 'Enrollments']; | ||
expect(downloadCsv).toHaveBeenCalledWith(expectedFileName, mockData.results, expectedHeaders, expect.any(Function)); | ||
}); | ||
it('download button should handle error returned by the API endpoint.', 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.
Can we add in a test for filtered results?
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 point! The DownloadCsvButton component doesn't have awareness of filters, and this would have to be tested at the level of the PeopleManagementPage. As of right now there are no unit tests for the table part of the PeopleManagementPage, and adding that I feel is a substantial enough task outside the scope of this change to address it as a separate ticket.
test: Add DownloadCsvButton unit test refactor: move getTimeStampedFilename into utils
2524094
to
e611011
Compare
Jira Ticket
This change adds a button for downloading the filtered results of the people table on the People Management tab.
Testing Instructions
npm run start:stage
For all changes
Only if submitting a visual change