From 54b1ab12cdbcd0c22a869ef67782fbc9fcf35238 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Dec 2020 20:53:31 +0100 Subject: [PATCH 1/8] Passed API error while creating URLs to display proper error messages --- .../helpers/CreateShortUrlResult.tsx | 6 ++-- src/short-urls/reducers/shortUrlCreation.ts | 12 ++++++-- src/utils/services/types.ts | 30 ++++++++++++------- 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/short-urls/helpers/CreateShortUrlResult.tsx b/src/short-urls/helpers/CreateShortUrlResult.tsx index 916f622e..ad4fa3bc 100644 --- a/src/short-urls/helpers/CreateShortUrlResult.tsx +++ b/src/short-urls/helpers/CreateShortUrlResult.tsx @@ -9,6 +9,7 @@ import { ShortUrlCreation } from '../reducers/shortUrlCreation'; import { StateFlagTimeout } from '../../utils/helpers/hooks'; import { Result } from '../../utils/Result'; import './CreateShortUrlResult.scss'; +import { isInvalidArgumentError } from '../../utils/services/types'; export interface CreateShortUrlResultProps extends ShortUrlCreation { resetCreateShortUrl: () => void; @@ -16,7 +17,7 @@ export interface CreateShortUrlResultProps extends ShortUrlCreation { } const CreateShortUrlResult = (useStateFlagTimeout: StateFlagTimeout) => ( - { error, result, resetCreateShortUrl, canBeClosed = false }: CreateShortUrlResultProps, + { error, errorData, result, resetCreateShortUrl, canBeClosed = false }: CreateShortUrlResultProps, ) => { const [ showCopyTooltip, setShowCopyTooltip ] = useStateFlagTimeout(); @@ -28,7 +29,8 @@ const CreateShortUrlResult = (useStateFlagTimeout: StateFlagTimeout) => ( return ( {canBeClosed && } - An error occurred while creating the URL :( + {errorData?.detail ?? 'An error occurred while creating the URL :('} + {isInvalidArgumentError(errorData) &&

Invalid elements: [{errorData.invalidElements.join(', ')}]

}
); } diff --git a/src/short-urls/reducers/shortUrlCreation.ts b/src/short-urls/reducers/shortUrlCreation.ts index 3f066655..0b617f45 100644 --- a/src/short-urls/reducers/shortUrlCreation.ts +++ b/src/short-urls/reducers/shortUrlCreation.ts @@ -3,6 +3,7 @@ import { GetState } from '../../container/types'; import { ShortUrl, ShortUrlData } from '../data'; import { buildReducer, buildActionCreator } from '../../utils/helpers/redux'; import { ShlinkApiClientBuilder } from '../../utils/services/ShlinkApiClientBuilder'; +import { ProblemDetailsError } from '../../utils/services/types'; /* eslint-disable padding-line-between-statements */ export const CREATE_SHORT_URL_START = 'shlink/createShortUrl/CREATE_SHORT_URL_START'; @@ -15,21 +16,26 @@ export interface ShortUrlCreation { result: ShortUrl | null; saving: boolean; error: boolean; + errorData?: ProblemDetailsError; } export interface CreateShortUrlAction extends Action { result: ShortUrl; } +export interface CreateShortUrlFailedAction extends Action { + errorData?: ProblemDetailsError; +} + const initialState: ShortUrlCreation = { result: null, saving: false, error: false, }; -export default buildReducer({ +export default buildReducer({ [CREATE_SHORT_URL_START]: (state) => ({ ...state, saving: true, error: false }), - [CREATE_SHORT_URL_ERROR]: (state) => ({ ...state, saving: false, error: true }), + [CREATE_SHORT_URL_ERROR]: (state, { errorData }) => ({ ...state, saving: false, error: true, errorData }), [CREATE_SHORT_URL]: (_, { result }) => ({ result, saving: false, error: false }), [RESET_CREATE_SHORT_URL]: () => initialState, }, initialState); @@ -46,7 +52,7 @@ export const createShortUrl = (buildShlinkApiClient: ShlinkApiClientBuilder) => dispatch({ type: CREATE_SHORT_URL, result }); } catch (e) { - dispatch({ type: CREATE_SHORT_URL_ERROR }); + dispatch({ type: CREATE_SHORT_URL_ERROR, errorData: e.response?.data }); throw e; } diff --git a/src/utils/services/types.ts b/src/utils/services/types.ts index a098750b..ae50c352 100644 --- a/src/utils/services/types.ts +++ b/src/utils/services/types.ts @@ -25,12 +25,12 @@ interface ShlinkTagsStats { export interface ShlinkTags { tags: string[]; - stats?: ShlinkTagsStats[]; // Is only optional in old Shlink versions + stats?: ShlinkTagsStats[]; // Is only optional in Shlink older than v2.2 } export interface ShlinkTagsResponse { data: string[]; - stats?: ShlinkTagsStats[]; // Is only optional in old Shlink versions + stats?: ShlinkTagsStats[]; // Is only optional in Shlink older than v2.2 } export interface ShlinkPaginator { @@ -41,7 +41,7 @@ export interface ShlinkPaginator { export interface ShlinkVisits { data: Visit[]; - pagination?: ShlinkPaginator; // Is only optional in old Shlink versions + pagination: ShlinkPaginator; } export interface ShlinkVisitsOverview { @@ -60,14 +60,6 @@ export interface ShlinkShortUrlMeta extends ShortUrlMeta { longUrl?: string; } -export interface ProblemDetailsError { - type: string; - detail: string; - title: string; - status: number; - [extraProps: string]: any; -} - export interface ShlinkDomain { domain: string; isDefault: boolean; @@ -76,3 +68,19 @@ export interface ShlinkDomain { export interface ShlinkDomainsResponse { data: ShlinkDomain[]; } + +export interface ProblemDetailsError { + type: string; + detail: string; + title: string; + status: number; + [extraProps: string]: any; +} + +export interface InvalidArgumentError extends ProblemDetailsError { + type: 'INVALID_ARGUMENT'; + invalidElements: string[]; +} + +export const isInvalidArgumentError = (error?: ProblemDetailsError): error is InvalidArgumentError => + error?.type === 'INVALID_ARGUMENT'; From f69f791790238b7c2d633f8886fc6a8cc1c895fd Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Dec 2020 21:02:30 +0100 Subject: [PATCH 2/8] Improved handling of short URL deletion errors --- src/short-urls/helpers/DeleteShortUrlModal.tsx | 16 ++++++---------- src/utils/services/types.ts | 10 +++++++++- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/short-urls/helpers/DeleteShortUrlModal.tsx b/src/short-urls/helpers/DeleteShortUrlModal.tsx index 35f99cf4..9fd714f9 100644 --- a/src/short-urls/helpers/DeleteShortUrlModal.tsx +++ b/src/short-urls/helpers/DeleteShortUrlModal.tsx @@ -5,8 +5,7 @@ import { ShortUrlDeletion } from '../reducers/shortUrlDeletion'; import { ShortUrlModalProps } from '../data'; import { handleEventPreventingDefault, OptionalString } from '../../utils/utils'; import { Result } from '../../utils/Result'; - -const THRESHOLD_REACHED = 'INVALID_SHORTCODE_DELETION'; +import { isInvalidDeletionError } from '../../utils/services/types'; interface DeleteShortUrlModalConnectProps extends ShortUrlModalProps { shortUrlDeletion: ShortUrlDeletion; @@ -22,9 +21,6 @@ const DeleteShortUrlModal = ( useEffect(() => resetDeleteShortUrl, []); const { error, errorData } = shortUrlDeletion; - const errorCode = error && errorData?.type; - const hasThresholdError = errorCode === THRESHOLD_REACHED; - const hasErrorOtherThanThreshold = error && errorCode !== THRESHOLD_REACHED; const close = pipe(resetDeleteShortUrl, toggle); const handleDeleteUrl = handleEventPreventingDefault(() => { const { shortCode, domain } = shortUrl; @@ -53,15 +49,15 @@ const DeleteShortUrlModal = ( onChange={(e) => setInputValue(e.target.value)} /> - {hasThresholdError && ( + {error && isInvalidDeletionError(errorData) && ( - {errorData?.threshold && `This short URL has received more than ${errorData.threshold} visits, and therefore, it cannot be deleted.`} - {!errorData?.threshold && 'This short URL has received too many visits, and therefore, it cannot be deleted.'} + {errorData.threshold && `This short URL has received more than ${errorData.threshold} visits, and therefore, it cannot be deleted.`} + {!errorData.threshold && 'This short URL has received too many visits, and therefore, it cannot be deleted.'} )} - {hasErrorOtherThanThreshold && ( + {error && !isInvalidDeletionError(errorData) && ( - Something went wrong while deleting the URL :( + {errorData?.detail ?? 'Something went wrong while deleting the URL :('} )} diff --git a/src/utils/services/types.ts b/src/utils/services/types.ts index ae50c352..b515ff06 100644 --- a/src/utils/services/types.ts +++ b/src/utils/services/types.ts @@ -77,10 +77,18 @@ export interface ProblemDetailsError { [extraProps: string]: any; } -export interface InvalidArgumentError extends ProblemDetailsError { +interface InvalidArgumentError extends ProblemDetailsError { type: 'INVALID_ARGUMENT'; invalidElements: string[]; } +interface InvalidShortUrlDeletion extends ProblemDetailsError { + type: 'INVALID_SHORTCODE_DELETION'; + threshold?: number; +} + export const isInvalidArgumentError = (error?: ProblemDetailsError): error is InvalidArgumentError => error?.type === 'INVALID_ARGUMENT'; + +export const isInvalidDeletionError = (error?: ProblemDetailsError): error is InvalidShortUrlDeletion => + error?.type === 'INVALID_SHORTCODE_DELETION'; From 51379eb2a0b9e3930849b1554a402f08a7e885e4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Dec 2020 21:19:02 +0100 Subject: [PATCH 3/8] Created component holding the logic to render Shlink API errors --- src/api/ShlinkApiError.tsx | 15 +++++++++++++ .../helpers/CreateShortUrlResult.tsx | 5 ++--- .../helpers/DeleteShortUrlModal.tsx | 13 ++++------- src/short-urls/helpers/EditMetaModal.tsx | 9 ++++++-- src/short-urls/reducers/shortUrlMeta.ts | 12 +++++++--- src/utils/services/ShlinkApiClient.ts | 2 ++ src/utils/services/ShlinkApiClientBuilder.ts | 2 ++ src/utils/services/types.ts | 2 ++ .../helpers/DeleteShortUrlModal.test.tsx | 22 ------------------- 9 files changed, 43 insertions(+), 39 deletions(-) create mode 100644 src/api/ShlinkApiError.tsx diff --git a/src/api/ShlinkApiError.tsx b/src/api/ShlinkApiError.tsx new file mode 100644 index 00000000..8c9e51f5 --- /dev/null +++ b/src/api/ShlinkApiError.tsx @@ -0,0 +1,15 @@ +import { isInvalidArgumentError, ProblemDetailsError } from '../utils/services/types'; + +interface ShlinkApiErrorProps { + errorData?: ProblemDetailsError; + fallbackMessage?: string; +} + +export const ShlinkApiError = ({ errorData, fallbackMessage }: ShlinkApiErrorProps) => ( + <> + {errorData?.detail ?? fallbackMessage} + {isInvalidArgumentError(errorData) && +

Invalid elements: [{errorData.invalidElements.join(', ')}]

+ } + +); diff --git a/src/short-urls/helpers/CreateShortUrlResult.tsx b/src/short-urls/helpers/CreateShortUrlResult.tsx index ad4fa3bc..f5a75ded 100644 --- a/src/short-urls/helpers/CreateShortUrlResult.tsx +++ b/src/short-urls/helpers/CreateShortUrlResult.tsx @@ -9,7 +9,7 @@ import { ShortUrlCreation } from '../reducers/shortUrlCreation'; import { StateFlagTimeout } from '../../utils/helpers/hooks'; import { Result } from '../../utils/Result'; import './CreateShortUrlResult.scss'; -import { isInvalidArgumentError } from '../../utils/services/types'; +import { ShlinkApiError } from '../../api/ShlinkApiError'; export interface CreateShortUrlResultProps extends ShortUrlCreation { resetCreateShortUrl: () => void; @@ -29,8 +29,7 @@ const CreateShortUrlResult = (useStateFlagTimeout: StateFlagTimeout) => ( return ( {canBeClosed && } - {errorData?.detail ?? 'An error occurred while creating the URL :('} - {isInvalidArgumentError(errorData) &&

Invalid elements: [{errorData.invalidElements.join(', ')}]

} +
); } diff --git a/src/short-urls/helpers/DeleteShortUrlModal.tsx b/src/short-urls/helpers/DeleteShortUrlModal.tsx index 9fd714f9..119892d2 100644 --- a/src/short-urls/helpers/DeleteShortUrlModal.tsx +++ b/src/short-urls/helpers/DeleteShortUrlModal.tsx @@ -6,6 +6,7 @@ import { ShortUrlModalProps } from '../data'; import { handleEventPreventingDefault, OptionalString } from '../../utils/utils'; import { Result } from '../../utils/Result'; import { isInvalidDeletionError } from '../../utils/services/types'; +import { ShlinkApiError } from '../../api/ShlinkApiError'; interface DeleteShortUrlModalConnectProps extends ShortUrlModalProps { shortUrlDeletion: ShortUrlDeletion; @@ -49,15 +50,9 @@ const DeleteShortUrlModal = ( onChange={(e) => setInputValue(e.target.value)} /> - {error && isInvalidDeletionError(errorData) && ( - - {errorData.threshold && `This short URL has received more than ${errorData.threshold} visits, and therefore, it cannot be deleted.`} - {!errorData.threshold && 'This short URL has received too many visits, and therefore, it cannot be deleted.'} - - )} - {error && !isInvalidDeletionError(errorData) && ( - - {errorData?.detail ?? 'Something went wrong while deleting the URL :('} + {error && ( + + )} diff --git a/src/short-urls/helpers/EditMetaModal.tsx b/src/short-urls/helpers/EditMetaModal.tsx index ca22c2a1..688cf9be 100644 --- a/src/short-urls/helpers/EditMetaModal.tsx +++ b/src/short-urls/helpers/EditMetaModal.tsx @@ -11,6 +11,7 @@ import { formatIsoDate } from '../../utils/helpers/date'; import { ShortUrl, ShortUrlMeta, ShortUrlModalProps } from '../data'; import { handleEventPreventingDefault, Nullable, OptionalString } from '../../utils/utils'; import { Result } from '../../utils/Result'; +import { ShlinkApiError } from '../../api/ShlinkApiError'; interface EditMetaModalConnectProps extends ShortUrlModalProps { shortUrlMeta: ShortUrlMetaEdition; @@ -27,7 +28,7 @@ const dateOrNull = (shortUrl: ShortUrl | undefined, dateName: 'validSince' | 'va const EditMetaModal = ( { isOpen, toggle, shortUrl, shortUrlMeta, editShortUrlMeta, resetShortUrlMeta }: EditMetaModalConnectProps, ) => { - const { saving, error } = shortUrlMeta; + const { saving, error, errorData } = shortUrlMeta; const url = shortUrl && (shortUrl.shortUrl || ''); const [ validSince, setValidSince ] = useState(dateOrNull(shortUrl, 'validSince')); const [ validUntil, setValidUntil ] = useState(dateOrNull(shortUrl, 'validUntil')); @@ -78,9 +79,13 @@ const EditMetaModal = ( onChange={(e: ChangeEvent) => setMaxVisits(Number(e.target.value))} /> + {error && ( - Something went wrong while saving the metadata :( + )} diff --git a/src/short-urls/reducers/shortUrlMeta.ts b/src/short-urls/reducers/shortUrlMeta.ts index 8b4e8a51..c867bf0d 100644 --- a/src/short-urls/reducers/shortUrlMeta.ts +++ b/src/short-urls/reducers/shortUrlMeta.ts @@ -4,6 +4,7 @@ import { GetState } from '../../container/types'; import { buildActionCreator, buildReducer } from '../../utils/helpers/redux'; import { OptionalString } from '../../utils/utils'; import { ShlinkApiClientBuilder } from '../../utils/services/ShlinkApiClientBuilder'; +import { ProblemDetailsError } from '../../utils/services/types'; /* eslint-disable padding-line-between-statements */ export const EDIT_SHORT_URL_META_START = 'shlink/shortUrlMeta/EDIT_SHORT_URL_META_START'; @@ -17,12 +18,17 @@ export interface ShortUrlMetaEdition { meta: ShortUrlMeta; saving: boolean; error: boolean; + errorData?: ProblemDetailsError; } export interface ShortUrlMetaEditedAction extends Action, ShortUrlIdentifier { meta: ShortUrlMeta; } +export interface ShortUrlMetaEditionFailedAction extends Action { + errorData?: ProblemDetailsError; +} + const initialState: ShortUrlMetaEdition = { shortCode: null, meta: {}, @@ -30,9 +36,9 @@ const initialState: ShortUrlMetaEdition = { error: false, }; -export default buildReducer({ +export default buildReducer({ [EDIT_SHORT_URL_META_START]: (state) => ({ ...state, saving: true, error: false }), - [EDIT_SHORT_URL_META_ERROR]: (state) => ({ ...state, saving: false, error: true }), + [EDIT_SHORT_URL_META_ERROR]: (state, { errorData }) => ({ ...state, saving: false, error: true, errorData }), [SHORT_URL_META_EDITED]: (_, { shortCode, meta }) => ({ shortCode, meta, saving: false, error: false }), [RESET_EDIT_SHORT_URL_META]: () => initialState, }, initialState); @@ -49,7 +55,7 @@ export const editShortUrlMeta = (buildShlinkApiClient: ShlinkApiClientBuilder) = await updateShortUrlMeta(shortCode, domain, meta); dispatch({ shortCode, meta, domain, type: SHORT_URL_META_EDITED }); } catch (e) { - dispatch({ type: EDIT_SHORT_URL_META_ERROR }); + dispatch({ type: EDIT_SHORT_URL_META_ERROR, errorData: e.response?.data }); throw e; } diff --git a/src/utils/services/ShlinkApiClient.ts b/src/utils/services/ShlinkApiClient.ts index 706a72eb..b04b7738 100644 --- a/src/utils/services/ShlinkApiClient.ts +++ b/src/utils/services/ShlinkApiClient.ts @@ -18,6 +18,8 @@ import { ShlinkVisitsOverview, } from './types'; +// TODO Move this file to api module + const buildShlinkBaseUrl = (url: string, apiVersion: number) => url ? `${url}/rest/v${apiVersion}` : ''; const rejectNilProps = reject(isNil); diff --git a/src/utils/services/ShlinkApiClientBuilder.ts b/src/utils/services/ShlinkApiClientBuilder.ts index d2ba24cd..328a1c8a 100644 --- a/src/utils/services/ShlinkApiClientBuilder.ts +++ b/src/utils/services/ShlinkApiClientBuilder.ts @@ -4,6 +4,8 @@ import { hasServerData, SelectedServer, ServerWithId } from '../../servers/data' import { GetState } from '../../container/types'; import ShlinkApiClient from './ShlinkApiClient'; +// TODO Move this file to api module + const apiClients: Record = {}; const isGetState = (getStateOrSelectedServer: GetState | ServerWithId): getStateOrSelectedServer is GetState => diff --git a/src/utils/services/types.ts b/src/utils/services/types.ts index b515ff06..9f5f9225 100644 --- a/src/utils/services/types.ts +++ b/src/utils/services/types.ts @@ -2,6 +2,8 @@ import { Visit } from '../../visits/types'; // FIXME Should be defined as part o import { ShortUrl, ShortUrlMeta } from '../../short-urls/data'; // FIXME Should be defined as part of this module import { OptionalString } from '../utils'; +// TODO Move this file to api module + export interface ShlinkShortUrlsResponse { data: ShortUrl[]; pagination: ShlinkPaginator; diff --git a/test/short-urls/helpers/DeleteShortUrlModal.test.tsx b/test/short-urls/helpers/DeleteShortUrlModal.test.tsx index 06ce4864..d39fdb27 100644 --- a/test/short-urls/helpers/DeleteShortUrlModal.test.tsx +++ b/test/short-urls/helpers/DeleteShortUrlModal.test.tsx @@ -33,28 +33,6 @@ describe('', () => { afterEach(() => wrapper?.unmount()); afterEach(jest.clearAllMocks); - it.each([ - [ - { type: 'INVALID_SHORTCODE_DELETION' }, - 'This short URL has received too many visits, and therefore, it cannot be deleted.', - ], - [ - { type: 'INVALID_SHORTCODE_DELETION', threshold: 8 }, - 'This short URL has received more than 8 visits, and therefore, it cannot be deleted.', - ], - ])('shows threshold error message when threshold error occurs', (errorData: Partial, expectedMessage) => { - const wrapper = createWrapper({ - loading: false, - error: true, - shortCode: 'abc123', - errorData: Mock.of(errorData), - }); - const warning = wrapper.find(Result).filterWhere((result) => result.prop('type') === 'warning'); - - expect(warning).toHaveLength(1); - expect(warning.html()).toContain(expectedMessage); - }); - it('shows generic error when non-threshold error occurs', () => { const wrapper = createWrapper({ loading: false, From ee95d5a1b7d8220f8716b11f2d922ea18eee1d61 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Dec 2020 21:26:45 +0100 Subject: [PATCH 4/8] Improved handling of errors in several API interactions --- src/short-urls/helpers/EditShortUrlModal.tsx | 8 ++++++-- src/short-urls/helpers/EditTagsModal.tsx | 10 ++++++---- src/short-urls/reducers/shortUrlEdition.ts | 12 +++++++++--- src/short-urls/reducers/shortUrlTags.ts | 12 +++++++++--- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/short-urls/helpers/EditShortUrlModal.tsx b/src/short-urls/helpers/EditShortUrlModal.tsx index 1a24d640..0a09e27e 100644 --- a/src/short-urls/helpers/EditShortUrlModal.tsx +++ b/src/short-urls/helpers/EditShortUrlModal.tsx @@ -5,6 +5,7 @@ import { ShortUrlEdition } from '../reducers/shortUrlEdition'; import { handleEventPreventingDefault, hasValue, OptionalString } from '../../utils/utils'; import { ShortUrlModalProps } from '../data'; import { Result } from '../../utils/Result'; +import { ShlinkApiError } from '../../api/ShlinkApiError'; interface EditShortUrlModalProps extends ShortUrlModalProps { shortUrlEdition: ShortUrlEdition; @@ -12,7 +13,7 @@ interface EditShortUrlModalProps extends ShortUrlModalProps { } const EditShortUrlModal = ({ isOpen, toggle, shortUrl, shortUrlEdition, editShortUrl }: EditShortUrlModalProps) => { - const { saving, error } = shortUrlEdition; + const { saving, error, errorData } = shortUrlEdition; const url = shortUrl?.shortUrl ?? ''; const [ longUrl, setLongUrl ] = useState(shortUrl.longUrl); @@ -36,7 +37,10 @@ const EditShortUrlModal = ({ isOpen, toggle, shortUrl, shortUrlEdition, editShor {error && ( - Something went wrong while saving the long URL :( + )} diff --git a/src/short-urls/helpers/EditTagsModal.tsx b/src/short-urls/helpers/EditTagsModal.tsx index a41a4b98..a390499c 100644 --- a/src/short-urls/helpers/EditTagsModal.tsx +++ b/src/short-urls/helpers/EditTagsModal.tsx @@ -6,6 +6,7 @@ import { ShortUrlModalProps } from '../data'; import { OptionalString } from '../../utils/utils'; import { TagsSelectorProps } from '../../tags/helpers/TagsSelector'; import { Result } from '../../utils/Result'; +import { ShlinkApiError } from '../../api/ShlinkApiError'; interface EditTagsModalProps extends ShortUrlModalProps { shortUrlTags: ShortUrlTags; @@ -20,6 +21,7 @@ const EditTagsModal = (TagsSelector: FC) => ( useEffect(() => resetShortUrlsTags, []); + const { saving, error, errorData } = shortUrlTags; const url = shortUrl?.shortUrl ?? ''; const saveTags = async () => editShortUrlTags(shortUrl.shortCode, shortUrl.domain, selectedTags) .then(toggle) @@ -32,16 +34,16 @@ const EditTagsModal = (TagsSelector: FC) => ( - {shortUrlTags.error && ( + {error && ( - Something went wrong while saving the tags :( + )} - diff --git a/src/short-urls/reducers/shortUrlEdition.ts b/src/short-urls/reducers/shortUrlEdition.ts index 44feac7b..c326c1ab 100644 --- a/src/short-urls/reducers/shortUrlEdition.ts +++ b/src/short-urls/reducers/shortUrlEdition.ts @@ -4,6 +4,7 @@ import { GetState } from '../../container/types'; import { OptionalString } from '../../utils/utils'; import { ShortUrlIdentifier } from '../data'; import { ShlinkApiClientBuilder } from '../../utils/services/ShlinkApiClientBuilder'; +import { ProblemDetailsError } from '../../utils/services/types'; /* eslint-disable padding-line-between-statements */ export const EDIT_SHORT_URL_START = 'shlink/shortUrlEdition/EDIT_SHORT_URL_START'; @@ -16,12 +17,17 @@ export interface ShortUrlEdition { longUrl: string | null; saving: boolean; error: boolean; + errorData?: ProblemDetailsError; } export interface ShortUrlEditedAction extends Action, ShortUrlIdentifier { longUrl: string; } +export interface ShortUrlEditionFailedAction extends Action { + errorData?: ProblemDetailsError; +} + const initialState: ShortUrlEdition = { shortCode: null, longUrl: null, @@ -29,9 +35,9 @@ const initialState: ShortUrlEdition = { error: false, }; -export default buildReducer({ +export default buildReducer({ [EDIT_SHORT_URL_START]: (state) => ({ ...state, saving: true, error: false }), - [EDIT_SHORT_URL_ERROR]: (state) => ({ ...state, saving: false, error: true }), + [EDIT_SHORT_URL_ERROR]: (state, { errorData }) => ({ ...state, saving: false, error: true, errorData }), [SHORT_URL_EDITED]: (_, { shortCode, longUrl }) => ({ shortCode, longUrl, saving: false, error: false }), }, initialState); @@ -47,7 +53,7 @@ export const editShortUrl = (buildShlinkApiClient: ShlinkApiClientBuilder) => ( await updateShortUrlMeta(shortCode, domain, { longUrl }); dispatch({ shortCode, longUrl, domain, type: SHORT_URL_EDITED }); } catch (e) { - dispatch({ type: EDIT_SHORT_URL_ERROR }); + dispatch({ type: EDIT_SHORT_URL_ERROR, errorData: e.response?.data }); throw e; } diff --git a/src/short-urls/reducers/shortUrlTags.ts b/src/short-urls/reducers/shortUrlTags.ts index b81380f1..0c38462c 100644 --- a/src/short-urls/reducers/shortUrlTags.ts +++ b/src/short-urls/reducers/shortUrlTags.ts @@ -4,6 +4,7 @@ import { GetState } from '../../container/types'; import { OptionalString } from '../../utils/utils'; import { ShortUrlIdentifier } from '../data'; import { ShlinkApiClientBuilder } from '../../utils/services/ShlinkApiClientBuilder'; +import { ProblemDetailsError } from '../../utils/services/types'; /* eslint-disable padding-line-between-statements */ export const EDIT_SHORT_URL_TAGS_START = 'shlink/shortUrlTags/EDIT_SHORT_URL_TAGS_START'; @@ -17,12 +18,17 @@ export interface ShortUrlTags { tags: string[]; saving: boolean; error: boolean; + errorData?: ProblemDetailsError; } export interface EditShortUrlTagsAction extends Action, ShortUrlIdentifier { tags: string[]; } +export interface EditShortUrlTagsFailedAction extends Action { + errorData?: ProblemDetailsError; +} + const initialState: ShortUrlTags = { shortCode: null, tags: [], @@ -30,9 +36,9 @@ const initialState: ShortUrlTags = { error: false, }; -export default buildReducer({ +export default buildReducer({ [EDIT_SHORT_URL_TAGS_START]: (state) => ({ ...state, saving: true, error: false }), - [EDIT_SHORT_URL_TAGS_ERROR]: (state) => ({ ...state, saving: false, error: true }), + [EDIT_SHORT_URL_TAGS_ERROR]: (state, { errorData }) => ({ ...state, saving: false, error: true, errorData }), [SHORT_URL_TAGS_EDITED]: (_, { shortCode, tags }) => ({ shortCode, tags, saving: false, error: false }), [RESET_EDIT_SHORT_URL_TAGS]: () => initialState, }, initialState); @@ -50,7 +56,7 @@ export const editShortUrlTags = (buildShlinkApiClient: ShlinkApiClientBuilder) = dispatch({ tags: normalizedTags, shortCode, domain, type: SHORT_URL_TAGS_EDITED }); } catch (e) { - dispatch({ type: EDIT_SHORT_URL_TAGS_ERROR }); + dispatch({ type: EDIT_SHORT_URL_TAGS_ERROR, errorData: e.response?.data }); throw e; } From 4c3772d5c8aa65becd2153c111967366958d679d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Dec 2020 23:41:50 +0100 Subject: [PATCH 5/8] 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); From d534a4e441a67f4a0e5eb33b8512e865fe056e5a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Dec 2020 23:51:35 +0100 Subject: [PATCH 6/8] Moved logic to parse API errors to a helper function --- src/api/util.ts | 4 ++++ src/short-urls/reducers/shortUrlCreation.ts | 3 ++- src/short-urls/reducers/shortUrlDeletion.ts | 5 +++-- src/short-urls/reducers/shortUrlEdition.ts | 3 ++- src/short-urls/reducers/shortUrlMeta.ts | 3 ++- src/short-urls/reducers/shortUrlTags.ts | 3 ++- src/tags/reducers/tagDelete.ts | 3 ++- src/tags/reducers/tagEdit.ts | 3 ++- src/tags/reducers/tagsList.ts | 3 ++- src/visits/reducers/common.ts | 3 ++- 10 files changed, 23 insertions(+), 10 deletions(-) create mode 100644 src/api/util.ts diff --git a/src/api/util.ts b/src/api/util.ts new file mode 100644 index 00000000..91901f1a --- /dev/null +++ b/src/api/util.ts @@ -0,0 +1,4 @@ +import { AxiosError } from 'axios'; +import { ProblemDetailsError } from '../utils/services/types'; + +export const parseApiError = (e: AxiosError) => e.response?.data; diff --git a/src/short-urls/reducers/shortUrlCreation.ts b/src/short-urls/reducers/shortUrlCreation.ts index 0b617f45..e6f54cec 100644 --- a/src/short-urls/reducers/shortUrlCreation.ts +++ b/src/short-urls/reducers/shortUrlCreation.ts @@ -4,6 +4,7 @@ import { ShortUrl, ShortUrlData } from '../data'; import { buildReducer, buildActionCreator } from '../../utils/helpers/redux'; import { ShlinkApiClientBuilder } from '../../utils/services/ShlinkApiClientBuilder'; import { ProblemDetailsError } from '../../utils/services/types'; +import { parseApiError } from '../../api/util'; /* eslint-disable padding-line-between-statements */ export const CREATE_SHORT_URL_START = 'shlink/createShortUrl/CREATE_SHORT_URL_START'; @@ -52,7 +53,7 @@ export const createShortUrl = (buildShlinkApiClient: ShlinkApiClientBuilder) => dispatch({ type: CREATE_SHORT_URL, result }); } catch (e) { - dispatch({ type: CREATE_SHORT_URL_ERROR, errorData: e.response?.data }); + dispatch({ type: CREATE_SHORT_URL_ERROR, errorData: parseApiError(e) }); throw e; } diff --git a/src/short-urls/reducers/shortUrlDeletion.ts b/src/short-urls/reducers/shortUrlDeletion.ts index a21eb5e4..b4bf8966 100644 --- a/src/short-urls/reducers/shortUrlDeletion.ts +++ b/src/short-urls/reducers/shortUrlDeletion.ts @@ -3,6 +3,7 @@ import { buildActionCreator, buildReducer } from '../../utils/helpers/redux'; import { ProblemDetailsError } from '../../utils/services/types'; import { GetState } from '../../container/types'; import { ShlinkApiClientBuilder } from '../../utils/services/ShlinkApiClientBuilder'; +import { parseApiError } from '../../api/util'; /* eslint-disable padding-line-between-statements */ export const DELETE_SHORT_URL_START = 'shlink/deleteShortUrl/DELETE_SHORT_URL_START'; @@ -24,7 +25,7 @@ export interface DeleteShortUrlAction extends Action { } interface DeleteShortUrlErrorAction extends Action { - errorData: ProblemDetailsError; + errorData?: ProblemDetailsError; } const initialState: ShortUrlDeletion = { @@ -51,7 +52,7 @@ export const deleteShortUrl = (buildShlinkApiClient: ShlinkApiClientBuilder) => await deleteShortUrl(shortCode, domain); dispatch({ type: SHORT_URL_DELETED, shortCode, domain }); } catch (e) { - dispatch({ type: DELETE_SHORT_URL_ERROR, errorData: e.response.data }); + dispatch({ type: DELETE_SHORT_URL_ERROR, errorData: parseApiError(e) }); throw e; } diff --git a/src/short-urls/reducers/shortUrlEdition.ts b/src/short-urls/reducers/shortUrlEdition.ts index c326c1ab..126b1287 100644 --- a/src/short-urls/reducers/shortUrlEdition.ts +++ b/src/short-urls/reducers/shortUrlEdition.ts @@ -5,6 +5,7 @@ import { OptionalString } from '../../utils/utils'; import { ShortUrlIdentifier } from '../data'; import { ShlinkApiClientBuilder } from '../../utils/services/ShlinkApiClientBuilder'; import { ProblemDetailsError } from '../../utils/services/types'; +import { parseApiError } from '../../api/util'; /* eslint-disable padding-line-between-statements */ export const EDIT_SHORT_URL_START = 'shlink/shortUrlEdition/EDIT_SHORT_URL_START'; @@ -53,7 +54,7 @@ export const editShortUrl = (buildShlinkApiClient: ShlinkApiClientBuilder) => ( await updateShortUrlMeta(shortCode, domain, { longUrl }); dispatch({ shortCode, longUrl, domain, type: SHORT_URL_EDITED }); } catch (e) { - dispatch({ type: EDIT_SHORT_URL_ERROR, errorData: e.response?.data }); + dispatch({ type: EDIT_SHORT_URL_ERROR, errorData: parseApiError(e) }); throw e; } diff --git a/src/short-urls/reducers/shortUrlMeta.ts b/src/short-urls/reducers/shortUrlMeta.ts index c867bf0d..dc55dbaa 100644 --- a/src/short-urls/reducers/shortUrlMeta.ts +++ b/src/short-urls/reducers/shortUrlMeta.ts @@ -5,6 +5,7 @@ import { buildActionCreator, buildReducer } from '../../utils/helpers/redux'; import { OptionalString } from '../../utils/utils'; import { ShlinkApiClientBuilder } from '../../utils/services/ShlinkApiClientBuilder'; import { ProblemDetailsError } from '../../utils/services/types'; +import { parseApiError } from '../../api/util'; /* eslint-disable padding-line-between-statements */ export const EDIT_SHORT_URL_META_START = 'shlink/shortUrlMeta/EDIT_SHORT_URL_META_START'; @@ -55,7 +56,7 @@ export const editShortUrlMeta = (buildShlinkApiClient: ShlinkApiClientBuilder) = await updateShortUrlMeta(shortCode, domain, meta); dispatch({ shortCode, meta, domain, type: SHORT_URL_META_EDITED }); } catch (e) { - dispatch({ type: EDIT_SHORT_URL_META_ERROR, errorData: e.response?.data }); + dispatch({ type: EDIT_SHORT_URL_META_ERROR, errorData: parseApiError(e) }); throw e; } diff --git a/src/short-urls/reducers/shortUrlTags.ts b/src/short-urls/reducers/shortUrlTags.ts index 0c38462c..6df90782 100644 --- a/src/short-urls/reducers/shortUrlTags.ts +++ b/src/short-urls/reducers/shortUrlTags.ts @@ -5,6 +5,7 @@ import { OptionalString } from '../../utils/utils'; import { ShortUrlIdentifier } from '../data'; import { ShlinkApiClientBuilder } from '../../utils/services/ShlinkApiClientBuilder'; import { ProblemDetailsError } from '../../utils/services/types'; +import { parseApiError } from '../../api/util'; /* eslint-disable padding-line-between-statements */ export const EDIT_SHORT_URL_TAGS_START = 'shlink/shortUrlTags/EDIT_SHORT_URL_TAGS_START'; @@ -56,7 +57,7 @@ export const editShortUrlTags = (buildShlinkApiClient: ShlinkApiClientBuilder) = dispatch({ tags: normalizedTags, shortCode, domain, type: SHORT_URL_TAGS_EDITED }); } catch (e) { - dispatch({ type: EDIT_SHORT_URL_TAGS_ERROR, errorData: e.response?.data }); + dispatch({ type: EDIT_SHORT_URL_TAGS_ERROR, errorData: parseApiError(e) }); throw e; } diff --git a/src/tags/reducers/tagDelete.ts b/src/tags/reducers/tagDelete.ts index c5f3b2e4..4fb54621 100644 --- a/src/tags/reducers/tagDelete.ts +++ b/src/tags/reducers/tagDelete.ts @@ -3,6 +3,7 @@ import { buildReducer } from '../../utils/helpers/redux'; import { GetState } from '../../container/types'; import { ShlinkApiClientBuilder } from '../../utils/services/ShlinkApiClientBuilder'; import { ProblemDetailsError } from '../../utils/services/types'; +import { parseApiError } from '../../api/util'; /* eslint-disable padding-line-between-statements */ export const DELETE_TAG_START = 'shlink/deleteTag/DELETE_TAG_START'; @@ -47,7 +48,7 @@ export const deleteTag = (buildShlinkApiClient: ShlinkApiClientBuilder) => (tag: await deleteTags([ tag ]); dispatch({ type: DELETE_TAG }); } catch (e) { - dispatch({ type: DELETE_TAG_ERROR, errorData: e.response?.data }); + dispatch({ type: DELETE_TAG_ERROR, errorData: parseApiError(e) }); throw e; } diff --git a/src/tags/reducers/tagEdit.ts b/src/tags/reducers/tagEdit.ts index 6e4f85c6..b6f1f964 100644 --- a/src/tags/reducers/tagEdit.ts +++ b/src/tags/reducers/tagEdit.ts @@ -5,6 +5,7 @@ import { GetState } from '../../container/types'; import ColorGenerator from '../../utils/services/ColorGenerator'; import { ShlinkApiClientBuilder } from '../../utils/services/ShlinkApiClientBuilder'; import { ProblemDetailsError } from '../../utils/services/types'; +import { parseApiError } from '../../api/util'; /* eslint-disable padding-line-between-statements */ export const EDIT_TAG_START = 'shlink/editTag/EDIT_TAG_START'; @@ -62,7 +63,7 @@ export const editTag = (buildShlinkApiClient: ShlinkApiClientBuilder, colorGener colorGenerator.setColorForKey(newName, color); dispatch({ type: EDIT_TAG, oldName, newName }); } catch (e) { - dispatch({ type: EDIT_TAG_ERROR, errorData: e.response?.data }); + dispatch({ type: EDIT_TAG_ERROR, errorData: parseApiError(e) }); throw e; } diff --git a/src/tags/reducers/tagsList.ts b/src/tags/reducers/tagsList.ts index e3e80d1d..826d333f 100644 --- a/src/tags/reducers/tagsList.ts +++ b/src/tags/reducers/tagsList.ts @@ -9,6 +9,7 @@ import { TagStats } from '../data'; import { CreateVisit, Stats } from '../../visits/types'; import { DeleteTagAction, TAG_DELETED } from './tagDelete'; import { EditTagAction, TAG_EDITED } from './tagEdit'; +import { parseApiError } from '../../api/util'; /* eslint-disable padding-line-between-statements */ export const LIST_TAGS_START = 'shlink/tagsList/LIST_TAGS_START'; @@ -130,7 +131,7 @@ export const listTags = (buildShlinkApiClient: ShlinkApiClientBuilder, force = t dispatch({ tags, stats: processedStats, type: LIST_TAGS }); } catch (e) { - dispatch({ type: LIST_TAGS_ERROR, errorData: e.response?.data }); + dispatch({ type: LIST_TAGS_ERROR, errorData: parseApiError(e) }); } }; diff --git a/src/visits/reducers/common.ts b/src/visits/reducers/common.ts index 4e1313f7..765203c3 100644 --- a/src/visits/reducers/common.ts +++ b/src/visits/reducers/common.ts @@ -2,6 +2,7 @@ import { flatten, prop, range, splitEvery } from 'ramda'; import { Action, Dispatch } from 'redux'; import { ShlinkPaginator, ShlinkVisits } from '../../utils/services/types'; import { Visit, VisitsLoadFailedAction } from '../types'; +import { parseApiError } from '../../api/util'; const ITEMS_PER_PAGE = 5000; const PARALLEL_REQUESTS_COUNT = 4; @@ -71,6 +72,6 @@ export const getVisitsWithLoader = async & { visits: V dispatch({ ...extraFinishActionData, visits, type: actionMap.finish }); } catch (e) { - dispatch({ type: actionMap.error, errorData: e.response?.data }); + dispatch({ type: actionMap.error, errorData: parseApiError(e) }); } }; From 89f6c6c2833b37f39f4b685332a7772c86fc2910 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Dec 2020 23:53:15 +0100 Subject: [PATCH 7/8] Updated changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0ff9857..c3ca7860 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Changed * [#267](https://github.com/shlinkio/shlink-web-client/issues/267) Added some subtle but important improvements on UI/UX. * [#352](https://github.com/shlinkio/shlink-web-client/issues/352) Moved from Scrutinizer to Codecov as the code coverage backend. -* [#217](https://github.com/shlinkio/shlink-web-client/issues/217) Improved how messages are displayed, by centralizing it in the `Message` and `Result` components.. +* [#217](https://github.com/shlinkio/shlink-web-client/issues/217) Improved how messages are displayed, by centralizing it in the `Message` and `Result` components. +* [#219](https://github.com/shlinkio/shlink-web-client/issues/219) Improved error messages when something fails while interacting with Shlink's API. ### Deprecated * *Nothing* From 6a354c277c81b02b2a9db9334af051948cfeb597 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Dec 2020 23:55:54 +0100 Subject: [PATCH 8/8] Set API response as per Shlink v2 --- src/utils/services/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/services/types.ts b/src/utils/services/types.ts index 9f5f9225..36f2fe82 100644 --- a/src/utils/services/types.ts +++ b/src/utils/services/types.ts @@ -86,7 +86,7 @@ interface InvalidArgumentError extends ProblemDetailsError { interface InvalidShortUrlDeletion extends ProblemDetailsError { type: 'INVALID_SHORTCODE_DELETION'; - threshold?: number; + threshold: number; } export const isInvalidArgumentError = (error?: ProblemDetailsError): error is InvalidArgumentError =>