Skip to content
This repository has been archived by the owner on May 10, 2023. It is now read-only.

fix: correctly handle language fetch error and inform user #597

Merged
merged 1 commit into from
Feb 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion server/lib/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,13 @@ module.exports = {
};

async function enhanceLanguages(languageCodes) {
const allLanguages = await languages.getAllLanguages();
let allLanguages = [];
try {
allLanguages = await languages.getAllLanguages();
} catch (error) {
// Not returning any users languages is better than completely failing
}

const validLanguageCodes = languageCodes.filter((code) => allLanguages.find((language) => language.id === code));
return validLanguageCodes.map((languageCode) => ({
id: languageCode,
Expand Down
9 changes: 9 additions & 0 deletions web/css/root.css
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
--grey-color: #d7d7db;
--dark-grey-color: #5e5c5b;
--error-color: #ff4f5e;
--error-border-color: #ff7070;
--error-background-color: #ffbfc5;
--warning-border-color: #ffff00;
--warning-background-color: #ffffb9;
--review-selected-color: #000000;
Expand Down Expand Up @@ -218,6 +220,13 @@ form button:not(.standalone) {
font-size: 0.7rem;
}

.error-box {
margin: 1.5rem 0;
padding: 1rem;
border: 2px solid var(--error-border-color);
background-color: var(--error-background-color);
}

.warning-box {
margin: 1.5rem 0;
padding: 1rem;
Expand Down
3 changes: 3 additions & 0 deletions web/locales/en/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ sc-home-collect-text = Help us by writing or collecting Public Domain sentences.
sc-home-review-title = Review sentences
sc-home-review-text = Help us by reviewing sentences for correctness according to the guidelines.

## GENERAL
sc-languages-fetch-error = We failed to fetch available languages. Contributions are currently not possible. Please try again later.

## HOW-TO
sc-howto-title = How to
sc-howto-addlang-title = Adding languages to work with
Expand Down
5 changes: 4 additions & 1 deletion web/src/actions/languages.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe('getLanguages', () => {
});
});

test('should not throw on error', async () => {
test('should dispatch on error', async () => {
const error = new Error('NOPE');
(console.error as jest.Mock).mockImplementation(() => {
/* ignore */
Expand All @@ -35,6 +35,9 @@ describe('getLanguages', () => {
});
expect(languages.getLanguages()(dispatch, getState, null)).resolves.not.toThrow();
expect((console.error as jest.Mock).mock.calls[0][0]).toEqual('Failed to fetch languages');
expect(dispatch.mock.calls[0][0]).toEqual({
type: languages.ACTION_GET_LANGUAGES_FAILURE,
});
});
});

Expand Down
8 changes: 8 additions & 0 deletions web/src/actions/languages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const ACTION_REMOVE_LANGUAGE_SUCCESS = 'REMOVE_LANGUAGE_SUCCESS';
export const ACTION_REMOVE_LANGUAGE_FAILURE = 'REMOVE_LANGUAGE_FAILURE';

export const ACTION_GOT_LANGUAGES = 'ACTION_GOT_LANGUAGES';
export const ACTION_GET_LANGUAGES_FAILURE = 'ACTION_GET_LANGUAGES_FAILURE';

export const ACTION_SET_CURRENT_UI_LOCALE = 'ACTION_SET_CURRENT_UI_LOCALE';

Expand All @@ -23,6 +24,7 @@ export function getLanguages(): ThunkAction<void, RootState, unknown, AnyAction>
dispatch(gotLanguages(languages));
} catch (error) {
console.error('Failed to fetch languages', error);
dispatch(getLanguagesFailure());
}
};
}
Expand Down Expand Up @@ -65,6 +67,12 @@ export function gotLanguages(languages: Language[]) {
};
}

export function getLanguagesFailure() {
return {
type: ACTION_GET_LANGUAGES_FAILURE,
};
}

export function sendAddLanguage() {
return {
type: ACTION_ADD_LANGUAGE_REQUEST,
Expand Down
7 changes: 7 additions & 0 deletions web/src/components/add/submit-form.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,13 @@ test('should submit form if valid', async () => {
});
});

test('should show error if no languages fetched', async () => {
await renderWithLocalization(
<SubmitForm languages={languages} onSubmit={onSubmit} languageFetchFailure={true} />
);
expect(screen.queryByText(/We failed to fetch available languages./)).toBeTruthy();
});

test('should show error if no language', async () => {
await renderWithLocalization(<SubmitForm languages={languages} onSubmit={onSubmit} />);

Expand Down
5 changes: 5 additions & 0 deletions web/src/components/add/submit-form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Localized, useLocalization } from '@fluent/react';

import { useLocaleUrl } from '../../urls';
import type { Language, SubmissionFailures } from '../../types';
import Error from '../error';
import LanguageSelector from '../language-selector';
import Sentence from '../sentence';
import SubmitButton from '../submit-button';
Expand Down Expand Up @@ -33,6 +34,7 @@ type Props = {
message?: string;
error?: string;
sentenceSubmissionFailures?: SubmissionFailures;
languageFetchFailure?: boolean;
};

type FormFields = {
Expand All @@ -47,6 +49,7 @@ export default function SubmitForm({
message,
error,
sentenceSubmissionFailures,
languageFetchFailure = false,
}: Props) {
const firstLanguage = languages.length === 1 && languages[0];
const [formError, setError] = useState('');
Expand Down Expand Up @@ -125,6 +128,8 @@ export default function SubmitForm({
<h1></h1>
</Localized>

{languageFetchFailure && <Error translationKey="sc-languages-fetch-error" />}

{message && <section className="form-message">{message}</section>}
{formError && <section className="form-error">{formError}</section>}
{error && <section className="form-error">{error}</section>}
Expand Down
10 changes: 10 additions & 0 deletions web/src/components/error.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import React from 'react';
import { Localized } from '@fluent/react';

export default function Footer({ translationKey }: { translationKey: string }) {
return (
<Localized id={translationKey}>
<div className="error-box"></div>
</Localized>
);
}
12 changes: 12 additions & 0 deletions web/src/components/pages/add.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,18 @@ beforeEach(() => {
}));
});

test('should render error if no languages fetched', async () => {
(redux.useSelector as jest.Mock).mockImplementation(() => ({
allLanguages: [],
languages: [],
pendingLanguages: false,
fetchFailure: true,
}));

await renderWithLocalization(<Add />);
expect(screen.queryByText(/We failed to fetch available languages./)).toBeTruthy();
});

test('should submit sentences including review', async () => {
const dispatchMock = jest.fn(() =>
Promise.resolve({
Expand Down
5 changes: 4 additions & 1 deletion web/src/components/pages/add.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ export default function Add() {
const [validated, setValidated] = useState<string[]>([]);
const [invalidated, setInvalidated] = useState<string[]>([]);

const { allLanguages, languages } = useSelector((state: RootState) => state.languages);
const { allLanguages, languages, fetchFailure } = useSelector(
(state: RootState) => state.languages
);
const { isUploadingSentences, sentenceSubmissionFailures } = useSelector(
(state: RootState) => state.sentences
);
Expand Down Expand Up @@ -159,6 +161,7 @@ export default function Add() {
error={error}
languages={availableLanguages}
sentenceSubmissionFailures={sentenceSubmissionFailures}
languageFetchFailure={fetchFailure}
/>
);
}
12 changes: 12 additions & 0 deletions web/src/components/pages/review.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,18 @@ test('should set language from single user language', async () => {
expect(screen.queryByText('Please select a language to review sentences.')).toBeNull();
});

test('should render error if no languages fetched', async () => {
(redux.useSelector as jest.Mock).mockImplementation(() => ({
allLanguages: [],
languages: [],
pendingLanguages: false,
fetchFailure: true,
}));

await renderWithLocalization(<Review />);
expect(screen.queryByText(/We failed to fetch available languages./)).toBeTruthy();
});

test('should ask to set language', async () => {
(redux.useSelector as jest.Mock).mockImplementation(() => ({
allLanguages: [],
Expand Down
8 changes: 6 additions & 2 deletions web/src/components/pages/review.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
import { useLocaleUrl, getReviewUrl } from '../../urls';
import type { RootState, ReviewedState } from '../../types';

import Error from '../error';
import LanguageSelector from '../language-selector';
import ReviewForm from '../review/review-form';
import ReviewCriteria from '../review/review-criteria';
Expand All @@ -24,7 +25,7 @@ type ReviewRouteMatch = {
export default function Review() {
const match = useRouteMatch<ReviewRouteMatch>();
const history = useHistory();
const { languages = [] } = useSelector((state: RootState) => state.languages);
const { languages = [], fetchFailure } = useSelector((state: RootState) => state.languages);
const {
sentencesLoading,
sentences = [],
Expand Down Expand Up @@ -52,7 +53,7 @@ export default function Review() {
}, [language]);

// If user hasn't added any languages, ask them to do so.
if (languages.length === 0) {
if (languages.length === 0 && !fetchFailure) {
return (
<Localized
id="sc-review-lang-not-selected"
Expand Down Expand Up @@ -106,6 +107,9 @@ export default function Review() {
<Localized id="sc-review-title">
<h1></h1>
</Localized>

{fetchFailure && <Error translationKey="sc-languages-fetch-error" />}

<div className="row">
<LanguageSelector
labelText=""
Expand Down
12 changes: 12 additions & 0 deletions web/src/components/pages/stats.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,18 @@ test('should render stats', async () => {
expect(screen.getByText(/\u206844\u2069 rejected sentences/)).toBeTruthy();
});

test('should render error if no languages fetched', async () => {
(redux.useSelector as jest.Mock).mockImplementation(() => ({
allLanguages: [],
languages: [],
pendingLanguages: false,
fetchFailure: true,
}));

await renderWithLocalization(<Stats />);
expect(screen.queryByText(/We failed to fetch available languages./)).toBeTruthy();
});

test('should render error', async () => {
(backend.sendRequest as jest.Mock).mockImplementation(() => {
throw new Error('oh no');
Expand Down
7 changes: 6 additions & 1 deletion web/src/components/pages/stats.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Localized } from '@fluent/react';

import { sendRequest } from '../../backend';
import type { LanguageStats, RootState } from '../../types';
import Error from '../error';
import LanguageInfo from '../language-info';
import LanguageSelector from '../language-selector';

Expand All @@ -20,7 +21,9 @@ export default function Stats() {
const [hasError, setHasError] = useState(false);
const [loading, setLoading] = useState(false);
const [language, setLanguage] = useState<string>();
const { languages, pendingLanguages } = useSelector((state: RootState) => state.languages);
const { languages, pendingLanguages, fetchFailure } = useSelector(
(state: RootState) => state.languages
);

const isLoading = loading || pendingLanguages;

Expand Down Expand Up @@ -58,6 +61,8 @@ export default function Stats() {
<h1></h1>
</Localized>

{fetchFailure && <Error translationKey="sc-languages-fetch-error" />}

<div className="row">
<LanguageSelector
labelText=""
Expand Down
13 changes: 13 additions & 0 deletions web/src/components/profile/personal-language-info.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,22 @@ beforeEach(() => {
},
],
pendingLanguages: false,
fetchFailure: false,
}));
});

test('should render error', async () => {
(redux.useSelector as jest.Mock).mockImplementation(() => ({
allLanguages,
languages: [],
pendingLanguages: false,
fetchFailure: true,
}));

await renderWithLocalization(<PersonalLanguageInfo />);
expect(screen.queryByText(/We failed to fetch available languages./)).toBeTruthy();
});

test('should render if not added languages', async () => {
(redux.useSelector as jest.Mock).mockImplementation(() => ({
allLanguages,
Expand Down
7 changes: 6 additions & 1 deletion web/src/components/profile/personal-language-info.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@ import { Localized, useLocalization } from '@fluent/react';

import { removeLanguage } from '../../actions/languages';
import { RootState } from '../../types';
import Error from '../error';

export default function PersonalLanguageInfo() {
const { languages, pendingLanguages } = useSelector((state: RootState) => state.languages);
const { languages, pendingLanguages, fetchFailure } = useSelector(
(state: RootState) => state.languages
);
const [error, setError] = useState('');
const dispatch = useDispatch();

Expand All @@ -30,6 +33,8 @@ export default function PersonalLanguageInfo() {
<section>
{error && <p className="error-message">{error}</p>}

{fetchFailure && <Error translationKey="sc-languages-fetch-error" />}

{languages && languages.length > 0 ? (
<section>
<Localized id="sc-personal-your-languages">
Expand Down
18 changes: 18 additions & 0 deletions web/src/reducers/languages.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ test('should use initial state', async () => {
allLanguages: [],
pendingLanguages: false,
currentUILocale: 'en',
fetchFailure: false,
});
});

Expand All @@ -36,6 +37,23 @@ test('should reduce languages', async () => {
expect(newState.allLanguages).toEqual(mockLanguages);
});

test('should reset fetch failure on successful fetch', async () => {
const newState = languageReducer(combineState({ fetchFailure: true }), {
type: languages.ACTION_GOT_LANGUAGES,
languages: mockLanguages,
});

expect(newState.fetchFailure).toEqual(false);
});

test('should reduce language fetch error', async () => {
const newState = languageReducer(combineState({}), {
type: languages.ACTION_GET_LANGUAGES_FAILURE,
});

expect(newState.fetchFailure).toEqual(true);
});

test('should reduce add language request', async () => {
const newState = languageReducer(combineState({}), {
type: languages.ACTION_ADD_LANGUAGE_REQUEST,
Expand Down
Loading