From 595148a82bd5b8ed6718d764cfb3ea6d07fc1448 Mon Sep 17 00:00:00 2001 From: Ben Warzeski Date: Tue, 9 Nov 2021 15:49:20 -0500 Subject: [PATCH] fix: separate local and static grade data --- .../CriterionContainer/CriterionFeedback.jsx | 7 +- .../CriterionFeedback.test.jsx | 2 +- .../CriterionContainer/RadioCriterion.jsx | 11 +-- .../RadioCriterion.test.jsx | 12 +-- .../CriterionFeedback.test.jsx.snap | 26 +----- .../RadioCriterion.test.jsx.snap | 4 +- src/containers/Rubric/RubricFeedback.jsx | 4 +- src/containers/Rubric/RubricFeedback.test.jsx | 4 +- .../RubricFeedback.test.jsx.snap | 4 +- src/data/redux/grading/reducer.js | 60 +++++++++---- src/data/redux/grading/selectors.js | 86 +++++++++++++++---- src/data/redux/thunkActions/grading.js | 2 +- src/data/redux/thunkActions/grading.test.js | 4 +- 13 files changed, 144 insertions(+), 82 deletions(-) diff --git a/src/containers/CriterionContainer/CriterionFeedback.jsx b/src/containers/CriterionContainer/CriterionFeedback.jsx index d3f50f1..5a41740 100644 --- a/src/containers/CriterionContainer/CriterionFeedback.jsx +++ b/src/containers/CriterionContainer/CriterionFeedback.jsx @@ -29,7 +29,10 @@ export class CriterionFeedback extends React.Component { render() { const { - config, isGrading, value, valueIsInvalid, + config, + isGrading, + value, + valueIsInvalid, } = this.props; if (config === feedbackRequirement.disabled) { return null; @@ -57,7 +60,7 @@ export class CriterionFeedback extends React.Component { } CriterionFeedback.defaultProps = { - value: '', + value: { local: '', review: '' }, }; CriterionFeedback.propTypes = { diff --git a/src/containers/CriterionContainer/CriterionFeedback.test.jsx b/src/containers/CriterionContainer/CriterionFeedback.test.jsx index 3b4c537..781e7b0 100644 --- a/src/containers/CriterionContainer/CriterionFeedback.test.jsx +++ b/src/containers/CriterionContainer/CriterionFeedback.test.jsx @@ -35,7 +35,7 @@ describe('Criterion Feedback', () => { orderNum: 1, config: 'config string', isGrading: true, - value: 'some value', + value: { grading: 'grading value', review: 'review value' }, gradeStatus: gradeStatuses.ungraded, setValue: jest.fn().mockName('this.props.setValue'), valueIsInvalid: false, diff --git a/src/containers/CriterionContainer/RadioCriterion.jsx b/src/containers/CriterionContainer/RadioCriterion.jsx index 43db992..41b328d 100644 --- a/src/containers/CriterionContainer/RadioCriterion.jsx +++ b/src/containers/CriterionContainer/RadioCriterion.jsx @@ -32,9 +32,10 @@ export class RadioCriterion extends React.Component { isGrading, radioIsInvalid, } = this.props; + const value = isGrading ? data.grading : data.review; return ( <> - + {config.options.map((option) => ( ({ })); jest.mock('data/redux/grading/selectors', () => ({ selected: { - criterionGradeData: jest.fn((...args) => ({ - selectedCriterionGradeData: args, + criterionSelectedOption: jest.fn((...args) => ({ + selectedCriterionSelectedOption: args, })), criterionSelectedIsInvalid: jest.fn((...args) => ({ selectedCriterionSelectedIsInvalid: args, @@ -54,8 +54,8 @@ describe('Radio Criterion Container', () => { ], }, data: { - selectedOption: 'selected option', - feedback: 'data feedback', + review: 'selected review option', + grading: 'selected grading option', }, setCriterionOption: jest.fn().mockName('this.props.setCriterionOption'), radioIsInvalid: false, @@ -142,9 +142,9 @@ describe('Radio Criterion Container', () => { ); }); - test('selectors.grading.selected.criterionGradeData', () => { + test('selectors.grading.selected.criterionSelectedOption', () => { expect(mapped.data).toEqual( - selectors.grading.selected.criterionGradeData(testState, ownProps), + selectors.grading.selected.criterionSelectedOption(testState, ownProps), ); }); test('selectors.grading.selected.criterionSelectedIsInvalid', () => { diff --git a/src/containers/CriterionContainer/__snapshots__/CriterionFeedback.test.jsx.snap b/src/containers/CriterionContainer/__snapshots__/CriterionFeedback.test.jsx.snap index 30332df..7c4460b 100644 --- a/src/containers/CriterionContainer/__snapshots__/CriterionFeedback.test.jsx.snap +++ b/src/containers/CriterionContainer/__snapshots__/CriterionFeedback.test.jsx.snap @@ -21,28 +21,4 @@ exports[`Criterion Feedback snapshot feedback value is invalid 1`] = ` exports[`Criterion Feedback snapshot is configure to disabled 1`] = `null`; -exports[`Criterion Feedback snapshot is graded 1`] = ` - - - -`; - -exports[`Criterion Feedback snapshot is grading 1`] = ` - - - -`; +exports[`Criterion Feedback snapshot is graded 1`] = ``; diff --git a/src/containers/CriterionContainer/__snapshots__/RadioCriterion.test.jsx.snap b/src/containers/CriterionContainer/__snapshots__/RadioCriterion.test.jsx.snap index 1d6ad07..94daf6f 100644 --- a/src/containers/CriterionContainer/__snapshots__/RadioCriterion.test.jsx.snap +++ b/src/containers/CriterionContainer/__snapshots__/RadioCriterion.test.jsx.snap @@ -4,7 +4,7 @@ exports[`Radio Criterion Container snapshot is grading 1`] = ` @@ -71,7 +71,7 @@ export class RubricFeedback extends React.Component { } RubricFeedback.defaultProps = { - value: '', + value: { grading: '', review: '' }, }; RubricFeedback.propTypes = { diff --git a/src/containers/Rubric/RubricFeedback.test.jsx b/src/containers/Rubric/RubricFeedback.test.jsx index 0e6a656..efa0654 100644 --- a/src/containers/Rubric/RubricFeedback.test.jsx +++ b/src/containers/Rubric/RubricFeedback.test.jsx @@ -89,7 +89,7 @@ describe('Rubric Feedback component', () => { expect(el.isEmptyRender()).toEqual(false); const input = el.find('.rubric-feedback.feedback-input'); expect(input.prop('disabled')).toEqual(false); - expect(input.prop('value')).toEqual(props.value); + expect(input.prop('value')).toEqual(props.value.grading); }); test('is graded (the input are disabled)', () => { @@ -100,7 +100,7 @@ describe('Rubric Feedback component', () => { expect(el.isEmptyRender()).toEqual(false); const input = el.find('.rubric-feedback.feedback-input'); expect(input.prop('disabled')).toEqual(true); - expect(input.prop('value')).toEqual(props.value); + expect(input.prop('value')).toEqual(props.value.review); }); test('is having invalid feedback (feedback get render)', () => { diff --git a/src/containers/Rubric/__snapshots__/RubricFeedback.test.jsx.snap b/src/containers/Rubric/__snapshots__/RubricFeedback.test.jsx.snap index f698267..e3c3fe5 100644 --- a/src/containers/Rubric/__snapshots__/RubricFeedback.test.jsx.snap +++ b/src/containers/Rubric/__snapshots__/RubricFeedback.test.jsx.snap @@ -69,7 +69,7 @@ exports[`Rubric Feedback component snapshot is graded 1`] = ` disabled={true} floatingLabel="Comments" onChange={[MockFunction this.onChange]} - value="some value" + value="review value" /> `; @@ -100,7 +100,7 @@ exports[`Rubric Feedback component snapshot is grading 1`] = ` disabled={false} floatingLabel="Add comments" onChange={[MockFunction this.onChange]} - value="some value" + value="grading value" /> `; diff --git a/src/data/redux/grading/reducer.js b/src/data/redux/grading/reducer.js index 5d42a7b..7990288 100644 --- a/src/data/redux/grading/reducer.js +++ b/src/data/redux/grading/reducer.js @@ -28,6 +28,18 @@ const initialState = { * } */ }, + localGradeData: { + /** + * : { + * overallFeedback: '', + * criteria: [{ + * orderNum: 0, + * points: 0, + * comments: '', + * }], + * } + */ + }, activeIndex: null, current: { /** @@ -59,35 +71,49 @@ const initialState = { }; /** - * Updates the given state's gradeData entry for the seleted submission, - * overlaying the passed data on top of the existing data for the that - * submission. + * Updates the given state's gradeData entry for the seleted submission. * @return {object} - new state */ -export const updateGradeData = (state, data) => ({ +export const loadGradeData = (state, data) => ({ ...state, gradeData: { ...state.gradeData, - [state.current.submissionId]: { - ...state.gradeData[state.current.submissionId], - ...data, - }, + [state.current.submissionId]: { ...data }, }, }); /** - * Updates the given state's gradeData entry for the seleted submission, + * Updates the state's localGradeData entry for the seleted submission, + * overlaying the passed data on top of the existing data for the that + * submission. + * @return {object} - new state + */ +export const updateLocalGradeData = (state, data) => { + const currentId = state.current.submissionId; + return { + ...state, + localGradeData: { + ...state.localGradeData, + [currentId]: state.localGradeData[currentId] + ? { ...state.localGradeData[currentId], ...data } + : { ...data }, + }, + }; +}; + +/** + * Updates the given state's localGradeData entry for the seleted submission, * overlaying the passed data on top of the existing data for the criterion * at the given index (orderNum) for the rubric. * @return {object} - new state */ export const updateCriterion = (state, orderNum, data) => { - const entry = state.gradeData[state.current.submissionId]; + const entry = state.localGradeData[state.current.submissionId]; const criteria = { ...entry.criteria, [orderNum]: { ...entry.criteria[orderNum], ...data }, }; - return updateGradeData(state, { ...entry, criteria }); + return updateLocalGradeData(state, { ...entry, criteria }); }; // eslint-disable-next-line no-unused-vars @@ -129,7 +155,7 @@ const grading = createSlice({ selected: payload, activeIndex: 0, }), - startGrading: (state, { payload }) => updateGradeData( + startGrading: (state, { payload }) => updateLocalGradeData( { ...state, current: { ...state.current, lockStatus: lockStatuses.inProgress }, @@ -137,7 +163,7 @@ const grading = createSlice({ { ...payload }, ), setRubricFeedback: (state, { payload }) => ( - updateGradeData(state, { overallFeedback: payload }) + updateLocalGradeData(state, { overallFeedback: payload }) ), setCriterionOption: (state, { payload: { orderNum, value } }) => ( updateCriterion(state, orderNum, { selectedOption: value }) @@ -166,12 +192,12 @@ const grading = createSlice({ lockStatus: lockStatuses.unlocked, }, }), - clearGrade: (state) => { - const gradeData = { ...state.gradeData }; - delete gradeData[state.current.submissionId]; + stopGrading: (state) => { + const localGradeData = { ...state.localGradeData }; + delete localGradeData[state.current.submissionId]; return { ...state, - gradeData, + localGradeData, current: { ...state.current, lockStatus: lockStatuses.unlocked, diff --git a/src/data/redux/grading/selectors.js b/src/data/redux/grading/selectors.js index 16ff9a0..0b486b5 100644 --- a/src/data/redux/grading/selectors.js +++ b/src/data/redux/grading/selectors.js @@ -10,6 +10,7 @@ export const simpleSelectors = { activeIndex: state => state.grading.activeIndex, current: state => state.grading.current, gradeData: state => state.grading.gradeData, + localGradeData: state => state.grading.localGradeData, }; /** @@ -121,12 +122,39 @@ selected.gradeData = createSelector( ); /** - * Returns list of criterion grade data for the current selection - * @return {obj[]} criterion grade data entries + * Returns the local grade data for the selected submission + * @return {obj} local grade data + * { score, overallFeedback, criteria } + */ +selected.localGradeData = createSelector( + [module.selected.submissionId, module.simpleSelectors.localGradeData], + (submissionId, localGradeData) => localGradeData[submissionId], +); + +/** + * Returns list of criterion grade data for the current selection for review + * and grading views. + * @return {obj} criterion grade data entries ({ review: [{}], grading: [{}] }) */ selected.criteriaGradeData = createSelector( - [module.selected.gradeData], - (data) => (data ? data.criteria : []), + [module.selected.gradeData, module.selected.localGradeData], + (data, localData) => ({ + grading: (localData ? localData.criteria : []), + review: (data ? data.criteria : []), + }), +); + +/** + * Returns list of criterion grade data for the current selection for both + * review and grading views. + * @return {obj} criterion grade data entries ({ review: [{}], grading: [{}] }) + */ +selected.criteriaGradeData = createSelector( + [module.selected.gradeData, module.selected.localGradeData], + (data, localData) => ({ + grading: (localData ? localData.criteria : {}), + review: (data ? data.criteria : {}), + }), ); /** @@ -139,12 +167,17 @@ selected.score = createSelector( ); /** - * Returns the rubric-level feedback for the selected submission - * @return {string} selected submission's associated rubric-level feedback + * Returns the rubric-level feedback for the selected submission for both review + * and grading views. + * @return {obj} selected submission's associated rubric-level feedback + * ({ review: '', grading: '' }) */ selected.overallFeedback = createSelector( - [module.selected.gradeData], - (data) => (data ? data.overallFeedback : ''), + [module.selected.gradeData, module.selected.localGradeData], + (data, localData) => ({ + review: (data ? data.overallFeedback : ''), + grading: (localData ? localData.overallFeedback : ''), + }), ); /** @@ -170,24 +203,47 @@ selected.isValidForSubmit = createSelector( /** * Returns the grade data for the given criterion of the current - * selection + * selection for both review and grading views. * @param {number} orderNum - criterion orderNum (and index) * @return {obj} - Grade Data associated with the criterion + * ({ review: {}, grading: {} }) */ selected.criterionGradeData = (state, { orderNum }) => { - const data = module.selected.criteriaGradeData(state); - return data ? data[orderNum] : {}; + const { grading, review } = module.selected.criteriaGradeData(state); + return { + review: review ? review[orderNum] : {}, + grading: grading ? grading[orderNum] : {}, + }; +}; + +/** + * Returns the selected option for the given criterion of the current selection for + * both review and grading views. + * @param {number} orderNum - criterion orderNum (and index) + * @return {obj} - selected option associated with the criterion + * ({ review: '', grading: '' }) + */ +selected.criterionSelectedOption = (state, { orderNum }) => { + const { grading, review } = module.selected.criterionGradeData(state, { orderNum }); + return { + grading: grading ? grading.selectedOption : '', + review: review ? review.selectedOption : '', + }; }; /** * Returns the critierion-level feedback for the selected submission, given the - * orderNum of the criterion. + * orderNum of the criterion for both review and grading views. * @param {number} orderNum - criterion index - * @return {string} - criterion-level feedback response for the given criterion. + * @return {obj} - criterion-level feedback response for the given criterion. + * ({ review: '', grading: '' }), */ selected.criterionFeedback = (state, { orderNum }) => { - const data = module.selected.criterionGradeData(state, { orderNum }); - return data ? data.feedback : ''; + const { grading, review } = module.selected.criterionGradeData(state, { orderNum }); + return { + grading: grading ? grading.feedback : '', + review: review ? review.feedback : '', + }; }; /** diff --git a/src/data/redux/thunkActions/grading.js b/src/data/redux/thunkActions/grading.js index 092a85d..88c1f76 100644 --- a/src/data/redux/thunkActions/grading.js +++ b/src/data/redux/thunkActions/grading.js @@ -159,7 +159,7 @@ export const cancelGrading = () => (dispatch, getState) => { * to False */ export const stopGrading = () => (dispatch) => { - dispatch(actions.grading.clearGrade()); + dispatch(actions.grading.stopGrading()); dispatch(actions.app.setGrading(false)); }; diff --git a/src/data/redux/thunkActions/grading.test.js b/src/data/redux/thunkActions/grading.test.js index 687acca..c48a4f0 100644 --- a/src/data/redux/thunkActions/grading.test.js +++ b/src/data/redux/thunkActions/grading.test.js @@ -339,10 +339,10 @@ describe('grading thunkActions', () => { }); describe('stopGrading', () => { - it('dispatches grading.clearGrade and app.setGrading(false)', () => { + it('dispatches grading.stopGrading and app.setGrading(false)', () => { thunkActions.stopGrading()(dispatch, getState); expect(dispatch.mock.calls).toEqual([ - [actions.grading.clearGrade()], + [actions.grading.stopGrading()], [actions.app.setGrading(false)], ]); });