From 4c3772d5c8aa65becd2153c111967366958d679d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Dec 2020 23:41:50 +0100 Subject: [PATCH] Added meaningful error messages for the rest of API calls --- src/tags/TagsList.tsx | 7 ++++++- src/tags/helpers/DeleteTagConfirmModal.tsx | 10 ++++++---- src/tags/helpers/EditTagModal.tsx | 12 ++++++------ src/tags/reducers/tagDelete.ts | 12 +++++++++--- src/tags/reducers/tagEdit.ts | 12 +++++++++--- src/tags/reducers/tagsList.ts | 19 +++++++++++++++---- src/visits/VisitsStats.tsx | 9 +++++++-- src/visits/reducers/common.ts | 4 ++-- src/visits/reducers/shortUrlVisits.ts | 9 ++++++--- src/visits/reducers/tagVisits.ts | 11 ++++++++--- src/visits/types/index.ts | 6 ++++++ test/visits/reducers/shortUrlVisits.test.ts | 2 +- test/visits/reducers/tagVisits.test.ts | 2 +- 13 files changed, 82 insertions(+), 33 deletions(-) diff --git a/src/tags/TagsList.tsx b/src/tags/TagsList.tsx index b72a74f6..a7d21bce 100644 --- a/src/tags/TagsList.tsx +++ b/src/tags/TagsList.tsx @@ -5,6 +5,7 @@ import SearchField from '../utils/SearchField'; import { SelectedServer } from '../servers/data'; import { boundToMercureHub } from '../mercure/helpers/boundToMercureHub'; import { Result } from '../utils/Result'; +import { ShlinkApiError } from '../api/ShlinkApiError'; import { TagsList as TagsListState } from './reducers/tagsList'; import { TagCardProps } from './TagCard'; @@ -33,7 +34,11 @@ const TagsList = (TagCard: FC) => boundToMercureHub(( } if (tagsList.error) { - return Error loading tags :(; + return ( + + + + ); } const tagsCount = tagsList.filteredTags.length; diff --git a/src/tags/helpers/DeleteTagConfirmModal.tsx b/src/tags/helpers/DeleteTagConfirmModal.tsx index eb95cd55..c4ef46ab 100644 --- a/src/tags/helpers/DeleteTagConfirmModal.tsx +++ b/src/tags/helpers/DeleteTagConfirmModal.tsx @@ -2,6 +2,7 @@ import { Modal, ModalBody, ModalFooter, ModalHeader } from 'reactstrap'; import { TagDeletion } from '../reducers/tagDelete'; import { TagModalProps } from '../data'; import { Result } from '../../utils/Result'; +import { ShlinkApiError } from '../../api/ShlinkApiError'; interface DeleteTagConfirmModalProps extends TagModalProps { deleteTag: (tag: string) => Promise; @@ -12,6 +13,7 @@ interface DeleteTagConfirmModalProps extends TagModalProps { const DeleteTagConfirmModal = ( { tag, toggle, isOpen, deleteTag, tagDelete, tagDeleted }: DeleteTagConfirmModalProps, ) => { + const { deleting, error, errorData } = tagDelete; const doDelete = async () => { await deleteTag(tag); tagDeleted(tag); @@ -25,16 +27,16 @@ const DeleteTagConfirmModal = ( Are you sure you want to delete tag {tag}? - {tagDelete.error && ( + {error && ( - Something went wrong while deleting the tag :( + )} - diff --git a/src/tags/helpers/EditTagModal.tsx b/src/tags/helpers/EditTagModal.tsx index 5779520b..7cbd5169 100644 --- a/src/tags/helpers/EditTagModal.tsx +++ b/src/tags/helpers/EditTagModal.tsx @@ -8,8 +8,9 @@ import { handleEventPreventingDefault } from '../../utils/utils'; import ColorGenerator from '../../utils/services/ColorGenerator'; import { TagModalProps } from '../data'; import { TagEdition } from '../reducers/tagEdit'; -import './EditTagModal.scss'; import { Result } from '../../utils/Result'; +import { ShlinkApiError } from '../../api/ShlinkApiError'; +import './EditTagModal.scss'; interface EditTagModalProps extends TagModalProps { tagEdit: TagEdition; @@ -23,6 +24,7 @@ const EditTagModal = ({ getColorForKey }: ColorGenerator) => ( const [ newTagName, setNewTagName ] = useState(tag); const [ color, setColor ] = useState(getColorForKey(tag)); const [ showColorPicker, toggleColorPicker, , hideColorPicker ] = useToggle(); + const { editing, error, errorData } = tagEdit; const saveTag = handleEventPreventingDefault(async () => editTag(tag, newTagName, color) .then(() => tagEdited(tag, newTagName, color)) .then(toggle) @@ -55,17 +57,15 @@ const EditTagModal = ({ getColorForKey }: ColorGenerator) => ( /> - {tagEdit.error && ( + {error && ( - Something went wrong while editing the tag :( + )} - + diff --git a/src/tags/reducers/tagDelete.ts b/src/tags/reducers/tagDelete.ts index 55c08b55..c5f3b2e4 100644 --- a/src/tags/reducers/tagDelete.ts +++ b/src/tags/reducers/tagDelete.ts @@ -2,6 +2,7 @@ import { Action, Dispatch } from 'redux'; import { buildReducer } from '../../utils/helpers/redux'; import { GetState } from '../../container/types'; import { ShlinkApiClientBuilder } from '../../utils/services/ShlinkApiClientBuilder'; +import { ProblemDetailsError } from '../../utils/services/types'; /* eslint-disable padding-line-between-statements */ export const DELETE_TAG_START = 'shlink/deleteTag/DELETE_TAG_START'; @@ -13,20 +14,25 @@ export const TAG_DELETED = 'shlink/deleteTag/TAG_DELETED'; export interface TagDeletion { deleting: boolean; error: boolean; + errorData?: ProblemDetailsError; } export interface DeleteTagAction extends Action { tag: string; } +export interface DeleteTagFailedAction extends Action { + errorData?: ProblemDetailsError; +} + const initialState: TagDeletion = { deleting: false, error: false, }; -export default buildReducer({ +export default buildReducer({ [DELETE_TAG_START]: () => ({ deleting: true, error: false }), - [DELETE_TAG_ERROR]: () => ({ deleting: false, error: true }), + [DELETE_TAG_ERROR]: (_, { errorData }) => ({ deleting: false, error: true, errorData }), [DELETE_TAG]: () => ({ deleting: false, error: false }), }, initialState); @@ -41,7 +47,7 @@ export const deleteTag = (buildShlinkApiClient: ShlinkApiClientBuilder) => (tag: await deleteTags([ tag ]); dispatch({ type: DELETE_TAG }); } catch (e) { - dispatch({ type: DELETE_TAG_ERROR }); + dispatch({ type: DELETE_TAG_ERROR, errorData: e.response?.data }); throw e; } diff --git a/src/tags/reducers/tagEdit.ts b/src/tags/reducers/tagEdit.ts index 44825200..6e4f85c6 100644 --- a/src/tags/reducers/tagEdit.ts +++ b/src/tags/reducers/tagEdit.ts @@ -4,6 +4,7 @@ import { buildReducer } from '../../utils/helpers/redux'; import { GetState } from '../../container/types'; import ColorGenerator from '../../utils/services/ColorGenerator'; import { ShlinkApiClientBuilder } from '../../utils/services/ShlinkApiClientBuilder'; +import { ProblemDetailsError } from '../../utils/services/types'; /* eslint-disable padding-line-between-statements */ export const EDIT_TAG_START = 'shlink/editTag/EDIT_TAG_START'; @@ -18,6 +19,7 @@ export interface TagEdition { newName: string; editing: boolean; error: boolean; + errorData?: ProblemDetailsError; } export interface EditTagAction extends Action { @@ -26,6 +28,10 @@ export interface EditTagAction extends Action { color: string; } +export interface EditTagFailedAction extends Action { + errorData?: ProblemDetailsError; +} + const initialState: TagEdition = { oldName: '', newName: '', @@ -33,9 +39,9 @@ const initialState: TagEdition = { error: false, }; -export default buildReducer({ +export default buildReducer({ [EDIT_TAG_START]: (state) => ({ ...state, editing: true, error: false }), - [EDIT_TAG_ERROR]: (state) => ({ ...state, editing: false, error: true }), + [EDIT_TAG_ERROR]: (state, { errorData }) => ({ ...state, editing: false, error: true, errorData }), [EDIT_TAG]: (_, action) => ({ ...pick([ 'oldName', 'newName' ], action), editing: false, @@ -56,7 +62,7 @@ export const editTag = (buildShlinkApiClient: ShlinkApiClientBuilder, colorGener colorGenerator.setColorForKey(newName, color); dispatch({ type: EDIT_TAG, oldName, newName }); } catch (e) { - dispatch({ type: EDIT_TAG_ERROR }); + dispatch({ type: EDIT_TAG_ERROR, errorData: e.response?.data }); throw e; } diff --git a/src/tags/reducers/tagsList.ts b/src/tags/reducers/tagsList.ts index e33ba2ff..e3e80d1d 100644 --- a/src/tags/reducers/tagsList.ts +++ b/src/tags/reducers/tagsList.ts @@ -2,7 +2,7 @@ import { isEmpty, reject } from 'ramda'; import { Action, Dispatch } from 'redux'; import { CREATE_VISITS, CreateVisitsAction } from '../../visits/reducers/visitCreation'; import { buildReducer } from '../../utils/helpers/redux'; -import { ShlinkTags } from '../../utils/services/types'; +import { ProblemDetailsError, ShlinkTags } from '../../utils/services/types'; import { GetState } from '../../container/types'; import { ShlinkApiClientBuilder } from '../../utils/services/ShlinkApiClientBuilder'; import { TagStats } from '../data'; @@ -25,6 +25,7 @@ export interface TagsList { stats: TagsStatsMap; loading: boolean; error: boolean; + errorData?: ProblemDetailsError; } interface ListTagsAction extends Action { @@ -32,11 +33,21 @@ interface ListTagsAction extends Action { stats: TagsStatsMap; } + +interface ListTagsFailedAction extends Action { + errorData?: ProblemDetailsError; +} + interface FilterTagsAction extends Action { searchTerm: string; } -type ListTagsCombinedAction = ListTagsAction & DeleteTagAction & CreateVisitsAction & EditTagAction & FilterTagsAction; +type ListTagsCombinedAction = ListTagsAction + & DeleteTagAction + & CreateVisitsAction + & EditTagAction + & FilterTagsAction + & ListTagsFailedAction; const initialState = { tags: [], @@ -74,7 +85,7 @@ const calculateVisitsPerTag = (createdVisits: CreateVisit[]): TagIncrease[] => O export default buildReducer({ [LIST_TAGS_START]: () => ({ ...initialState, loading: true }), - [LIST_TAGS_ERROR]: () => ({ ...initialState, error: true }), + [LIST_TAGS_ERROR]: (_, { errorData }) => ({ ...initialState, error: true, errorData }), [LIST_TAGS]: (_, { tags, stats }) => ({ ...initialState, stats, tags, filteredTags: tags }), [TAG_DELETED]: (state, { tag }) => ({ ...state, @@ -119,7 +130,7 @@ export const listTags = (buildShlinkApiClient: ShlinkApiClientBuilder, force = t dispatch({ tags, stats: processedStats, type: LIST_TAGS }); } catch (e) { - dispatch({ type: LIST_TAGS_ERROR }); + dispatch({ type: LIST_TAGS_ERROR, errorData: e.response?.data }); } }; diff --git a/src/visits/VisitsStats.tsx b/src/visits/VisitsStats.tsx index 83e267e0..ceb11522 100644 --- a/src/visits/VisitsStats.tsx +++ b/src/visits/VisitsStats.tsx @@ -12,6 +12,7 @@ import { formatIsoDate } from '../utils/helpers/date'; import { ShlinkVisitsParams } from '../utils/services/types'; import { DateInterval, DateRange, intervalToDateRange } from '../utils/dates/types'; import { Result } from '../utils/Result'; +import { ShlinkApiError } from '../api/ShlinkApiError'; import SortableBarGraph from './helpers/SortableBarGraph'; import GraphCard from './helpers/GraphCard'; import LineChartCard from './helpers/LineChartCard'; @@ -83,7 +84,7 @@ const VisitsStats: FC = ({ children, visitsInfo, getVisits, ca return !subPath ? `${baseUrl}${query}` : `${baseUrl}${subPath}${query}`; }; - const { visits, loading, loadingLarge, error, progress } = visitsInfo; + const { visits, loading, loadingLarge, error, errorData, progress } = visitsInfo; const normalizedVisits = useMemo(() => normalizeVisits(visits), [ visits ]); const { os, browsers, referrers, countries, cities, citiesForMap } = useMemo( () => processStatsFromVisits(normalizedVisits), @@ -131,7 +132,11 @@ const VisitsStats: FC = ({ children, visitsInfo, getVisits, ca } if (error) { - return An error occurred while loading visits :(; + return ( + + + + ); } if (isEmpty(visits)) { diff --git a/src/visits/reducers/common.ts b/src/visits/reducers/common.ts index f7e0afbd..4e1313f7 100644 --- a/src/visits/reducers/common.ts +++ b/src/visits/reducers/common.ts @@ -1,7 +1,7 @@ import { flatten, prop, range, splitEvery } from 'ramda'; import { Action, Dispatch } from 'redux'; import { ShlinkPaginator, ShlinkVisits } from '../../utils/services/types'; -import { Visit } from '../types'; +import { Visit, VisitsLoadFailedAction } from '../types'; const ITEMS_PER_PAGE = 5000; const PARALLEL_REQUESTS_COUNT = 4; @@ -71,6 +71,6 @@ export const getVisitsWithLoader = async & { visits: V dispatch({ ...extraFinishActionData, visits, type: actionMap.finish }); } catch (e) { - dispatch({ type: actionMap.error }); + dispatch({ type: actionMap.error, errorData: e.response?.data }); } }; diff --git a/src/visits/reducers/shortUrlVisits.ts b/src/visits/reducers/shortUrlVisits.ts index 4cc689b2..2126513e 100644 --- a/src/visits/reducers/shortUrlVisits.ts +++ b/src/visits/reducers/shortUrlVisits.ts @@ -1,6 +1,6 @@ import { Action, Dispatch } from 'redux'; import { shortUrlMatches } from '../../short-urls/helpers'; -import { Visit, VisitsInfo, VisitsLoadProgressChangedAction } from '../types'; +import { Visit, VisitsInfo, VisitsLoadFailedAction, VisitsLoadProgressChangedAction } from '../types'; import { ShortUrlIdentifier } from '../../short-urls/data'; import { buildActionCreator, buildReducer } from '../../utils/helpers/redux'; import { ShlinkApiClientBuilder } from '../../utils/services/ShlinkApiClientBuilder'; @@ -24,7 +24,10 @@ interface ShortUrlVisitsAction extends Action, ShortUrlIdentifier { visits: Visit[]; } -type ShortUrlVisitsCombinedAction = ShortUrlVisitsAction & VisitsLoadProgressChangedAction & CreateVisitsAction; +type ShortUrlVisitsCombinedAction = ShortUrlVisitsAction +& VisitsLoadProgressChangedAction +& CreateVisitsAction +& VisitsLoadFailedAction; const initialState: ShortUrlVisits = { visits: [], @@ -39,7 +42,7 @@ const initialState: ShortUrlVisits = { export default buildReducer({ [GET_SHORT_URL_VISITS_START]: () => ({ ...initialState, loading: true }), - [GET_SHORT_URL_VISITS_ERROR]: () => ({ ...initialState, error: true }), + [GET_SHORT_URL_VISITS_ERROR]: (_, { errorData }) => ({ ...initialState, error: true, errorData }), [GET_SHORT_URL_VISITS]: (_, { visits, shortCode, domain }) => ({ ...initialState, visits, diff --git a/src/visits/reducers/tagVisits.ts b/src/visits/reducers/tagVisits.ts index 691c8b14..3f361acb 100644 --- a/src/visits/reducers/tagVisits.ts +++ b/src/visits/reducers/tagVisits.ts @@ -1,5 +1,5 @@ import { Action, Dispatch } from 'redux'; -import { Visit, VisitsInfo, VisitsLoadProgressChangedAction } from '../types'; +import { Visit, VisitsInfo, VisitsLoadFailedAction, VisitsLoadProgressChangedAction } from '../types'; import { buildActionCreator, buildReducer } from '../../utils/helpers/redux'; import { ShlinkApiClientBuilder } from '../../utils/services/ShlinkApiClientBuilder'; import { GetState } from '../../container/types'; @@ -24,6 +24,11 @@ export interface TagVisitsAction extends Action { tag: string; } +type TagsVisitsCombinedAction = TagVisitsAction +& VisitsLoadProgressChangedAction +& CreateVisitsAction +& VisitsLoadFailedAction; + const initialState: TagVisits = { visits: [], tag: '', @@ -34,9 +39,9 @@ const initialState: TagVisits = { progress: 0, }; -export default buildReducer({ +export default buildReducer({ [GET_TAG_VISITS_START]: () => ({ ...initialState, loading: true }), - [GET_TAG_VISITS_ERROR]: () => ({ ...initialState, error: true }), + [GET_TAG_VISITS_ERROR]: (_, { errorData }) => ({ ...initialState, error: true, errorData }), [GET_TAG_VISITS]: (_, { visits, tag }) => ({ ...initialState, visits, tag }), [GET_TAG_VISITS_LARGE]: (state) => ({ ...state, loadingLarge: true }), [GET_TAG_VISITS_CANCEL]: (state) => ({ ...state, cancelLoad: true }), diff --git a/src/visits/types/index.ts b/src/visits/types/index.ts index b54c19f7..91540c4a 100644 --- a/src/visits/types/index.ts +++ b/src/visits/types/index.ts @@ -1,11 +1,13 @@ import { Action } from 'redux'; import { ShortUrl } from '../../short-urls/data'; +import { ProblemDetailsError } from '../../utils/services/types'; export interface VisitsInfo { visits: Visit[]; loading: boolean; loadingLarge: boolean; error: boolean; + errorData?: ProblemDetailsError; progress: number; cancelLoad: boolean; } @@ -14,6 +16,10 @@ export interface VisitsLoadProgressChangedAction extends Action { progress: number; } +export interface VisitsLoadFailedAction extends Action { + errorData?: ProblemDetailsError; +} + interface VisitLocation { countryCode: string | null; countryName: string | null; diff --git a/test/visits/reducers/shortUrlVisits.test.ts b/test/visits/reducers/shortUrlVisits.test.ts index 1e33c5b9..3467290b 100644 --- a/test/visits/reducers/shortUrlVisits.test.ts +++ b/test/visits/reducers/shortUrlVisits.test.ts @@ -103,7 +103,7 @@ describe('shortUrlVisitsReducer', () => { beforeEach(() => dispatchMock.mockReset()); it('dispatches start and error when promise is rejected', async () => { - const ShlinkApiClient = buildApiClientMock(Promise.reject() as any); + const ShlinkApiClient = buildApiClientMock(Promise.reject({})); await getShortUrlVisits(() => ShlinkApiClient)('abc123')(dispatchMock, getState); diff --git a/test/visits/reducers/tagVisits.test.ts b/test/visits/reducers/tagVisits.test.ts index 30304001..2b419483 100644 --- a/test/visits/reducers/tagVisits.test.ts +++ b/test/visits/reducers/tagVisits.test.ts @@ -103,7 +103,7 @@ describe('tagVisitsReducer', () => { beforeEach(jest.resetAllMocks); it('dispatches start and error when promise is rejected', async () => { - const ShlinkApiClient = buildApiClientMock(Promise.reject()); + const ShlinkApiClient = buildApiClientMock(Promise.reject({})); await getTagVisits(() => ShlinkApiClient)('foo')(dispatchMock, getState);