From f435e8d2c2967adaaa059a13e6a668ab716e922e Mon Sep 17 00:00:00 2001 From: Ben Warzeski Date: Fri, 2 Dec 2022 15:10:11 -0500 Subject: [PATCH] fix: unenroll messaging (#84) --- src/__snapshots__/index.test.jsx.snap | 4 +- .../components/ReasonPane.jsx | 9 +- .../components/ReasonPane.test.jsx | 4 +- .../__snapshots__/ReasonPane.test.jsx.snap | 4 +- .../components/messages.js | 4 +- .../UnenrollConfirmModal/hooks/index.js | 20 +--- .../UnenrollConfirmModal/hooks/index.test.js | 10 +- .../UnenrollConfirmModal/hooks/reasons.js | 46 ++++++-- .../hooks/reasons.test.js | 109 +++++++++++++----- src/containers/UnenrollConfirmModal/index.jsx | 3 +- src/data/redux/thunkActions/app.js | 5 +- src/data/redux/thunkActions/app.test.js | 7 +- 12 files changed, 144 insertions(+), 81 deletions(-) diff --git a/src/__snapshots__/index.test.jsx.snap b/src/__snapshots__/index.test.jsx.snap index fed133f..fcad384 100644 --- a/src/__snapshots__/index.test.jsx.snap +++ b/src/__snapshots__/index.test.jsx.snap @@ -23,12 +23,12 @@ exports[`app registry subscribe: APP_READY. links App to root element 1`] = ` > diff --git a/src/containers/UnenrollConfirmModal/components/ReasonPane.jsx b/src/containers/UnenrollConfirmModal/components/ReasonPane.jsx index 8ab0977..22acb88 100644 --- a/src/containers/UnenrollConfirmModal/components/ReasonPane.jsx +++ b/src/containers/UnenrollConfirmModal/components/ReasonPane.jsx @@ -13,7 +13,6 @@ import messages from './messages'; export const ReasonPane = ({ reason, - handleSubmit, }) => { const { formatMessage } = useIntl(); const option = (key) => ( @@ -38,10 +37,10 @@ export const ReasonPane = ({ - - @@ -51,15 +50,15 @@ export const ReasonPane = ({ ReasonPane.propTypes = { reason: PropTypes.shape({ value: PropTypes.string, - skip: PropTypes.func, + handleSkip: PropTypes.func, selectOption: PropTypes.func, customOption: PropTypes.shape({ value: PropTypes.string, onChange: PropTypes.func, }), selected: PropTypes.string, + handleSubmit: PropTypes.func.isRequired, }).isRequired, - handleSubmit: PropTypes.func.isRequired, }; export default ReasonPane; diff --git a/src/containers/UnenrollConfirmModal/components/ReasonPane.test.jsx b/src/containers/UnenrollConfirmModal/components/ReasonPane.test.jsx index 19b092a..ddb5033 100644 --- a/src/containers/UnenrollConfirmModal/components/ReasonPane.test.jsx +++ b/src/containers/UnenrollConfirmModal/components/ReasonPane.test.jsx @@ -6,15 +6,15 @@ import { ReasonPane } from './ReasonPane'; describe('UnenrollConfirmModal ReasonPane', () => { const props = { reason: { - skip: jest.fn().mockName('props.reason.skip'), + handleSkip: jest.fn().mockName('props.reason.handleSkip'), selectOption: jest.fn().mockName('props.reason.selectOption'), customOption: { value: 'props.reason.customOption.value', onChange: jest.fn().mockName('props.reason.customOption.onChange'), }, selected: 'props.reason.selected', + handleSubmit: jest.fn().mockName('props.reason.handleSubmit'), }, - handleSubmit: jest.fn().mockName('props.handleSubmit'), }; test('snapshot', () => { expect(shallow()).toMatchSnapshot(); diff --git a/src/containers/UnenrollConfirmModal/components/__snapshots__/ReasonPane.test.jsx.snap b/src/containers/UnenrollConfirmModal/components/__snapshots__/ReasonPane.test.jsx.snap index 184bcb3..e8d9760 100644 --- a/src/containers/UnenrollConfirmModal/components/__snapshots__/ReasonPane.test.jsx.snap +++ b/src/containers/UnenrollConfirmModal/components/__snapshots__/ReasonPane.test.jsx.snap @@ -76,13 +76,13 @@ exports[`UnenrollConfirmModal ReasonPane snapshot 1`] = ` diff --git a/src/containers/UnenrollConfirmModal/components/messages.js b/src/containers/UnenrollConfirmModal/components/messages.js index f6f88b2..1f15e94 100644 --- a/src/containers/UnenrollConfirmModal/components/messages.js +++ b/src/containers/UnenrollConfirmModal/components/messages.js @@ -30,12 +30,12 @@ export const messages = StrictDict({ reasonSkip: { id: 'learner-dash.unenrollConfirm.confirm.reason.skip', description: 'Skip action for unenroll reason modal', - defaultMessage: 'Skip', + defaultMessage: 'Skip survey', }, reasonSubmit: { id: 'learner-dash.unenrollConfirm.confirm.reason.submit', description: 'Submit action for unenroll reason modal', - defaultMessage: 'Submit', + defaultMessage: 'Submit reason', }, finishHeading: { id: 'learner-dash.unenrollConfirm.confirm.finish.heading', diff --git a/src/containers/UnenrollConfirmModal/hooks/index.js b/src/containers/UnenrollConfirmModal/hooks/index.js index a0ef637..9cab3a9 100644 --- a/src/containers/UnenrollConfirmModal/hooks/index.js +++ b/src/containers/UnenrollConfirmModal/hooks/index.js @@ -1,8 +1,7 @@ import React from 'react'; import { StrictDict } from 'utils'; -import { hooks as appHooks, thunkActions } from 'data/redux'; -import track from 'tracking'; +import { thunkActions } from 'data/redux'; import { useUnenrollReasons } from './reasons'; import * as module from '.'; @@ -21,29 +20,19 @@ export const useUnenrollData = ({ closeModal, dispatch, cardId }) => { const [isConfirmed, setIsConfirmed] = module.state.confirmed(false); const confirm = () => setIsConfirmed(true); const reason = useUnenrollReasons({ dispatch, cardId }); - const { isEntitlement } = appHooks.useCardEntitlementData(cardId); - const handleTrackReasons = appHooks.useTrackCourseEvent( - track.engagement.unenrollReason, - cardId, - reason.submittedReason, - isEntitlement, - ); let modalState; if (isConfirmed) { - modalState = reason.isSubmitted ? modalStates.finished : modalStates.reason; + modalState = (reason.isSubmitted || reason.isSkipped) + ? modalStates.finished : modalStates.reason; } else { modalState = modalStates.confirm; } - const handleSubmitReason = () => { - handleTrackReasons(); - dispatch(thunkActions.app.unenrollFromCourse(cardId, reason.submittedReason)); - }; const close = () => { closeModal(); setIsConfirmed(false); - reason.clear(); + reason.handleClear(); }; const closeAndRefresh = () => { dispatch(thunkActions.app.refreshList()); @@ -57,7 +46,6 @@ export const useUnenrollData = ({ closeModal, dispatch, cardId }) => { close, closeAndRefresh, modalState, - handleSubmitReason, }; }; diff --git a/src/containers/UnenrollConfirmModal/hooks/index.test.js b/src/containers/UnenrollConfirmModal/hooks/index.test.js index 1abea23..8b4be0d 100644 --- a/src/containers/UnenrollConfirmModal/hooks/index.test.js +++ b/src/containers/UnenrollConfirmModal/hooks/index.test.js @@ -18,7 +18,7 @@ const testValue = 'test-value'; let out; const mockReason = { - clear: jest.fn(), + handleClear: jest.fn(), isSubmitted: false, submittedReason: 'test-submitted-reason', }; @@ -58,19 +58,19 @@ describe('UnenrollConfirmModal hooks', () => { expect(out.reason).toEqual(mockReason); }); describe('close', () => { - it('calls closeModal, sets isConfirmed to false, and calls reason.clear', () => { + it('calls closeModal, sets isConfirmed to false, and calls reason.handleClear', () => { out.close(); expect(closeModal).toHaveBeenCalled(); expect(state.setState.confirmed).toHaveBeenCalledWith(false); - expect(mockReason.clear).toHaveBeenCalled(); + expect(mockReason.handleClear).toHaveBeenCalled(); }); }); describe('closeAndRefresh', () => { - it('calls closeModal, sets isConfirmed to false, and calls reason.clear', () => { + it('calls closeModal, sets isConfirmed to false, and calls reason.handleClear', () => { out.closeAndRefresh(); expect(closeModal).toHaveBeenCalled(); expect(state.setState.confirmed).toHaveBeenCalledWith(false); - expect(mockReason.clear).toHaveBeenCalled(); + expect(mockReason.handleClear).toHaveBeenCalled(); }); it('dispatches refreshList thunkAction', () => { out.closeAndRefresh(); diff --git a/src/containers/UnenrollConfirmModal/hooks/reasons.js b/src/containers/UnenrollConfirmModal/hooks/reasons.js index 235e4c0..725b53f 100644 --- a/src/containers/UnenrollConfirmModal/hooks/reasons.js +++ b/src/containers/UnenrollConfirmModal/hooks/reasons.js @@ -1,8 +1,9 @@ import React from 'react'; -import { thunkActions } from 'data/redux'; +import { thunkActions, hooks as appHooks } from 'data/redux'; import { useValueCallback } from 'hooks'; import { StrictDict } from 'utils'; +import track from 'tracking'; import * as module from './reasons'; @@ -10,34 +11,63 @@ export const state = StrictDict({ customOption: (val) => React.useState(val), // eslint-disable-line isSkipped: (val) => React.useState(val), // eslint-disable-line selectedReason: (val) => React.useState(val), // eslint-disable-line + isSubmitted: (val) => React.useState(val), //eslint-disable-line }); +export const useTrackUnenrollReasons = ({ cardId, submittedReason }) => { + const { isEntitlement } = appHooks.useCardEntitlementData(cardId); + return appHooks.useTrackCourseEvent( + track.engagement.unenrollReason, + cardId, + submittedReason, + isEntitlement, + ); +}; + export const useUnenrollReasons = ({ dispatch, cardId, }) => { + // The selected option element from the menu const [selectedReason, setSelectedReason] = module.state.selectedReason(null); - const [isSkipped, setIsSkipped] = module.state.isSkipped(false); + // Custom option element entry value const [customOption, setCustomOption] = module.state.customOption(''); + + // Did the user choose to skip selecting a reason? + const [isSkipped, setIsSkipped] = module.state.isSkipped(false); + // Did the user submit an unenrollment reason + const [isSubmitted, setIsSubmitted] = module.state.isSubmitted(false); + const submittedReason = selectedReason === 'custom' ? customOption : selectedReason; - const clear = () => { + const handleTrackReasons = module.useTrackUnenrollReasons({ cardId, submittedReason }); + + const handleClear = () => { setSelectedReason(null); setCustomOption(''); setIsSkipped(false); + setIsSubmitted(false); }; - const skip = () => { + + const handleSkip = () => { setIsSkipped(true); dispatch(thunkActions.app.unenrollFromCourse(cardId)); }; + const handleSubmit = (e) => { + handleTrackReasons(e); + setIsSubmitted(true); + dispatch(thunkActions.app.unenrollFromCourse(cardId, submittedReason)); + }; + return { - clear, customOption: { value: customOption, onChange: useValueCallback(setCustomOption) }, - selectOption: useValueCallback(setSelectedReason), + handleClear, + handleSkip, + handleSubmit, isSkipped, - skip, - isSubmitted: isSkipped, + isSubmitted, + selectOption: useValueCallback(setSelectedReason), submittedReason, }; }; diff --git a/src/containers/UnenrollConfirmModal/hooks/reasons.test.js b/src/containers/UnenrollConfirmModal/hooks/reasons.test.js index 7a7be37..e8d3135 100644 --- a/src/containers/UnenrollConfirmModal/hooks/reasons.test.js +++ b/src/containers/UnenrollConfirmModal/hooks/reasons.test.js @@ -1,20 +1,32 @@ -import { thunkActions } from 'data/redux'; +import { keyStore } from 'utils'; +import { thunkActions, hooks as appHooks } from 'data/redux'; import { useValueCallback } from 'hooks'; import { MockUseState } from 'testUtils'; +import track from 'tracking'; import * as hooks from './reasons'; +jest.mock('data/redux', () => ({ + thunkActions: { + app: { + refreshList: jest.fn((args) => ({ refreshList: args })), + unenrollFromCourse: jest.fn((...args) => ({ unenrollFromCourse: args })), + }, + }, + hooks: { + useCardEntitlementData: jest.fn(), + useTrackCourseEvent: jest.fn(), + }, +})); + jest.mock('hooks', () => ({ useValueCallback: jest.fn((cb, prereqs) => ({ useValueCallback: { cb, prereqs } })), })); -jest.mock('data/redux/thunkActions/app', () => ({ - refreshList: jest.fn((args) => ({ refreshList: args })), - unenrollFromCourse: jest.fn((...args) => ({ unenrollFromCourse: args })), -})); - const state = new MockUseState(hooks); const testValue = 'test-value'; +const testValue2 = 'test-value2'; +const moduleKeys = keyStore(hooks); let out; describe('UnenrollConfirmModal reasons hooks', () => { @@ -26,50 +38,93 @@ describe('UnenrollConfirmModal reasons hooks', () => { describe('state fields', () => { state.testGetter(state.keys.customOption); state.testGetter(state.keys.isSkipped); + state.testGetter(state.keys.isSubmitted); state.testGetter(state.keys.selectedReason); }); + describe('useTrackUnenrollReasons', () => { + it('returns trackCourseEvent for unenroll with submitted reason and isEntitlement', () => { + appHooks.useCardEntitlementData.mockReturnValue({ isEntitlement: false }); + const args = { cardId, submittedReason: testValue }; + expect(hooks.useTrackUnenrollReasons(args)).toEqual(appHooks.useTrackCourseEvent( + track.engagement.unenrollReason, + args.cardId, + args.submittedReason, + false, + )); + }); + }); describe('useUnenrollReasons', () => { + const trackReasonsEvent = jest.fn((e) => ({ trackReasonsEvent: e })); + let useTrackUnenrollReasons; beforeEach(() => { state.mock(); + appHooks.useCardEntitlementData.mockReturnValue({ isEntitlement: false }); + useTrackUnenrollReasons = jest.spyOn(hooks, moduleKeys.useTrackUnenrollReasons) + .mockImplementation(() => trackReasonsEvent); out = createUseUnenrollReasons(); }); afterEach(() => { state.restore(); }); - describe('clear method', () => { + describe('customOption', () => { + test('customOption.value returns custom option', () => { + state.mockVal(state.keys.customOption, testValue); + expect(createUseUnenrollReasons().customOption.value).toEqual(testValue); + }); + test('customOption.onChange returns valueCallback for setCustomOption', () => { + expect(out.customOption.onChange).toEqual(useValueCallback(state.setState.customOption)); + }); + }); + describe('handleClear method', () => { it('resets selected and submitted reasons, custom option and isSkipped', () => { - out.clear(); + out.handleClear(); expect(state.setState.selectedReason).toHaveBeenCalledWith(null); expect(state.setState.customOption).toHaveBeenCalledWith(''); expect(state.setState.isSkipped).toHaveBeenCalledWith(false); + expect(state.setState.isSubmitted).toHaveBeenCalledWith(false); }); }); - test('customOption.value returns custom option', () => { - state.mockVal(state.keys.customOption, testValue); - expect(createUseUnenrollReasons().customOption.value).toEqual(testValue); + test('handleSkip sets isSkipped and isSubmitted, and unenrolls w/out a reason', () => { + out.handleSkip(); + expect(state.setState.isSkipped).toHaveBeenCalledWith(true); + expect(dispatch).toHaveBeenCalledWith(thunkActions.app.unenrollFromCourse(cardId)); }); - test('customOption.onChange returns valueCallback for setCustomOption', () => { - expect(out.customOption.onChange).toEqual(useValueCallback(state.setState.customOption)); - }); - test('selectedOption returns valueCallback for setSelectedReason', () => { - expect(out.selectOption).toEqual(useValueCallback(state.setState.selectedReason)); + describe('handleSubmit', () => { + it('tracks reason event and dispatches unenroll thunk action', () => { + state.mockVal(state.keys.selectedReason, testValue); + out = createUseUnenrollReasons(); + expect(useTrackUnenrollReasons).toHaveBeenCalledWith({ + cardId, + submittedReason: testValue, + }); + expect(trackReasonsEvent).not.toHaveBeenCalled(); + const event = { test: 'event' }; + out.handleSubmit(event); + expect(trackReasonsEvent).toHaveBeenCalledWith(event); + expect(dispatch).toHaveBeenCalledWith(thunkActions.app.unenrollFromCourse(cardId)); + }); }); test('isSkipped returns state value', () => { state.mockVal(state.keys.isSkipped, testValue); expect(createUseUnenrollReasons().isSkipped).toEqual(testValue); }); - test('skip returns callback that sets isSkipped to true and unenrolls with no reason', () => { - out.skip(); - expect(state.setState.isSkipped).toHaveBeenCalledWith(true); - expect(dispatch).toHaveBeenCalledWith(thunkActions.app.unenrollFromCourse(cardId)); + test('isSubmitted returns state value', () => { + state.mockVal(state.keys.isSubmitted, testValue); + expect(createUseUnenrollReasons().isSubmitted).toEqual(testValue); }); - describe('isSubmitted', () => { - it('returns false if submittedReason is null and not isSkipped', () => { - expect(out.isSubmitted).toEqual(false); - }); - it('returns true if isSkipped', () => { - state.mockVal(state.keys.isSkipped, true); - expect(createUseUnenrollReasons().isSubmitted).toEqual(true); + test('selectedOption returns valueCallback for setSelectedReason', () => { + expect(out.selectOption).toEqual(useValueCallback(state.setState.selectedReason)); + }); + describe('submittedReason', () => { + it('returns the selected reason unless custom is selcted, then shows custom option', () => { + state.mockVal(state.keys.selectedReason, testValue); + state.mockVal(state.keys.customOption, testValue2); + out = createUseUnenrollReasons(); + expect(out.submittedReason).toEqual(testValue); + state.mockVal(state.keys.selectedReason, 'custom'); + state.mockVal(state.keys.customOption, testValue2); + out = createUseUnenrollReasons(); + expect(out.submittedReason).toEqual(testValue2); }); }); }); diff --git a/src/containers/UnenrollConfirmModal/index.jsx b/src/containers/UnenrollConfirmModal/index.jsx index 1e159d1..dc66dd2 100644 --- a/src/containers/UnenrollConfirmModal/index.jsx +++ b/src/containers/UnenrollConfirmModal/index.jsx @@ -27,7 +27,6 @@ export const UnenrollConfirmModal = ({ closeAndRefresh, close, modalState, - handleSubmitReason, } = useUnenrollData({ dispatch, closeModal, cardId }); const showFullscreen = modalState === modalStates.reason; return ( @@ -49,7 +48,7 @@ export const UnenrollConfirmModal = ({ )} {(modalState === modalStates.reason) && ( - + )} diff --git a/src/data/redux/thunkActions/app.js b/src/data/redux/thunkActions/app.js index b62a523..5de5640 100644 --- a/src/data/redux/thunkActions/app.js +++ b/src/data/redux/thunkActions/app.js @@ -59,10 +59,7 @@ export const leaveEntitlementSession = (cardId) => (dispatch, getState) => { export const unenrollFromCourse = (cardId) => (dispatch, getState) => { const { courseId } = selectors.app.courseCard.courseRun(getState(), cardId); - dispatch(requests.unenrollFromCourse({ - courseId, - onSuccess: () => dispatch(module.initialize()), - })); + dispatch(requests.unenrollFromCourse({ courseId })); }; export const masqueradeAs = (user) => (dispatch) => ( diff --git a/src/data/redux/thunkActions/app.test.js b/src/data/redux/thunkActions/app.test.js index d24c4a0..e731fe6 100644 --- a/src/data/redux/thunkActions/app.test.js +++ b/src/data/redux/thunkActions/app.test.js @@ -145,15 +145,10 @@ describe('app thunk actions', () => { }); describe('unenrollFromCourse', () => { const reason = 'test-reason'; - beforeEach(() => { - initializeSpy.mockImplementationOnce(mockInitialize); - }); - it('dispatches unenrollFromCourse request action, re-initializing on success', () => { + it('dispatches unenrollFromCourse request action', () => { module.unenrollFromCourse(cardId, reason)(dispatch, getState); const request = dispatch.mock.calls[0][0]; expect(request.unenrollFromCourse.courseId).toEqual(courseId); - request.unenrollFromCourse.onSuccess(); - expect(dispatch).toHaveBeenCalledWith(mockInitialize()); }); }); describe('masqueradeAs', () => {