From 7e49670a95af76d6ff99ae467bc039eb9be4d781 Mon Sep 17 00:00:00 2001 From: leangseu-edx <83240113+leangseu-edx@users.noreply.github.com> Date: Fri, 10 Dec 2021 14:57:24 -0500 Subject: [PATCH] feat: update lock/unlock logistic (#38) * feat: update lock/unlock logistic * chore: disable submit grade on lock pending * chore: update mock for contested lock * chore: update set lock success response * chore: add failure reducer/action for set lock --- .../components/StartGradingButton.jsx | 8 +- .../components/StartGradingButton.test.jsx | 24 ++++-- .../StartGradingButton.test.jsx.snap | 34 +++++++- .../ReviewModal/ReviewErrors/LockErrors.jsx | 65 +++++++++++++++ .../ReviewErrors/LockErrors.test.jsx | 62 +++++++++++++++ .../ReviewModal/ReviewErrors/ReviewError.jsx | 5 +- .../__snapshots__/LockErrors.test.jsx.snap | 39 +++++++++ .../__snapshots__/index.test.jsx.snap | 1 + .../ReviewModal/ReviewErrors/index.jsx | 2 + .../ReviewModal/ReviewErrors/index.test.jsx | 1 + .../ReviewModal/ReviewErrors/messages.js | 21 ++++- .../Rubric/__snapshots__/index.test.jsx.snap | 79 +++++++++++++++++++ src/containers/Rubric/index.jsx | 14 ++-- src/containers/Rubric/index.test.jsx | 20 ++++- src/data/redux/grading/reducer.js | 4 + src/data/redux/thunkActions/grading.js | 10 +++ src/data/services/lms/fakeData/testUtils.js | 15 ++++ 17 files changed, 382 insertions(+), 22 deletions(-) create mode 100644 src/containers/ReviewModal/ReviewErrors/LockErrors.jsx create mode 100644 src/containers/ReviewModal/ReviewErrors/LockErrors.test.jsx create mode 100644 src/containers/ReviewModal/ReviewErrors/__snapshots__/LockErrors.test.jsx.snap diff --git a/src/containers/ReviewActions/components/StartGradingButton.jsx b/src/containers/ReviewActions/components/StartGradingButton.jsx index ce40ea4..db80cbe 100644 --- a/src/containers/ReviewActions/components/StartGradingButton.jsx +++ b/src/containers/ReviewActions/components/StartGradingButton.jsx @@ -94,7 +94,7 @@ export class StartGradingButton extends React.Component { variant="primary" iconAfter={args.iconAfter} onClick={this.handleClick} - disabled={this.props.isPending} + disabled={this.props.gradeIsPending || this.props.lockIsPending} > @@ -119,11 +119,13 @@ StartGradingButton.propTypes = { gradingStatus: PropTypes.string.isRequired, startGrading: PropTypes.func.isRequired, stopGrading: PropTypes.func.isRequired, - isPending: PropTypes.bool.isRequired, + gradeIsPending: PropTypes.bool.isRequired, + lockIsPending: PropTypes.bool.isRequired, }; export const mapStateToProps = (state) => ({ - isPending: selectors.requests.isPending(state, { requestKey: RequestKeys.submitGrade }), + gradeIsPending: selectors.requests.isPending(state, { requestKey: RequestKeys.submitGrade }), + lockIsPending: selectors.requests.isPending(state, { requestKey: RequestKeys.setLock }), gradeStatus: selectors.grading.selected.gradeStatus(state), gradingStatus: selectors.grading.selected.gradingStatus(state), }); diff --git a/src/containers/ReviewActions/components/StartGradingButton.test.jsx b/src/containers/ReviewActions/components/StartGradingButton.test.jsx index 6afef81..2a29d42 100644 --- a/src/containers/ReviewActions/components/StartGradingButton.test.jsx +++ b/src/containers/ReviewActions/components/StartGradingButton.test.jsx @@ -36,7 +36,8 @@ let el; describe('StartGradingButton component', () => { describe('component', () => { const props = { - isPending: false, + gradeIsPending: false, + lockIsPending: false, }; beforeEach(() => { props.startGrading = jest.fn().mockName('this.props.startGrading'); @@ -69,9 +70,14 @@ describe('StartGradingButton component', () => { test('snapshot: ungraded (startGrading callback)', () => { expect(mockedEl(statuses.ungraded).instance().render()).toMatchSnapshot(); }); - test('snapshot: pending (disabled)', () => { + test('snapshot: grade pending (disabled)', () => { el = mockedEl(statuses.ungraded); - el.setProps({ isPending: true }); + el.setProps({ gradeIsPending: true }); + expect(el.instance().render()).toMatchSnapshot(); + }); + test('snapshot: lock pending (disabled)', () => { + el = mockedEl(statuses.ungraded); + el.setProps({ lockIsPending: true }); expect(el.instance().render()).toMatchSnapshot(); }); test('snapshot: graded, confirmOverride (startGrading callback)', () => { @@ -92,14 +98,22 @@ describe('StartGradingButton component', () => { beforeEach(() => { mapped = mapStateToProps(testState); }); - test('isPending loads from requests.isPending(submitGrade)', () => { - expect(mapped.isPending).toEqual( + test('gradeIsPending loads from requests.gradeIsPending(submitGrade)', () => { + expect(mapped.gradeIsPending).toEqual( selectors.requests.isPending( testState, { requestKey: RequestKeys.submitGrade }, ), ); }); + test('lockIsPending loads from requests.lockIsPending(setLock)', () => { + expect(mapped.lockIsPending).toEqual( + selectors.requests.isPending( + testState, + { requestKey: RequestKeys.setLock }, + ), + ); + }); test('gradeStatus loads from grading.selected.gradeStatus', () => { expect(mapped.gradeStatus).toEqual(selectors.grading.selected.gradeStatus(testState)); }); diff --git a/src/containers/ReviewActions/components/__snapshots__/StartGradingButton.test.jsx.snap b/src/containers/ReviewActions/components/__snapshots__/StartGradingButton.test.jsx.snap index be5f631..bff99fc 100644 --- a/src/containers/ReviewActions/components/__snapshots__/StartGradingButton.test.jsx.snap +++ b/src/containers/ReviewActions/components/__snapshots__/StartGradingButton.test.jsx.snap @@ -1,5 +1,33 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`StartGradingButton component component snapshotes snapshot: grade pending (disabled) 1`] = ` + + + + + + + +`; + exports[`StartGradingButton component component snapshotes snapshot: graded, confirmOverride (startGrading callback) 1`] = ` `; -exports[`StartGradingButton component component snapshotes snapshot: locked (null) 1`] = `null`; - -exports[`StartGradingButton component component snapshotes snapshot: pending (disabled) 1`] = ` +exports[`StartGradingButton component component snapshotes snapshot: lock pending (disabled) 1`] = ` `; +exports[`StartGradingButton component component snapshotes snapshot: locked (null) 1`] = `null`; + exports[`StartGradingButton component component snapshotes snapshot: ungraded (startGrading callback) 1`] = ` + */ +export class LockErrors extends React.Component { + get errorProp() { + if (this.errorStatus === ErrorStatuses.forbidden) { + return { + heading: messages.errorLockContestedHeading, + message: messages.errorLockContested, + }; + } + + return { + heading: messages.errorLockBadRequestHeading, + message: messages.errorLockBadRequest, + }; + } + + render() { + if (!this.props.isFailed) { return null; } + const { heading, message } = this.errorProp; + return ( + + + + ); + } +} +LockErrors.defaultProps = { + errorStatus: undefined, +}; +LockErrors.propTypes = { + // redux + isFailed: PropTypes.bool.isRequired, + errorStatus: PropTypes.number, +}; + +export const mapStateToProps = (state) => ({ + isFailed: selectors.requests.isFailed(state, { + requestKey: RequestKeys.setLock, + }), + errorStatus: selectors.requests.errorStatus(state, { + requestKey: RequestKeys.setLock, + }), +}); + +export const mapDispatchToProps = {}; + +export default connect(mapStateToProps, mapDispatchToProps)(LockErrors); diff --git a/src/containers/ReviewModal/ReviewErrors/LockErrors.test.jsx b/src/containers/ReviewModal/ReviewErrors/LockErrors.test.jsx new file mode 100644 index 0000000..9f14b72 --- /dev/null +++ b/src/containers/ReviewModal/ReviewErrors/LockErrors.test.jsx @@ -0,0 +1,62 @@ +import React from 'react'; +import { shallow } from 'enzyme'; + +import { selectors } from 'data/redux'; +import { ErrorStatuses, RequestKeys } from 'data/constants/requests'; + +import { + LockErrors, + mapStateToProps, +} from './LockErrors'; + +jest.mock('data/redux', () => ({ + selectors: { + requests: { + errorStatus: (...args) => ({ errorStatus: args }), + isFailed: (...args) => ({ isFailed: args }), + }, + }, +})); + +let el; +jest.mock('./ReviewError', () => 'ReviewError'); + +const requestKey = RequestKeys.setLock; + +describe('LockErrors component', () => { + const props = { + isFailed: true, + }; + describe('component', () => { + beforeEach(() => { + el = shallow(); + el.instance().dismissError = jest.fn().mockName('this.dismissError'); + }); + describe('snapshots', () => { + test('no failure', () => { + el.setProps({ isFailed: false }); + expect(el.instance().render()).toMatchSnapshot(); + }); + test('snapshot: error with bad request', () => { + el.setProps({ errorStatus: ErrorStatuses.badRequest }); + expect(el.instance().render()).toMatchSnapshot(); + }); + test('snapshot: error with conflicted lock', () => { + el.setProps({ errorStatus: ErrorStatuses.forbidden }); + expect(el.instance().render()).toMatchSnapshot(); + }); + }); + }); + describe('mapStateToProps', () => { + let mapped; + const testState = { some: 'test-state' }; + beforeEach(() => { + mapped = mapStateToProps(testState); + }); + test('errorStatus loads from requests.errorStatus(setLock)', () => { + expect(mapped.errorStatus).toEqual( + selectors.requests.errorStatus(testState, { requestKey }), + ); + }); + }); +}); diff --git a/src/containers/ReviewModal/ReviewErrors/ReviewError.jsx b/src/containers/ReviewModal/ReviewErrors/ReviewError.jsx index 050756a..354165a 100644 --- a/src/containers/ReviewModal/ReviewErrors/ReviewError.jsx +++ b/src/containers/ReviewModal/ReviewErrors/ReviewError.jsx @@ -44,6 +44,9 @@ const ReviewError = ({ ); }; +ReviewError.defaultProps = { + actions: {}, +}; ReviewError.propTypes = { actions: PropTypes.shape({ cancel: PropTypes.shape({ @@ -54,7 +57,7 @@ ReviewError.propTypes = { onClick: PropTypes.func, message: messageShape, }), - }).isRequired, + }), headingMessage: messageShape.isRequired, children: PropTypes.node.isRequired, }; diff --git a/src/containers/ReviewModal/ReviewErrors/__snapshots__/LockErrors.test.jsx.snap b/src/containers/ReviewModal/ReviewErrors/__snapshots__/LockErrors.test.jsx.snap new file mode 100644 index 0000000..054c91f --- /dev/null +++ b/src/containers/ReviewModal/ReviewErrors/__snapshots__/LockErrors.test.jsx.snap @@ -0,0 +1,39 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`LockErrors component component snapshots no failure 1`] = `null`; + +exports[`LockErrors component component snapshots snapshot: error with bad request 1`] = ` + + + +`; + +exports[`LockErrors component component snapshots snapshot: error with conflicted lock 1`] = ` + + + +`; diff --git a/src/containers/ReviewModal/ReviewErrors/__snapshots__/index.test.jsx.snap b/src/containers/ReviewModal/ReviewErrors/__snapshots__/index.test.jsx.snap index 4bf64fd..44ecc2c 100644 --- a/src/containers/ReviewModal/ReviewErrors/__snapshots__/index.test.jsx.snap +++ b/src/containers/ReviewModal/ReviewErrors/__snapshots__/index.test.jsx.snap @@ -4,5 +4,6 @@ exports[`ReviewErrors component component snapshot: no failure 1`] = ` + `; diff --git a/src/containers/ReviewModal/ReviewErrors/index.jsx b/src/containers/ReviewModal/ReviewErrors/index.jsx index d0852e0..519093e 100644 --- a/src/containers/ReviewModal/ReviewErrors/index.jsx +++ b/src/containers/ReviewModal/ReviewErrors/index.jsx @@ -1,6 +1,7 @@ import React from 'react'; import FetchErrors from './FetchErrors'; +import LockErrors from './LockErrors'; import SubmitErrors from './SubmitErrors'; /** @@ -10,6 +11,7 @@ export const ReviewErrors = () => ( <> + > ); ReviewErrors.defaultProps = { diff --git a/src/containers/ReviewModal/ReviewErrors/index.test.jsx b/src/containers/ReviewModal/ReviewErrors/index.test.jsx index 4e11d18..6aa4903 100644 --- a/src/containers/ReviewModal/ReviewErrors/index.test.jsx +++ b/src/containers/ReviewModal/ReviewErrors/index.test.jsx @@ -5,6 +5,7 @@ import { ReviewErrors } from '.'; jest.mock('./FetchErrors', () => 'FetchErrors'); jest.mock('./SubmitErrors', () => 'SubmitErrors'); +jest.mock('./LockErrors', () => 'LockErrors'); describe('ReviewErrors component', () => { describe('component', () => { diff --git a/src/containers/ReviewModal/ReviewErrors/messages.js b/src/containers/ReviewModal/ReviewErrors/messages.js index 56b165d..c37b112 100644 --- a/src/containers/ReviewModal/ReviewErrors/messages.js +++ b/src/containers/ReviewModal/ReviewErrors/messages.js @@ -47,7 +47,26 @@ const messages = defineMessages({ defaultMessage: 'It looks like someone else got here first! Your grade submission has been rejected', description: 'Error Submitting Grade content', }, - + errorLockContestedHeading: { + id: 'ora-grading.ReviewModal.errorLockContestedHeading', + defaultMessage: 'The lock owned by another user', + description: 'Error lock by someone else', + }, + errorLockContested: { + id: 'ora-grading.ReviewModal.errorLockContested', + defaultMessage: 'The lock owned by another user', + description: 'Error lock by someone else', + }, + errorLockBadRequestHeading: { + id: 'ora-grading.ReviewModal.errorLockBadRequestHeading', + defaultMessage: 'Invalid request. Please check your input.', + description: 'Error lock request for missing params', + }, + errorLockBadRequest: { + id: 'ora-grading.ReviewModal.errorLockBadRequest', + defaultMessage: 'Invalid request. Please check your input.', + description: 'Error lock request for missing params', + }, }); export default StrictDict(messages); diff --git a/src/containers/Rubric/__snapshots__/index.test.jsx.snap b/src/containers/Rubric/__snapshots__/index.test.jsx.snap index 1fad1cb..d3d166d 100644 --- a/src/containers/Rubric/__snapshots__/index.test.jsx.snap +++ b/src/containers/Rubric/__snapshots__/index.test.jsx.snap @@ -47,6 +47,7 @@ exports[`Rubric Container snapshot is grading 1`] = ` disabledStates={ Array [ "pending", + "complete", ] } labels={ @@ -75,6 +76,82 @@ exports[`Rubric Container snapshot is grading 1`] = ` `; +exports[`Rubric Container snapshot is grading, lock is pending 1`] = ` + + + + + + + + + + + + + + + + , + "default": , + "pending": , + } + } + onClick={[MockFunction this.submitGradeHandler]} + state="pending" + /> + + +`; + exports[`Rubric Container snapshot is grading, submit pending 1`] = ` , [ButtonStates.pending]: , @@ -87,17 +87,17 @@ Rubric.defaultProps = { Rubric.propTypes = { isCompleted: PropTypes.bool.isRequired, isGrading: PropTypes.bool.isRequired, - isPending: PropTypes.bool.isRequired, + gradeIsPending: PropTypes.bool.isRequired, + lockIsPending: PropTypes.bool.isRequired, criteriaIndices: PropTypes.arrayOf(PropTypes.number), submitGrade: PropTypes.func.isRequired, }; -const requestKey = RequestKeys.submitGrade; - export const mapStateToProps = (state) => ({ - isCompleted: selectors.requests.isCompleted(state, { requestKey }), + isCompleted: selectors.requests.isCompleted(state, { requestKey: RequestKeys.submitGrade }), isGrading: selectors.grading.selected.isGrading(state), - isPending: selectors.requests.isPending(state, { requestKey }), + gradeIsPending: selectors.requests.isPending(state, { requestKey: RequestKeys.submitGrade }), + lockIsPending: selectors.requests.isPending(state, { requestKey: RequestKeys.setLock }), criteriaIndices: selectors.app.rubric.criteriaIndices(state), }); diff --git a/src/containers/Rubric/index.test.jsx b/src/containers/Rubric/index.test.jsx index da7c19d..24aabb7 100644 --- a/src/containers/Rubric/index.test.jsx +++ b/src/containers/Rubric/index.test.jsx @@ -2,6 +2,7 @@ import React from 'react'; import { shallow } from 'enzyme'; import { selectors, thunkActions } from 'data/redux'; +import { RequestKeys } from 'data/constants/requests'; import { Rubric, mapStateToProps, mapDispatchToProps } from '.'; jest.mock('containers/CriterionContainer', () => 'CriterionContainer'); @@ -36,7 +37,8 @@ jest.mock('data/redux', () => ({ describe('Rubric Container', () => { const props = { isCompleted: false, - isPending: false, + gradeIsPending: false, + lockIsPending: false, isGrading: true, criteriaIndices: [1, 2, 3, 4, 5], submitGrade: jest.fn().mockName('this.props.submitGrade'), @@ -60,7 +62,11 @@ describe('Rubric Container', () => { expect(el.instance().render()).toMatchSnapshot(); }); test('is grading, submit pending', () => { - el.setProps({ isPending: true }); + el.setProps({ gradeIsPending: true }); + expect(el.instance().render()).toMatchSnapshot(); + }); + test('is grading, lock is pending', () => { + el.setProps({ lockIsPending: true }); expect(el.instance().render()).toMatchSnapshot(); }); test('submit completed', () => { @@ -111,6 +117,16 @@ describe('Rubric Container', () => { test('isGrading from selectors.grading.selected.isGrading', () => { expect(mapped.isGrading).toEqual(selectors.grading.selected.isGrading(testState)); }); + test('gradeIsPending from selectors.requests.isPending(submitGrade)', () => { + expect(mapped.gradeIsPending).toEqual( + selectors.requests.isPending(testState, { requestKey: RequestKeys.submitGrade }), + ); + }); + test('lockIsPending from selectors.requests.isPending(setLock)', () => { + expect(mapped.lockIsPending).toEqual(selectors.requests.isPending( + testState, { requestKey: RequestKeys.setLock }, + )); + }); test('criteriaIndices from selectors.app.rubric.criteriaIndices', () => { expect(mapped.criteriaIndices).toEqual( selectors.app.rubric.criteriaIndices(testState), diff --git a/src/data/redux/grading/reducer.js b/src/data/redux/grading/reducer.js index efd0b72..375d2af 100644 --- a/src/data/redux/grading/reducer.js +++ b/src/data/redux/grading/reducer.js @@ -166,6 +166,10 @@ const grading = createSlice({ gradingData, }; }, + failSetLock: (state, { payload }) => ({ + ...state, + current: { ...state.current, lockStatus: payload.lockStatus }, + }), setRubricFeedback: (state, { payload }) => ( updateGradingData(state, { overallFeedback: payload }) ), diff --git a/src/data/redux/thunkActions/grading.js b/src/data/redux/thunkActions/grading.js index 585eda7..a1fdd6c 100644 --- a/src/data/redux/thunkActions/grading.js +++ b/src/data/redux/thunkActions/grading.js @@ -72,6 +72,11 @@ export const startGrading = () => (dispatch, getState) => { } dispatch(actions.grading.startGrading({ ...response, gradeData })); }, + onFailure: (error) => { + if (error.response.status === ErrorStatuses.forbidden) { + dispatch(actions.grading.failSetLock(error.response.data)); + } + }, })); }; @@ -87,6 +92,11 @@ export const cancelGrading = () => (dispatch, getState) => { onSuccess: () => { dispatch(module.stopGrading()); }, + onFailure: (error) => { + if (error.response.status === ErrorStatuses.forbidden) { + dispatch(actions.grading.failSetLock(error.response.data)); + } + }, })); }; diff --git a/src/data/services/lms/fakeData/testUtils.js b/src/data/services/lms/fakeData/testUtils.js index 55529ec..45e187e 100644 --- a/src/data/services/lms/fakeData/testUtils.js +++ b/src/data/services/lms/fakeData/testUtils.js @@ -67,6 +67,21 @@ export const genTestUtils = ({ dispatch }) => { }, ), }), + setLock: StrictDict({ + start: mockStart(RequestKeys.setLock), + success: () => { + dispatch(actions.requests.completeRequest({ + requestKey: RequestKeys.setLock, + response: { + lockStatus: lockStatuses.inProgress, + }, + })); + }, + badRequestError: mockError(RequestKeys.setLock, ErrorStatuses.badRequest), + contestedLockError: mockError(RequestKeys.setLock, ErrorStatuses.forbidden, { + lockStatus: lockStatuses.locked, + }), + }), }; };