Skip to content

Commit

Permalink
PRMDR-646 403_logout Status UI should be specific rather than stating…
Browse files Browse the repository at this point in the history
… its an unknown issue (#336)

* [PRMDR-646] Add a page to handle session expiry

* [PRMDR-646] Route 403 errors to new error page

* [PRMDR-646] Fix unit test

* [PRMDR-646] Fix unit tests related to route change

* [PRMDR-646] add unit test

* [PRMDR-646] bug fix

* [PRMDR-646] Amend the page to meet the design in mural

* [PRMDR-646] remove unused imports
  • Loading branch information
joefong-nhs authored Apr 10, 2024
1 parent 73f536e commit 9452dea
Show file tree
Hide file tree
Showing 16 changed files with 154 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ describe('DocumentSearchResultsOptions', () => {
});

describe('Navigation', () => {
it('navigates to home page when API returns 403', async () => {
it('navigates to session expire page when API returns 403', async () => {
const history = createMemoryHistory({
initialEntries: ['/example'],
initialIndex: 1,
Expand All @@ -155,7 +155,7 @@ describe('DocumentSearchResultsOptions', () => {
userEvent.click(screen.getByRole('button', { name: 'Download All Documents' }));

await waitFor(() => {
expect(mockedUseNavigate).toHaveBeenCalledWith(routes.START);
expect(mockedUseNavigate).toHaveBeenCalledWith(routes.SESSION_EXPIRED);
});
});
it('navigates to error page when API returns 5XX', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const DocumentSearchResultsOptions = (props: Props) => {
} catch (e) {
const error = e as AxiosError;
if (error.response?.status === 403) {
navigate(routes.START);
navigate(routes.SESSION_EXPIRED);
} else {
navigate(routes.SERVER_ERROR + errorToParams(error));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ describe('DeleteDocumentsStage', () => {
});

describe('Navigation', () => {
it('navigates to home page when API call returns 403', async () => {
it('navigates to session expire page when API call returns 403', async () => {
const errorResponse = {
response: {
status: 403,
Expand All @@ -218,7 +218,7 @@ describe('Navigation', () => {
});

await waitFor(() => {
expect(mockedUseNavigate).toHaveBeenCalledWith(routes.START);
expect(mockedUseNavigate).toHaveBeenCalledWith(routes.SESSION_EXPIRED);
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ function DeleteDocumentsStage({
onSuccess();
} else {
if (error.response?.status === 403) {
navigate(routes.START);
navigate(routes.SESSION_EXPIRED);
} else {
navigate(routes.SERVER_ERROR + errorToParams(error));
}
Expand Down
38 changes: 34 additions & 4 deletions app/src/components/blocks/feedbackForm/FeedbackForm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -254,11 +254,41 @@ describe('<FeedbackForm />', () => {
);
expect(mockSetStage).toBeCalledWith(SUBMISSION_STAGE.Submitting);

await waitFor(() => {
expect(mockedUseNavigate).toHaveBeenCalledWith(
routes.SERVER_ERROR + '?encodedError=WyJTUF8xMDAxIiwiMTU3NzgzNjgwMCJd',
);
expect(mockedUseNavigate).toHaveBeenCalledWith(
routes.SERVER_ERROR + '?encodedError=WyJTUF8xMDAxIiwiMTU3NzgzNjgwMCJd',
);
});

it('navigates to Session Expire page when call to feedback endpoint return 403', async () => {
const errorResponse = {
response: {
status: 403,
data: { message: 'Unauthorized' },
},
};
mockedAxios.post.mockImplementation(() => Promise.reject(errorResponse));

const mockInputData = {
feedbackContent: 'Mock feedback content',
howSatisfied: SATISFACTION_CHOICES.VerySatisfied,
respondentName: 'Jane Smith',
respondentEmail: '[email protected]',
};

renderComponent();

act(() => {
fillInForm(mockInputData);
clickSubmitButton();
});

await waitFor(() =>
expect(mockedAxios.post).toBeCalledWith(baseURL + '/Feedback', mockInputData, {
headers: {},
}),
);
expect(mockSetStage).toBeCalledWith(SUBMISSION_STAGE.Submitting);
expect(mockedUseNavigate).toHaveBeenCalledWith(routes.SESSION_EXPIRED);
});
});
});
5 changes: 4 additions & 1 deletion app/src/components/blocks/feedbackForm/FeedbackForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,15 @@ function FeedbackForm({ stage, setStage }: Props) {

const submit: SubmitHandler<FormData> = async (formData) => {
setStage(SUBMISSION_STAGE.Submitting);
// add tests for failing and passing cases when real email service is implemented
try {
await sendEmail({ formData, baseUrl, baseHeaders });
setStage(SUBMISSION_STAGE.Successful);
} catch (e) {
const error = e as AxiosError;
if (error.response?.status === 403) {
navigate(routes.SESSION_EXPIRED);
return;
}
navigate(routes.SERVER_ERROR + errorToParams(error));
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ describe('<DocumentSearchResultsPage />', () => {
);
});
});
it('navigates to Start page when a document search fails', async () => {
it('navigates to session expire page when a document search return 403 unauthorised error', async () => {
const errorResponse = {
response: {
status: 403,
Expand All @@ -218,7 +218,7 @@ describe('<DocumentSearchResultsPage />', () => {
render(<DocumentSearchResultsPage />);

await waitFor(() => {
expect(mockedUseNavigate).toHaveBeenCalledWith(routes.START);
expect(mockedUseNavigate).toHaveBeenCalledWith(routes.SESSION_EXPIRED);
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function DocumentSearchResultsPage() {
} catch (e) {
const error = e as AxiosError;
if (error.response?.status === 403) {
navigate(routes.START);
navigate(routes.SESSION_EXPIRED);
} else if (error.response?.status && error.response?.status >= 500) {
navigate(routes.SERVER_ERROR + errorToParams(error));
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ describe('LloydGeorgeUploadPage', () => {
);
});
});
it('navigates to start page when when call to lg record view return 403', async () => {
it('navigates to session expire page when when call to lg record view return 403', async () => {
const errorResponse = {
response: {
status: 403,
Expand All @@ -348,10 +348,10 @@ describe('LloydGeorgeUploadPage', () => {
expect(mockUploadDocuments).toHaveBeenCalled();

await waitFor(() => {
expect(mockNavigate).toHaveBeenCalledWith(routes.START);
expect(mockNavigate).toHaveBeenCalledWith(routes.SESSION_EXPIRED);
});
});
it('navigates to start page when confirmation returns 403', async () => {
it('navigates to session expire page when confirmation returns 403', async () => {
mockS3Upload.mockReturnValue(Promise.resolve());
mockVirusScan.mockReturnValue(DOCUMENT_UPLOAD_STATE.CLEAN);
mockUploadConfirmation.mockImplementation(() =>
Expand Down Expand Up @@ -383,7 +383,7 @@ describe('LloydGeorgeUploadPage', () => {
expect(mockUploadConfirmation).toHaveBeenCalled();
});
await waitFor(() => {
expect(mockNavigate).toHaveBeenCalledWith(routes.START);
expect(mockNavigate).toHaveBeenCalledWith(routes.SESSION_EXPIRED);
});
});
});
Expand Down
4 changes: 2 additions & 2 deletions app/src/pages/lloydGeorgeUploadPage/LloydGeorgeUploadPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ function LloydGeorgeUploadPage() {
} catch (e) {
const error = e as AxiosError;
if (error.response?.status === 403) {
navigate(routes.START);
navigate(routes.SESSION_EXPIRED);
return;
}
setStage(LG_UPLOAD_STAGE.FAILED);
Expand Down Expand Up @@ -196,7 +196,7 @@ function LloydGeorgeUploadPage() {
} catch (e) {
const error = e as AxiosError;
if (error.response?.status === 403) {
navigate(routes.START);
navigate(routes.SESSION_EXPIRED);
} else if (error.response?.status === 423) {
navigate(routes.SERVER_ERROR + errorToParams(error));
} else if (isMock(error)) {
Expand Down
4 changes: 2 additions & 2 deletions app/src/pages/patientSearchPage/PatientSearchPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ describe('PatientSearchPage', () => {
},
);

it('navigates to start page when user is unauthorized to make request', async () => {
it('navigates to session expired page page when user is unauthorized to make request', async () => {
const errorResponse = {
response: {
status: 403,
Expand All @@ -192,7 +192,7 @@ describe('PatientSearchPage', () => {
userEvent.click(screen.getByRole('button', { name: 'Search' }));

await waitFor(() => {
expect(mockedUseNavigate).toHaveBeenCalledWith(routes.START);
expect(mockedUseNavigate).toHaveBeenCalledWith(routes.SESSION_EXPIRED);
});
});

Expand Down
2 changes: 1 addition & 1 deletion app/src/pages/patientSearchPage/PatientSearchPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ function PatientSearchPage() {
if (error.response?.status === 400) {
setInputError('Enter a valid patient NHS number.');
} else if (error.response?.status === 403) {
navigate(routes.START);
navigate(routes.SESSION_EXPIRED);
} else if (error.response?.status === 404) {
setInputError('Sorry, patient data not found.');
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { render, screen, waitFor } from '@testing-library/react';
import { act } from 'react-dom/test-utils';
import SessionExpiredErrorPage from './SessionExpiredErrorPage';
import useBaseAPIUrl from '../../helpers/hooks/useBaseAPIUrl';
import { endpoints } from '../../types/generic/endpoints';

jest.mock('../../helpers/hooks/useBaseAPIUrl');

const originalWindowLocation = window.location;
const mockLocationReplace = jest.fn();
const mockUseBaseUrl = useBaseAPIUrl as jest.Mock;

describe('SessionExpiredErrorPage', () => {
afterAll(() => {
Object.defineProperty(window, 'location', {
value: originalWindowLocation,
});
});

it('render a page with a user friendly message to state that their session expired', () => {
render(<SessionExpiredErrorPage />);

expect(
screen.getByRole('heading', { name: 'We signed you out due to inactivity' }),
).toBeInTheDocument();

expect(
screen.getByText(
"This is to protect your information. You'll need to enter any information you submitted again.",
),
).toBeInTheDocument();
});

it('move to login endpoint when user click the button', async () => {
const mockBackendUrl = 'http://localhost/mock_url/';
mockUseBaseUrl.mockReturnValue(mockBackendUrl);

Object.defineProperty(window, 'location', {
value: {
replace: mockLocationReplace,
},
});

render(<SessionExpiredErrorPage />);

const signBackInButton = screen.getByRole('button', {
name: 'Sign back in',
});
expect(signBackInButton).toBeInTheDocument();

act(() => {
signBackInButton.click();
});

await waitFor(() =>
expect(mockLocationReplace).toBeCalledWith(mockBackendUrl + endpoints.LOGIN),
);
});
});
32 changes: 32 additions & 0 deletions app/src/pages/sessionExpiredErrorPage/SessionExpiredErrorPage.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { ButtonLink } from 'nhsuk-react-components';
import React, { MouseEvent, useState } from 'react';
import { endpoints } from '../../types/generic/endpoints';
import Spinner from '../../components/generic/spinner/Spinner';
import useBaseAPIUrl from '../../helpers/hooks/useBaseAPIUrl';

const SessionExpiredErrorPage = () => {
const baseAPIUrl = useBaseAPIUrl();
const [isLoading, setIsLoading] = useState(false);

const handleLogin = (e: MouseEvent<HTMLAnchorElement>) => {
setIsLoading(true);
e.preventDefault();
window.location.replace(`${baseAPIUrl}${endpoints.LOGIN}`);
};

return !isLoading ? (
<>
<h1>We signed you out due to inactivity</h1>
<p>
This is to protect your information. You'll need to enter any information you
submitted again.
</p>
<ButtonLink href="#" onClick={handleLogin}>
Sign back in
</ButtonLink>
</>
) : (
<Spinner status="Logging in..." />
);
};
export default SessionExpiredErrorPage;
6 changes: 6 additions & 0 deletions app/src/router/AppRouter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import FeedbackPage from '../pages/feedbackPage/FeedbackPage';
import ServerErrorPage from '../pages/serverErrorPage/ServerErrorPage';
import PrivacyPage from '../pages/privacyPage/PrivacyPage';
import LloydGeorgeUploadPage from '../pages/lloydGeorgeUploadPage/LloydGeorgeUploadPage';
import SessionExpiredErrorPage from '../pages/sessionExpiredErrorPage/SessionExpiredErrorPage';

const {
START,
Expand All @@ -33,6 +34,7 @@ const {
UNAUTHORISED_LOGIN,
AUTH_ERROR,
SERVER_ERROR,
SESSION_EXPIRED,
FEEDBACK,
LOGOUT,
SEARCH_PATIENT,
Expand Down Expand Up @@ -78,6 +80,10 @@ export const routeMap: Routes = {
page: <ServerErrorPage />,
type: ROUTE_TYPE.PUBLIC,
},
[SESSION_EXPIRED]: {
page: <SessionExpiredErrorPage />,
type: ROUTE_TYPE.PUBLIC,
},
[PRIVACY_POLICY]: {
page: <PrivacyPage />,
type: ROUTE_TYPE.PUBLIC,
Expand Down
1 change: 1 addition & 0 deletions app/src/types/generic/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export enum routes {
AUTH_ERROR = '/auth-error',
UNAUTHORISED_LOGIN = '/unauthorised-login',
SERVER_ERROR = '/server-error',
SESSION_EXPIRED = '/session-expired',
PRIVACY_POLICY = '/privacy-policy',
LOGOUT = '/logout',
FEEDBACK = '/feedback',
Expand Down

0 comments on commit 9452dea

Please sign in to comment.