From ab2bf89e255869d4293564e478c677a9f5e32d86 Mon Sep 17 00:00:00 2001 From: leangseu-edx <83240113+leangseu-edx@users.noreply.github.com> Date: Fri, 7 Oct 2022 11:08:00 -0400 Subject: [PATCH] fix: update logic for unenrollment (#36) --- .../components/ReasonPane.test.jsx | 1 - src/containers/UnenrollConfirmModal/hooks.js | 29 +++++----- .../UnenrollConfirmModal/hooks.test.js | 55 ++++++------------- src/containers/UnenrollConfirmModal/index.jsx | 4 +- .../UnenrollConfirmModal/index.test.jsx | 21 ++++--- src/data/redux/thunkActions/app.js | 3 +- src/data/services/lms/api.js | 8 +-- src/data/services/lms/api.test.js | 3 +- src/data/services/lms/utils.js | 5 +- src/data/services/lms/utils.test.js | 12 +++- 10 files changed, 66 insertions(+), 75 deletions(-) diff --git a/src/containers/UnenrollConfirmModal/components/ReasonPane.test.jsx b/src/containers/UnenrollConfirmModal/components/ReasonPane.test.jsx index 039974e..db5e490 100644 --- a/src/containers/UnenrollConfirmModal/components/ReasonPane.test.jsx +++ b/src/containers/UnenrollConfirmModal/components/ReasonPane.test.jsx @@ -6,7 +6,6 @@ import { ReasonPane } from './ReasonPane'; describe('UnenrollConfirmModal ReasonPane', () => { const props = { reason: { - value: 'props.reason.value', skip: jest.fn().mockName('props.reason.skip'), selectOption: jest.fn().mockName('props.reason.selectOption'), customOption: { diff --git a/src/containers/UnenrollConfirmModal/hooks.js b/src/containers/UnenrollConfirmModal/hooks.js index 6011989..2839edd 100644 --- a/src/containers/UnenrollConfirmModal/hooks.js +++ b/src/containers/UnenrollConfirmModal/hooks.js @@ -11,7 +11,6 @@ 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 - submittedReason: (val) => React.useState(val), // eslint-disable-line }); export const modalStates = StrictDict({ @@ -20,27 +19,25 @@ export const modalStates = StrictDict({ finished: 'finished', }); -export const useUnenrollReasons = () => { +export const useUnenrollReasons = ({ + dispatch, + cardId, +}) => { const [selectedReason, setSelectedReason] = module.state.selectedReason(null); - const [submittedReason, setSubmittedReason] = module.state.submittedReason(null); const [isSkipped, setIsSkipped] = module.state.isSkipped(false); const [customOption, setCustomOption] = module.state.customOption(''); return { clear: React.useCallback(() => { setSelectedReason(null); - setSubmittedReason(null); setCustomOption(''); setIsSkipped(false); }, [ setSelectedReason, - setSubmittedReason, setCustomOption, setIsSkipped, ]), - value: submittedReason, - customOption: { value: customOption, onChange: useValueCallback(setCustomOption), @@ -52,23 +49,23 @@ export const useUnenrollReasons = () => { isSkipped, skip: React.useCallback(() => setIsSkipped(true), [setIsSkipped]), - isSubmitted: submittedReason !== null || isSkipped, + isSubmitted: isSkipped, submit: React.useCallback(() => { - if (selectedReason === 'custom') { - setSubmittedReason(customOption); - } else { - setSubmittedReason(selectedReason); - } - }, [setSubmittedReason, customOption, selectedReason]), + const submittedReason = selectedReason === 'custom' ? customOption : selectedReason; + dispatch(thunkActions.app.unenrollFromCourse(cardId, submittedReason)); + }, [cardId, customOption, dispatch, selectedReason]), }; }; -export const useUnenrollData = ({ closeModal, dispatch }) => { +export const useUnenrollData = ({ closeModal, dispatch, cardId }) => { const [isConfirmed, setIsConfirmed] = module.state.confirmed(false); const confirm = React.useCallback(() => setIsConfirmed(true), [setIsConfirmed]); - const reason = module.useUnenrollReasons(); + const reason = module.useUnenrollReasons({ + dispatch, + cardId, + }); const close = React.useCallback(() => { closeModal(); diff --git a/src/containers/UnenrollConfirmModal/hooks.test.js b/src/containers/UnenrollConfirmModal/hooks.test.js index 366a421..b5f51a8 100644 --- a/src/containers/UnenrollConfirmModal/hooks.test.js +++ b/src/containers/UnenrollConfirmModal/hooks.test.js @@ -15,21 +15,25 @@ jest.mock('data/redux/thunkActions/app', () => ({ const state = new MockUseState(hooks); const testValue = 'test-value'; let out; -let hook; describe('UnenrollConfirmModal hooks', () => { + const dispatch = jest.fn(); + const closeModal = jest.fn(); + const cardId = 'test-card-id'; + + const createUseUnenrollReasons = () => hooks.useUnenrollReasons({ dispatch, cardId }); + const createUseUnenrollData = () => hooks.useUnenrollData({ closeModal, dispatch, cardId }); + describe('state fields', () => { state.testGetter(state.keys.confirmed); state.testGetter(state.keys.customOption); state.testGetter(state.keys.isSkipped); state.testGetter(state.keys.selectedReason); - state.testGetter(state.keys.submittedReason); }); describe('useUnenrollReasons', () => { beforeEach(() => { - hook = hooks.useUnenrollReasons; state.mock(); - out = hook(); + out = createUseUnenrollReasons(); }); afterEach(() => { state.restore(); @@ -39,38 +43,32 @@ describe('UnenrollConfirmModal hooks', () => { const { cb, prereqs } = out.clear.useCallback; expect(prereqs).toEqual([ state.setState.selectedReason, - state.setState.submittedReason, state.setState.customOption, state.setState.isSkipped, ]); cb(); expect(state.setState.selectedReason).toHaveBeenCalledWith(null); - expect(state.setState.submittedReason).toHaveBeenCalledWith(null); expect(state.setState.customOption).toHaveBeenCalledWith(''); expect(state.setState.isSkipped).toHaveBeenCalledWith(false); }); }); - test('value returns submitted reason', () => { - state.mockVal(state.keys.submittedReason, testValue); - expect(hook().value).toEqual(testValue); - }); test('customOption.value returns custom option', () => { state.mockVal(state.keys.customOption, testValue); - expect(hook().customOption.value).toEqual(testValue); + expect(createUseUnenrollReasons().customOption.value).toEqual(testValue); }); test('customOption.onChange returns valueCallback for setCustomOption', () => { expect(out.customOption.onChange).toEqual(useValueCallback(state.setState.customOption)); }); test('selected returns selectedReason', () => { state.mockVal(state.keys.selectedReason, testValue); - expect(hook().selected).toEqual(testValue); + expect(createUseUnenrollReasons().selected).toEqual(testValue); }); test('selectedOption returns valueCallback for setSelectedReason', () => { expect(out.selectOption).toEqual(useValueCallback(state.setState.selectedReason)); }); test('isSkipped returns state value', () => { state.mockVal(state.keys.isSkipped, testValue); - expect(hook().isSkipped).toEqual(testValue); + expect(createUseUnenrollReasons().isSkipped).toEqual(testValue); }); test('skip returns callback that sets isSkipped to true', () => { const { cb, prereqs } = out.skip.useCallback; @@ -82,44 +80,25 @@ describe('UnenrollConfirmModal hooks', () => { it('returns false if submittedReason is null and not isSkipped', () => { expect(out.isSubmitted).toEqual(false); }); - it('returns true if submittedReason is not null', () => { - state.mockVal(state.keys.submittedReason, testValue); - expect(hook().isSubmitted).toEqual(true); - }); it('returns true if isSkipped', () => { state.mockVal(state.keys.isSkipped, true); - expect(hook().isSubmitted).toEqual(true); + expect(createUseUnenrollReasons().isSubmitted).toEqual(true); }); }); describe('submit', () => { - it('sets customOption as submittedReason if selectedReason is custom', () => { - state.mockVal(state.keys.selectedReason, 'custom'); - state.mockVal(state.keys.customOption, testValue); - hook().submit.useCallback.cb(); - expect(state.setState.submittedReason).toHaveBeenCalledWith(testValue); - }); - it('sets selectedReason as submittedReason if selectedReason is not custom', () => { - state.mockVal(state.keys.selectedReason, testValue); - state.mockVal(state.keys.customOption, 'customValue'); - hook().submit.useCallback.cb(); - expect(state.setState.submittedReason).toHaveBeenCalledWith(testValue); - }); it('depends on customOption and selectedReason', () => { const customValue = 'custom-value'; state.mockVal(state.keys.selectedReason, testValue); state.mockVal(state.keys.customOption, customValue); - const { prereqs } = hook().submit.useCallback; + const { prereqs } = createUseUnenrollReasons().submit.useCallback; expect(prereqs).toContain(testValue); expect(prereqs).toContain(customValue); }); }); }); describe('modalHooks', () => { - const closeModal = jest.fn(); - const dispatch = jest.fn(); let mockReason; beforeEach(() => { - hook = hooks.useUnenrollData; mockReason = { isSubmitted: false, clear: jest.fn(), @@ -127,7 +106,7 @@ describe('UnenrollConfirmModal hooks', () => { state.mock(); state.mockVal(state.keys.confirmed, testValue); hooks.useUnenrollReasons = jest.fn(() => mockReason); - out = hook({ closeModal, dispatch }); + out = createUseUnenrollData(); }); afterEach(() => { state.restore(); @@ -184,17 +163,17 @@ describe('UnenrollConfirmModal hooks', () => { it('returns modalStates.finished if confirmed and submitted', () => { state.mockVal(state.keys.confirmed, true); hooks.useUnenrollReasons = jest.fn(() => ({ ...mockReason, isSubmitted: true })); - out = hook({ closeModal, dispatch }); + out = createUseUnenrollData(); expect(out.modalState).toEqual(hooks.modalStates.finished); }); it('returns modalStates.reason if confirmed and not submitted', () => { state.mockVal(state.keys.confirmed, true); - out = hook({ closeModal, dispatch }); + out = createUseUnenrollData(); expect(out.modalState).toEqual(hooks.modalStates.reason); }); it('returns modalStates.confirm if not confirmed', () => { state.mockVal(state.keys.confirmed, false); - out = hook({ closeModal, dispatch }); + out = createUseUnenrollData(); expect(out.modalState).toEqual(hooks.modalStates.confirm); }); }); diff --git a/src/containers/UnenrollConfirmModal/index.jsx b/src/containers/UnenrollConfirmModal/index.jsx index 41df9a5..dc66dd2 100644 --- a/src/containers/UnenrollConfirmModal/index.jsx +++ b/src/containers/UnenrollConfirmModal/index.jsx @@ -18,6 +18,7 @@ import { useUnenrollData, modalStates } from './hooks'; export const UnenrollConfirmModal = ({ closeModal, show, + cardId, }) => { const dispatch = useDispatch(); const { @@ -26,7 +27,7 @@ export const UnenrollConfirmModal = ({ closeAndRefresh, close, modalState, - } = useUnenrollData({ dispatch, closeModal }); + } = useUnenrollData({ dispatch, closeModal, cardId }); const showFullscreen = modalState === modalStates.reason; return ( { closeAndRefresh: jest.fn().mockName('hooks.closeAndRefresh'), modalState: hooks.modalStates.confirm, }; - const closeModal = jest.fn().mockName('props.closeModal'); - const show = true; + const closeModal = jest.fn().mockName('closeModal'); + const cardId = 'cardId'; + const props = { + closeModal, + show: true, + cardId, + }; test('hooks called with dispatch and closeModal props', () => { hooks.useUnenrollData.mockReturnValueOnce(hookProps); - shallow(); - expect(hooks.useUnenrollData).toHaveBeenCalledWith({ dispatch, closeModal }); + shallow(); + expect(hooks.useUnenrollData).toHaveBeenCalledWith({ dispatch, closeModal, cardId }); }); test('snapshot: modalStates.confirm', () => { hooks.useUnenrollData.mockReturnValueOnce(hookProps); - expect(shallow()).toMatchSnapshot(); + expect(shallow()).toMatchSnapshot(); }); test('snapshot: modalStates.finished, reason given', () => { hooks.useUnenrollData.mockReturnValueOnce({ ...hookProps, modalState: hooks.modalStates.finished }); - expect(shallow()).toMatchSnapshot(); + expect(shallow()).toMatchSnapshot(); }); test('snapshot: modalStates.finished, reason skipped', () => { hooks.useUnenrollData.mockReturnValueOnce({ @@ -49,10 +54,10 @@ describe('UnenrollConfirmModal component', () => { modalState: hooks.modalStates.finished, isSkipped: true, }); - expect(shallow()).toMatchSnapshot(); + expect(shallow()).toMatchSnapshot(); }); test('snapshot: modalStates.reason, should be fullscreen with no shadow', () => { hooks.useUnenrollData.mockReturnValueOnce({ ...hookProps, modalState: hooks.modalStates.reason }); - expect(shallow()).toMatchSnapshot(); + expect(shallow()).toMatchSnapshot(); }); }); diff --git a/src/data/redux/thunkActions/app.js b/src/data/redux/thunkActions/app.js index dbf5461..8425a52 100644 --- a/src/data/redux/thunkActions/app.js +++ b/src/data/redux/thunkActions/app.js @@ -67,7 +67,8 @@ export const leaveEntitlementSession = (cardId) => (dispatch, getState) => { return dispatch(requests.leaveEntitlementSession({ uuid })); }; -export const unenrollFromCourse = (courseId, reason) => (dispatch) => { +export const unenrollFromCourse = (cardId, reason) => (dispatch, getState) => { + const { courseId } = selectors.app.courseCard.courseRun(getState(), cardId); handleEvent(eventNames.unenrollReason, { category: 'user-engagement', displayName: 'v1', diff --git a/src/data/services/lms/api.js b/src/data/services/lms/api.js index 01521a9..ba7a820 100644 --- a/src/data/services/lms/api.js +++ b/src/data/services/lms/api.js @@ -34,10 +34,10 @@ const updateEmailSettings = ({ courseId, enable }) => post(stringifyUrl( { [apiKeys.courseId]: courseId, ...(enable && enableEmailsAction) }, )); -const unenrollFromCourse = ({ courseId }) => post(stringifyUrl( - urls.courseUnenroll, - { [apiKeys.courseId]: courseId, ...unenrollmentAction }, -)); +const unenrollFromCourse = ({ courseId }) => post(stringifyUrl(urls.courseUnenroll), { + [apiKeys.courseId]: courseId, + ...unenrollmentAction, +}); export default { initializeList, diff --git a/src/data/services/lms/api.test.js b/src/data/services/lms/api.test.js index e3bab74..47fe1b5 100644 --- a/src/data/services/lms/api.test.js +++ b/src/data/services/lms/api.test.js @@ -90,8 +90,7 @@ describe('lms api methods', () => { ).toEqual( utils.post(utils.stringifyUrl( urls.courseUnenroll, - { [apiKeys.courseId]: testCourseId, ...unenrollmentAction }, - )), + ), { [apiKeys.courseId]: testCourseId, ...unenrollmentAction }), ); }); }); diff --git a/src/data/services/lms/utils.js b/src/data/services/lms/utils.js index 4b0dff5..65a5ff6 100644 --- a/src/data/services/lms/utils.js +++ b/src/data/services/lms/utils.js @@ -10,10 +10,11 @@ export const get = (...args) => getAuthenticatedHttpClient().get(...args); /** * post(url, data) * simple wrapper providing an authenticated Http client post action + * queryString.stringify is used to convert the object to query string with = and & * @param {string} url - target url - * @param {object|string} data - post payload + * @param {object|string} body - post payload */ -export const post = (...args) => getAuthenticatedHttpClient().post(...args); +export const post = (url, body) => getAuthenticatedHttpClient().post(url, queryString.stringify(body)); export const client = getAuthenticatedHttpClient; diff --git a/src/data/services/lms/utils.test.js b/src/data/services/lms/utils.test.js index a033bba..fbb0a93 100644 --- a/src/data/services/lms/utils.test.js +++ b/src/data/services/lms/utils.test.js @@ -4,6 +4,7 @@ import * as utils from './utils'; jest.mock('query-string', () => ({ stringifyUrl: jest.fn((url, options) => ({ url, options })), + stringify: jest.fn((data) => data), })); jest.mock('@edx/frontend-platform/auth', () => ({ getAuthenticatedHttpClient: jest.fn(), @@ -22,8 +23,15 @@ describe('lms service utils', () => { it('forwards arguments to authenticatedHttpClient().post', () => { const post = jest.fn((...args) => ({ post: args })); getAuthenticatedHttpClient.mockReturnValue({ post }); - const args = ['some', 'args', 'for', 'the', 'test']; - expect(utils.post(...args)).toEqual(post(...args)); + const url = 'some url'; + const body = { + some: 'body', + for: 'the', + test: 'yay', + }; + const expectedUrl = utils.post(url, body); + expect(queryString.stringify).toHaveBeenCalledWith(body); + expect(expectedUrl).toEqual(post(url, body)); }); }); describe('stringifyUrl', () => {