From c82c49ea598af290b84d93d59cd363620508acfc Mon Sep 17 00:00:00 2001 From: Zach Hancock Date: Wed, 23 Jan 2019 15:55:16 -0500 Subject: [PATCH] persist assignment type filter --- src/components/Gradebook/index.jsx | 24 +++++----- src/components/PageButtons/index.jsx | 11 +++-- src/containers/GradebookPage/index.jsx | 13 ++--- src/data/actions/grades.js | 35 ++++++++++---- src/data/actions/grades.test.js | 66 ++++++++++++++++++++++---- src/data/actions/roles.js | 2 +- src/data/actions/utils.js | 2 + src/data/reducers/grades.js | 1 + src/data/store.js | 1 + 9 files changed, 117 insertions(+), 38 deletions(-) diff --git a/src/components/Gradebook/index.jsx b/src/components/Gradebook/index.jsx index 0453020..8f68918 100644 --- a/src/components/Gradebook/index.jsx +++ b/src/components/Gradebook/index.jsx @@ -120,6 +120,8 @@ export default class Gradebook extends React.Component { updateAssignmentTypes = (event) => { this.props.filterColumns(event, this.props.grades[0]); + const updatedQueryStrings = this.updateQueryParams('assignmentType', event); + this.props.history.push(updatedQueryStrings); } updateTracks = (event) => { @@ -132,8 +134,10 @@ export default class Gradebook extends React.Component { this.props.match.params.courseId, this.props.selectedCohort, selectedTrackSlug, + this.props.selectedAssignmentType, ); - this.updateQueryParams('track', selectedTrackSlug); + const updatedQueryStrings = this.updateQueryParams('track', selectedTrackSlug); + this.props.history.push(updatedQueryStrings); }; updateCohorts = (event) => { @@ -146,19 +150,11 @@ export default class Gradebook extends React.Component { this.props.match.params.courseId, selectedCohortId, this.props.selectedTrack, + this.props.selectedAssignmentType, ); this.updateQueryParams('cohort', selectedCohortId); }; - mapSelectedAssignmentTypeEntry = (entry) => { - const selectedAssignmentTypeEntry = this.props.assignmentTypes - .find(x => x.id === parseInt(entry, 10)); - if (selectedAssignmentTypeEntry) { - return selectedAssignmentTypeEntry.name; - } - return 'All'; - }; - mapSelectedCohortEntry = (entry) => { const selectedCohortEntry = this.props.cohorts.find(x => x.id === parseInt(entry, 10)); if (selectedCohortEntry) { @@ -296,7 +292,7 @@ export default class Gradebook extends React.Component { @@ -335,6 +331,7 @@ export default class Gradebook extends React.Component { value, this.props.selectedCohort, this.props.selectedTrack, + this.props.selectedAssignmentType, ) } onChange={filterValue => this.setState({ filterValue })} @@ -343,6 +340,7 @@ export default class Gradebook extends React.Component { this.props.match.params.courseId, this.props.selectedCohort, this.props.selectedTrack, + this.props.selectedAssignmentType, ) } value={this.state.filterValue} @@ -457,6 +455,9 @@ Gradebook.propTypes = { columnSortable: PropTypes.bool, onSort: PropTypes.func, })).isRequired, + history: PropTypes.shape({ + push: PropTypes.func, + }).isRequired, location: PropTypes.shape({ search: PropTypes.string, }), @@ -466,6 +467,7 @@ Gradebook.propTypes = { }), }), searchForUser: PropTypes.func.isRequired, + selectedAssignmentType: PropTypes.string.isRequired, selectedCohort: PropTypes.shape({ name: PropTypes.string, }), diff --git a/src/components/PageButtons/index.jsx b/src/components/PageButtons/index.jsx index e79981e..005f4a5 100644 --- a/src/components/PageButtons/index.jsx +++ b/src/components/PageButtons/index.jsx @@ -4,7 +4,8 @@ import { Button } from '@edx/paragon'; export default function PageButtons({ - prevPage, nextPage, selectedTrack, selectedCohort, getPrevNextGrades, match, + prevPage, nextPage, selectedTrack, selectedCohort, selectedAssignmentType, + getPrevNextGrades, match, }) { return (
getPrevNextGrades( prevPage, + match.params.courseId, selectedCohort, selectedTrack, - match.params.courseId, + selectedAssignmentType, )} />
@@ -51,6 +54,7 @@ PageButtons.defaultProps = { prevPage: '', selectedCohort: null, selectedTrack: null, + selectedAssignmentType: null, }; PageButtons.propTypes = { @@ -62,6 +66,7 @@ PageButtons.propTypes = { }), nextPage: PropTypes.string, prevPage: PropTypes.string, + selectedAssignmentType: PropTypes.string, selectedCohort: PropTypes.shape({ name: PropTypes.string, }), diff --git a/src/containers/GradebookPage/index.jsx b/src/containers/GradebookPage/index.jsx index 164e201..ef68cc0 100644 --- a/src/containers/GradebookPage/index.jsx +++ b/src/containers/GradebookPage/index.jsx @@ -32,6 +32,7 @@ const mapStateToProps = state => ( cohorts: state.cohorts.results, selectedTrack: state.grades.selectedTrack, selectedCohort: state.grades.selectedCohort, + selectedAssignmentType: state.grades.selectedAssignmentType, format: state.grades.gradeFormat, showSuccess: state.grades.showSuccess, prevPage: state.grades.prevPage, @@ -45,14 +46,14 @@ const mapStateToProps = state => ( const mapDispatchToProps = dispatch => ( { - getUserGrades: (courseId, cohort, track) => { - dispatch(fetchGrades(courseId, cohort, track)); + getUserGrades: (courseId, cohort, track, assignmentType) => { + dispatch(fetchGrades(courseId, cohort, track, assignmentType)); }, - searchForUser: (courseId, searchText, cohort, track) => { - dispatch(fetchMatchingUserGrades(courseId, searchText, cohort, track, false)); + searchForUser: (courseId, searchText, cohort, track, assignmentType) => { + dispatch(fetchMatchingUserGrades(courseId, searchText, cohort, track, assignmentType, false)); }, - getPrevNextGrades: (endpoint, cohort, track, courseId) => { - dispatch(fetchPrevNextGrades(endpoint, cohort, track, courseId)); + getPrevNextGrades: (endpoint, courseId, cohort, track, assignmentType) => { + dispatch(fetchPrevNextGrades(endpoint, courseId, cohort, track, assignmentType)); }, getCohorts: (courseId) => { dispatch(fetchCohorts(courseId)); diff --git a/src/data/actions/grades.js b/src/data/actions/grades.js index f2b8e8c..a2a69c3 100644 --- a/src/data/actions/grades.js +++ b/src/data/actions/grades.js @@ -32,11 +32,12 @@ const sortGrades = (columnName, direction) => { const startedFetchingGrades = () => ({ type: STARTED_FETCHING_GRADES }); const finishedFetchingGrades = () => ({ type: FINISHED_FETCHING_GRADES }); const errorFetchingGrades = () => ({ type: ERROR_FETCHING_GRADES }); -const gotGrades = (grades, cohort, track, headings, prev, next, courseId) => ({ +const gotGrades = (grades, cohort, track, assignmentType, headings, prev, next, courseId) => ({ type: GOT_GRADES, grades, cohort, track, + assignmentType, headings, prev, next, @@ -67,7 +68,7 @@ const filterColumns = (filterType, exampleUser) => ( const updateBanner = showSuccess => ({ type: UPDATE_BANNER, showSuccess }); -const fetchGrades = (courseId, cohort, track, showSuccess) => ( +const fetchGrades = (courseId, cohort, track, assignmentType, showSuccess) => ( (dispatch) => { dispatch(startedFetchingGrades()); return LmsApiService.fetchGradebookData(courseId, null, cohort, track) @@ -77,7 +78,8 @@ const fetchGrades = (courseId, cohort, track, showSuccess) => ( data.results.sort(sortAlphaAsc), cohort, track, - headingMapper(defaultAssignmentFilter)(dispatch, data.results[0]), + assignmentType, + headingMapper(assignmentType || defaultAssignmentFilter)(dispatch, data.results[0]), data.previous, data.next, courseId, @@ -91,7 +93,14 @@ const fetchGrades = (courseId, cohort, track, showSuccess) => ( } ); -const fetchMatchingUserGrades = (courseId, searchText, cohort, track, showSuccess) => ( +const fetchMatchingUserGrades = ( + courseId, + searchText, + cohort, + track, + assignmentType, + showSuccess, +) => ( (dispatch) => { dispatch(startedFetchingGrades()); return LmsApiService.fetchGradebookData(courseId, searchText, cohort, track) @@ -101,7 +110,8 @@ const fetchMatchingUserGrades = (courseId, searchText, cohort, track, showSucces data.results.sort(sortAlphaAsc), cohort, track, - headingMapper(defaultAssignmentFilter)(dispatch, data.results[0]), + assignmentType, + headingMapper(assignmentType || defaultAssignmentFilter)(dispatch, data.results[0]), data.previous, data.next, courseId, @@ -115,7 +125,7 @@ const fetchMatchingUserGrades = (courseId, searchText, cohort, track, showSucces } ); -const fetchPrevNextGrades = (endpoint, cohort, track, courseId) => ( +const fetchPrevNextGrades = (endpoint, courseId, cohort, track, assignmentType) => ( (dispatch) => { dispatch(startedFetchingGrades()); return apiClient.get(endpoint) @@ -125,7 +135,8 @@ const fetchPrevNextGrades = (endpoint, cohort, track, courseId) => ( data.results.sort(sortAlphaAsc), cohort, track, - headingMapper(defaultAssignmentFilter)(dispatch, data.results[0]), + assignmentType, + headingMapper(assignmentType || defaultAssignmentFilter)(dispatch, data.results[0]), data.previous, data.next, courseId, @@ -138,7 +149,6 @@ const fetchPrevNextGrades = (endpoint, cohort, track, courseId) => ( } ); - const updateGrades = (courseId, updateData, searchText, cohort, track) => ( (dispatch) => { dispatch(gradeUpdateRequest()); @@ -146,7 +156,14 @@ const updateGrades = (courseId, updateData, searchText, cohort, track) => ( .then(response => response.data) .then((data) => { dispatch(gradeUpdateSuccess(courseId, data)); - dispatch(fetchMatchingUserGrades(courseId, searchText, cohort, track, true)); + dispatch(fetchMatchingUserGrades( + courseId, + searchText, + cohort, + track, + defaultAssignmentFilter, + true, + )); }) .catch((error) => { dispatch(gradeUpdateFailure(courseId, error)); diff --git a/src/data/actions/grades.test.js b/src/data/actions/grades.test.js index b029ed7..48dc4d6 100644 --- a/src/data/actions/grades.test.js +++ b/src/data/actions/grades.test.js @@ -27,6 +27,7 @@ describe('actions', () => { const courseId = 'course-v1:edX+DemoX+Demo_Course'; const expectedCohort = 1; const expectedTrack = 'verified'; + const expectedAssignmentType = 'Exam'; const fetchGradesURL = `${configuration.LMS_BASE_URL}/api/grades/v1/gradebook/${courseId}/?page_size=10&cohort_id=${expectedCohort}&enrollment_mode=${expectedTrack}`; const responseData = { next: `${fetchGradesURL}&cursor=2344fda`, @@ -94,6 +95,7 @@ describe('actions', () => { grades: responseData.results.sort(sortAlphaAsc), cohort: expectedCohort, track: expectedTrack, + assignmentType: expectedAssignmentType, headings: [ { columnSortable: true, @@ -120,10 +122,15 @@ describe('actions', () => { axiosMock.onGet(fetchGradesURL) .replyOnce(200, JSON.stringify(responseData)); - return store.dispatch(fetchGrades(courseId, expectedCohort, expectedTrack, false)) - .then(() => { - expect(store.getActions()).toEqual(expectedActions); - }); + return store.dispatch(fetchGrades( + courseId, + expectedCohort, + expectedTrack, + expectedAssignmentType, + false, + )).then(() => { + expect(store.getActions()).toEqual(expectedActions); + }); }); it('dispatches failure action after fetching grades', () => { @@ -136,10 +143,53 @@ describe('actions', () => { axiosMock.onGet(fetchGradesURL) .replyOnce(500, JSON.stringify({})); - return store.dispatch(fetchGrades(courseId, expectedCohort, expectedTrack, false)) - .then(() => { - expect(store.getActions()).toEqual(expectedActions); - }); + return store.dispatch(fetchGrades( + courseId, + expectedCohort, + expectedTrack, + expectedAssignmentType, + false, + )).then(() => { + expect(store.getActions()).toEqual(expectedActions); + }); + }); + + it('dispatches success action on empty response after fetching grades', () => { + const emptyResponseData = { + next: responseData.next, + previous: responseData.previous, + results: [], + }; + const expectedActions = [ + { type: STARTED_FETCHING_GRADES }, + { + type: GOT_GRADES, + grades: [], + cohort: expectedCohort, + track: expectedTrack, + assignmentType: expectedAssignmentType, + headings: [], + prev: responseData.previous, + next: responseData.next, + courseId, + }, + { type: FINISHED_FETCHING_GRADES }, + { type: UPDATE_BANNER, showSuccess: false }, + ]; + const store = mockStore(); + + axiosMock.onGet(fetchGradesURL) + .replyOnce(200, JSON.stringify(emptyResponseData)); + + return store.dispatch(fetchGrades( + courseId, + expectedCohort, + expectedTrack, + expectedAssignmentType, + false, + )).then(() => { + expect(store.getActions()).toEqual(expectedActions); + }); }); }); }); diff --git a/src/data/actions/roles.js b/src/data/actions/roles.js index 79af33a..f66f88a 100644 --- a/src/data/actions/roles.js +++ b/src/data/actions/roles.js @@ -26,7 +26,7 @@ const getRoles = (courseId, urlQuery) => ( && allowedRoles.includes(role.role))); dispatch(gotRoles(canUserViewGradebook, courseId)); if (canUserViewGradebook) { - dispatch(fetchGrades(courseId, urlQuery.cohort, urlQuery.track)); + dispatch(fetchGrades(courseId, urlQuery.cohort, urlQuery.track, urlQuery.assignmentType)); dispatch(fetchTracks(courseId)); dispatch(fetchCohorts(courseId)); dispatch(fetchAssignmentTypes(courseId)); diff --git a/src/data/actions/utils.js b/src/data/actions/utils.js index cf5d677..fe747d1 100644 --- a/src/data/actions/utils.js +++ b/src/data/actions/utils.js @@ -92,6 +92,8 @@ const headingMapper = (filterKey) => { } function some(dispatch, entry) { + if (!entry) return []; + const results = [{ label: 'Username', key: 'username', diff --git a/src/data/reducers/grades.js b/src/data/reducers/grades.js index 51a7167..6bf355e 100644 --- a/src/data/reducers/grades.js +++ b/src/data/reducers/grades.js @@ -32,6 +32,7 @@ const grades = (state = initialState, action) => { errorFetching: false, selectedTrack: action.track, selectedCohort: action.cohort, + selectedAssignmentType: action.assignmentType, prevPage: action.prev, nextPage: action.next, showSpinner: false, diff --git a/src/data/store.js b/src/data/store.js index 48a2eb4..44fef0a 100755 --- a/src/data/store.js +++ b/src/data/store.js @@ -24,6 +24,7 @@ const eventsMap = { courseId: action.courseId, track: action.track, cohort: action.cohort, + assignmentType: action.assignmentType, prev: action.prev, next: action.next, },