diff --git a/src/api/services/ShlinkApiClient.ts b/src/api/services/ShlinkApiClient.ts index 66d36332..8617a30a 100644 --- a/src/api/services/ShlinkApiClient.ts +++ b/src/api/services/ShlinkApiClient.ts @@ -24,11 +24,9 @@ import { HttpClient } from '../../common/services/HttpClient'; const buildShlinkBaseUrl = (url: string, version: 2 | 3) => `${url}/rest/v${version}`; const rejectNilProps = reject(isNil); -const normalizeOrderByInParams = (params: ShlinkShortUrlsListParams): ShlinkShortUrlsListNormalizedParams => { - const { orderBy = {}, ...rest } = params; - - return { ...rest, orderBy: orderToString(orderBy) }; -}; +const normalizeOrderByInParams = ( + { orderBy = {}, ...rest }: ShlinkShortUrlsListParams, +): ShlinkShortUrlsListNormalizedParams => ({ ...rest, orderBy: orderToString(orderBy) }); export class ShlinkApiClient { private apiVersion: 2 | 3; @@ -47,7 +45,6 @@ export class ShlinkApiClient { public readonly createShortUrl = async (options: ShortUrlData): Promise => { const filteredOptions = reject((value) => isEmpty(value) || isNil(value), options as any); - return this.performRequest('/short-urls', 'POST', {}, filteredOptions); }; @@ -56,31 +53,25 @@ export class ShlinkApiClient { .then(({ visits }) => visits); public readonly getTagVisits = async (tag: string, query?: Omit): Promise => - this.performRequest<{ visits: ShlinkVisits }>(`/tags/${tag}/visits`, 'GET', query) - .then(({ visits }) => visits); + this.performRequest<{ visits: ShlinkVisits }>(`/tags/${tag}/visits`, 'GET', query).then(({ visits }) => visits); public readonly getDomainVisits = async (domain: string, query?: Omit): Promise => - this.performRequest<{ visits: ShlinkVisits }>(`/domains/${domain}/visits`, 'GET', query) - .then(({ visits }) => visits); + this.performRequest<{ visits: ShlinkVisits }>(`/domains/${domain}/visits`, 'GET', query).then(({ visits }) => visits); public readonly getOrphanVisits = async (query?: Omit): Promise => - this.performRequest<{ visits: ShlinkVisits }>('/visits/orphan', 'GET', query) - .then(({ visits }) => visits); + this.performRequest<{ visits: ShlinkVisits }>('/visits/orphan', 'GET', query).then(({ visits }) => visits); public readonly getNonOrphanVisits = async (query?: Omit): Promise => - this.performRequest<{ visits: ShlinkVisits }>('/visits/non-orphan', 'GET', query) - .then(({ visits }) => visits); + this.performRequest<{ visits: ShlinkVisits }>('/visits/non-orphan', 'GET', query).then(({ visits }) => visits); public readonly getVisitsOverview = async (): Promise => - this.performRequest<{ visits: ShlinkVisitsOverview }>('/visits') - .then(({ visits }) => visits); + this.performRequest<{ visits: ShlinkVisitsOverview }>('/visits').then(({ visits }) => visits); public readonly getShortUrl = async (shortCode: string, domain?: OptionalString): Promise => this.performRequest(`/short-urls/${shortCode}`, 'GET', { domain }); public readonly deleteShortUrl = async (shortCode: string, domain?: OptionalString): Promise => - this.performRequest(`/short-urls/${shortCode}`, 'DELETE', { domain }) - .then(() => {}); + this.performEmptyRequest(`/short-urls/${shortCode}`, 'DELETE', { domain }); public readonly updateShortUrl = async ( shortCode: string, @@ -95,12 +86,10 @@ export class ShlinkApiClient { .then(({ data, stats }) => ({ tags: data, stats })); public readonly deleteTags = async (tags: string[]): Promise<{ tags: string[] }> => - this.performRequest('/tags', 'DELETE', { tags }) - .then(() => ({ tags })); + this.performEmptyRequest('/tags', 'DELETE', { tags }).then(() => ({ tags })); public readonly editTag = async (oldName: string, newName: string): Promise<{ oldName: string; newName: string }> => - this.performRequest('/tags', 'PUT', {}, { oldName, newName }) - .then(() => ({ oldName, newName })); + this.performEmptyRequest('/tags', 'PUT', {}, { oldName, newName }).then(() => ({ oldName, newName })); public readonly health = async (): Promise => this.performRequest('/health', 'GET'); @@ -115,26 +104,35 @@ export class ShlinkApiClient { ): Promise => this.performRequest('/domains/redirects', 'PATCH', {}, domainRedirects); - private readonly performRequest = async (url: string, method = 'GET', query = {}, body?: object): Promise => { + private readonly performRequest = async (url: string, method = 'GET', query = {}, body?: object): Promise => + this.httpClient.fetchJson(...this.toFetchParams(url, method, query, body)).catch( + this.handleFetchError(() => this.httpClient.fetchJson(...this.toFetchParams(url, method, query, body))), + ); + + private readonly performEmptyRequest = async (url: string, method = 'GET', query = {}, body?: object): Promise => + this.httpClient.fetchEmpty(...this.toFetchParams(url, method, query, body)).catch( + this.handleFetchError(() => this.httpClient.fetchEmpty(...this.toFetchParams(url, method, query, body))), + ); + + private readonly toFetchParams = (url: string, method: string, query = {}, body?: object): [string, RequestInit] => { const normalizedQuery = stringifyQuery(rejectNilProps(query)); const stringifiedQuery = isEmpty(normalizedQuery) ? '' : `?${normalizedQuery}`; - return this.httpClient.fetchJson( - `${buildShlinkBaseUrl(this.baseUrl, this.apiVersion)}${url}${stringifiedQuery}`, - { - method, - body: body && JSON.stringify(body), - headers: { 'X-Api-Key': this.apiKey }, - }, - ).catch((e: unknown) => { - if (!isRegularNotFound(parseApiError(e))) { - throw e; - } + return [`${buildShlinkBaseUrl(this.baseUrl, this.apiVersion)}${url}${stringifiedQuery}`, { + method, + body: body && JSON.stringify(body), + headers: { 'X-Api-Key': this.apiKey }, + }]; + }; - // If we capture a not found error, let's assume this Shlink version does not support API v3, so we decrease to - // v2 and retry - this.apiVersion = 2; - return this.performRequest(url, method, query, body); - }); + private readonly handleFetchError = (retryFetch: Function) => (e: unknown) => { + if (!isRegularNotFound(parseApiError(e))) { + throw e; + } + + // If we capture a not found error, let's assume this Shlink version does not support API v3, so we decrease to + // v2 and retry + this.apiVersion = 2; + return retryFetch(); }; } diff --git a/src/common/services/HttpClient.ts b/src/common/services/HttpClient.ts index bd9b47f9..af8aedf3 100644 --- a/src/common/services/HttpClient.ts +++ b/src/common/services/HttpClient.ts @@ -3,19 +3,23 @@ import { Fetch } from '../../utils/types'; export class HttpClient { constructor(private readonly fetch: Fetch) {} - public fetchJson(url: string, options?: RequestInit): Promise { - return this.fetch(url, options).then(async (resp) => { - const parsed = await resp.json(); + public readonly fetchJson = (url: string, options?: RequestInit): Promise => + this.fetch(url, options).then(async (resp) => { + const json = await resp.json(); if (!resp.ok) { - throw parsed; + throw json; } - return parsed as T; + return json as T; }); - } - public fetchBlob(url: string): Promise { - return this.fetch(url).then((resp) => resp.blob()); - } + public readonly fetchEmpty = (url: string, options?: RequestInit): Promise => + this.fetch(url, options).then(async (resp) => { + if (!resp.ok) { + throw await resp.json(); + } + }); + + public readonly fetchBlob = (url: string): Promise => this.fetch(url).then((resp) => resp.blob()); } diff --git a/src/index.scss b/src/index.scss index ee9a8abb..3c8e2ba6 100644 --- a/src/index.scss +++ b/src/index.scss @@ -39,6 +39,10 @@ a:not(.nav-link):not(.navbar-brand):not(.page-link):not(.highlight-card):not(.bt background-color: $mainColor !important; } +.bg-warning { + color: $lightTextColor; +} + .card-body, .card-header, .list-group-item { diff --git a/src/tags/helpers/EditTagModal.tsx b/src/tags/helpers/EditTagModal.tsx index 59297b6c..06dbefe5 100644 --- a/src/tags/helpers/EditTagModal.tsx +++ b/src/tags/helpers/EditTagModal.tsx @@ -27,9 +27,10 @@ export const EditTagModal = ({ getColorForKey }: ColorGenerator) => ( const [showColorPicker, toggleColorPicker, , hideColorPicker] = useToggle(); const { editing, error, edited, errorData } = tagEdit; const saveTag = handleEventPreventingDefault( - async () => editTag({ oldName: tag, newName: newTagName, color }) - .then(toggle) - .catch(() => {}), + async () => { + await editTag({ oldName: tag, newName: newTagName, color }); + toggle(); + }, ); const onClosed = pipe(hideColorPicker, () => edited && tagEdited({ oldName: tag, newName: newTagName, color })); diff --git a/src/tags/reducers/tagEdit.ts b/src/tags/reducers/tagEdit.ts index 7c32b3ee..675f8898 100644 --- a/src/tags/reducers/tagEdit.ts +++ b/src/tags/reducers/tagEdit.ts @@ -33,35 +33,34 @@ const initialState: TagEdition = { export const tagEdited = createAction(`${REDUCER_PREFIX}/tagEdited`); -export const tagEditReducerCreator = (buildShlinkApiClient: ShlinkApiClientBuilder, colorGenerator: ColorGenerator) => { - const editTag = createAsyncThunk( - `${REDUCER_PREFIX}/editTag`, - async ({ oldName, newName, color }: EditTag, { getState }): Promise => { - await buildShlinkApiClient(getState).editTag(oldName, newName); - colorGenerator.setColorForKey(newName, color); +export const editTag = ( + buildShlinkApiClient: ShlinkApiClientBuilder, + colorGenerator: ColorGenerator, +) => createAsyncThunk( + `${REDUCER_PREFIX}/editTag`, + async ({ oldName, newName, color }: EditTag, { getState }): Promise => { + await buildShlinkApiClient(getState).editTag(oldName, newName); + colorGenerator.setColorForKey(newName, color); - return { oldName, newName, color }; - }, - ); + return { oldName, newName, color }; + }, +); - const { reducer } = createSlice({ - name: REDUCER_PREFIX, - initialState, - reducers: {}, - extraReducers: (builder) => { - builder.addCase(editTag.pending, () => ({ editing: true, edited: false, error: false })); - builder.addCase( - editTag.rejected, - (_, { error }) => ({ editing: false, edited: false, error: true, errorData: parseApiError(error) }), - ); - builder.addCase(editTag.fulfilled, (_, { payload }) => ({ - ...pick(['oldName', 'newName'], payload), - editing: false, - edited: true, - error: false, - })); - }, - }); - - return { reducer, editTag }; -}; +export const tagEditReducerCreator = (editTagThunk: ReturnType) => createSlice({ + name: REDUCER_PREFIX, + initialState, + reducers: {}, + extraReducers: (builder) => { + builder.addCase(editTagThunk.pending, () => ({ editing: true, edited: false, error: false })); + builder.addCase( + editTagThunk.rejected, + (_, { error }) => ({ editing: false, edited: false, error: true, errorData: parseApiError(error) }), + ); + builder.addCase(editTagThunk.fulfilled, (_, { payload }) => ({ + ...pick(['oldName', 'newName'], payload), + editing: false, + edited: true, + error: false, + })); + }, +}); diff --git a/src/tags/reducers/tagsList.ts b/src/tags/reducers/tagsList.ts index da15e123..d348674c 100644 --- a/src/tags/reducers/tagsList.ts +++ b/src/tags/reducers/tagsList.ts @@ -111,15 +111,19 @@ export const tagsListReducerCreator = ( { ...initialState, stats: payload.stats, tags: payload.tags, filteredTags: payload.tags } )); - builder.addCase(tagDeleted, (state, { payload: tag }) => ({ - ...state, - tags: rejectTag(state.tags, tag), - filteredTags: rejectTag(state.filteredTags, tag), + builder.addCase(tagDeleted, ({ tags, filteredTags, ...rest }, { payload: tag }) => ({ + ...rest, + tags: rejectTag(tags, tag), + filteredTags: rejectTag(filteredTags, tag), })); - builder.addCase(tagEdited, (state, { payload }) => ({ - ...state, - tags: state.tags.map(renameTag(payload.oldName, payload.newName)).sort(), - filteredTags: state.filteredTags.map(renameTag(payload.oldName, payload.newName)).sort(), + builder.addCase(tagEdited, ({ tags, filteredTags, stats, ...rest }, { payload }) => ({ + ...rest, + stats: { + ...stats, + [payload.newName]: stats[payload.oldName], + }, + tags: tags.map(renameTag(payload.oldName, payload.newName)).sort(), + filteredTags: filteredTags.map(renameTag(payload.oldName, payload.newName)).sort(), })); builder.addCase(createNewVisits, (state, { payload }) => ({ ...state, diff --git a/src/tags/services/provideServices.ts b/src/tags/services/provideServices.ts index 92980f72..2d81afb7 100644 --- a/src/tags/services/provideServices.ts +++ b/src/tags/services/provideServices.ts @@ -7,7 +7,7 @@ import { EditTagModal } from '../helpers/EditTagModal'; import { TagsList } from '../TagsList'; import { filterTags, listTags, tagsListReducerCreator } from '../reducers/tagsList'; import { tagDeleted, tagDeleteReducerCreator } from '../reducers/tagDelete'; -import { tagEdited, tagEditReducerCreator } from '../reducers/tagEdit'; +import { editTag, tagEdited, tagEditReducerCreator } from '../reducers/tagEdit'; import { ConnectDecorator } from '../../container/types'; import { TagsCards } from '../TagsCards'; import { TagsTable } from '../TagsTable'; @@ -38,7 +38,7 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { )); // Reducers - bottle.serviceFactory('tagEditReducerCreator', tagEditReducerCreator, 'buildShlinkApiClient', 'ColorGenerator'); + bottle.serviceFactory('tagEditReducerCreator', tagEditReducerCreator, 'editTag'); bottle.serviceFactory('tagEditReducer', prop('reducer'), 'tagEditReducerCreator'); bottle.serviceFactory('tagDeleteReducerCreator', tagDeleteReducerCreator, 'buildShlinkApiClient'); @@ -58,7 +58,7 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { bottle.serviceFactory('deleteTag', prop('deleteTag'), 'tagDeleteReducerCreator'); bottle.serviceFactory('tagDeleted', () => tagDeleted); - bottle.serviceFactory('editTag', prop('editTag'), 'tagEditReducerCreator'); + bottle.serviceFactory('editTag', editTag, 'buildShlinkApiClient', 'ColorGenerator'); bottle.serviceFactory('tagEdited', () => tagEdited); }; diff --git a/test/api/services/ShlinkApiClient.test.ts b/test/api/services/ShlinkApiClient.test.ts index 17c6ac49..0b15299e 100644 --- a/test/api/services/ShlinkApiClient.test.ts +++ b/test/api/services/ShlinkApiClient.test.ts @@ -8,7 +8,8 @@ import { HttpClient } from '../../../src/common/services/HttpClient'; describe('ShlinkApiClient', () => { const fetchJson = jest.fn().mockResolvedValue({}); - const httpClient = Mock.of({ fetchJson }); + const fetchEmpty = jest.fn().mockResolvedValue(undefined); + const httpClient = Mock.of({ fetchJson, fetchEmpty }); const buildApiClient = () => new ShlinkApiClient(httpClient, '', ''); const shortCodesWithDomainCombinations: [string, OptionalString][] = [ ['abc123', null], @@ -196,7 +197,7 @@ describe('ShlinkApiClient', () => { await deleteTags(tags); - expect(fetchJson).toHaveBeenCalledWith( + expect(fetchEmpty).toHaveBeenCalledWith( expect.stringContaining(`/tags?${tags.map((tag) => `tags%5B%5D=${tag}`).join('&')}`), expect.objectContaining({ method: 'DELETE' }), ); @@ -211,7 +212,7 @@ describe('ShlinkApiClient', () => { await editTag(oldName, newName); - expect(fetchJson).toHaveBeenCalledWith(expect.stringContaining('/tags'), expect.objectContaining({ + expect(fetchEmpty).toHaveBeenCalledWith(expect.stringContaining('/tags'), expect.objectContaining({ method: 'PUT', body: JSON.stringify({ oldName, newName }), })); @@ -225,7 +226,7 @@ describe('ShlinkApiClient', () => { await deleteShortUrl(shortCode, domain); - expect(fetchJson).toHaveBeenCalledWith( + expect(fetchEmpty).toHaveBeenCalledWith( expect.stringContaining(`/short-urls/${shortCode}${expectedQuery}`), expect.objectContaining({ method: 'DELETE' }), ); diff --git a/test/common/services/HttpClient.test.ts b/test/common/services/HttpClient.test.ts index 63a2307d..b894cd32 100644 --- a/test/common/services/HttpClient.test.ts +++ b/test/common/services/HttpClient.test.ts @@ -7,14 +7,14 @@ describe('HttpClient', () => { beforeEach(jest.clearAllMocks); describe('fetchJson', () => { - it('throws json when response is not ok', async () => { + it('throws json on success', async () => { const theError = { error: true, foo: 'bar' }; fetch.mockResolvedValue({ json: () => theError, ok: false }); await expect(httpClient.fetchJson('')).rejects.toEqual(theError); }); - it('return json when response is ok', async () => { + it('return json on failure', async () => { const theJson = { foo: 'bar' }; fetch.mockResolvedValue({ json: () => theJson, ok: true }); @@ -24,6 +24,23 @@ describe('HttpClient', () => { }); }); + describe('fetchEmpty', () => { + it('returns empty on success', async () => { + fetch.mockResolvedValue({ ok: true }); + + const result = await httpClient.fetchEmpty(''); + + expect(result).not.toBeDefined(); + }); + + it('throws error on failure', async () => { + const theError = { error: true, foo: 'bar' }; + fetch.mockResolvedValue({ json: () => theError, ok: false }); + + await expect(httpClient.fetchJson('')).rejects.toEqual(theError); + }); + }); + describe('fetchBlob', () => { it('returns response as blob', async () => { const theBlob = new Blob(); diff --git a/test/tags/reducers/tagEdit.test.ts b/test/tags/reducers/tagEdit.test.ts index 7f496dd1..b421c579 100644 --- a/test/tags/reducers/tagEdit.test.ts +++ b/test/tags/reducers/tagEdit.test.ts @@ -1,5 +1,5 @@ import { Mock } from 'ts-mockery'; -import { tagEdited, EditTagAction, tagEditReducerCreator } from '../../../src/tags/reducers/tagEdit'; +import { tagEdited, editTag as editTagCreator, EditTagAction, tagEditReducerCreator } from '../../../src/tags/reducers/tagEdit'; import { ShlinkApiClient } from '../../../src/api/services/ShlinkApiClient'; import { ColorGenerator } from '../../../src/utils/services/ColorGenerator'; import { ShlinkState } from '../../../src/container/types'; @@ -11,7 +11,8 @@ describe('tagEditReducer', () => { const editTagCall = jest.fn(); const buildShlinkApiClient = () => Mock.of({ editTag: editTagCall }); const colorGenerator = Mock.of({ setColorForKey: jest.fn() }); - const { reducer, editTag } = tagEditReducerCreator(buildShlinkApiClient, colorGenerator); + const editTag = editTagCreator(buildShlinkApiClient, colorGenerator); + const { reducer } = tagEditReducerCreator(editTag); describe('reducer', () => { it('returns loading on EDIT_TAG_START', () => { diff --git a/test/tags/reducers/tagsList.test.ts b/test/tags/reducers/tagsList.test.ts index a3f1cbb2..2bea89bf 100644 --- a/test/tags/reducers/tagsList.test.ts +++ b/test/tags/reducers/tagsList.test.ts @@ -70,14 +70,30 @@ describe('tagsListReducer', () => { const expectedTags = ['foo', 'renamed', 'baz'].sort(); expect(reducer( - state({ tags, filteredTags: tags }), - { - type: tagEdited.toString(), - payload: { oldName, newName }, - }, + state({ + tags, + filteredTags: tags, + stats: { + [oldName]: { + shortUrlsCount: 35, + visitsCount: 35, + }, + }, + }), + { type: tagEdited.toString(), payload: { oldName, newName } }, )).toEqual({ tags: expectedTags, filteredTags: expectedTags, + stats: { + [oldName]: { + shortUrlsCount: 35, + visitsCount: 35, + }, + [newName]: { + shortUrlsCount: 35, + visitsCount: 35, + }, + }, }); });