From e246ec0d5b401871cf6a64fbb7360a0b2f913b4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Mon, 17 May 2021 09:15:41 +0200 Subject: [PATCH] ForgottenPassword: Adds better error handling (#34174) * ForgottenPassword: Adds better error handling * Chore: updates after PR comments --- .../ForgottenPassword.test.tsx | 118 ++++++++++++++ .../ForgottenPassword/ForgottenPassword.tsx | 149 ++++++++++++------ 2 files changed, 223 insertions(+), 44 deletions(-) create mode 100644 public/app/core/components/ForgottenPassword/ForgottenPassword.test.tsx diff --git a/public/app/core/components/ForgottenPassword/ForgottenPassword.test.tsx b/public/app/core/components/ForgottenPassword/ForgottenPassword.test.tsx new file mode 100644 index 00000000000..46d601ee1cf --- /dev/null +++ b/public/app/core/components/ForgottenPassword/ForgottenPassword.test.tsx @@ -0,0 +1,118 @@ +import React from 'react'; +import { asyncScheduler, scheduled, throwError } from 'rxjs'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import { ForgottenPassword, SendResetEmailDTO } from './ForgottenPassword'; +import { backendSrv } from '../../services/backend_srv'; +import { toAsyncOfResult } from '../../../features/query/state/DashboardQueryRunner/testHelpers'; +import { createFetchResponse } from '../../../../test/helpers/createFetchResponse'; + +jest.mock('@grafana/runtime', () => ({ + ...((jest.requireActual('@grafana/runtime') as unknown) as object), + getBackendSrv: () => backendSrv, +})); + +function getTestContext() { + jest.clearAllMocks(); + const fetchMock = jest.spyOn(backendSrv, 'fetch'); + const { rerender } = render(); + + return { fetchMock, rerender }; +} + +function getInputField() { + return screen.getByRole('textbox', { name: /user enter your information to get a reset link sent to you/i }); +} + +function getSubmitButton() { + return screen.getByRole('button', { name: /send reset email/i }); +} + +async function enterUserNameAndSubmitForm(fetchMock: jest.SpyInstance) { + await userEvent.type(getInputField(), 'JaneDoe'); + userEvent.click(getSubmitButton()); + + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); + + expect(fetchMock).toHaveBeenCalledWith({ + url: '/api/user/password/send-reset-email', + method: 'POST', + data: { userOrEmail: 'JaneDoe' }, + showSuccessAlert: false, + showErrorAlert: false, + }); +} + +describe('ForgottenPassword', () => { + describe('when mounted', () => { + it('then it should show input field', () => { + getTestContext(); + + expect(getInputField()).toBeInTheDocument(); + }); + + it('then it should show send button', () => { + getTestContext(); + + expect(getSubmitButton()).toBeInTheDocument(); + }); + + it('then it should show back to login link', () => { + getTestContext(); + + expect(screen.getByRole('link', { name: /back to login/i })).toBeInTheDocument(); + }); + }); + + describe('when user submits form', () => { + describe('and response is ok', () => { + it('then it should show success message', async () => { + const { fetchMock } = getTestContext(); + fetchMock.mockImplementation(() => + toAsyncOfResult( + createFetchResponse({ message: 'Success' }) + ) + ); + + await enterUserNameAndSubmitForm(fetchMock); + + expect( + screen.getByText( + /an email with a reset link has been sent to the email address\. you should receive it shortly\./i + ) + ).toBeInTheDocument(); + }); + }); + + describe('and response is ok but contains an error', () => { + it('then it should show alert', async () => { + const { fetchMock } = getTestContext(); + fetchMock.mockImplementation(() => + toAsyncOfResult( + createFetchResponse({ message: 'Success', error: 'Something went wrong' }) + ) + ); + + await enterUserNameAndSubmitForm(fetchMock); + + expect(screen.getByLabelText(/alert error/i)).toBeInTheDocument(); + expect(screen.getByText(/couldn't send reset link to the email address/i)).toBeInTheDocument(); + expect(screen.getByText(/something went wrong/i)).toBeInTheDocument(); + }); + }); + + describe('and response is not ok', () => { + it('then it should show alert', async () => { + const { fetchMock } = getTestContext(); + fetchMock.mockImplementation(() => scheduled(throwError('Server error'), asyncScheduler)); + + await enterUserNameAndSubmitForm(fetchMock); + + expect(screen.getByLabelText(/alert error/i)).toBeInTheDocument(); + expect(screen.getByText(/couldn't send reset link to the email address/i)).toBeInTheDocument(); + expect(screen.getByText(/server error/i)).toBeInTheDocument(); + }); + }); + }); +}); diff --git a/public/app/core/components/ForgottenPassword/ForgottenPassword.tsx b/public/app/core/components/ForgottenPassword/ForgottenPassword.tsx index 268e9d767f7..ae51a816aa6 100644 --- a/public/app/core/components/ForgottenPassword/ForgottenPassword.tsx +++ b/public/app/core/components/ForgottenPassword/ForgottenPassword.tsx @@ -1,32 +1,68 @@ -import React, { FC, useState } from 'react'; -import { Form, Field, Input, Button, Legend, Container, useStyles, HorizontalGroup, LinkButton } from '@grafana/ui'; -import { getBackendSrv } from '@grafana/runtime'; +import React, { useState } from 'react'; +import { Unsubscribable } from 'rxjs'; import { css } from '@emotion/css'; -import { GrafanaTheme } from '@grafana/data'; -import config from 'app/core/config'; +import { + Alert, + Button, + Container, + FadeTransition, + Field, + Form, + HorizontalGroup, + Input, + Legend, + LinkButton, + useStyles2, +} from '@grafana/ui'; +import { getBackendSrv, toDataQueryError } from '@grafana/runtime'; +import { GrafanaTheme2 } from '@grafana/data'; + +import { getConfig } from 'app/core/config'; interface EmailDTO { userOrEmail: string; } -const paragraphStyles = (theme: GrafanaTheme) => css` - color: ${theme.colors.formDescription}; - font-size: ${theme.typography.size.sm}; - font-weight: ${theme.typography.weight.regular}; - margin-top: ${theme.spacing.sm}; - display: block; -`; +export interface SendResetEmailDTO { + message: string; + error?: string; +} -export const ForgottenPassword: FC = () => { +export function ForgottenPassword(): JSX.Element { const [emailSent, setEmailSent] = useState(false); - const styles = useStyles(paragraphStyles); - const loginHref = `${config.appSubUrl}/login`; + const [error, setError] = useState(null); + const styles = useStyles2(getStyles); + const loginHref = `${getConfig().appSubUrl}/login`; - const sendEmail = async (formModel: EmailDTO) => { - const res = await getBackendSrv().post('/api/user/password/send-reset-email', formModel); - if (res) { - setEmailSent(true); - } + const sendEmail = (formModel: EmailDTO) => { + setError(null); + // using fetch here so we can disable showSuccessAlert, showErrorAlert and handle those + const subscription: Unsubscribable = getBackendSrv() + .fetch({ + url: '/api/user/password/send-reset-email', + method: 'POST', + data: formModel, + showSuccessAlert: false, + showErrorAlert: false, + }) + .subscribe({ + next: (response) => { + const { error, message } = response.data; // the backend api can respond with 200 Ok but still have an error set + + if (error) { + setError(error); + } else if (message) { + setEmailSent(true); + } + + subscription.unsubscribe(); // unsubscribe as soon as a value is received + }, + error: (err) => { + const dataError = toDataQueryError(err); + setError(dataError.message ?? 'Unknown error'); + subscription.unsubscribe(); // unsubscribe as soon as an error is received + }, + }); }; if (emailSent) { @@ -40,29 +76,54 @@ export const ForgottenPassword: FC = () => { ); } - return ( -
- {({ register, errors }) => ( - <> - Reset password - - - - - - - Back to login - - -

Did you forget your username or email? Contact your Grafana administrator.

- - )} -
+ return ( + <> +
+ {({ register, errors }) => ( + <> + Reset password + + + + + + + Back to login + + + +

Did you forget your username or email? Contact your Grafana administrator.

+ + )} +
+ + + + Reason:  + {error} + + + + ); -}; +} + +function getStyles(theme: GrafanaTheme2) { + return css` + color: ${theme.colors.text.secondary}; + font-size: ${theme.typography.size.sm}; + font-weight: ${theme.typography.fontWeightRegular}; + margin-top: ${theme.spacing(1)}; + display: block; + `; +}