From 0557f29e0bc0735fdf9325b5268069e0d7fc2612 Mon Sep 17 00:00:00 2001 From: Matt Hughes Date: Mon, 1 Jul 2019 17:28:28 -0400 Subject: [PATCH 1/4] Add table corresponding to what new history endpoint supports --- src/components/Gradebook/index.jsx | 16 ++++++++++++++++ src/containers/GradebookPage/index.jsx | 6 ++++-- src/data/actions/grades.js | 14 +++++++++++++- src/data/actions/tracks.js | 5 +++++ src/data/constants/actionTypes/grades.js | 2 ++ src/data/reducers/grades.js | 14 +++++++++++++- src/data/selectors/tracks.js | 15 ++++++++++++--- src/data/services/LmsApiService.js | 7 ++++++- 8 files changed, 71 insertions(+), 8 deletions(-) diff --git a/src/components/Gradebook/index.jsx b/src/components/Gradebook/index.jsx index 7253000..63aaf3c 100644 --- a/src/components/Gradebook/index.jsx +++ b/src/components/Gradebook/index.jsx @@ -487,6 +487,16 @@ export default class Gradebook extends React.Component { buttonType="primary" onClick={this.handleClickImportGrades} /> + )} @@ -513,6 +523,7 @@ Gradebook.defaultProps = { tracks: [], bulkImportError: '', showBulkManagement: false, + bulkManagementHistory: [], }; Gradebook.propTypes = { @@ -565,4 +576,9 @@ Gradebook.propTypes = { submitFileUploadFormData: PropTypes.func.isRequired, bulkImportError: PropTypes.string, showBulkManagement: PropTypes.bool, + bulkManagementHistory: PropTypes.arrayOf(PropTypes.shape({ + operation: PropTypes.oneOf(['commit', 'error']), + user: PropTypes.number, + modified: PropTypes.string, + })), }; diff --git a/src/containers/GradebookPage/index.jsx b/src/containers/GradebookPage/index.jsx index 76524e6..72e375f 100644 --- a/src/containers/GradebookPage/index.jsx +++ b/src/containers/GradebookPage/index.jsx @@ -13,7 +13,7 @@ import { } from '../../data/actions/grades'; import { fetchCohorts } from '../../data/actions/cohorts'; import { fetchTracks } from '../../data/actions/tracks'; -import hasMastersTrack from '../../data/selectors/tracks'; +import stateHasMastersTrack from '../../data/selectors/tracks'; import { fetchAssignmentTypes } from '../../data/actions/assignmentTypes'; import { getRoles } from '../../data/actions/roles'; import LmsApiService from '../../data/services/LmsApiService'; @@ -53,7 +53,9 @@ const mapStateToProps = (state, ownProps) => ( state.grades.bulkManagement.errorMessages ? `Errors while processing: ${state.grades.bulkManagement.errorMessages.join(', ')}` : '', - showBulkManagement: hasMastersTrack(state), + showBulkManagement: stateHasMastersTrack(state), + // TODO: selector + bulkManagementHistory: state.grades.bulkManagement.history, } ); diff --git a/src/data/actions/grades.js b/src/data/actions/grades.js index 20a98a5..045f640 100644 --- a/src/data/actions/grades.js +++ b/src/data/actions/grades.js @@ -13,6 +13,7 @@ import { START_UPLOAD, UPLOAD_COMPLETE, UPLOAD_ERR, + GOT_BULK_HISTORY, } from '../constants/actionTypes/grades'; import LmsApiService from '../services/LmsApiService'; import { headingMapper, sortAlphaAsc } from './utils'; @@ -23,6 +24,7 @@ const defaultAssignmentFilter = 'All'; const startedCsvUpload = () => ({ type: START_UPLOAD }); const finishedCsvUpload = () => ({ type: UPLOAD_COMPLETE }); const csvUploadError = data => ({ type: UPLOAD_ERR, data }); +const gotBulkHistory = data => ({ type: GOT_BULK_HISTORY, data }); const startedFetchingGrades = () => ({ type: STARTED_FETCHING_GRADES }); const finishedFetchingGrades = () => ({ type: FINISHED_FETCHING_GRADES }); @@ -163,7 +165,7 @@ const submitFileUploadFormData = (courseId, formData) => ( return LmsApiService.uploadGradeCsv(courseId, formData).then(() => ( dispatch(finishedCsvUpload()) )).catch((err) => { - if (err.status === 200 && err.data.error_messages.length) { + if (err.response.status === 200 && err.response.error_messages.length) { const { error_messages: errorMessages, saved, total } = err.data; return dispatch(csvUploadError({ errorMessages, saved, total })); } @@ -172,6 +174,15 @@ const submitFileUploadFormData = (courseId, formData) => ( } ); +const fetchBulkUpgradeHistory = courseId => ( + dispatch => + // todo add loading effect + LmsApiService.fetchGradeBulkOperationHistory(courseId).then((response) => { + dispatch(gotBulkHistory(response)); + // todo: thread action through action + }).catch(() => dispatch({ type: 'BULK_HISTORY_ERR' })) +); + export { startedFetchingGrades, finishedFetchingGrades, @@ -188,4 +199,5 @@ export { filterColumns, closeBanner, submitFileUploadFormData, + fetchBulkUpgradeHistory, }; diff --git a/src/data/actions/tracks.js b/src/data/actions/tracks.js index 81f43cc..39eb79a 100644 --- a/src/data/actions/tracks.js +++ b/src/data/actions/tracks.js @@ -3,6 +3,8 @@ import { GOT_TRACKS, ERROR_FETCHING_TRACKS, } from '../constants/actionTypes/tracks'; +import { hasMastersTrack } from '../selectors/tracks'; +import { fetchBulkUpgradeHistory } from './grades'; import LmsApiService from '../services/LmsApiService'; const startedFetchingTracks = () => ({ type: STARTED_FETCHING_TRACKS }); @@ -16,6 +18,9 @@ const fetchTracks = courseId => ( .then(response => response.data) .then((data) => { dispatch(gotTracks(data.course_modes)); + if (hasMastersTrack(data.course_modes)) { + dispatch(fetchBulkUpgradeHistory(courseId)); + } }) .catch(() => { dispatch(errorFetchingTracks()); diff --git a/src/data/constants/actionTypes/grades.js b/src/data/constants/actionTypes/grades.js index 77921f7..87aa3a3 100644 --- a/src/data/constants/actionTypes/grades.js +++ b/src/data/constants/actionTypes/grades.js @@ -15,6 +15,7 @@ const OPEN_BANNER = 'OPEN_BANNER'; const START_UPLOAD = 'START_UPLOAD'; const UPLOAD_COMPLETE = 'UPLOAD_COMPLETE'; const UPLOAD_ERR = 'UPLOAD_ERR'; +const GOT_BULK_HISTORY = 'GOT_BULK_HISTORY'; export { STARTED_FETCHING_GRADES, @@ -31,4 +32,5 @@ export { START_UPLOAD, UPLOAD_COMPLETE, UPLOAD_ERR, + GOT_BULK_HISTORY, }; diff --git a/src/data/reducers/grades.js b/src/data/reducers/grades.js index e6f7339..9c8f70d 100644 --- a/src/data/reducers/grades.js +++ b/src/data/reducers/grades.js @@ -9,6 +9,7 @@ import { START_UPLOAD, UPLOAD_COMPLETE, UPLOAD_ERR, + GOT_BULK_HISTORY, } from '../constants/actionTypes/grades'; const initialState = { @@ -22,6 +23,7 @@ const initialState = { prevPage: null, nextPage: null, showSpinner: true, + bulkManagement: {}, }; const grades = (state = initialState, action) => { @@ -90,10 +92,20 @@ const grades = (state = initialState, action) => { ...state, showSpinner: false, bulkManagement: { - ...(state.bulkManagement || {}), + ...state.bulkManagement, ...action.data, }, }; + case GOT_BULK_HISTORY: + return { + ...state, + // TODO: this will be cleared if we successfully upload a new one; + // probably want to trigger a reload of this info instead + bulkManagement: { + ...state.bulkManagement, + history: action.data, + }, + }; default: return state; } diff --git a/src/data/selectors/tracks.js b/src/data/selectors/tracks.js index 6d755f6..d7e40b0 100644 --- a/src/data/selectors/tracks.js +++ b/src/data/selectors/tracks.js @@ -1,4 +1,13 @@ -const getTracks = state => state.tracks.results || []; -const hasMastersTrack = state => getTracks(state).some(track => track.slug === 'masters'); +const compose = (...fns) => { + const [firstFunc, ...rest] = fns.reverse(); + return (...args) => + rest.reduce((accum, fn) => fn(accum), firstFunc(...args)); +}; -export default hasMastersTrack; +const getTracks = state => state.tracks.results || []; +const trackIsMasters = track => track.slug === 'masters'; +const hasMastersTrack = tracks => tracks.some(trackIsMasters); +const stateHasMastersTrack = compose(hasMastersTrack, getTracks); + +export { hasMastersTrack, trackIsMasters }; +export default stateHasMastersTrack; diff --git a/src/data/services/LmsApiService.js b/src/data/services/LmsApiService.js index b8852e0..1409c8d 100644 --- a/src/data/services/LmsApiService.js +++ b/src/data/services/LmsApiService.js @@ -72,7 +72,7 @@ class LmsApiService { const trackQueryParam = options.track ? [`track=${options.track}`] : []; const cohortQueryParam = options.cohort ? [`cohort=${options.cohort}`] : []; const queryParams = [...trackQueryParam, ...cohortQueryParam].join('&'); - const downloadUrl = `${LmsApiService.baseUrl}/api/bulk_grades/course/${courseId}?${queryParams}`; + const downloadUrl = `${LmsApiService.baseUrl}/api/bulk_grades/course/${courseId}/?${queryParams}`; return downloadUrl; } @@ -87,6 +87,11 @@ class LmsApiService { return Promise.reject(result); }); } + + static fetchGradeBulkOperationHistory(courseId) { + const url = `${LmsApiService.baseUrl}/api/bulk_grades/course/${courseId}/history/`; + return apiClient.get(url).then(response => response.data).catch(() => Promise.reject(Error('unhandled response error'))); + } } export default LmsApiService; From e31495b00b3e06dd57da05c093984ff9063f3577 Mon Sep 17 00:00:00 2001 From: Matt Hughes Date: Tue, 2 Jul 2019 11:34:26 -0400 Subject: [PATCH 2/4] address some TODOs & fix a unit test's expectation --- src/components/Gradebook/index.jsx | 4 +--- src/data/reducers/grades.js | 9 +++++---- src/data/reducers/grades.test.js | 15 +-------------- 3 files changed, 7 insertions(+), 21 deletions(-) diff --git a/src/components/Gradebook/index.jsx b/src/components/Gradebook/index.jsx index 63aaf3c..bb2a5a8 100644 --- a/src/components/Gradebook/index.jsx +++ b/src/components/Gradebook/index.jsx @@ -490,8 +490,6 @@ export default class Gradebook extends React.Component {
{ ...state, showSpinner: true, }; - case UPLOAD_COMPLETE: + case UPLOAD_COMPLETE: { + const { errorMessages, ...rest } = state.bulkManagement; return { ...state, showSpinner: false, - bulkManagement: {}, + bulkManagement: { ...rest }, }; + } case UPLOAD_ERR: return { ...state, @@ -99,8 +101,6 @@ const grades = (state = initialState, action) => { case GOT_BULK_HISTORY: return { ...state, - // TODO: this will be cleared if we successfully upload a new one; - // probably want to trigger a reload of this info instead bulkManagement: { ...state.bulkManagement, history: action.data, @@ -111,4 +111,5 @@ const grades = (state = initialState, action) => { } }; +export { initialState as initialGradesState }; export default grades; diff --git a/src/data/reducers/grades.test.js b/src/data/reducers/grades.test.js index 4a8ecb2..2cc3002 100644 --- a/src/data/reducers/grades.test.js +++ b/src/data/reducers/grades.test.js @@ -1,4 +1,4 @@ -import grades from './grades'; +import grades, { initialGradesState as initialState } from './grades'; import { STARTED_FETCHING_GRADES, ERROR_FETCHING_GRADES, @@ -8,19 +8,6 @@ import { OPEN_BANNER, } from '../constants/actionTypes/grades'; -const initialState = { - results: [], - headings: [], - startedFetching: false, - finishedFetching: false, - errorFetching: false, - gradeFormat: 'percent', - showSuccess: false, - prevPage: null, - nextPage: null, - showSpinner: true, -}; - const courseId = 'course-v1:edX+DemoX+Demo_Course'; const headingsData = [ { name: 'exam' }, From fb09bee8d9a2194ff5797b74e699c3b030ad6a7c Mon Sep 17 00:00:00 2001 From: Matt Hughes Date: Tue, 2 Jul 2019 11:37:54 -0400 Subject: [PATCH 3/4] addressed another todo starting a selector module for grades --- src/containers/GradebookPage/index.jsx | 4 ++-- src/data/selectors/grades.js | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) create mode 100644 src/data/selectors/grades.js diff --git a/src/containers/GradebookPage/index.jsx b/src/containers/GradebookPage/index.jsx index 72e375f..ab362fc 100644 --- a/src/containers/GradebookPage/index.jsx +++ b/src/containers/GradebookPage/index.jsx @@ -14,6 +14,7 @@ import { import { fetchCohorts } from '../../data/actions/cohorts'; import { fetchTracks } from '../../data/actions/tracks'; import stateHasMastersTrack from '../../data/selectors/tracks'; +import getBulkManagementHistory from '../../data/selectors/grades'; import { fetchAssignmentTypes } from '../../data/actions/assignmentTypes'; import { getRoles } from '../../data/actions/roles'; import LmsApiService from '../../data/services/LmsApiService'; @@ -54,8 +55,7 @@ const mapStateToProps = (state, ownProps) => ( `Errors while processing: ${state.grades.bulkManagement.errorMessages.join(', ')}` : '', showBulkManagement: stateHasMastersTrack(state), - // TODO: selector - bulkManagementHistory: state.grades.bulkManagement.history, + bulkManagementHistory: getBulkManagementHistory(state), } ); diff --git a/src/data/selectors/grades.js b/src/data/selectors/grades.js new file mode 100644 index 0000000..28c71c5 --- /dev/null +++ b/src/data/selectors/grades.js @@ -0,0 +1,3 @@ +const getBulkManagementHistory = state => state.grades.bulkManagement.history; + +export default getBulkManagementHistory; From 90f66d37590086361864325afd3ea0ea0318a9cc Mon Sep 17 00:00:00 2001 From: Matt Hughes Date: Tue, 2 Jul 2019 12:19:48 -0400 Subject: [PATCH 4/4] added an action for errors (currently unhandled) --- src/data/actions/grades.js | 5 +++-- src/data/constants/actionTypes/grades.js | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/data/actions/grades.js b/src/data/actions/grades.js index 045f640..307d48b 100644 --- a/src/data/actions/grades.js +++ b/src/data/actions/grades.js @@ -14,6 +14,7 @@ import { UPLOAD_COMPLETE, UPLOAD_ERR, GOT_BULK_HISTORY, + BULK_HISTORY_ERR, } from '../constants/actionTypes/grades'; import LmsApiService from '../services/LmsApiService'; import { headingMapper, sortAlphaAsc } from './utils'; @@ -25,6 +26,7 @@ const startedCsvUpload = () => ({ type: START_UPLOAD }); const finishedCsvUpload = () => ({ type: UPLOAD_COMPLETE }); const csvUploadError = data => ({ type: UPLOAD_ERR, data }); const gotBulkHistory = data => ({ type: GOT_BULK_HISTORY, data }); +const bulkHistoryError = () => ({ type: BULK_HISTORY_ERR }); const startedFetchingGrades = () => ({ type: STARTED_FETCHING_GRADES }); const finishedFetchingGrades = () => ({ type: FINISHED_FETCHING_GRADES }); @@ -179,8 +181,7 @@ const fetchBulkUpgradeHistory = courseId => ( // todo add loading effect LmsApiService.fetchGradeBulkOperationHistory(courseId).then((response) => { dispatch(gotBulkHistory(response)); - // todo: thread action through action - }).catch(() => dispatch({ type: 'BULK_HISTORY_ERR' })) + }).catch(() => dispatch(bulkHistoryError())) ); export { diff --git a/src/data/constants/actionTypes/grades.js b/src/data/constants/actionTypes/grades.js index 87aa3a3..6838397 100644 --- a/src/data/constants/actionTypes/grades.js +++ b/src/data/constants/actionTypes/grades.js @@ -16,6 +16,7 @@ const START_UPLOAD = 'START_UPLOAD'; const UPLOAD_COMPLETE = 'UPLOAD_COMPLETE'; const UPLOAD_ERR = 'UPLOAD_ERR'; const GOT_BULK_HISTORY = 'GOT_BULK_HISTORY'; +const BULK_HISTORY_ERR = 'BULK_HISTORY_ERR'; export { STARTED_FETCHING_GRADES, @@ -33,4 +34,5 @@ export { UPLOAD_COMPLETE, UPLOAD_ERR, GOT_BULK_HISTORY, + BULK_HISTORY_ERR, };