From aada46f6ebcc39b6bd1e9b2c5c1b9000052ef9e3 Mon Sep 17 00:00:00 2001 From: Bianca Severino Date: Thu, 7 Oct 2021 16:22:42 -0400 Subject: [PATCH 1/2] fix: redirect user to original location after IDV --- package.json | 1 + .../name-change/NameChange.jsx | 2 +- .../IdVerification.messages.js | 5 ++ src/id-verification/IdVerificationPage.jsx | 11 +++-- .../panels/RequestCameraAccessPanel.jsx | 17 ++++--- src/id-verification/panels/SubmittedPanel.jsx | 20 ++++++-- .../tests/IdVerificationPage.test.jsx | 47 +++++++------------ .../tests/panels/SubmittedPanel.test.jsx | 43 ++++++++++++++++- 8 files changed, 99 insertions(+), 47 deletions(-) diff --git a/package.json b/package.json index 7386f91..b74410e 100644 --- a/package.json +++ b/package.json @@ -50,6 +50,7 @@ "formdata-polyfill": "4.0.10", "history": "4.10.1", "jslib-html5-camera-photo": "3.1.8", + "lodash.camelcase": "^4.3.0", "lodash.debounce": "4.0.8", "lodash.findindex": "4.6.0", "lodash.get": "4.4.2", diff --git a/src/account-settings/name-change/NameChange.jsx b/src/account-settings/name-change/NameChange.jsx index 797e9a1..9c2f285 100644 --- a/src/account-settings/name-change/NameChange.jsx +++ b/src/account-settings/name-change/NameChange.jsx @@ -69,7 +69,7 @@ function NameChangeModal({ useEffect(() => { if (saveState === 'complete') { handleClose(); - push('/id-verification'); + push('/id-verification?next=account%2Fsettings'); } }, [saveState]); diff --git a/src/id-verification/IdVerification.messages.js b/src/id-verification/IdVerification.messages.js index a7f0879..39a2ec0 100644 --- a/src/id-verification/IdVerification.messages.js +++ b/src/id-verification/IdVerification.messages.js @@ -701,6 +701,11 @@ const messages = defineMessages({ defaultMessage: 'Return to Course', description: 'Return to the course which ID verification was accessed from.', }, + 'id.verification.return.generic': { + id: 'id.verification.return.generic', + defaultMessage: 'Return', + description: 'Button to return to the user\'s original location.', + }, 'id.verification.photo.upload.help.title': { id: 'id.verification.photo.upload.help.title', defaultMessage: 'Upload a Photo Instead', diff --git a/src/id-verification/IdVerificationPage.jsx b/src/id-verification/IdVerificationPage.jsx index 97fdbd7..a8a7944 100644 --- a/src/id-verification/IdVerificationPage.jsx +++ b/src/id-verification/IdVerificationPage.jsx @@ -3,6 +3,7 @@ import { connect } from 'react-redux'; import { Route, Switch, Redirect, useRouteMatch, useLocation, } from 'react-router-dom'; +import camelCase from 'lodash.camelcase'; import qs from 'qs'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import { Modal, Button } from '@edx/paragon'; @@ -32,16 +33,16 @@ function IdVerificationPage(props) { const [isModalOpen, setIsModalOpen] = useState(false); - // Course run key is passed as a query string + // Save query params in order to route back to the correct location later useEffect(() => { if (search) { - const parsed = qs.parse(search, { + const parsedQueryParams = qs.parse(search, { ignoreQueryPrefix: true, interpretNumericEntities: true, }); - if (Object.prototype.hasOwnProperty.call(parsed, 'course_id') && parsed.course_id) { - sessionStorage.setItem('courseRunKey', parsed.course_id); - } + Object.entries(parsedQueryParams).forEach(([key, value]) => { + sessionStorage.setItem(camelCase(key), value); + }); } }, [search]); diff --git a/src/id-verification/panels/RequestCameraAccessPanel.jsx b/src/id-verification/panels/RequestCameraAccessPanel.jsx index 57728d2..3e43543 100644 --- a/src/id-verification/panels/RequestCameraAccessPanel.jsx +++ b/src/id-verification/panels/RequestCameraAccessPanel.jsx @@ -38,12 +38,15 @@ function RequestCameraAccessPanel(props) { } }, [mediaAccess, userId]); - // If the user accessed IDV through a course, - // link back to that course rather than the dashboard + // Link back to the correct location if the user accessed IDV somewhere other + // than the dashboard useEffect(() => { - if (sessionStorage.getItem('courseRunKey')) { - setReturnUrl(`courses/${sessionStorage.getItem('courseRunKey')}`); + if (sessionStorage.getItem('courseId')) { + setReturnUrl(`courses/${sessionStorage.getItem('courseId')}`); setReturnText('id.verification.return.course'); + } else if (sessionStorage.getItem('next')) { + setReturnUrl(sessionStorage.getItem('next')); + setReturnText('id.verification.return.generic'); } }, []); @@ -57,7 +60,7 @@ function RequestCameraAccessPanel(props) { return props.intl.formatMessage(messages['id.verification.camera.access.title']); }; - const returnToDashboardLink = ( + const returnLink = ( {props.intl.formatMessage(messages[returnText])} @@ -114,7 +117,7 @@ function RequestCameraAccessPanel(props) {

- {optimizelyExperimentName ? nextButtonLink : returnToDashboardLink} + {optimizelyExperimentName ? nextButtonLink : returnLink}
)} @@ -126,7 +129,7 @@ function RequestCameraAccessPanel(props) {

- {optimizelyExperimentName ? nextButtonLink : returnToDashboardLink} + {optimizelyExperimentName ? nextButtonLink : returnLink}
)} diff --git a/src/id-verification/panels/SubmittedPanel.jsx b/src/id-verification/panels/SubmittedPanel.jsx index d54e3ac..09c5481 100644 --- a/src/id-verification/panels/SubmittedPanel.jsx +++ b/src/id-verification/panels/SubmittedPanel.jsx @@ -1,4 +1,4 @@ -import React, { useEffect, useContext } from 'react'; +import React, { useContext, useEffect, useState } from 'react'; import { getConfig } from '@edx/frontend-platform'; import { sendTrackEvent } from '@edx/frontend-platform/analytics'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; @@ -10,6 +10,8 @@ import messages from '../IdVerification.messages'; function SubmittedPanel(props) { const { userId } = useContext(IdVerificationContext); + const [returnUrl, setReturnUrl] = useState('dashboard'); + const [returnText, setReturnText] = useState('id.verification.return.dashboard'); const panelSlug = 'submitted'; useEffect(() => { @@ -19,6 +21,18 @@ function SubmittedPanel(props) { }); }, [userId]); + // Link back to the correct location if the user accessed IDV somewhere other + // than the dashboard + useEffect(() => { + if (sessionStorage.getItem('courseId')) { + setReturnUrl(`courses/${sessionStorage.getItem('courseId')}`); + setReturnText('id.verification.return.course'); + } else if (sessionStorage.getItem('next')) { + setReturnUrl(sessionStorage.getItem('next')); + setReturnText('id.verification.return.generic'); + } + }, []); + return ( - {props.intl.formatMessage(messages['id.verification.return.dashboard'])} + {props.intl.formatMessage(messages[returnText])} ); diff --git a/src/id-verification/tests/IdVerificationPage.test.jsx b/src/id-verification/tests/IdVerificationPage.test.jsx index 09ce597..4479729 100644 --- a/src/id-verification/tests/IdVerificationPage.test.jsx +++ b/src/id-verification/tests/IdVerificationPage.test.jsx @@ -41,34 +41,6 @@ describe('IdVerificationPage', () => { intl: {}, }; - it('does not store irrelevant query params', async () => { - history.push('/?test=irrelevant'); - await act(async () => render(( - - - - - - - - ))); - expect(sessionStorage.setItem).toHaveBeenCalledTimes(0); - }); - - it('does not store empty course_id', async () => { - history.push('/?course_id='); - await act(async () => render(( - - - - - - - - ))); - expect(sessionStorage.setItem).toHaveBeenCalledTimes(0); - }); - it('decodes and stores course_id', async () => { history.push('/?course_id=course-v1%3AedX%2BDemoX%2BDemo_Course'); await act(async () => render(( @@ -81,8 +53,25 @@ describe('IdVerificationPage', () => { ))); expect(sessionStorage.setItem).toHaveBeenCalledWith( - 'courseRunKey', + 'courseId', 'course-v1:edX+DemoX+Demo_Course', ); }); + + it('stores `next` value', async () => { + history.push('/?next=dashboard'); + await act(async () => render(( + + + + + + + + ))); + expect(sessionStorage.setItem).toHaveBeenCalledWith( + 'next', + 'dashboard', + ); + }); }); diff --git a/src/id-verification/tests/panels/SubmittedPanel.test.jsx b/src/id-verification/tests/panels/SubmittedPanel.test.jsx index 42dd0f2..e7f88f5 100644 --- a/src/id-verification/tests/panels/SubmittedPanel.test.jsx +++ b/src/id-verification/tests/panels/SubmittedPanel.test.jsx @@ -28,14 +28,20 @@ describe('SubmittedPanel', () => { }; beforeEach(() => { - global.sessionStorage.getItem = jest.fn(); + const mockStorage = {}; + global.Storage.prototype.setItem = jest.fn((key, value) => { + mockStorage[key] = value; + }); + global.Storage.prototype.getItem = jest.fn(key => mockStorage[key]); }); afterEach(() => { + global.Storage.prototype.setItem.mockReset(); + global.Storage.prototype.getItem.mockReset(); cleanup(); }); - it('links to dashboard without courseRunKey', async () => { + it('links to dashboard without courseId or next value', async () => { await act(async () => render(( @@ -47,5 +53,38 @@ describe('SubmittedPanel', () => { ))); const button = await screen.findByTestId('return-button'); expect(button).toHaveTextContent(/Return to Your Dashboard/); + expect(button).toHaveAttribute('href', `${process.env.LMS_BASE_URL}/dashboard`); + }); + + it('links to course when courseId is stored', async () => { + sessionStorage.setItem('courseId', 'course-v1:edX+DemoX+Demo_Course'); + await act(async () => render(( + + + + + + + + ))); + const button = await screen.findByTestId('return-button'); + expect(button).toHaveTextContent(/Return to Course/); + expect(button).toHaveAttribute('href', `${process.env.LMS_BASE_URL}/courses/course-v1:edX+DemoX+Demo_Course`); + }); + + it('links to specified page when `next` value is provided', async () => { + sessionStorage.setItem('next', 'some_page'); + await act(async () => render(( + + + + + + + + ))); + const button = await screen.findByTestId('return-button'); + expect(button).toHaveTextContent(/Return/); + expect(button).toHaveAttribute('href', `${process.env.LMS_BASE_URL}/some_page`); }); }); From dfb13c42866ac9cadbfb2f5889883dac7e16e3fe Mon Sep 17 00:00:00 2001 From: Bianca Severino Date: Fri, 8 Oct 2021 11:49:06 -0400 Subject: [PATCH 2/2] fix: convert duplicate useEffects to custom hook --- .../name-change/NameChange.jsx | 2 +- src/hooks.js | 24 +++++++++++++++++++ .../panels/RequestCameraAccessPanel.jsx | 18 +++----------- src/id-verification/panels/SubmittedPanel.jsx | 21 ++++------------ .../tests/IdVerificationPage.test.jsx | 2 +- 5 files changed, 34 insertions(+), 33 deletions(-) diff --git a/src/account-settings/name-change/NameChange.jsx b/src/account-settings/name-change/NameChange.jsx index 9c2f285..0159c4a 100644 --- a/src/account-settings/name-change/NameChange.jsx +++ b/src/account-settings/name-change/NameChange.jsx @@ -69,7 +69,7 @@ function NameChangeModal({ useEffect(() => { if (saveState === 'complete') { handleClose(); - push('/id-verification?next=account%2Fsettings'); + push(`/id-verification?next=${encodeURIComponent('account/settings')}`); } }, [saveState]); diff --git a/src/hooks.js b/src/hooks.js index 6cec54c..0c9e677 100644 --- a/src/hooks.js +++ b/src/hooks.js @@ -21,3 +21,27 @@ export function useAsyncCall(asyncFunc) { return [isLoading, data]; } + +// Redirect the user to their original location based on session storage +export function useRedirect() { + const [redirect, setRedirect] = useState({ + location: 'dashboard', + text: 'id.verification.return.dashboard', + }); + + useEffect(() => { + if (sessionStorage.getItem('courseId')) { + setRedirect({ + location: `courses/${sessionStorage.getItem('courseId')}`, + text: 'id.verification.return.course', + }); + } else if (sessionStorage.getItem('next')) { + setRedirect({ + location: sessionStorage.getItem('next'), + text: 'id.verification.return.generic', + }); + } + }, []); + + return redirect; +} diff --git a/src/id-verification/panels/RequestCameraAccessPanel.jsx b/src/id-verification/panels/RequestCameraAccessPanel.jsx index 3e43543..739902a 100644 --- a/src/id-verification/panels/RequestCameraAccessPanel.jsx +++ b/src/id-verification/panels/RequestCameraAccessPanel.jsx @@ -1,10 +1,11 @@ -import React, { useState, useEffect, useContext } from 'react'; +import React, { useEffect, useContext } from 'react'; import { Link } from 'react-router-dom'; import Bowser from 'bowser'; import { getConfig } from '@edx/frontend-platform'; import { sendTrackEvent } from '@edx/frontend-platform/analytics'; import { injectIntl, intlShape, FormattedMessage } from '@edx/frontend-platform/i18n'; +import { useRedirect } from '../../hooks'; import { useNextPanelSlug } from '../routing-utilities'; import BasePanel from './BasePanel'; import IdVerificationContext, { MEDIA_ACCESS } from '../IdVerificationContext'; @@ -14,8 +15,7 @@ import { UnsupportedCameraDirectionsPanel } from './UnsupportedCameraDirectionsP import messages from '../IdVerification.messages'; function RequestCameraAccessPanel(props) { - const [returnUrl, setReturnUrl] = useState('dashboard'); - const [returnText, setReturnText] = useState('id.verification.return.dashboard'); + const { location: returnUrl, text: returnText } = useRedirect(); const panelSlug = 'request-camera-access'; const nextPanelSlug = useNextPanelSlug(panelSlug); const { @@ -38,18 +38,6 @@ function RequestCameraAccessPanel(props) { } }, [mediaAccess, userId]); - // Link back to the correct location if the user accessed IDV somewhere other - // than the dashboard - useEffect(() => { - if (sessionStorage.getItem('courseId')) { - setReturnUrl(`courses/${sessionStorage.getItem('courseId')}`); - setReturnText('id.verification.return.course'); - } else if (sessionStorage.getItem('next')) { - setReturnUrl(sessionStorage.getItem('next')); - setReturnText('id.verification.return.generic'); - } - }, []); - const getTitle = () => { if (mediaAccess === MEDIA_ACCESS.GRANTED) { return props.intl.formatMessage(messages['id.verification.camera.access.title.success']); diff --git a/src/id-verification/panels/SubmittedPanel.jsx b/src/id-verification/panels/SubmittedPanel.jsx index 09c5481..682cd33 100644 --- a/src/id-verification/panels/SubmittedPanel.jsx +++ b/src/id-verification/panels/SubmittedPanel.jsx @@ -1,17 +1,18 @@ -import React, { useContext, useEffect, useState } from 'react'; +import React, { useContext, useEffect } from 'react'; import { getConfig } from '@edx/frontend-platform'; import { sendTrackEvent } from '@edx/frontend-platform/analytics'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; -import BasePanel from './BasePanel'; +import { useRedirect } from '../../hooks'; import IdVerificationContext from '../IdVerificationContext'; import messages from '../IdVerification.messages'; +import BasePanel from './BasePanel'; + function SubmittedPanel(props) { const { userId } = useContext(IdVerificationContext); - const [returnUrl, setReturnUrl] = useState('dashboard'); - const [returnText, setReturnText] = useState('id.verification.return.dashboard'); + const { location: returnUrl, text: returnText } = useRedirect(); const panelSlug = 'submitted'; useEffect(() => { @@ -21,18 +22,6 @@ function SubmittedPanel(props) { }); }, [userId]); - // Link back to the correct location if the user accessed IDV somewhere other - // than the dashboard - useEffect(() => { - if (sessionStorage.getItem('courseId')) { - setReturnUrl(`courses/${sessionStorage.getItem('courseId')}`); - setReturnText('id.verification.return.course'); - } else if (sessionStorage.getItem('next')) { - setReturnUrl(sessionStorage.getItem('next')); - setReturnText('id.verification.return.generic'); - } - }, []); - return ( { }; it('decodes and stores course_id', async () => { - history.push('/?course_id=course-v1%3AedX%2BDemoX%2BDemo_Course'); + history.push(`/?course_id=${encodeURIComponent('course-v1:edX+DemoX+Demo_Course')}`); await act(async () => render((