From 4ebe23e89f71e7686de5f1159402a192879fc14c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 19 Mar 2023 10:32:11 +0100 Subject: [PATCH 1/6] Add logic to dynamically exclude bots visits in tags table --- package.json | 3 ++- src/api/types/index.ts | 3 +++ src/tags/TagsList.tsx | 21 ++++++++++++++------- src/tags/TagsTableRow.tsx | 4 ++-- src/tags/data/TagsListChildrenProps.ts | 4 ++-- src/tags/data/index.ts | 5 ++++- src/tags/reducers/tagsList.ts | 8 ++++---- src/tags/services/provideServices.ts | 1 + test/tags/TagsList.test.tsx | 2 +- test/tags/TagsTable.test.tsx | 4 ++-- 10 files changed, 35 insertions(+), 20 deletions(-) diff --git a/package.json b/package.json index e9d4c8bd..66ebcd44 100644 --- a/package.json +++ b/package.json @@ -12,9 +12,10 @@ "lint:fix": "npm run lint:css:fix && npm run lint:js:fix", "lint:css:fix": "npm run lint:css -- --fix", "lint:js:fix": "npm run lint:js -- --fix", + "types": "tsc", "start": "vite serve --host=0.0.0.0", "preview": "vite preview --host=0.0.0.0", - "build": "tsc --noEmit && vite build && node scripts/replace-version.mjs", + "build": "npm run types && vite build && node scripts/replace-version.mjs", "build:dist": "npm run build && node scripts/create-dist-file.mjs", "test": "jest --env=jsdom --colors", "test:coverage": "npm run test -- --coverage --coverageReporters=text --coverageReporters=text-summary", diff --git a/src/api/types/index.ts b/src/api/types/index.ts index 59c4e05b..e97dddee 100644 --- a/src/api/types/index.ts +++ b/src/api/types/index.ts @@ -21,6 +21,9 @@ export interface ShlinkHealth { interface ShlinkTagsStats { tag: string; shortUrlsCount: number; + visitsSummary?: ShlinkVisitsSummary; // Optional only before Shlink 3.5.0 + + /** @deprecated */ visitsCount: number; } diff --git a/src/tags/TagsList.tsx b/src/tags/TagsList.tsx index a2e35501..13aad995 100644 --- a/src/tags/TagsList.tsx +++ b/src/tags/TagsList.tsx @@ -12,7 +12,7 @@ import { Message } from '../utils/Message'; import { OrderingDropdown } from '../utils/OrderingDropdown'; import { Result } from '../utils/Result'; import { SearchField } from '../utils/SearchField'; -import type { NormalizedTag } from './data'; +import type { SimplifiedTag } from './data'; import type { TagsOrder, TagsOrderableFields } from './data/TagsListChildrenProps'; import { TAGS_ORDERABLE_FIELDS } from './data/TagsListChildrenProps'; import type { TagsList as TagsListState } from './reducers/tagsList'; @@ -31,12 +31,19 @@ export const TagsList = (TagsTable: FC) => boundToMercureHub(( ) => { const [order, setOrder] = useState(settings.tags?.defaultOrdering ?? {}); const resolveSortedTags = pipe( - () => tagsList.filteredTags.map((tag): NormalizedTag => ({ - tag, - shortUrls: tagsList.stats[tag]?.shortUrlsCount ?? 0, - visits: tagsList.stats[tag]?.visitsCount ?? 0, - })), - (normalizedTags) => sortList(normalizedTags, order), + () => tagsList.filteredTags.map((tag): SimplifiedTag => { + const theTag = tagsList.stats[tag]; + const visits = ( + settings.visits?.excludeBots ? theTag?.visitsSummary?.nonBots : theTag?.visitsSummary?.total + ) ?? theTag?.visitsCount ?? 0; + + return { + tag, + visits, + shortUrls: theTag?.shortUrlsCount ?? 0, + }; + }), + (simplifiedTags) => sortList(simplifiedTags, order), ); useEffect(() => { diff --git a/src/tags/TagsTableRow.tsx b/src/tags/TagsTableRow.tsx index aa8ea5a5..d3257bae 100644 --- a/src/tags/TagsTableRow.tsx +++ b/src/tags/TagsTableRow.tsx @@ -9,11 +9,11 @@ import { DropdownBtnMenu } from '../utils/DropdownBtnMenu'; import { useToggle } from '../utils/helpers/hooks'; import { prettify } from '../utils/helpers/numbers'; import type { ColorGenerator } from '../utils/services/ColorGenerator'; -import type { NormalizedTag, TagModalProps } from './data'; +import type { SimplifiedTag, TagModalProps } from './data'; import { TagBullet } from './helpers/TagBullet'; export interface TagsTableRowProps { - tag: NormalizedTag; + tag: SimplifiedTag; selectedServer: SelectedServer; } diff --git a/src/tags/data/TagsListChildrenProps.ts b/src/tags/data/TagsListChildrenProps.ts index a20720bc..3e2843eb 100644 --- a/src/tags/data/TagsListChildrenProps.ts +++ b/src/tags/data/TagsListChildrenProps.ts @@ -1,6 +1,6 @@ import type { SelectedServer } from '../../servers/data'; import type { Order } from '../../utils/helpers/ordering'; -import type { NormalizedTag } from './index'; +import type { SimplifiedTag } from './index'; export const TAGS_ORDERABLE_FIELDS = { tag: 'Tag', @@ -13,6 +13,6 @@ export type TagsOrderableFields = keyof typeof TAGS_ORDERABLE_FIELDS; export type TagsOrder = Order; export interface TagsListChildrenProps { - sortedTags: NormalizedTag[]; + sortedTags: SimplifiedTag[]; selectedServer: SelectedServer; } diff --git a/src/tags/data/index.ts b/src/tags/data/index.ts index 8210a98b..acc59a5b 100644 --- a/src/tags/data/index.ts +++ b/src/tags/data/index.ts @@ -1,6 +1,9 @@ +import type { ShlinkVisitsSummary } from '../../api/types'; + export interface TagStats { shortUrlsCount: number; visitsCount: number; + visitsSummary?: ShlinkVisitsSummary; // Optional only before Shlink 3.5.0 } export interface TagModalProps { @@ -9,7 +12,7 @@ export interface TagModalProps { toggle: () => void; } -export interface NormalizedTag { +export interface SimplifiedTag { tag: string; shortUrls: number; visits: number; diff --git a/src/tags/reducers/tagsList.ts b/src/tags/reducers/tagsList.ts index e96c8a23..9582659f 100644 --- a/src/tags/reducers/tagsList.ts +++ b/src/tags/reducers/tagsList.ts @@ -50,6 +50,7 @@ const increaseVisitsForTags = (tags: TagIncrease[], stats: TagsStatsMap) => tags const tagStats = theStats[tag]; + // TODO take into consideration bots, nonBots and total return { ...theStats, [tag]: { @@ -78,12 +79,11 @@ export const listTags = (buildShlinkApiClient: ShlinkApiClientBuilder, force = t } const { listTags: shlinkListTags, tagsStats } = buildShlinkApiClient(getState); - const { tags, stats = [] }: ShlinkTags = await ( + const { tags, stats }: ShlinkTags = await ( supportedFeatures.tagsStats(selectedServer) ? tagsStats() : shlinkListTags() ); - const processedStats = stats.reduce((acc, { tag, shortUrlsCount, visitsCount }) => { - acc[tag] = { shortUrlsCount, visitsCount }; - + const processedStats = stats.reduce((acc, { tag, ...rest }) => { + acc[tag] = rest; return acc; }, {}); diff --git a/src/tags/services/provideServices.ts b/src/tags/services/provideServices.ts index 8fbbe9cb..50d32e49 100644 --- a/src/tags/services/provideServices.ts +++ b/src/tags/services/provideServices.ts @@ -24,6 +24,7 @@ export const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { bottle.decorator('EditTagModal', connect(['tagEdit'], ['editTag', 'tagEdited'])); bottle.serviceFactory('TagsTableRow', TagsTableRow, 'DeleteTagConfirmModal', 'EditTagModal', 'ColorGenerator'); + bottle.decorator('TagsTableRow', connect(['settings'])); bottle.serviceFactory('TagsTable', TagsTable, 'TagsTableRow'); diff --git a/test/tags/TagsList.test.tsx b/test/tags/TagsList.test.tsx index b20a8115..64dd7afa 100644 --- a/test/tags/TagsList.test.tsx +++ b/test/tags/TagsList.test.tsx @@ -10,7 +10,7 @@ import { renderWithEvents } from '../__helpers__/setUpTest'; describe('', () => { const filterTags = jest.fn(); - const TagsListComp = createTagsList(() => <>TagsTable); + const TagsListComp = createTagsList(({ sortedTags }) => <>TagsTable ({sortedTags.map((t) => t.visits).join(',')})); const setUp = (tagsList: Partial) => renderWithEvents( ()} diff --git a/test/tags/TagsTable.test.tsx b/test/tags/TagsTable.test.tsx index 4c212cf0..da187a6a 100644 --- a/test/tags/TagsTable.test.tsx +++ b/test/tags/TagsTable.test.tsx @@ -2,7 +2,7 @@ import { screen } from '@testing-library/react'; import { useLocation } from 'react-router-dom'; import { Mock } from 'ts-mockery'; import type { SelectedServer } from '../../src/servers/data'; -import type { NormalizedTag } from '../../src/tags/data'; +import type { SimplifiedTag } from '../../src/tags/data'; import { TagsTable as createTagsTable } from '../../src/tags/TagsTable'; import { rangeOf } from '../../src/utils/utils'; import { renderWithEvents } from '../__helpers__/setUpTest'; @@ -17,7 +17,7 @@ describe('', () => { (useLocation as any).mockReturnValue({ search }); return renderWithEvents( Mock.of({ tag }))} + sortedTags={sortedTags.map((tag) => Mock.of({ tag }))} selectedServer={Mock.all()} currentOrder={{}} orderByColumn={() => orderByColumn} From 34cfe2077b9f4410d4817410761ea0205e66eb6f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 19 Mar 2023 10:44:09 +0100 Subject: [PATCH 2/6] Test proper amount of visits is displayed in TagsList --- test/tags/TagsList.test.tsx | 49 +++++++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/test/tags/TagsList.test.tsx b/test/tags/TagsList.test.tsx index 64dd7afa..19f59b25 100644 --- a/test/tags/TagsList.test.tsx +++ b/test/tags/TagsList.test.tsx @@ -11,14 +11,14 @@ import { renderWithEvents } from '../__helpers__/setUpTest'; describe('', () => { const filterTags = jest.fn(); const TagsListComp = createTagsList(({ sortedTags }) => <>TagsTable ({sortedTags.map((t) => t.visits).join(',')})); - const setUp = (tagsList: Partial) => renderWithEvents( + const setUp = (tagsList: Partial, excludeBots = false) => renderWithEvents( ()} {...Mock.of({ mercureInfo: {} })} forceListTags={identity} filterTags={filterTags} tagsList={Mock.of(tagsList)} - settings={Mock.all()} + settings={Mock.of({ visits: { excludeBots } })} />, ); @@ -53,4 +53,49 @@ describe('', () => { await user.type(screen.getByPlaceholderText('Search...'), 'Hello'); await waitFor(() => expect(filterTags).toHaveBeenCalledTimes(1)); }); + + it.each([ + [false, undefined, '25,25,25'], + [true, undefined, '25,25,25'], + [ + false, + { + total: 20, + nonBots: 15, + bots: 5, + }, + '20,20,20', + ], + [ + true, + { + total: 20, + nonBots: 15, + bots: 5, + }, + '15,15,15', + ], + ])('displays proper amount of visits', (excludeBots, visitsSummary, expectedAmounts) => { + setUp({ + filteredTags: ['foo', 'bar', 'baz'], + stats: { + foo: { + visitsSummary, + visitsCount: 25, + shortUrlsCount: 1, + }, + bar: { + visitsSummary, + visitsCount: 25, + shortUrlsCount: 1, + }, + baz: { + visitsSummary, + visitsCount: 25, + shortUrlsCount: 1, + }, + }, + }, excludeBots); + expect(screen.getByText(`TagsTable (${expectedAmounts})`)).toBeInTheDocument(); + }); }); From 927ab76dbd6aed8c80198243b252d42d2a6e4316 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 19 Mar 2023 10:46:52 +0100 Subject: [PATCH 3/6] Increase required coverage --- jest.config.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jest.config.js b/jest.config.js index 4f2d9e03..d212d872 100644 --- a/jest.config.js +++ b/jest.config.js @@ -10,8 +10,8 @@ module.exports = { coverageThreshold: { global: { statements: 90, - branches: 80, - functions: 85, + branches: 85, + functions: 90, lines: 90, }, }, From 1d6464fefb69d0e62dd2a5411fc6a99d33ddb638 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 19 Mar 2023 11:44:40 +0100 Subject: [PATCH 4/6] Take into consideration types of visits when increasing tags visits --- src/tags/reducers/tagsList.ts | 26 ++++++++--- test/tags/reducers/tagsList.test.ts | 71 +++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/src/tags/reducers/tagsList.ts b/src/tags/reducers/tagsList.ts index 9582659f..e92cb589 100644 --- a/src/tags/reducers/tagsList.ts +++ b/src/tags/reducers/tagsList.ts @@ -8,7 +8,7 @@ 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'; +import type { CreateVisit } from '../../visits/types'; import type { TagStats } from '../data'; import { tagDeleted } from './tagDelete'; import { tagEdited } from './tagEdit'; @@ -39,7 +39,8 @@ const initialState: TagsList = { error: false, }; -type TagIncrease = [string, number]; +type TagIncreaseRecord = Record; +type TagIncrease = [string, { bots: number; nonBots: number }]; const renameTag = (oldName: string, newName: string) => (tag: string) => (tag === oldName ? newName : tag); const rejectTag = (tags: string[], tagToReject: string) => reject((tag) => tag === tagToReject, tags); @@ -48,21 +49,34 @@ const increaseVisitsForTags = (tags: TagIncrease[], stats: TagsStatsMap) => tags return theStats; } + const { bots, nonBots } = increase; const tagStats = theStats[tag]; - // TODO take into consideration bots, nonBots and total return { ...theStats, [tag]: { ...tagStats, - visitsCount: tagStats.visitsCount + increase, + visitsSummary: tagStats.visitsSummary && { + total: tagStats.visitsSummary.total + bots + nonBots, + bots: tagStats.visitsSummary.bots + bots, + nonBots: tagStats.visitsSummary.nonBots + nonBots, + }, + visitsCount: tagStats.visitsCount + bots + nonBots, }, }; }, { ...stats }); const calculateVisitsPerTag = (createdVisits: CreateVisit[]): TagIncrease[] => Object.entries( - createdVisits.reduce((acc, { shortUrl }) => { + createdVisits.reduce((acc, { shortUrl, visit }) => { shortUrl?.tags.forEach((tag) => { - acc[tag] = (acc[tag] || 0) + 1; + if (!acc[tag]) { + acc[tag] = { bots: 0, nonBots: 0 }; + } + + if (visit.potentialBot) { + acc[tag].bots += 1; + } else { + acc[tag].nonBots += 1; + } }); return acc; diff --git a/test/tags/reducers/tagsList.test.ts b/test/tags/reducers/tagsList.test.ts index 54746842..d8f9cfe1 100644 --- a/test/tags/reducers/tagsList.test.ts +++ b/test/tags/reducers/tagsList.test.ts @@ -11,6 +11,8 @@ import { listTags as listTagsCreator, tagsListReducerCreator, } from '../../../src/tags/reducers/tagsList'; +import { createNewVisits } from '../../../src/visits/reducers/visitCreation'; +import type { CreateVisit, Visit } from '../../../src/visits/types'; describe('tagsListReducer', () => { const state = (props: Partial) => Mock.of(props); @@ -118,6 +120,75 @@ describe('tagsListReducer', () => { tags: expectedTags, }); }); + + it('increases amounts when visits are created', () => { + const createdVisits = [ + Mock.of({ + shortUrl: Mock.of({ tags: ['foo', 'bar'] }), + visit: Mock.of({ potentialBot: true }), + }), + Mock.of({ + shortUrl: Mock.of({ tags: ['foo', 'bar'] }), + visit: Mock.all(), + }), + Mock.of({ + shortUrl: Mock.of({ tags: ['bar'] }), + visit: Mock.all(), + }), + Mock.of({ + shortUrl: Mock.of({ tags: ['baz'] }), + visit: Mock.of({ potentialBot: true }), + }), + ]; + const tagStats = (total: number) => ({ + shortUrlsCount: 1, + visitsCount: total, + visitsSummary: { + total, + nonBots: total - 10, + bots: 10, + }, + }); + const stateBefore = state({ + stats: { + foo: tagStats(100), + bar: tagStats(200), + baz: tagStats(150), + }, + }); + + expect(reducer(stateBefore, createNewVisits(createdVisits))).toEqual(expect.objectContaining({ + stats: { + foo: { + shortUrlsCount: 1, + visitsCount: 100 + 2, + visitsSummary: { + total: 100 + 2, + nonBots: 90 + 1, + bots: 10 + 1, + }, + }, + bar: { + shortUrlsCount: 1, + visitsCount: 200 + 3, + visitsSummary: { + total: 200 + 3, + nonBots: 190 + 2, + bots: 10 + 1, + }, + }, + baz: { + shortUrlsCount: 1, + visitsCount: 150 + 1, + visitsSummary: { + total: 150 + 1, + nonBots: 140, + bots: 10 + 1, + }, + }, + }, + })); + }); }); describe('filterTags', () => { From 43b29260637b160f3db5916b080f42f464e4bc44 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 19 Mar 2023 11:47:02 +0100 Subject: [PATCH 5/6] Update changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e47fbd0..d5928ee8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,10 +4,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org). -## [Unreleased] +## [3.10.0] - 2023-03-19 ### Added * [#807](https://github.com/shlinkio/shlink-web-client/issues/807) Add support for device-specific long-URLs when creating or editing short URLs. * [#808](https://github.com/shlinkio/shlink-web-client/issues/808) Respect settings on excluding bots in the overview section, for visits cards. +* [#809](https://github.com/shlinkio/shlink-web-client/issues/809) Respect settings on excluding bots in the tags list. ### Changed * [#798](https://github.com/shlinkio/shlink-web-client/issues/798) Remove stryker and mutation testing. From cf27de965ecce24e80346259b6b5d71894a74f05 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 19 Mar 2023 11:50:06 +0100 Subject: [PATCH 6/6] Remove redundant type --- src/api/types/index.ts | 2 +- src/tags/data/index.ts | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/api/types/index.ts b/src/api/types/index.ts index e97dddee..06473dff 100644 --- a/src/api/types/index.ts +++ b/src/api/types/index.ts @@ -18,7 +18,7 @@ export interface ShlinkHealth { version: string; } -interface ShlinkTagsStats { +export interface ShlinkTagsStats { tag: string; shortUrlsCount: number; visitsSummary?: ShlinkVisitsSummary; // Optional only before Shlink 3.5.0 diff --git a/src/tags/data/index.ts b/src/tags/data/index.ts index acc59a5b..744fcd47 100644 --- a/src/tags/data/index.ts +++ b/src/tags/data/index.ts @@ -1,10 +1,6 @@ -import type { ShlinkVisitsSummary } from '../../api/types'; +import type { ShlinkTagsStats } from '../../api/types'; -export interface TagStats { - shortUrlsCount: number; - visitsCount: number; - visitsSummary?: ShlinkVisitsSummary; // Optional only before Shlink 3.5.0 -} +export type TagStats = Omit; export interface TagModalProps { tag: string;