From 1d8189369c1c6d645d51869cbe0cf9d735b7f0c3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 18 Mar 2023 10:54:14 +0100 Subject: [PATCH] Enhance visits overview reducer to handle bot and non-bots visits amounts --- src/visits/reducers/visitsOverview.ts | 69 ++++++++-- src/visits/types/helpers.ts | 6 +- test/visits/reducers/visitsOverview.test.ts | 141 +++++++++++++++----- test/visits/types/helpers.test.ts | 14 +- 4 files changed, 176 insertions(+), 54 deletions(-) diff --git a/src/visits/reducers/visitsOverview.ts b/src/visits/reducers/visitsOverview.ts index bab2b58b..073fa3d5 100644 --- a/src/visits/reducers/visitsOverview.ts +++ b/src/visits/reducers/visitsOverview.ts @@ -3,14 +3,24 @@ import { createSlice } from '@reduxjs/toolkit'; import type { ShlinkApiClientBuilder } from '../../api/services/ShlinkApiClientBuilder'; import type { ShlinkVisitsOverview } from '../../api/types'; import { createAsyncThunk } from '../../utils/helpers/redux'; +import type { CreateVisit } from '../types'; import { groupNewVisitsByType } from '../types/helpers'; import { createNewVisits } from './visitCreation'; const REDUCER_PREFIX = 'shlink/visitsOverview'; -export interface VisitsOverview { - visitsCount: number; - orphanVisitsCount: number; +export type PartialVisitsSummary = { + total: number; + nonBots?: number; + bots?: number; +}; + +export type ParsedVisitsOverview = { + nonOrphanVisits: PartialVisitsSummary; + orphanVisits: PartialVisitsSummary; +}; + +export interface VisitsOverview extends ParsedVisitsOverview { loading: boolean; error: boolean; } @@ -18,15 +28,34 @@ export interface VisitsOverview { export type GetVisitsOverviewAction = PayloadAction; const initialState: VisitsOverview = { - visitsCount: 0, - orphanVisitsCount: 0, + nonOrphanVisits: { + total: 0, + }, + orphanVisits: { + total: 0, + }, loading: false, error: false, }; +const countBots = (visits: CreateVisit[]) => visits.filter(({ visit }) => visit.potentialBot).length; + export const loadVisitsOverview = (buildShlinkApiClient: ShlinkApiClientBuilder) => createAsyncThunk( `${REDUCER_PREFIX}/loadVisitsOverview`, - (_: void, { getState }): Promise => buildShlinkApiClient(getState).getVisitsOverview(), + (_: void, { getState }): Promise => buildShlinkApiClient(getState).getVisitsOverview().then( + (resp) => ({ + nonOrphanVisits: { + total: resp.nonOrphanVisits?.total ?? resp.visitsCount, + nonBots: resp.nonOrphanVisits?.nonBots, + bots: resp.nonOrphanVisits?.bots, + }, + orphanVisits: { + total: resp.orphanVisits?.total ?? resp.orphanVisitsCount, + nonBots: resp.orphanVisits?.nonBots, + bots: resp.orphanVisits?.bots, + }, + }), + ), ); export const visitsOverviewReducerCreator = ( @@ -40,13 +69,31 @@ export const visitsOverviewReducerCreator = ( builder.addCase(loadVisitsOverviewThunk.rejected, () => ({ ...initialState, error: true })); builder.addCase(loadVisitsOverviewThunk.fulfilled, (_, { payload }) => ({ ...initialState, ...payload })); - builder.addCase(createNewVisits, ({ visitsCount, orphanVisitsCount = 0, ...rest }, { payload }) => { - const { createdVisits } = payload; - const { regularVisits, orphanVisits } = groupNewVisitsByType(createdVisits); + builder.addCase(createNewVisits, ({ nonOrphanVisits, orphanVisits, ...rest }, { payload }) => { + const { nonOrphanVisits: newNonOrphanVisits, orphanVisits: newOrphanVisits } = groupNewVisitsByType( + payload.createdVisits, + ); + + const newNonOrphanTotalVisits = newNonOrphanVisits.length; + const newNonOrphanBotVisits = countBots(newNonOrphanVisits); + const newNonOrphanNonBotVisits = newNonOrphanTotalVisits - newNonOrphanBotVisits; + + const newOrphanTotalVisits = newOrphanVisits.length; + const newOrphanBotVisits = countBots(newOrphanVisits); + const newOrphanNonBotVisits = newOrphanTotalVisits - newOrphanBotVisits; + return { ...rest, - visitsCount: visitsCount + regularVisits.length, - orphanVisitsCount: orphanVisitsCount + orphanVisits.length, + nonOrphanVisits: { + total: nonOrphanVisits.total + newNonOrphanTotalVisits, + bots: nonOrphanVisits.bots && nonOrphanVisits.bots + newNonOrphanBotVisits, + nonBots: nonOrphanVisits.nonBots && nonOrphanVisits.nonBots + newNonOrphanNonBotVisits, + }, + orphanVisits: { + total: orphanVisits.total + newOrphanTotalVisits, + bots: orphanVisits.bots && orphanVisits.bots + newOrphanBotVisits, + nonBots: orphanVisits.nonBots && orphanVisits.nonBots + newOrphanNonBotVisits, + }, }; }); }, diff --git a/src/visits/types/helpers.ts b/src/visits/types/helpers.ts index 77714262..4cda7222 100644 --- a/src/visits/types/helpers.ts +++ b/src/visits/types/helpers.ts @@ -10,13 +10,13 @@ export const isNormalizedOrphanVisit = (visit: NormalizedVisit): visit is Normal export interface GroupedNewVisits { orphanVisits: CreateVisit[]; - regularVisits: CreateVisit[]; + nonOrphanVisits: CreateVisit[]; } export const groupNewVisitsByType = pipe( - groupBy((newVisit: CreateVisit) => (isOrphanVisit(newVisit.visit) ? 'orphanVisits' : 'regularVisits')), + groupBy((newVisit: CreateVisit) => (isOrphanVisit(newVisit.visit) ? 'orphanVisits' : 'nonOrphanVisits')), // @ts-expect-error Type declaration on groupBy is not correct. It can return undefined props - (result): GroupedNewVisits => ({ orphanVisits: [], regularVisits: [], ...result }), + (result): GroupedNewVisits => ({ orphanVisits: [], nonOrphanVisits: [], ...result }), ); export type HighlightableProps = T extends NormalizedOrphanVisit diff --git a/test/visits/reducers/visitsOverview.test.ts b/test/visits/reducers/visitsOverview.test.ts index 4bf3568f..a5fbf757 100644 --- a/test/visits/reducers/visitsOverview.test.ts +++ b/test/visits/reducers/visitsOverview.test.ts @@ -5,8 +5,8 @@ import type { ShlinkState } from '../../../src/container/types'; import type { CreateVisitsAction } from '../../../src/visits/reducers/visitCreation'; import { createNewVisits } from '../../../src/visits/reducers/visitCreation'; import type { - GetVisitsOverviewAction, - VisitsOverview } from '../../../src/visits/reducers/visitsOverview'; + GetVisitsOverviewAction, ParsedVisitsOverview, + PartialVisitsSummary, VisitsOverview } from '../../../src/visits/reducers/visitsOverview'; import { loadVisitsOverview as loadVisitsOverviewCreator, visitsOverviewReducerCreator, @@ -46,45 +46,98 @@ describe('visitsOverviewReducer', () => { }); it('return visits overview on GET_OVERVIEW', () => { - const { loading, error, visitsCount } = reducer(state({ loading: true, error: false }), { - type: loadVisitsOverview.fulfilled.toString(), - payload: { visitsCount: 100 }, - }); + const action = loadVisitsOverview.fulfilled(Mock.of({ + nonOrphanVisits: { total: 100 }, + }), 'requestId'); + const { loading, error, nonOrphanVisits } = reducer(state({ loading: true, error: false }), action); expect(loading).toEqual(false); expect(error).toEqual(false); - expect(visitsCount).toEqual(100); + expect(nonOrphanVisits.total).toEqual(100); }); it.each([ [50, 53], [0, 3], - [undefined, 3], ])('returns updated amounts on CREATE_VISITS', (providedOrphanVisitsCount, expectedOrphanVisitsCount) => { - const { visitsCount, orphanVisitsCount } = reducer( - state({ visitsCount: 100, orphanVisitsCount: providedOrphanVisitsCount }), - { - type: createNewVisits.toString(), - payload: { - createdVisits: [ - Mock.of({ visit: Mock.all() }), - Mock.of({ visit: Mock.all() }), - Mock.of({ - visit: Mock.of({ visitedUrl: '' }), - }), - Mock.of({ - visit: Mock.of({ visitedUrl: '' }), - }), - Mock.of({ - visit: Mock.of({ visitedUrl: '' }), - }), - ], - }, - } as unknown as GetVisitsOverviewAction & CreateVisitsAction, + const { nonOrphanVisits, orphanVisits } = reducer( + state({ + nonOrphanVisits: { total: 100 }, + orphanVisits: { total: providedOrphanVisitsCount }, + }), + createNewVisits([ + Mock.of({ visit: Mock.all() }), + Mock.of({ visit: Mock.all() }), + Mock.of({ + visit: Mock.of({ visitedUrl: '' }), + }), + Mock.of({ + visit: Mock.of({ visitedUrl: '' }), + }), + Mock.of({ + visit: Mock.of({ visitedUrl: '' }), + }), + ]), ); - expect(visitsCount).toEqual(102); - expect(orphanVisitsCount).toEqual(expectedOrphanVisitsCount); + expect(nonOrphanVisits.total).toEqual(102); + expect(orphanVisits.total).toEqual(expectedOrphanVisitsCount); + }); + + it.each([ + [ + {} satisfies Omit, + {} satisfies Omit, + { total: 103 } satisfies PartialVisitsSummary, + { total: 203 } satisfies PartialVisitsSummary, + ], + [ + { bots: 35 } satisfies Omit, + { bots: 35 } satisfies Omit, + { total: 103, bots: 37 } satisfies PartialVisitsSummary, + { total: 203, bots: 36 } satisfies PartialVisitsSummary, + ], + [ + { nonBots: 41, bots: 85 } satisfies Omit, + { nonBots: 63, bots: 27 } satisfies Omit, + { total: 103, nonBots: 42, bots: 87 } satisfies PartialVisitsSummary, + { total: 203, nonBots: 65, bots: 28 } satisfies PartialVisitsSummary, + ], + [ + { nonBots: 56 } satisfies Omit, + { nonBots: 99 } satisfies Omit, + { total: 103, nonBots: 57 } satisfies PartialVisitsSummary, + { total: 203, nonBots: 101 } satisfies PartialVisitsSummary, + ], + ])('takes bots and non-bots into consideration when creating visits', ( + initialNonOrphanVisits, + initialOrphanVisits, + expectedNonOrphanVisits, + expectedOrphanVisits, + ) => { + const { nonOrphanVisits, orphanVisits } = reducer( + state({ + nonOrphanVisits: { total: 100, ...initialNonOrphanVisits }, + orphanVisits: { total: 200, ...initialOrphanVisits }, + }), + createNewVisits([ + Mock.of({ visit: Mock.all() }), + Mock.of({ visit: Mock.of({ potentialBot: true }) }), + Mock.of({ visit: Mock.of({ potentialBot: true }) }), + Mock.of({ + visit: Mock.of({ visitedUrl: '' }), + }), + Mock.of({ + visit: Mock.of({ visitedUrl: '' }), + }), + Mock.of({ + visit: Mock.of({ visitedUrl: '', potentialBot: true }), + }), + ]), + ); + + expect(nonOrphanVisits).toEqual(expectedNonOrphanVisits); + expect(orphanVisits).toEqual(expectedOrphanVisits); }); }); @@ -109,8 +162,30 @@ describe('visitsOverviewReducer', () => { expect(getVisitsOverview).toHaveBeenCalledTimes(1); }); - it('dispatches start and success when promise is resolved', async () => { - const resolvedOverview = Mock.of({ visitsCount: 50 }); + it.each([ + [ + // Shlink <3.5.0 + { visitsCount: 50, orphanVisitsCount: 20 } satisfies ShlinkVisitsOverview, + { + nonOrphanVisits: { total: 50, nonBots: undefined, bots: undefined }, + orphanVisits: { total: 20, nonBots: undefined, bots: undefined }, + }, + ], + [ + // Shlink >=3.5.0 + { + nonOrphanVisits: { total: 50, nonBots: 20, bots: 30 }, + orphanVisits: { total: 50, nonBots: 20, bots: 30 }, + visitsCount: 3, + orphanVisitsCount: 3, + } satisfies ShlinkVisitsOverview, + { + nonOrphanVisits: { total: 50, nonBots: 20, bots: 30 }, + orphanVisits: { total: 50, nonBots: 20, bots: 30 }, + }, + ], + ])('dispatches start and success when promise is resolved', async (serverResult, dispatchedPayload) => { + const resolvedOverview = Mock.of(serverResult); getVisitsOverview.mockResolvedValue(resolvedOverview); await loadVisitsOverview()(dispatchMock, getState, {}); @@ -121,7 +196,7 @@ describe('visitsOverviewReducer', () => { })); expect(dispatchMock).toHaveBeenNthCalledWith(2, expect.objectContaining({ type: loadVisitsOverview.fulfilled.toString(), - payload: { visitsCount: 50 }, + payload: dispatchedPayload, })); expect(getVisitsOverview).toHaveBeenCalledTimes(1); }); diff --git a/test/visits/types/helpers.test.ts b/test/visits/types/helpers.test.ts index de00bddd..75f915c3 100644 --- a/test/visits/types/helpers.test.ts +++ b/test/visits/types/helpers.test.ts @@ -8,7 +8,7 @@ import { groupNewVisitsByType, toApiParams } from '../../../src/visits/types/hel describe('visitsTypeHelpers', () => { describe('groupNewVisitsByType', () => { it.each([ - [[], { orphanVisits: [], regularVisits: [] }], + [[], { orphanVisits: [], nonOrphanVisits: [] }], ((): [CreateVisit[], GroupedNewVisits] => { const orphanVisits: CreateVisit[] = [ Mock.of({ @@ -18,7 +18,7 @@ describe('visitsTypeHelpers', () => { visit: Mock.of({ visitedUrl: '' }), }), ]; - const regularVisits: CreateVisit[] = [ + const nonOrphanVisits: CreateVisit[] = [ Mock.of({ visit: Mock.all() }), Mock.of({ visit: Mock.all() }), Mock.of({ visit: Mock.all() }), @@ -27,8 +27,8 @@ describe('visitsTypeHelpers', () => { ]; return [ - [...orphanVisits, ...regularVisits], - { orphanVisits, regularVisits }, + [...orphanVisits, ...nonOrphanVisits], + { orphanVisits, nonOrphanVisits }, ]; })(), ((): [CreateVisit[], GroupedNewVisits] => { @@ -44,16 +44,16 @@ describe('visitsTypeHelpers', () => { }), ]; - return [orphanVisits, { orphanVisits, regularVisits: [] }]; + return [orphanVisits, { orphanVisits, nonOrphanVisits: [] }]; })(), ((): [CreateVisit[], GroupedNewVisits] => { - const regularVisits: CreateVisit[] = [ + const nonOrphanVisits: CreateVisit[] = [ Mock.of({ visit: Mock.all() }), Mock.of({ visit: Mock.all() }), Mock.of({ visit: Mock.all() }), ]; - return [regularVisits, { orphanVisits: [], regularVisits }]; + return [nonOrphanVisits, { orphanVisits: [], nonOrphanVisits }]; })(), ])('groups new visits as expected', (createdVisits, expectedResult) => { expect(groupNewVisitsByType(createdVisits)).toEqual(expectedResult);