diff --git a/package-lock.json b/package-lock.json index 48e48e85..5f4f5eb7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -29,6 +29,7 @@ "core-js": "3.31.1", "filesize": "^8.0.6", "jest-when": "^3.6.0", + "lodash.camelcase": "^4.3.0", "prop-types": "15.8.1", "query-string": "^8.1.0", "react": "^17.0.2", diff --git a/package.json b/package.json index 6c806107..6cd1e040 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ "core-js": "3.31.1", "filesize": "^8.0.6", "jest-when": "^3.6.0", + "lodash.camelcase": "^4.3.0", "prop-types": "15.8.1", "query-string": "^8.1.0", "react": "^17.0.2", diff --git a/src/components/Rubric/CriterionContainer/CriterionFeedback.jsx b/src/components/Rubric/CriterionContainer/CriterionFeedback.jsx index 98bee746..f1ee0e0b 100644 --- a/src/components/Rubric/CriterionContainer/CriterionFeedback.jsx +++ b/src/components/Rubric/CriterionContainer/CriterionFeedback.jsx @@ -3,14 +3,11 @@ import PropTypes from 'prop-types'; import { Form } from '@edx/paragon'; import { useIntl } from '@edx/frontend-platform/i18n'; -import { StrictDict, useKeyedState } from '@edx/react-unit-test-utils'; import { feedbackRequirement } from 'data/services/lms/constants'; -import messages from './messages'; +import { useCriterionData } from './hooks'; -export const stateKeys = StrictDict({ - value: 'value', -}); +import messages from './messages'; /** * @@ -18,9 +15,23 @@ export const stateKeys = StrictDict({ const CriterionFeedback = ({ criterion, }) => { - const [value, setValue] = useKeyedState(stateKeys.value, ''); const { formatMessage } = useIntl(); + let commentMessage = formatMessage(messages.addComments); + if (criterion.feedbackRequired === feedbackRequirement.optional) { + commentMessage += ` ${formatMessage(messages.optional)}`; + } + + const { + feedback: { + value, + isInvalid, + onChange, + }, + } = useCriterionData({ + criterion, + }); + if ( !criterion.feedbackEnabled || criterion.feedbackRequired === feedbackRequirement.disabled @@ -28,14 +39,6 @@ const CriterionFeedback = ({ return null; } - const onChange = ({ target }) => { setValue(target.value); }; - let commentMessage = formatMessage(messages.addComments); - if (criterion.feedbackRequired === feedbackRequirement.optional) { - commentMessage += ` ${formatMessage(messages.optional)}`; - } - - const isInvalid = value === ''; - return ( @@ -18,10 +15,17 @@ const RadioCriterion = ({ isGrading, criterion, }) => { - const [value, setValue] = useKeyedState(stateKeys.value, ''); const { formatMessage } = useIntl(); - const onChange = ({ target }) => { setValue(target.value); }; - const isInvalid = value === ''; + + const { + radio: { + value, + isInvalid, + onChange, + }, + } = useCriterionData({ + criterion, + }); return ( diff --git a/src/components/Rubric/CriterionContainer/hooks.test.ts b/src/components/Rubric/CriterionContainer/hooks.test.ts new file mode 100644 index 00000000..b9a9b117 --- /dev/null +++ b/src/components/Rubric/CriterionContainer/hooks.test.ts @@ -0,0 +1,85 @@ +import { mocked } from 'ts-jest/utils'; + +import { usePageData } from 'data/services/lms/hooks/selectors'; +import { updatePageData } from 'data/services/lms/hooks/actions'; + +jest.mock('data/services/lms/hooks/selectors'); +jest.mock('data/services/lms/hooks/actions'); + +const mockedUsePageData = mocked(usePageData); +const mockedUpdatePageData = mocked(updatePageData); + +import { useCriterionData } from './hooks'; + +describe('useCriterionData', () => { + const criterion = { + name: 'criterion-1', + } + + const mutateFn = jest.fn(); + + mockedUsePageData.mockReturnValue({ + rubric: { + optionsSelected: { + 'criterion-1': 'option-1', + 'criterion-2': 'option-2', + }, + criterionFeedback: { + 'criterion-1': 'feedback-1', + 'criterion-2': 'feedback-2', + }, + }, + } as any); + + mockedUpdatePageData.mockReturnValue({ + mutate: mutateFn, + } as any); + + it('should return radio and feedback values', () => { + const { radio, feedback } = useCriterionData({ criterion }); + + expect(radio.value).toBe('option-1'); + expect(radio.isInvalid).toBe(false); + expect(feedback.value).toBe('feedback-1'); + expect(feedback.isInvalid).toBe(false); + }); + + test('radio onChange should call mutate with updated rubric', () => { + const { radio } = useCriterionData({ criterion }); + + radio.onChange({ target: { value: 'option-3' } }); + + expect(mutateFn).toHaveBeenCalledWith({ + rubric: { + optionsSelected: { + 'criterion-1': 'option-3', + 'criterion-2': 'option-2', + }, + criterionFeedback: { + 'criterion-1': 'feedback-1', + 'criterion-2': 'feedback-2', + }, + }, + }); + }); + + test('feedback onChange should call mutate with updated rubric', () => { + const { feedback } = useCriterionData({ criterion }); + + feedback.onChange({ target: { value: 'feedback-3' } }); + + expect(mutateFn).toHaveBeenCalledWith({ + rubric: { + optionsSelected: { + 'criterion-1': 'option-1', + 'criterion-2': 'option-2', + }, + criterionFeedback: { + 'criterion-1': 'feedback-3', + 'criterion-2': 'feedback-2', + }, + }, + }); + }); +}); + diff --git a/src/components/Rubric/CriterionContainer/hooks.ts b/src/components/Rubric/CriterionContainer/hooks.ts new file mode 100644 index 00000000..e4d8d46c --- /dev/null +++ b/src/components/Rubric/CriterionContainer/hooks.ts @@ -0,0 +1,48 @@ +import { usePageData } from 'data/services/lms/hooks/selectors'; +import { updatePageData } from 'data/services/lms/hooks/actions'; + +export const useCriterionData = ({ + criterion, +}) => { + const data = usePageData(); + const mutation = updatePageData(); + + const updateRadio = ({ target }) => { + mutation.mutate({ + ...data, + rubric: { + ...data.rubric, + optionsSelected: { + ...data.rubric.optionsSelected, + [criterion.name]: target.value, + } + } + } as any); + }; + + const updateFeedback = ({ target }) => { + mutation.mutate({ + ...data, + rubric: { + ...data.rubric, + criterionFeedback: { + ...data.rubric.criterionFeedback, + [criterion.name]: target.value, + } + } + } as any); + }; + + return { + radio: { + value: data.rubric.optionsSelected[criterion.name], + isInvalid: false, + onChange: updateRadio, + }, + feedback: { + value: data.rubric.criterionFeedback[criterion.name], + isInvalid: false, + onChange: updateFeedback, + }, + }; +}; diff --git a/src/components/Rubric/RubricFeedback.jsx b/src/components/Rubric/RubricFeedback.jsx index 6d215df2..d72d75f1 100644 --- a/src/components/Rubric/RubricFeedback.jsx +++ b/src/components/Rubric/RubricFeedback.jsx @@ -3,31 +3,22 @@ import PropTypes from 'prop-types'; import { Form } from '@edx/paragon'; import { useIntl } from '@edx/frontend-platform/i18n'; -import { StrictDict, useKeyedState } from '@edx/react-unit-test-utils'; - -import { useRubricConfig } from 'data/services/lms/hooks/selectors'; import InfoPopover from 'components/InfoPopover'; import messages from './messages'; - -export const stateKeys = StrictDict({ - value: 'value', -}); +import { useRubricData } from './hooks'; /** * */ -const RubricFeedback = ({ - isGrading, -}) => { - const [value, setValue] = useKeyedState(''); +const RubricFeedback = ({ isGrading }) => { const { formatMessage } = useIntl(); - const feedbackPrompt = useRubricConfig().feedbackConfig.defaultText; - - const onChange = (event) => { - setValue(event.target.value); - }; + const { + feedbackPrompt, value, isInvalid, onChange, + } = useRubricData({ + isGrading, + }); const inputLabel = formatMessage( isGrading ? messages.addComments : messages.comments, @@ -51,15 +42,11 @@ const RubricFeedback = ({ onChange={onChange} disabled={!isGrading} /> - { - /* - {isInvalid && ( - - - - )} - */ - } + {isInvalid && ( + + {formatMessage(messages.overallFeedbackError)} + + )} ); }; diff --git a/src/components/Rubric/hooks.js b/src/components/Rubric/hooks.js deleted file mode 100644 index 2d6b074e..00000000 --- a/src/components/Rubric/hooks.js +++ /dev/null @@ -1,32 +0,0 @@ -import { StrictDict } from 'utils'; -import { useRubricConfig } from 'data/services/lms/hooks/selectors'; - -export const ButtonStates = StrictDict({ - default: 'default', - pending: 'pending', - complete: 'complete', - error: 'error', -}); - -export const useRubricData = ({ - isGrading, -}) => { - const criteria = useRubricConfig().criteria.map( - (_, index) => ({ - isGrading, - key: index, - orderNum: index, - }), - ); - const submitButtonState = ButtonStates.default; - - return { - criteria, - showFooter: isGrading, - buttonProps: { - onClick: () => ({}), - state: submitButtonState, - disabledStates: [ButtonStates.pending, ButtonStates.complete], - }, - }; -}; diff --git a/src/components/Rubric/hooks.ts b/src/components/Rubric/hooks.ts new file mode 100644 index 00000000..5e38b1d9 --- /dev/null +++ b/src/components/Rubric/hooks.ts @@ -0,0 +1,28 @@ +import { usePageData, useRubricConfig } from 'data/services/lms/hooks/selectors'; +import { updatePageData } from 'data/services/lms/hooks/actions'; + +export const useRubricData = ({ + isGrading, +}) => { + const { feedbackConfig } = useRubricConfig(); + const data = usePageData(); + const mutation = updatePageData(); + + const onOverallFeedbackChange = ({ target }) => { + mutation.mutate({ + ...data, + rubric: { + ...data.rubric, + overallFeedback: target.value, + } + } as any); + }; + + return { + feedbackPrompt: feedbackConfig.defaultText, + value: data.rubric.overallFeedback, + onChange: onOverallFeedbackChange, + disabled: !isGrading, + isInvalid: false, + }; +}; diff --git a/src/components/Rubric/index.jsx b/src/components/Rubric/index.jsx index 56f9551f..13747dd5 100644 --- a/src/components/Rubric/index.jsx +++ b/src/components/Rubric/index.jsx @@ -36,7 +36,7 @@ export const Rubric = ({ isGrading }) => { ))}
- {isGrading && } + {isGrading && } {isGrading && (
diff --git a/src/data/services/lms/fakeData/pageData/index.jsx b/src/data/services/lms/fakeData/pageData/index.jsx index 3afe557c..b3ac5afe 100644 --- a/src/data/services/lms/fakeData/pageData/index.jsx +++ b/src/data/services/lms/fakeData/pageData/index.jsx @@ -10,7 +10,7 @@ export const emptySubmission = { export const peerAssessment = { progress: progressStates.peer(), - rubric: rubricStates.criteriaFeedbackEnabled.empty, + rubric: rubricStates.criteriaFeedbackEnabled.filled, submission: submissionStates.individialSubmission, }; diff --git a/src/data/services/lms/hooks/actions.test.ts b/src/data/services/lms/hooks/actions.test.ts new file mode 100644 index 00000000..5c286bc0 --- /dev/null +++ b/src/data/services/lms/hooks/actions.test.ts @@ -0,0 +1,48 @@ +import { mocked } from 'ts-jest/utils'; +import { useQueryClient, useMutation } from '@tanstack/react-query'; +import { queryKeys } from '../constants'; + +import { createMutationAction, updatePageData } from './actions'; + +jest.mock('@tanstack/react-query', () => ({ + useQueryClient: jest.fn(), + useMutation: jest.fn(), +})); + +const mockUseMutation = mocked(useMutation, true); +const mockUseQueryClient = mocked(useQueryClient, true); + +describe('actions', () => { + const queryClient = { setQueryData: jest.fn() }; + + // @ts-ignore + mockUseQueryClient.mockReturnValue(queryClient); + // @ts-ignore + mockUseMutation.mockImplementation(({ mutationFn }) => { + return { + mutate: mutationFn, + }; + }); + + describe('createMutationAction', () => { + it('returns a mutation function', () => { + const aribtraryMutationFn = jest.fn(); + const mutation = createMutationAction(aribtraryMutationFn) as any; + + mutation.mutate('foo', 'bar'); + expect(aribtraryMutationFn).toHaveBeenCalledWith('foo', 'bar', queryClient); + }); + }); + + describe('updatePageData', () => { + const mutation = updatePageData(); + + it('returns a mutation function', () => { + // @ts-ignore + mutation.mutate({'foo': 'bar'}); + expect(queryClient.setQueryData).toHaveBeenCalledWith([queryKeys.pageData, true], { + foo: 'bar', + }); + }); + }); +}); diff --git a/src/data/services/lms/hooks/actions.ts b/src/data/services/lms/hooks/actions.ts new file mode 100644 index 00000000..ce942a6d --- /dev/null +++ b/src/data/services/lms/hooks/actions.ts @@ -0,0 +1,20 @@ +import { useQueryClient, useMutation } from '@tanstack/react-query'; +import { queryKeys } from '../constants'; +import { ActionMutationFunction, PageData } from '../types'; + +export const createMutationAction = (mutationFn: ActionMutationFunction) => { + const queryClient = useQueryClient(); + + return useMutation({ + mutationFn: (...args) => mutationFn(...args, queryClient), + }); +}; + +export const updatePageData = () => + createMutationAction((data: PageData, queryClient) => { + queryClient.setQueryData([queryKeys.pageData, true], { + ...data, + }); + + return Promise.resolve(data); + }); diff --git a/src/data/services/lms/hooks/api.test.ts b/src/data/services/lms/hooks/api.test.ts index d87ec826..0364f5db 100644 --- a/src/data/services/lms/hooks/api.test.ts +++ b/src/data/services/lms/hooks/api.test.ts @@ -9,6 +9,7 @@ import { queryKeys } from '../constants'; import fakeData from '../fakeData'; import { useORAConfig, usePageData } from './api'; +import smartCamelize from 'utils/smartCamelize'; jest.mock('@tanstack/react-query', () => ({ useQuery: jest.fn() })); @@ -74,8 +75,13 @@ describe('lms api hooks', () => { }); }); describe('usePageData', () => { + const pageDataCamelCase = (data) => smartCamelize(data, [ + 'options_selected', + 'criterion_feedback', + 'overall_feedback', + ]); const mockUseQuery = (data?: types.PageData): MockUsePageDataQuery => ({ queryKey, queryFn }) => ({ - data: data ? camelCaseObject(data) : undefined, + data: data ? pageDataCamelCase(data) : {}, queryKey, queryFn, }); @@ -104,10 +110,10 @@ describe('lms api hooks', () => { }); it('initializes query with promise pointing to empty submission page data', async () => { const response = await out.queryFn(); - expect(response).toEqual(fakeData.pageData.shapes.emptySubmission); + expect(response).toEqual(pageDataCamelCase(fakeData.pageData.shapes.emptySubmission)); }); it('returns camelCase object from data if data has been returned', () => { - expect(out.data).toEqual(camelCaseObject(fakeData.pageData.shapes.emptySubmission)); + expect(out.data).toEqual(pageDataCamelCase(fakeData.pageData.shapes.emptySubmission)); }); }); describe('assessment', () => { @@ -121,10 +127,10 @@ describe('lms api hooks', () => { }); it('initializes query with promise pointing to peer assessment page data', async () => { const response = await out.queryFn(); - expect(response).toEqual(fakeData.pageData.shapes.peerAssessment); + expect(response).toEqual(pageDataCamelCase(fakeData.pageData.shapes.peerAssessment)); }); it('returns camelCase object from data if data has been returned', () => { - expect(out.data).toEqual(camelCaseObject(fakeData.pageData.shapes.peerAssessment)); + expect(out.data).toEqual(pageDataCamelCase(fakeData.pageData.shapes.peerAssessment)); }); }); it('returns empty object from data if data has not been returned', () => { diff --git a/src/data/services/lms/hooks/api.ts b/src/data/services/lms/hooks/api.ts index ba539eb5..cb4c11f1 100644 --- a/src/data/services/lms/hooks/api.ts +++ b/src/data/services/lms/hooks/api.ts @@ -7,13 +7,16 @@ import routes from 'routes'; import * as types from '../types'; import { queryKeys } from '../constants'; import fakeData from '../fakeData'; +import smartCamelize from 'utils/smartCamelize'; export const useORAConfig = (): types.QueryData => { const { data, ...status } = useQuery({ queryKey: [queryKeys.oraConfig], // queryFn: () => getAuthenticatedClient().get(...), queryFn: () => { - const result = window.location.pathname.endsWith('text') ? fakeData.oraConfig.assessmentText : fakeData.oraConfig.assessmentTinyMCE; + const result = window.location.pathname.endsWith('text') + ? fakeData.oraConfig.assessmentText + : fakeData.oraConfig.assessmentTinyMCE; return Promise.resolve(result); }, }); @@ -26,17 +29,28 @@ export const useORAConfig = (): types.QueryData => { export const usePageData = (): types.QueryData => { const route = useRouteMatch(); const isAssessment = route.path === routes.peerAssessment; - const returnData = isAssessment - ? fakeData.pageData.shapes.peerAssessment - : fakeData.pageData.shapes.emptySubmission; const { data, ...status } = useQuery({ queryKey: [queryKeys.pageData, isAssessment], // queryFn: () => getAuthenticatedClient().get(...), - queryFn: () => Promise.resolve(returnData), + queryFn: () => { + const assessmentData = isAssessment + ? fakeData.pageData.shapes.peerAssessment + : fakeData.pageData.shapes.emptySubmission; + + const returnData = assessmentData + ? smartCamelize(assessmentData, [ + 'options_selected', + 'criterion_feedback', + 'overall_feedback', + ]) + : {}; + + return Promise.resolve(returnData); + }, }); return { ...status, - data: data ? camelCaseObject(data) : {}, + data, }; }; diff --git a/src/data/services/lms/index.js b/src/data/services/lms/index.js index 08e4502d..ef9c1ab0 100644 --- a/src/data/services/lms/index.js +++ b/src/data/services/lms/index.js @@ -2,9 +2,11 @@ import { StrictDict } from '@edx/react-unit-test-utils'; import urls from './urls'; import api from './hooks/api'; import selectors from './hooks/selectors'; +import actions from './hooks/actions'; export default StrictDict({ api, selectors, urls, + actions, }); diff --git a/src/data/services/lms/types/react-query.ts b/src/data/services/lms/types/react-query.ts index 400e0fc7..df8eb99e 100644 --- a/src/data/services/lms/types/react-query.ts +++ b/src/data/services/lms/types/react-query.ts @@ -1,3 +1,5 @@ +import { QueryClient } from "@tanstack/query-core"; + // React-Query fields export interface QueryStatus { isLoading: boolean, @@ -14,3 +16,5 @@ export interface QueryData extends QueryStatus { error: unknown, data: Response, } + +export type ActionMutationFunction = (args: any, queryClient: QueryClient) => Promise diff --git a/src/utils/smartCamelize.test.ts b/src/utils/smartCamelize.test.ts new file mode 100644 index 00000000..089e49c2 --- /dev/null +++ b/src/utils/smartCamelize.test.ts @@ -0,0 +1,28 @@ +import smartCamelize from "./smartCamelize"; + +describe("smartCamelize", () => { + test('should return the same object if it is not an object', () => { + expect(smartCamelize(undefined)).toBe(undefined); + expect(smartCamelize(null)).toBe(null); + expect(smartCamelize(1)).toBe(1); + expect(smartCamelize('string')).toBe('string'); + expect(smartCamelize(true)).toBe(true); + expect(smartCamelize(false)).toBe(false); + }); + + test('use stoppageKeys to stop camelizing', () => { + const object = { + 'snake_case': { + 'snake_case': 'value', + }, + 'camelCase': 'value', + }; + + expect(smartCamelize(object, ['snake_case'])).toEqual({ + 'snakeCase': { + 'snake_case': 'value', + }, + 'camelCase': 'value', + }); + }); +}); \ No newline at end of file diff --git a/src/utils/smartCamelize.ts b/src/utils/smartCamelize.ts new file mode 100644 index 00000000..e2f1cdd3 --- /dev/null +++ b/src/utils/smartCamelize.ts @@ -0,0 +1,30 @@ +import camelCase from 'lodash.camelcase'; + +export function smartCamelize(object: any, stoppageKeys: string[] = []) { + // If the passed in object is not an Object, return it. + if ( + object === undefined || + object === null || + (typeof object !== 'object' && !Array.isArray(object)) + ) { + return object; + } + + if (Array.isArray(object)) { + return object.map((value) => smartCamelize(value)); + } + + // Otherwise, process all its keys. + const result = {}; + Object.entries(object).forEach(([key, value]) => { + if ( stoppageKeys.includes(key) ) { + result[camelCase(key)] = value; + } + else { + result[camelCase(key)] = smartCamelize(value, stoppageKeys); + } + }); + return result; +}; + +export default smartCamelize; diff --git a/src/views/PeerAssessmentView/AssessmentContentLayout.jsx b/src/views/PeerAssessmentView/AssessmentContentLayout.jsx index 55f600f5..80f0fc8f 100644 --- a/src/views/PeerAssessmentView/AssessmentContentLayout.jsx +++ b/src/views/PeerAssessmentView/AssessmentContentLayout.jsx @@ -18,7 +18,7 @@ const AssessmentContentLayout = () => { - {showRubric && ()} + {showRubric && ()}
diff --git a/tsconfig.json b/tsconfig.json index f168de28..173f94f6 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,7 +1,7 @@ { "extends": "@edx/typescript-config", "compilerOptions": { - "rootDir": ".", + "rootDir": "./src", "outDir": "dist", "baseUrl": "./src", "paths": {