From b87b108e530d26682a5a50cecae5c915d58114aa Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 18 Mar 2023 16:26:28 +0100 Subject: [PATCH 1/3] Use /tags/stats endpoint when the server supports it --- src/api/services/ShlinkApiClient.ts | 13 +++++++++---- src/api/types/index.ts | 5 +++++ src/tags/reducers/tagsList.ts | 5 +++-- src/utils/helpers/features.ts | 3 ++- test/api/services/ShlinkApiClient.test.ts | 22 +++++++++++++++++++++- 5 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/api/services/ShlinkApiClient.ts b/src/api/services/ShlinkApiClient.ts index 56f1f34b..99161c47 100644 --- a/src/api/services/ShlinkApiClient.ts +++ b/src/api/services/ShlinkApiClient.ts @@ -16,6 +16,7 @@ import type { ShlinkShortUrlsResponse, ShlinkTags, ShlinkTagsResponse, + ShlinkTagsStatsResponse, ShlinkVisits, ShlinkVisitsOverview, ShlinkVisitsParams, @@ -85,10 +86,14 @@ export class ShlinkApiClient { ): Promise => this.performRequest(`/short-urls/${shortCode}`, 'PATCH', { domain }, edit); - public readonly listTags = async (): Promise => - this.performRequest<{ tags: ShlinkTagsResponse }>('/tags', 'GET', { withStats: 'true' }) - .then(({ tags }) => tags) - .then(({ data, stats }) => ({ tags: data, stats })); + public readonly listTags = async (useTagsStatsEndpoint: boolean): Promise => + (useTagsStatsEndpoint + ? this.performRequest<{ tags: ShlinkTagsStatsResponse }>('/tags/stats', 'GET') + .then(({ tags }) => tags) + .then(({ data }) => ({ tags: data.map(({ tag }) => tag), stats: data })) + : this.performRequest<{ tags: ShlinkTagsResponse }>('/tags', 'GET', { withStats: 'true' }) + .then(({ tags }) => tags) + .then(({ data, stats }) => ({ tags: data, stats }))); public readonly deleteTags = async (tags: string[]): Promise<{ tags: string[] }> => this.performEmptyRequest('/tags', 'DELETE', { tags }).then(() => ({ tags })); diff --git a/src/api/types/index.ts b/src/api/types/index.ts index a06515f4..59c4e05b 100644 --- a/src/api/types/index.ts +++ b/src/api/types/index.ts @@ -31,9 +31,14 @@ export interface ShlinkTags { export interface ShlinkTagsResponse { data: string[]; + /** @deprecated Present only when withStats=true is provided, which is deprecated */ stats: ShlinkTagsStats[]; } +export interface ShlinkTagsStatsResponse { + data: ShlinkTagsStats[]; +} + export interface ShlinkPaginator { currentPage: number; pagesCount: number; diff --git a/src/tags/reducers/tagsList.ts b/src/tags/reducers/tagsList.ts index d6726817..b29a8264 100644 --- a/src/tags/reducers/tagsList.ts +++ b/src/tags/reducers/tagsList.ts @@ -5,6 +5,7 @@ import type { ShlinkTags } from '../../api/types'; import type { ProblemDetailsError } from '../../api/types/errors'; import { parseApiError } from '../../api/utils'; import type { createShortUrl } from '../../short-urls/reducers/shortUrlCreation'; +import { supportedFeatures } from '../../utils/helpers/features'; import { createAsyncThunk } from '../../utils/helpers/redux'; import { createNewVisits } from '../../visits/reducers/visitCreation'; import type { CreateVisit, Stats } from '../../visits/types'; @@ -70,14 +71,14 @@ const calculateVisitsPerTag = (createdVisits: CreateVisit[]): TagIncrease[] => O export const listTags = (buildShlinkApiClient: ShlinkApiClientBuilder, force = true) => createAsyncThunk( `${REDUCER_PREFIX}/listTags`, async (_: void, { getState }): Promise => { - const { tagsList } = getState(); + const { tagsList, selectedServer } = getState(); if (!force && !isEmpty(tagsList.tags)) { return tagsList; } const { listTags: shlinkListTags } = buildShlinkApiClient(getState); - const { tags, stats = [] }: ShlinkTags = await shlinkListTags(); + const { tags, stats = [] }: ShlinkTags = await shlinkListTags(supportedFeatures.tagsStats(selectedServer)); const processedStats = stats.reduce((acc, { tag, shortUrlsCount, visitsCount }) => { acc[tag] = { shortUrlsCount, visitsCount }; diff --git a/src/utils/helpers/features.ts b/src/utils/helpers/features.ts index 388c85e0..ca1010ec 100644 --- a/src/utils/helpers/features.ts +++ b/src/utils/helpers/features.ts @@ -14,11 +14,12 @@ export const supportedFeatures = { defaultDomainRedirectsEdition: matchesMinVersion('2.10.0'), nonOrphanVisits: matchesMinVersion('3.0.0'), allTagsFiltering: matchesMinVersion('3.0.0'), + tagsStats: matchesMinVersion('3.0.0'), domainVisits: matchesMinVersion('3.1.0'), excludeBotsOnShortUrls: matchesMinVersion('3.4.0'), filterDisabledUrls: matchesMinVersion('3.4.0'), deviceLongUrls: matchesMinVersion('3.5.0'), -} as const; +} as const satisfies Record>; Object.freeze(supportedFeatures); diff --git a/test/api/services/ShlinkApiClient.test.ts b/test/api/services/ShlinkApiClient.test.ts index 4215e088..5599fa3c 100644 --- a/test/api/services/ShlinkApiClient.test.ts +++ b/test/api/services/ShlinkApiClient.test.ts @@ -202,7 +202,7 @@ describe('ShlinkApiClient', () => { }); const { listTags } = buildApiClient(); - const result = await listTags(); + const result = await listTags(false); expect({ tags: expectedTags }).toEqual(result); expect(fetchJson).toHaveBeenCalledWith( @@ -210,6 +210,26 @@ describe('ShlinkApiClient', () => { expect.objectContaining({ method: 'GET' }), ); }); + + it('can use /tags/stats endpoint', async () => { + const expectedTags = ['foo', 'bar']; + const expectedStats = expectedTags.map((tag) => ({ tag, shortUrlsCount: 10, visitsCount: 10 })); + + fetchJson.mockResolvedValue({ + tags: { + data: expectedStats, + }, + }); + const { listTags } = buildApiClient(); + + const result = await listTags(true); + + expect({ tags: expectedTags, stats: expectedStats }).toEqual(result); + expect(fetchJson).toHaveBeenCalledWith( + expect.stringContaining('/tags/stats'), + expect.objectContaining({ method: 'GET' }), + ); + }); }); describe('deleteTags', () => { From a9af5163c14876fefddfd2cf177bf82d2395b190 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 18 Mar 2023 16:27:52 +0100 Subject: [PATCH 2/3] Update changelog --- CHANGELOG.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2844e1a..4e47fbd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * [#808](https://github.com/shlinkio/shlink-web-client/issues/808) Respect settings on excluding bots in the overview section, for visits cards. ### Changed -* Update to Vite 4.1 -* Update to coding standard v2.1.0 * [#798](https://github.com/shlinkio/shlink-web-client/issues/798) Remove stryker and mutation testing. +* [#800](https://github.com/shlinkio/shlink-web-client/issues/800) Use `/tags/stats` endpoint to load tags stats, when the server supports it. +* Update to Vite 4.2 +* Update to TypeScript 5 +* Update to coding standard v2.1.0 +* Decouple tests from RTK internals. ### Deprecated * *Nothing* From 96c20b36a518a73f8acf43866fbd807ff206cfb2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 18 Mar 2023 16:32:04 +0100 Subject: [PATCH 3/3] Split tagsList and tagsStats methods in ShlinkApiClient for clarity --- src/api/services/ShlinkApiClient.ts | 17 +++++++++-------- src/tags/reducers/tagsList.ts | 6 ++++-- test/api/services/ShlinkApiClient.test.ts | 8 +++++--- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/api/services/ShlinkApiClient.ts b/src/api/services/ShlinkApiClient.ts index 99161c47..97b6774a 100644 --- a/src/api/services/ShlinkApiClient.ts +++ b/src/api/services/ShlinkApiClient.ts @@ -86,14 +86,15 @@ export class ShlinkApiClient { ): Promise => this.performRequest(`/short-urls/${shortCode}`, 'PATCH', { domain }, edit); - public readonly listTags = async (useTagsStatsEndpoint: boolean): Promise => - (useTagsStatsEndpoint - ? this.performRequest<{ tags: ShlinkTagsStatsResponse }>('/tags/stats', 'GET') - .then(({ tags }) => tags) - .then(({ data }) => ({ tags: data.map(({ tag }) => tag), stats: data })) - : this.performRequest<{ tags: ShlinkTagsResponse }>('/tags', 'GET', { withStats: 'true' }) - .then(({ tags }) => tags) - .then(({ data, stats }) => ({ tags: data, stats }))); + public readonly listTags = async (): Promise => + this.performRequest<{ tags: ShlinkTagsResponse }>('/tags', 'GET', { withStats: 'true' }) + .then(({ tags }) => tags) + .then(({ data, stats }) => ({ tags: data, stats })); + + public readonly tagsStats = async (): Promise => + this.performRequest<{ tags: ShlinkTagsStatsResponse }>('/tags/stats', 'GET') + .then(({ tags }) => tags) + .then(({ data }) => ({ tags: data.map(({ tag }) => tag), stats: data })); public readonly deleteTags = async (tags: string[]): Promise<{ tags: string[] }> => this.performEmptyRequest('/tags', 'DELETE', { tags }).then(() => ({ tags })); diff --git a/src/tags/reducers/tagsList.ts b/src/tags/reducers/tagsList.ts index b29a8264..e96c8a23 100644 --- a/src/tags/reducers/tagsList.ts +++ b/src/tags/reducers/tagsList.ts @@ -77,8 +77,10 @@ export const listTags = (buildShlinkApiClient: ShlinkApiClientBuilder, force = t return tagsList; } - const { listTags: shlinkListTags } = buildShlinkApiClient(getState); - const { tags, stats = [] }: ShlinkTags = await shlinkListTags(supportedFeatures.tagsStats(selectedServer)); + const { listTags: shlinkListTags, tagsStats } = buildShlinkApiClient(getState); + const { tags, stats = [] }: ShlinkTags = await ( + supportedFeatures.tagsStats(selectedServer) ? tagsStats() : shlinkListTags() + ); const processedStats = stats.reduce((acc, { tag, shortUrlsCount, visitsCount }) => { acc[tag] = { shortUrlsCount, visitsCount }; diff --git a/test/api/services/ShlinkApiClient.test.ts b/test/api/services/ShlinkApiClient.test.ts index 5599fa3c..408f6903 100644 --- a/test/api/services/ShlinkApiClient.test.ts +++ b/test/api/services/ShlinkApiClient.test.ts @@ -202,7 +202,7 @@ describe('ShlinkApiClient', () => { }); const { listTags } = buildApiClient(); - const result = await listTags(false); + const result = await listTags(); expect({ tags: expectedTags }).toEqual(result); expect(fetchJson).toHaveBeenCalledWith( @@ -210,7 +210,9 @@ describe('ShlinkApiClient', () => { expect.objectContaining({ method: 'GET' }), ); }); + }); + describe('tagsStats', () => { it('can use /tags/stats endpoint', async () => { const expectedTags = ['foo', 'bar']; const expectedStats = expectedTags.map((tag) => ({ tag, shortUrlsCount: 10, visitsCount: 10 })); @@ -220,9 +222,9 @@ describe('ShlinkApiClient', () => { data: expectedStats, }, }); - const { listTags } = buildApiClient(); + const { tagsStats } = buildApiClient(); - const result = await listTags(true); + const result = await tagsStats(); expect({ tags: expectedTags, stats: expectedStats }).toEqual(result); expect(fetchJson).toHaveBeenCalledWith(